try to keep type and value symbols separate

This commit is contained in:
Folkert 2022-03-13 16:50:32 +01:00
parent d6c9e0c625
commit 8bc31e82c3
No known key found for this signature in database
GPG key ID: 1F17F6FFD112B97C
9 changed files with 114 additions and 79 deletions

View file

@ -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));
}
}

View file

@ -27,8 +27,11 @@ pub struct Env<'a> {
/// current closure name (if any)
pub closure_name_symbol: Option<Symbol>,
/// Symbols which were referenced by qualified lookups.
pub qualified_lookups: MutSet<Symbol>,
/// Symbols of values/functions which were referenced by qualified lookups.
pub qualified_value_lookups: MutSet<Symbol>,
/// Symbols of types which were referenced by qualified lookups.
pub qualified_type_lookups: MutSet<Symbol>,
pub top_level_symbols: MutSet<Symbol>,
@ -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)
}

View file

@ -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<Symbol> =
new_output.references.lookups.iter().copied().collect();
let mut captured_symbols: MutSet<Symbol> = 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<Symbol, References>,
) -> ImSet<Symbol> {
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<Symbol, References>) -
}
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)
}

View file

@ -23,7 +23,7 @@ pub struct Module {
pub module_id: ModuleId,
pub exposed_imports: MutMap<Symbol, Variable>,
pub exposed_symbols: MutSet<Symbol>,
pub references: MutSet<Symbol>,
pub referenced_values: MutSet<Symbol>,
pub aliases: MutMap<Symbol, Alias>,
pub rigid_variables: MutMap<Variable, Lowercase>,
}
@ -37,7 +37,8 @@ pub struct ModuleOutput {
pub lookups: Vec<(Symbol, Variable, Region)>,
pub problems: Vec<Problem>,
pub ident_ids: IdentIds,
pub references: MutSet<Symbol>,
pub referenced_values: MutSet<Symbol>,
pub referenced_types: MutSet<Symbol>,
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<Symbol> = references
let transitive_builtins: Vec<Symbol> = 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,

View file

@ -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(),

View file

@ -45,7 +45,8 @@ impl Procedure {
#[derive(Clone, Debug, Default, PartialEq)]
pub struct References {
pub bound_symbols: ImSet<Symbol>,
pub lookups: ImSet<Symbol>,
pub type_lookups: ImSet<Symbol>,
pub value_lookups: ImSet<Symbol>,
/// Aliases or opaque types referenced
pub referenced_type_defs: ImSet<Symbol>,
pub calls: ImSet<Symbol>,
@ -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)
}
}

View file

@ -107,7 +107,6 @@ pub fn constrain_imports(
pub struct ConstrainableImports {
pub imported_symbols: Vec<Import>,
pub imported_aliases: MutMap<Symbol, Alias>,
pub unused_imports: MutMap<ModuleId, Region>,
}
/// Run this before constraining imports.
@ -118,13 +117,11 @@ pub struct ConstrainableImports {
pub fn pre_constrain_imports(
home: ModuleId,
references: &MutSet<Symbol>,
imported_modules: MutMap<ModuleId, Region>,
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,
}
}

View file

@ -736,7 +736,7 @@ enum BuildTask<'a> {
var_store: VarStore,
declarations: Vec<Declaration>,
dep_idents: MutMap<ModuleId, IdentIds>,
unused_imports: MutMap<ModuleId, Region>,
unused_imported_modules: MutMap<ModuleId, Region>,
},
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,

View file

@ -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);
}