From 725eb217d85a87dd79dcb54216475e492ff0b816 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 21 Mar 2022 23:20:57 +0100 Subject: [PATCH] more vecs, less sets --- compiler/can/src/def.rs | 39 ++++++++++++++++++++++++--------------- compiler/can/src/expr.rs | 9 ++++++--- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 215aec3724..c9a002d4b6 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -3,12 +3,12 @@ use crate::annotation::IntroducedVariables; use crate::env::Env; use crate::expr::ClosureData; 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::procedure::References; use crate::scope::create_alias; 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_module::ident::Lowercase; use roc_module::symbol::Symbol; @@ -450,7 +450,7 @@ pub fn sort_can_defs( // recursive definitions. // All successors that occur in the body of a symbol. - let all_successors_without_self = |symbol: &Symbol| -> ImSet { + let all_successors_without_self = |symbol: &Symbol| -> Vec { // This may not be in refs_by_symbol. For example, the `f` in `f x` here: // // f = \z -> z @@ -471,7 +471,7 @@ pub fn sort_can_defs( // // In the above example, `f` cannot reference `a`, and in the closure // 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 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! // // 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 loc_succ.retain(|key| defined_symbols.contains(key)); + loc_succ.sort(); + loc_succ.dedup(); + 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 // (that are not faulty) always need a DeclareRec, even if there is just one symbol in the // group - let mut all_successors_with_self = |symbol: &Symbol| -> ImSet { + let mut all_successors_with_self = |symbol: &Symbol| -> Vec { // This may not be in refs_by_symbol. For example, the `f` in `f x` here: // // f = \z -> z @@ -521,31 +524,34 @@ pub fn sort_can_defs( // // In the above example, `f` cannot reference `a`, and in the closure // 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 let Some(References { value_lookups, .. }) = env.closures.get(symbol) { for lookup in value_lookups { - loc_succ.insert(*lookup); + loc_succ.push(*lookup); } } // remove anything that is not defined in the current block loc_succ.retain(|key| defined_symbols.contains(key)); + loc_succ.sort(); + loc_succ.dedup(); + loc_succ } - None => ImSet::default(), + None => vec![], } }; // 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, // but does consider direct self-recursion. - let direct_successors = |symbol: &Symbol| -> ImSet { + let direct_successors = |symbol: &Symbol| -> Vec { match refs_by_symbol.get(symbol) { 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 @@ -554,9 +560,12 @@ pub fn sort_can_defs( // NOTE: direct recursion does matter here: `x = x` is invalid recursion! + loc_succ.sort(); + loc_succ.dedup(); + loc_succ } - None => ImSet::default(), + None => vec![], } }; @@ -730,14 +739,14 @@ pub fn sort_can_defs( fn group_to_declaration( group: &[Symbol], closures: &MutMap, - successors: &mut dyn FnMut(&Symbol) -> ImSet, + successors: &mut dyn FnMut(&Symbol) -> Vec, can_defs_by_symbol: &mut MutMap, declarations: &mut Vec, ) { use Declaration::*; // We want only successors in the current group, otherwise definitions get duplicated - let filtered_successors = |symbol: &Symbol| -> ImSet { + let filtered_successors = |symbol: &Symbol| -> Vec { let mut result = successors(symbol); result.retain(|key| group.contains(key)); diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index bbfa1d14df..2d94b8069e 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -1113,10 +1113,10 @@ fn canonicalize_when_branch<'a>( ) } -pub fn local_successors<'a>( +pub fn local_successors_with_duplicates<'a>( references: &'a References, closures: &'a MutMap, -) -> ImSet { +) -> Vec { let mut answer: Vec<_> = references.value_lookups.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 {