From 0099e3e9fd8f0c82c6e5764210d30a4ccd90cac5 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 26 Dec 2020 17:39:16 +0100 Subject: [PATCH] rely on the symbol -> var mapping from solving; don't extract var from the def itself --- compiler/can/src/module.rs | 38 ++++++++++++------------------ compiler/load/src/effect_module.rs | 6 ++--- compiler/load/src/file.rs | 26 +++++++------------- 3 files changed, 27 insertions(+), 43 deletions(-) diff --git a/compiler/can/src/module.rs b/compiler/can/src/module.rs index 56ac071f6a..5f100a7a1b 100644 --- a/compiler/can/src/module.rs +++ b/compiler/can/src/module.rs @@ -36,7 +36,6 @@ pub struct ModuleOutput { pub lookups: Vec<(Symbol, Variable, Region)>, pub problems: Vec, pub ident_ids: IdentIds, - pub exposed_vars_by_symbol: Vec<(Symbol, Variable)>, pub references: MutSet, } @@ -51,7 +50,7 @@ pub fn canonicalize_module_defs<'a>( dep_idents: MutMap, aliases: MutMap, exposed_imports: MutMap, - mut exposed_symbols: MutSet, + exposed_symbols: &MutSet, var_store: &mut VarStore, ) -> Result { let mut can_exposed_imports = MutMap::default(); @@ -166,45 +165,39 @@ pub fn canonicalize_module_defs<'a>( // NOTE previously we inserted builtin defs into the list of defs here // this is now done later, in file.rs. + // assume all exposed symbols are not actually defined in the module + // then as we walk the module and encounter the definitions, remove + // symbols from this set + let mut exposed_but_not_defined = exposed_symbols.clone(); + match sort_can_defs(&mut env, defs, Output::default()) { (Ok(mut declarations), output) => { use crate::def::Declaration::*; - // Record the variables for all exposed symbols. - let mut exposed_vars_by_symbol = Vec::with_capacity(exposed_symbols.len()); - for decl in declarations.iter() { match decl { Declare(def) => { - for (symbol, variable) in def.pattern_vars.iter() { - if exposed_symbols.contains(symbol) { - // This is one of our exposed symbols; - // record the corresponding variable! - exposed_vars_by_symbol.push((*symbol, *variable)); - + for (symbol, _) in def.pattern_vars.iter() { + if exposed_but_not_defined.contains(symbol) { // Remove this from exposed_symbols, // so that at the end of the process, // we can see if there were any // exposed symbols which did not have // corresponding defs. - exposed_symbols.remove(symbol); + exposed_but_not_defined.remove(symbol); } } } DeclareRec(defs) => { for def in defs { - for (symbol, variable) in def.pattern_vars.iter() { - if exposed_symbols.contains(symbol) { - // This is one of our exposed symbols; - // record the corresponding variable! - exposed_vars_by_symbol.push((*symbol, *variable)); - + for (symbol, _) in def.pattern_vars.iter() { + if exposed_but_not_defined.contains(symbol) { // Remove this from exposed_symbols, // so that at the end of the process, // we can see if there were any // exposed symbols which did not have // corresponding defs. - exposed_symbols.remove(symbol); + exposed_but_not_defined.remove(symbol); } } } @@ -219,7 +212,7 @@ pub fn canonicalize_module_defs<'a>( debug_assert!(def .pattern_vars .iter() - .all(|(symbol, _)| !exposed_symbols.contains(symbol))); + .all(|(symbol, _)| !exposed_but_not_defined.contains(symbol))); } } } @@ -232,7 +225,7 @@ pub fn canonicalize_module_defs<'a>( // we can see if there were any // exposed symbols which did not have // corresponding defs. - exposed_symbols.remove(&symbol); + exposed_but_not_defined.remove(&symbol); aliases.insert(symbol, alias); } @@ -241,7 +234,7 @@ pub fn canonicalize_module_defs<'a>( // exposed_symbols and added to exposed_vars_by_symbol. If any were // not, that means they were declared as exposed but there was // no actual declaration with that name! - for symbol in exposed_symbols { + for symbol in exposed_but_not_defined { env.problem(Problem::ExposedButNotDefined(symbol)); // In case this exposed value is referenced by other modules, @@ -305,7 +298,6 @@ pub fn canonicalize_module_defs<'a>( exposed_imports: can_exposed_imports, problems: env.problems, lookups, - exposed_vars_by_symbol, ident_ids: env.ident_ids, }) } diff --git a/compiler/load/src/effect_module.rs b/compiler/load/src/effect_module.rs index 9de9f3b348..5dab90a4f8 100644 --- a/compiler/load/src/effect_module.rs +++ b/compiler/load/src/effect_module.rs @@ -3,7 +3,7 @@ use roc_can::env::Env; use roc_can::expr::{Expr, Recursive}; use roc_can::pattern::Pattern; use roc_can::scope::Scope; -use roc_collections::all::SendMap; +use roc_collections::all::{MutSet, SendMap}; use roc_module::ident::TagName; use roc_module::operator::CalledVia; use roc_module::symbol::Symbol; @@ -48,7 +48,7 @@ pub fn build_effect_builtins( scope: &mut Scope, effect_symbol: Symbol, var_store: &mut VarStore, - exposed_vars_by_symbol: &mut Vec<(Symbol, Variable)>, + exposed_symbols: &mut MutSet, declarations: &mut Vec, ) { for (_, f) in BUILTIN_EFFECT_FUNCTIONS.iter() { @@ -60,7 +60,7 @@ pub fn build_effect_builtins( var_store, ); - exposed_vars_by_symbol.push((symbol, def.expr_var)); + exposed_symbols.insert(symbol); declarations.push(Declaration::Declare(def)); } } diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index face032a27..c818d23d6d 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2800,6 +2800,7 @@ fn run_solve<'a>( let (solved_subs, solved_env, problems) = roc_solve::module::run_solve(aliases, rigid_variables, constraint, var_store); + // solved_env.vars_by_symbol.retain(|k, _| exposed_symbols.contains(k)); let exposed_vars_by_symbol: Vec<(Symbol, Variable)> = exposed_symbols .iter() .map(|s| (*s, solved_env.vars_by_symbol[s])) @@ -3006,8 +3007,8 @@ fn fabricate_effects_module<'a>( let mut declarations = Vec::new(); - let exposed_vars_by_symbol = { - let mut exposed_vars_by_symbol = Vec::new(); + let exposed_symbols: MutSet = { + let mut exposed_symbols = MutSet::default(); { for (ident, ann) in effect_entries { @@ -3040,7 +3041,7 @@ fn fabricate_effects_module<'a>( annotation, ); - exposed_vars_by_symbol.push((symbol, def.expr_var)); + exposed_symbols.insert(symbol); declarations.push(Declaration::Declare(def)); } @@ -3052,11 +3053,11 @@ fn fabricate_effects_module<'a>( &mut scope, effect_symbol, &mut var_store, - &mut exposed_vars_by_symbol, + &mut exposed_symbols, &mut declarations, ); - exposed_vars_by_symbol + exposed_symbols }; use roc_can::module::ModuleOutput; @@ -3068,7 +3069,6 @@ fn fabricate_effects_module<'a>( lookups: Vec::new(), problems: can_env.problems, ident_ids: can_env.ident_ids, - exposed_vars_by_symbol, references: MutSet::default(), }; @@ -3077,11 +3077,7 @@ fn fabricate_effects_module<'a>( let module = Module { module_id, exposed_imports: module_output.exposed_imports, - exposed_symbols: module_output - .exposed_vars_by_symbol - .iter() - .map(|t| t.0) - .collect(), + exposed_symbols, references: module_output.references, aliases: module_output.aliases, rigid_variables: module_output.rigid_variables, @@ -3191,7 +3187,7 @@ fn canonicalize_and_constrain<'a>( dep_idents, aliases, exposed_imports, - exposed_symbols, + &exposed_symbols, &mut var_store, ); let canonicalize_end = SystemTime::now(); @@ -3205,11 +3201,7 @@ fn canonicalize_and_constrain<'a>( let module = Module { module_id, exposed_imports: module_output.exposed_imports, - exposed_symbols: module_output - .exposed_vars_by_symbol - .iter() - .map(|t| t.0) - .collect(), + exposed_symbols, references: module_output.references, aliases: module_output.aliases, rigid_variables: module_output.rigid_variables,