diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 16b091a40e..e31b699107 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -287,7 +287,7 @@ pub fn canonicalize_defs<'a>( // Record all the annotation's references in output.references.lookups for symbol in can_ann.references { - output.references.lookups.insert(symbol); + output.references.type_lookups.insert(symbol); output.references.referenced_type_defs.insert(symbol); } @@ -411,7 +411,7 @@ pub fn sort_can_defs( // Determine the full set of references by traversing the graph. let mut visited_symbols = MutSet::default(); - let returned_lookups = ImSet::clone(&output.references.lookups); + let returned_lookups = ImSet::clone(&output.references.value_lookups); // Start with the return expression's referenced locals. They're the only ones that count! // @@ -484,10 +484,10 @@ pub fn sort_can_defs( let mut loc_succ = local_successors(references, &env.closures); // if the current symbol is a closure, peek into its body - if let Some(References { lookups, .. }) = env.closures.get(symbol) { + if let Some(References { value_lookups, .. }) = env.closures.get(symbol) { let home = env.home; - for lookup in lookups { + for lookup in value_lookups { if lookup != symbol && lookup.module_id() == home { // DO NOT register a self-call behind a lambda! // @@ -534,8 +534,8 @@ pub fn sort_can_defs( let mut loc_succ = local_successors(references, &env.closures); // if the current symbol is a closure, peek into its body - if let Some(References { lookups, .. }) = env.closures.get(symbol) { - for lookup in lookups { + if let Some(References { value_lookups, .. }) = env.closures.get(symbol) { + for lookup in value_lookups { loc_succ.insert(*lookup); } } @@ -921,7 +921,7 @@ fn canonicalize_pending_def<'a>( // Record all the annotation's references in output.references.lookups for symbol in type_annotation.references.iter() { - output.references.lookups.insert(*symbol); + output.references.type_lookups.insert(*symbol); output.references.referenced_type_defs.insert(*symbol); } @@ -1043,7 +1043,7 @@ fn canonicalize_pending_def<'a>( // Record all the annotation's references in output.references.lookups for symbol in type_annotation.references.iter() { - output.references.lookups.insert(*symbol); + output.references.type_lookups.insert(*symbol); output.references.referenced_type_defs.insert(*symbol); } @@ -1123,7 +1123,7 @@ fn canonicalize_pending_def<'a>( // Recursion doesn't count as referencing. (If it did, all recursive functions // would result in circular def errors!) refs_by_symbol.entry(symbol).and_modify(|(_, refs)| { - refs.lookups = refs.lookups.without(&symbol); + refs.value_lookups = refs.value_lookups.without(&symbol); }); // renamed_closure_def = Some(&symbol); @@ -1263,7 +1263,7 @@ fn canonicalize_pending_def<'a>( // Recursion doesn't count as referencing. (If it did, all recursive functions // would result in circular def errors!) refs_by_symbol.entry(symbol).and_modify(|(_, refs)| { - refs.lookups = refs.lookups.without(&symbol); + refs.value_lookups = refs.value_lookups.without(&symbol); }); loc_can_expr.value = Closure(ClosureData { @@ -1358,7 +1358,8 @@ pub fn can_defs_with_return<'a>( // Now that we've collected all the references, check to see if any of the new idents // we defined went unused by the return expression. If any were unused, report it. for (symbol, region) in symbols_introduced { - if !output.references.has_lookup(symbol) { + if !output.references.has_value_lookup(symbol) && !output.references.has_type_lookup(symbol) + { env.problem(Problem::UnusedDef(symbol, region)); } } diff --git a/compiler/can/src/env.rs b/compiler/can/src/env.rs index e1216c42eb..667817e11c 100644 --- a/compiler/can/src/env.rs +++ b/compiler/can/src/env.rs @@ -27,8 +27,11 @@ pub struct Env<'a> { /// current closure name (if any) pub closure_name_symbol: Option, - /// Symbols which were referenced by qualified lookups. - pub qualified_lookups: MutSet, + /// Symbols of values/functions which were referenced by qualified lookups. + pub qualified_value_lookups: MutSet, + + /// Symbols of types which were referenced by qualified lookups. + pub qualified_type_lookups: MutSet, pub top_level_symbols: MutSet, @@ -51,7 +54,8 @@ impl<'a> Env<'a> { exposed_ident_ids, problems: Vec::new(), closures: MutMap::default(), - qualified_lookups: MutSet::default(), + qualified_value_lookups: MutSet::default(), + qualified_type_lookups: MutSet::default(), tailcallable_symbol: None, closure_name_symbol: None, top_level_symbols: MutSet::default(), @@ -71,6 +75,8 @@ impl<'a> Env<'a> { ident ); + let is_type_name = ident.starts_with(|c: char| c.is_uppercase()); + let module_name = ModuleName::from(module_name_str); let ident = Ident::from(ident); @@ -83,7 +89,11 @@ impl<'a> Env<'a> { Some(ident_id) => { let symbol = Symbol::new(module_id, *ident_id); - self.qualified_lookups.insert(symbol); + if is_type_name { + self.qualified_type_lookups.insert(symbol); + } else { + self.qualified_value_lookups.insert(symbol); + } Ok(symbol) } @@ -104,7 +114,11 @@ impl<'a> Env<'a> { Some(ident_id) => { let symbol = Symbol::new(module_id, *ident_id); - self.qualified_lookups.insert(symbol); + if is_type_name { + self.qualified_type_lookups.insert(symbol); + } else { + self.qualified_value_lookups.insert(symbol); + } Ok(symbol) } diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index 8a54994f68..a1f9510b83 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -493,7 +493,7 @@ pub fn canonicalize_expr<'a>( Ok((name, opaque_def)) => { let argument = Box::new(args.pop().unwrap()); output.references.referenced_type_defs.insert(name); - output.references.lookups.insert(name); + output.references.type_lookups.insert(name); let (type_arguments, lambda_set_variables, specialized_def_type) = freshen_opaque_def(var_store, opaque_def); @@ -587,7 +587,7 @@ pub fn canonicalize_expr<'a>( } } ast::Expr::Var { module_name, ident } => { - canonicalize_lookup(env, scope, module_name, ident, region) + canonicalize_var_lookup(env, scope, module_name, ident, region) } ast::Expr::Underscore(name) => { // we parse underscores, but they are not valid expression syntax @@ -661,8 +661,12 @@ pub fn canonicalize_expr<'a>( &loc_body_expr.value, ); - let mut captured_symbols: MutSet = - new_output.references.lookups.iter().copied().collect(); + let mut captured_symbols: MutSet = new_output + .references + .value_lookups + .iter() + .copied() + .collect(); // filter out the closure's name itself captured_symbols.remove(&symbol); @@ -684,7 +688,10 @@ pub fn canonicalize_expr<'a>( output.union(new_output); // filter out aliases - captured_symbols.retain(|s| !output.references.referenced_type_defs.contains(s)); + debug_assert!(captured_symbols + .iter() + .all(|s| !output.references.referenced_type_defs.contains(s))); + // captured_symbols.retain(|s| !output.references.referenced_type_defs.contains(s)); // filter out functions that don't close over anything captured_symbols.retain(|s| !output.non_closures.contains(s)); @@ -693,7 +700,7 @@ pub fn canonicalize_expr<'a>( // went unreferenced. If any did, report them as unused arguments. for (sub_symbol, region) in scope.symbols() { if !original_scope.contains_symbol(*sub_symbol) { - if !output.references.has_lookup(*sub_symbol) { + if !output.references.has_value_lookup(*sub_symbol) { // The body never referenced this argument we declared. It's an unused argument! env.problem(Problem::UnusedArgument(symbol, *sub_symbol, *region)); } @@ -701,7 +708,7 @@ pub fn canonicalize_expr<'a>( // We shouldn't ultimately count arguments as referenced locals. Otherwise, // we end up with weird conclusions like the expression (\x -> x + 1) // references the (nonexistent) local variable x! - output.references.lookups.remove(sub_symbol); + output.references.value_lookups.remove(sub_symbol); } } @@ -1082,8 +1089,10 @@ fn canonicalize_when_branch<'a>( for (symbol, region) in scope.symbols() { let symbol = *symbol; - if !output.references.has_lookup(symbol) - && !branch_output.references.has_lookup(symbol) + if !output.references.has_value_lookup(symbol) + && !output.references.has_type_lookup(symbol) + && !branch_output.references.has_value_lookup(symbol) + && !branch_output.references.has_type_lookup(symbol) && !original_scope.contains_symbol(symbol) { env.problem(Problem::UnusedDef(symbol, *region)); @@ -1107,7 +1116,7 @@ pub fn local_successors<'a>( references: &'a References, closures: &'a MutMap, ) -> ImSet { - let mut answer = references.lookups.clone(); + let mut answer = references.value_lookups.clone(); for call_symbol in references.calls.iter() { answer = answer.union(call_successors(*call_symbol, closures)); @@ -1127,7 +1136,7 @@ fn call_successors(call_symbol: Symbol, closures: &MutMap) - } if let Some(references) = closures.get(&symbol) { - answer.extend(references.lookups.iter().copied()); + answer.extend(references.value_lookups.iter().copied()); queue.extend(references.calls.iter().copied()); seen.insert(symbol); @@ -1152,7 +1161,7 @@ where Some((_, refs)) => { visited.insert(defined_symbol); - for local in refs.lookups.iter() { + for local in refs.value_lookups.iter() { if !visited.contains(local) { let other_refs: References = references_from_local(*local, visited, refs_by_def, closures); @@ -1160,7 +1169,7 @@ where answer = answer.union(other_refs); } - answer.lookups.insert(*local); + answer.value_lookups.insert(*local); } for call in refs.calls.iter() { @@ -1194,7 +1203,7 @@ where visited.insert(call_symbol); - for closed_over_local in references.lookups.iter() { + for closed_over_local in references.value_lookups.iter() { if !visited.contains(closed_over_local) { let other_refs = references_from_local(*closed_over_local, visited, refs_by_def, closures); @@ -1202,7 +1211,7 @@ where answer = answer.union(other_refs); } - answer.lookups.insert(*closed_over_local); + answer.value_lookups.insert(*closed_over_local); } for call in references.calls.iter() { @@ -1335,7 +1344,7 @@ fn canonicalize_field<'a>( } } -fn canonicalize_lookup( +fn canonicalize_var_lookup( env: &mut Env<'_>, scope: &mut Scope, module_name: &str, @@ -1350,7 +1359,7 @@ fn canonicalize_lookup( // Look it up in scope! match scope.lookup(&(*ident).into(), region) { Ok(symbol) => { - output.references.lookups.insert(symbol); + output.references.value_lookups.insert(symbol); Var(symbol) } @@ -1365,7 +1374,7 @@ fn canonicalize_lookup( // Look it up in the env! match env.qualified_lookup(module_name, ident, region) { Ok(symbol) => { - output.references.lookups.insert(symbol); + output.references.value_lookups.insert(symbol); Var(symbol) } diff --git a/compiler/can/src/module.rs b/compiler/can/src/module.rs index baf4d9bb6c..416f284eb5 100644 --- a/compiler/can/src/module.rs +++ b/compiler/can/src/module.rs @@ -23,7 +23,7 @@ pub struct Module { pub module_id: ModuleId, pub exposed_imports: MutMap, pub exposed_symbols: MutSet, - pub references: MutSet, + pub referenced_values: MutSet, pub aliases: MutMap, pub rigid_variables: MutMap, } @@ -37,7 +37,8 @@ pub struct ModuleOutput { pub lookups: Vec<(Symbol, Variable, Region)>, pub problems: Vec, pub ident_ids: IdentIds, - pub references: MutSet, + pub referenced_values: MutSet, + pub referenced_types: MutSet, pub scope: Scope, } @@ -238,7 +239,11 @@ pub fn canonicalize_module_defs<'a>( // See if any of the new idents we defined went unused. // If any were unused and also not exposed, report it. for (symbol, region) in symbols_introduced { - if !output.references.has_lookup(symbol) && !exposed_symbols.contains(&symbol) { + if !output.references.has_value_lookup(symbol) + && !output.references.has_type_lookup(symbol) + && !exposed_symbols.contains(&symbol) + { + dbg!(symbol, region); env.problem(Problem::UnusedDef(symbol, region)); } } @@ -251,25 +256,25 @@ pub fn canonicalize_module_defs<'a>( rigid_variables.insert(var, "*".into()); } - let mut references = MutSet::default(); + let mut referenced_values = MutSet::default(); // Gather up all the symbols that were referenced across all the defs' lookups. - for symbol in output.references.lookups.iter() { - references.insert(*symbol); + for symbol in output.references.value_lookups.iter() { + referenced_values.insert(*symbol); } // Gather up all the symbols that were referenced across all the defs' calls. for symbol in output.references.calls.iter() { - references.insert(*symbol); + referenced_values.insert(*symbol); } // Gather up all the symbols that were referenced from other modules. - for symbol in env.qualified_lookups.iter() { - references.insert(*symbol); + for symbol in env.qualified_value_lookups.iter() { + referenced_values.insert(*symbol); } // add any builtins used by other builtins - let transitive_builtins: Vec = references + let transitive_builtins: Vec = referenced_values .iter() .filter(|s| s.is_builtin()) .map(|s| crate::builtins::builtin_dependencies(*s)) @@ -277,7 +282,7 @@ pub fn canonicalize_module_defs<'a>( .copied() .collect(); - references.extend(transitive_builtins); + referenced_values.extend(transitive_builtins); // NOTE previously we inserted builtin defs into the list of defs here // this is now done later, in file.rs. @@ -456,18 +461,18 @@ pub fn canonicalize_module_defs<'a>( } // Incorporate any remaining output.lookups entries into references. - for symbol in output.references.lookups { - references.insert(symbol); + for symbol in output.references.value_lookups { + referenced_values.insert(symbol); } // Incorporate any remaining output.calls entries into references. for symbol in output.references.calls { - references.insert(symbol); + referenced_values.insert(symbol); } // Gather up all the symbols that were referenced from other modules. - for symbol in env.qualified_lookups.iter() { - references.insert(*symbol); + for symbol in env.qualified_value_lookups.iter() { + referenced_values.insert(*symbol); } for declaration in declarations.iter_mut() { @@ -482,7 +487,7 @@ pub fn canonicalize_module_defs<'a>( // TODO this loops over all symbols in the module, we can speed it up by having an // iterator over all builtin symbols - for symbol in references.iter() { + for symbol in referenced_values.iter() { if symbol.is_builtin() { // this can fail when the symbol is for builtin types, or has no implementation yet if let Some(def) = crate::builtins::builtin_defs_map(*symbol, var_store) { @@ -496,7 +501,8 @@ pub fn canonicalize_module_defs<'a>( aliases, rigid_variables, declarations, - references, + referenced_values, + referenced_types: Default::default(), // TODO exposed_imports: can_exposed_imports, problems: env.problems, lookups, diff --git a/compiler/can/src/pattern.rs b/compiler/can/src/pattern.rs index 119f134f98..6e78fa5526 100644 --- a/compiler/can/src/pattern.rs +++ b/compiler/can/src/pattern.rs @@ -254,7 +254,7 @@ pub fn canonicalize_pattern<'a>( freshen_opaque_def(var_store, opaque_def); output.references.referenced_type_defs.insert(opaque); - output.references.lookups.insert(opaque); + output.references.type_lookups.insert(opaque); Pattern::UnwrappedOpaque { whole_var: var_store.fresh(), diff --git a/compiler/can/src/procedure.rs b/compiler/can/src/procedure.rs index 0794b86b3e..6f616206e0 100644 --- a/compiler/can/src/procedure.rs +++ b/compiler/can/src/procedure.rs @@ -45,7 +45,8 @@ impl Procedure { #[derive(Clone, Debug, Default, PartialEq)] pub struct References { pub bound_symbols: ImSet, - pub lookups: ImSet, + pub type_lookups: ImSet, + pub value_lookups: ImSet, /// Aliases or opaque types referenced pub referenced_type_defs: ImSet, pub calls: ImSet, @@ -57,7 +58,8 @@ impl References { } pub fn union(mut self, other: References) -> Self { - self.lookups = self.lookups.union(other.lookups); + self.value_lookups = self.value_lookups.union(other.value_lookups); + self.type_lookups = self.type_lookups.union(other.type_lookups); self.calls = self.calls.union(other.calls); self.bound_symbols = self.bound_symbols.union(other.bound_symbols); self.referenced_type_defs = self.referenced_type_defs.union(other.referenced_type_defs); @@ -66,13 +68,18 @@ impl References { } pub fn union_mut(&mut self, other: References) { - self.lookups.extend(other.lookups); + self.value_lookups.extend(other.value_lookups); + self.type_lookups.extend(other.type_lookups); self.calls.extend(other.calls); self.bound_symbols.extend(other.bound_symbols); self.referenced_type_defs.extend(other.referenced_type_defs); } - pub fn has_lookup(&self, symbol: Symbol) -> bool { - self.lookups.contains(&symbol) + pub fn has_value_lookup(&self, symbol: Symbol) -> bool { + self.value_lookups.contains(&symbol) + } + + pub fn has_type_lookup(&self, symbol: Symbol) -> bool { + self.type_lookups.contains(&symbol) } } diff --git a/compiler/constrain/src/module.rs b/compiler/constrain/src/module.rs index 18b251efeb..641d1f4d2f 100644 --- a/compiler/constrain/src/module.rs +++ b/compiler/constrain/src/module.rs @@ -107,7 +107,6 @@ pub fn constrain_imports( pub struct ConstrainableImports { pub imported_symbols: Vec, pub imported_aliases: MutMap, - pub unused_imports: MutMap, } /// Run this before constraining imports. @@ -118,13 +117,11 @@ pub struct ConstrainableImports { pub fn pre_constrain_imports( home: ModuleId, references: &MutSet, - imported_modules: MutMap, exposed_types: &mut SubsByModule, stdlib: &StdLib, ) -> ConstrainableImports { let mut imported_symbols = Vec::with_capacity(references.len()); let mut imported_aliases = MutMap::default(); - let mut unused_imports = imported_modules; // We'll remove these as we encounter them. // Translate referenced symbols into constraints. We do this on the main // thread because we need exclusive access to the exposed_types map, in order @@ -134,9 +131,6 @@ pub fn pre_constrain_imports( for &symbol in references.iter() { let module_id = symbol.module_id(); - // We used this module, so clearly it is not unused! - unused_imports.remove(&module_id); - if module_id.is_builtin() { // For builtin modules, we create imports from the // hardcoded builtin map. @@ -211,6 +205,5 @@ pub fn pre_constrain_imports( ConstrainableImports { imported_symbols, imported_aliases, - unused_imports, } } diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 918fe56330..e350285624 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -736,7 +736,7 @@ enum BuildTask<'a> { var_store: VarStore, declarations: Vec, dep_idents: MutMap, - unused_imports: MutMap, + unused_imported_modules: MutMap, }, BuildPendingSpecializations { module_timing: ModuleTiming, @@ -3048,14 +3048,19 @@ impl<'a> BuildTask<'a> { let ConstrainableImports { imported_symbols, imported_aliases: _, - unused_imports, - } = pre_constrain_imports( - home, - &module.references, - imported_modules, - exposed_types, - stdlib, - ); + } = pre_constrain_imports(home, &module.referenced_values, exposed_types, stdlib); + + // see if there are imported modules from which nothing is actually used + // this is an odd time to check this, it should be part of canonicalization. + let mut unused_imported_modules = imported_modules; + + for symbol in module.referenced_values.iter() { + unused_imported_modules.remove(&symbol.module_id()); + } + + // for symbol in module.referenced_types { + // unused_imports.remove(symbol.module_id()); + // } // Next, solve this module in the background. Self::Solve { @@ -3068,7 +3073,7 @@ impl<'a> BuildTask<'a> { declarations, dep_idents, module_timing, - unused_imports, + unused_imported_modules, } } } @@ -3266,7 +3271,7 @@ fn canonicalize_and_constrain<'a>( module_id, exposed_imports: module_output.exposed_imports, exposed_symbols, - references: module_output.references, + referenced_values: module_output.referenced_values, aliases: module_output.aliases, rigid_variables: module_output.rigid_variables, }; @@ -3765,7 +3770,7 @@ fn run_task<'a>( ident_ids, declarations, dep_idents, - unused_imports, + unused_imported_modules, } => Ok(run_solve( module, ident_ids, @@ -3776,7 +3781,7 @@ fn run_task<'a>( var_store, declarations, dep_idents, - unused_imports, + unused_imported_modules, )), BuildPendingSpecializations { module_id, diff --git a/reporting/src/report.rs b/reporting/src/report.rs index 406787a9c0..f7dc3c3ba8 100644 --- a/reporting/src/report.rs +++ b/reporting/src/report.rs @@ -534,7 +534,7 @@ impl<'a> RocDocAllocator<'a> { // debug_assert!(region.contains(&sub_region)); // If the outer region takes more than 1 full screen (~60 lines), only show the inner region - if region.end().line - region.start().line > 60 { + if region.end().line.saturating_sub(region.start().line) > 60 { return self.region_with_subregion(sub_region, sub_region); }