more vecs, less sets

This commit is contained in:
Folkert 2022-03-21 23:20:57 +01:00
parent 14b53c0ccf
commit 725eb217d8
No known key found for this signature in database
GPG key ID: 1F17F6FFD112B97C
2 changed files with 30 additions and 18 deletions

View file

@ -3,12 +3,12 @@ use crate::annotation::IntroducedVariables;
use crate::env::Env; use crate::env::Env;
use crate::expr::ClosureData; use crate::expr::ClosureData;
use crate::expr::Expr::{self, *}; use crate::expr::Expr::{self, *};
use crate::expr::{canonicalize_expr, local_successors, Output, Recursive}; use crate::expr::{canonicalize_expr, local_successors_with_duplicates, Output, Recursive};
use crate::pattern::{bindings_from_patterns, canonicalize_pattern, Pattern}; use crate::pattern::{bindings_from_patterns, canonicalize_pattern, Pattern};
use crate::procedure::References; use crate::procedure::References;
use crate::scope::create_alias; use crate::scope::create_alias;
use crate::scope::Scope; use crate::scope::Scope;
use roc_collections::all::{default_hasher, ImEntry, ImMap, ImSet, MutMap, MutSet, SendMap}; use roc_collections::all::{default_hasher, ImEntry, ImMap, MutMap, MutSet, SendMap};
use roc_error_macros::todo_abilities; use roc_error_macros::todo_abilities;
use roc_module::ident::Lowercase; use roc_module::ident::Lowercase;
use roc_module::symbol::Symbol; use roc_module::symbol::Symbol;
@ -450,7 +450,7 @@ pub fn sort_can_defs(
// recursive definitions. // recursive definitions.
// All successors that occur in the body of a symbol. // All successors that occur in the body of a symbol.
let all_successors_without_self = |symbol: &Symbol| -> ImSet<Symbol> { let all_successors_without_self = |symbol: &Symbol| -> Vec<Symbol> {
// This may not be in refs_by_symbol. For example, the `f` in `f x` here: // This may not be in refs_by_symbol. For example, the `f` in `f x` here:
// //
// f = \z -> z // f = \z -> z
@ -471,7 +471,7 @@ pub fn sort_can_defs(
// //
// In the above example, `f` cannot reference `a`, and in the closure // In the above example, `f` cannot reference `a`, and in the closure
// a call to `f` cannot cycle back to `a`. // a call to `f` cannot cycle back to `a`.
let mut loc_succ = local_successors(references, &env.closures); let mut loc_succ = local_successors_with_duplicates(references, &env.closures);
// if the current symbol is a closure, peek into its body // if the current symbol is a closure, peek into its body
if let Some(References { value_lookups, .. }) = env.closures.get(symbol) { if let Some(References { value_lookups, .. }) = env.closures.get(symbol) {
@ -482,7 +482,7 @@ pub fn sort_can_defs(
// DO NOT register a self-call behind a lambda! // DO NOT register a self-call behind a lambda!
// //
// We allow `boom = \_ -> boom {}`, but not `x = x` // We allow `boom = \_ -> boom {}`, but not `x = x`
loc_succ.insert(*lookup); loc_succ.push(*lookup);
} }
} }
} }
@ -490,9 +490,12 @@ pub fn sort_can_defs(
// remove anything that is not defined in the current block // remove anything that is not defined in the current block
loc_succ.retain(|key| defined_symbols.contains(key)); loc_succ.retain(|key| defined_symbols.contains(key));
loc_succ.sort();
loc_succ.dedup();
loc_succ loc_succ
} }
None => ImSet::default(), None => vec![],
} }
}; };
@ -500,7 +503,7 @@ pub fn sort_can_defs(
// This is required to determine whether a symbol is recursive. Recursive symbols // This is required to determine whether a symbol is recursive. Recursive symbols
// (that are not faulty) always need a DeclareRec, even if there is just one symbol in the // (that are not faulty) always need a DeclareRec, even if there is just one symbol in the
// group // group
let mut all_successors_with_self = |symbol: &Symbol| -> ImSet<Symbol> { let mut all_successors_with_self = |symbol: &Symbol| -> Vec<Symbol> {
// This may not be in refs_by_symbol. For example, the `f` in `f x` here: // This may not be in refs_by_symbol. For example, the `f` in `f x` here:
// //
// f = \z -> z // f = \z -> z
@ -521,31 +524,34 @@ pub fn sort_can_defs(
// //
// In the above example, `f` cannot reference `a`, and in the closure // In the above example, `f` cannot reference `a`, and in the closure
// a call to `f` cannot cycle back to `a`. // a call to `f` cannot cycle back to `a`.
let mut loc_succ = local_successors(references, &env.closures); let mut loc_succ = local_successors_with_duplicates(references, &env.closures);
// if the current symbol is a closure, peek into its body // if the current symbol is a closure, peek into its body
if let Some(References { value_lookups, .. }) = env.closures.get(symbol) { if let Some(References { value_lookups, .. }) = env.closures.get(symbol) {
for lookup in value_lookups { for lookup in value_lookups {
loc_succ.insert(*lookup); loc_succ.push(*lookup);
} }
} }
// remove anything that is not defined in the current block // remove anything that is not defined in the current block
loc_succ.retain(|key| defined_symbols.contains(key)); loc_succ.retain(|key| defined_symbols.contains(key));
loc_succ.sort();
loc_succ.dedup();
loc_succ loc_succ
} }
None => ImSet::default(), None => vec![],
} }
}; };
// If a symbol is a direct successor of itself, there is an invalid cycle. // If a symbol is a direct successor of itself, there is an invalid cycle.
// The difference with the function above is that this one does not look behind lambdas, // The difference with the function above is that this one does not look behind lambdas,
// but does consider direct self-recursion. // but does consider direct self-recursion.
let direct_successors = |symbol: &Symbol| -> ImSet<Symbol> { let direct_successors = |symbol: &Symbol| -> Vec<Symbol> {
match refs_by_symbol.get(symbol) { match refs_by_symbol.get(symbol) {
Some((_, references)) => { Some((_, references)) => {
let mut loc_succ = local_successors(references, &env.closures); let mut loc_succ = local_successors_with_duplicates(references, &env.closures);
// NOTE: if the symbol is a closure we DONT look into its body // NOTE: if the symbol is a closure we DONT look into its body
@ -554,9 +560,12 @@ pub fn sort_can_defs(
// NOTE: direct recursion does matter here: `x = x` is invalid recursion! // NOTE: direct recursion does matter here: `x = x` is invalid recursion!
loc_succ.sort();
loc_succ.dedup();
loc_succ loc_succ
} }
None => ImSet::default(), None => vec![],
} }
}; };
@ -730,14 +739,14 @@ pub fn sort_can_defs(
fn group_to_declaration( fn group_to_declaration(
group: &[Symbol], group: &[Symbol],
closures: &MutMap<Symbol, References>, closures: &MutMap<Symbol, References>,
successors: &mut dyn FnMut(&Symbol) -> ImSet<Symbol>, successors: &mut dyn FnMut(&Symbol) -> Vec<Symbol>,
can_defs_by_symbol: &mut MutMap<Symbol, Def>, can_defs_by_symbol: &mut MutMap<Symbol, Def>,
declarations: &mut Vec<Declaration>, declarations: &mut Vec<Declaration>,
) { ) {
use Declaration::*; use Declaration::*;
// We want only successors in the current group, otherwise definitions get duplicated // We want only successors in the current group, otherwise definitions get duplicated
let filtered_successors = |symbol: &Symbol| -> ImSet<Symbol> { let filtered_successors = |symbol: &Symbol| -> Vec<Symbol> {
let mut result = successors(symbol); let mut result = successors(symbol);
result.retain(|key| group.contains(key)); result.retain(|key| group.contains(key));

View file

@ -1113,10 +1113,10 @@ fn canonicalize_when_branch<'a>(
) )
} }
pub fn local_successors<'a>( pub fn local_successors_with_duplicates<'a>(
references: &'a References, references: &'a References,
closures: &'a MutMap<Symbol, References>, closures: &'a MutMap<Symbol, References>,
) -> ImSet<Symbol> { ) -> Vec<Symbol> {
let mut answer: Vec<_> = references.value_lookups.iter().copied().collect(); let mut answer: Vec<_> = references.value_lookups.iter().copied().collect();
let mut stack: Vec<_> = references.calls.iter().copied().collect(); let mut stack: Vec<_> = references.calls.iter().copied().collect();
@ -1135,7 +1135,10 @@ pub fn local_successors<'a>(
} }
} }
answer.into_iter().collect() answer.sort();
answer.dedup();
answer
} }
enum CanonicalizeRecordProblem { enum CanonicalizeRecordProblem {