step 4: new scope with passing reporting tests

compiler/can/src/annotation.rs

@@ -299,7 +299,7 @@ fn make_apply_symbol(
 /// `U8`, and `Str`.
 pub fn find_type_def_symbols(
     module_id: ModuleId,
-    ident_ids: &mut IdentIds,
+    ident_ids: &mut crate::scope::ScopedIdentIds,
     initial_annotation: &roc_parse::ast::TypeAnnotation,
 ) -> Vec<Symbol> {
     use roc_parse::ast::TypeAnnotation::*;
@@ -312,9 +312,8 @@ pub fn find_type_def_symbols(
         match annotation {
             Apply(_module_name, ident, arguments) => {
                 let ident: Ident = (*ident).into();
-                let ident_id = ident_ids.get_or_insert(&ident);
+                let symbol = ident_ids.scopeless_symbol(&ident, Region::zero());
 
-                let symbol = Symbol::new(module_id, ident_id);
                 result.push(symbol);
 
                 for t in arguments.iter() {

compiler/can/src/def.rs

@@ -279,8 +279,7 @@ pub(crate) fn canonicalize_defs<'a>(
             } => {
                 let referenced_symbols = crate::annotation::find_type_def_symbols(
                     env.home,
-                    // TODO IDENT_IDS
+                    &mut scope.locals,
-                    &mut scope.ident_ids,
                     &ann.value,
                 );
 
@@ -297,8 +296,7 @@ pub(crate) fn canonicalize_defs<'a>(
                     // definition.
                     referenced_symbols.extend(crate::annotation::find_type_def_symbols(
                         env.home,
-                        // TODO IDENT_IDS
+                        &mut scope.locals,
-                        &mut scope.ident_ids,
                         &member.typ.value,
                     ));
                 }

compiler/can/src/env.rs

@@ -77,7 +77,7 @@ impl<'a> Env<'a> {
                 // You can do qualified lookups on your own module, e.g.
                 // if I'm in the Foo module, I can do a `Foo.bar` lookup.
                 if module_id == self.home {
-                    match scope.ident_ids.get_id(&ident) {
+                    match scope.locals.ident_ids.get_id(&ident) {
                         Some(ident_id) => {
                             let symbol = Symbol::new(module_id, ident_id);
 
@@ -96,6 +96,7 @@ impl<'a> Env<'a> {
                                     region,
                                 },
                                 scope
+                                    .locals
                                     .ident_ids
                                     .ident_strs()
                                     .map(|(_, string)| string.into())

compiler/can/src/module.rs

@@ -404,8 +404,12 @@ pub fn canonicalize_module_defs<'a>(
                                 GeneratedInfo::Hosted { effect_symbol, .. } => {
                                     let symbol = def.pattern_vars.iter().next().unwrap().0;
                                     let ident_id = symbol.ident_id();
-                                    let ident =
+                                    let ident = scope
-                                        scope.ident_ids.get_name(ident_id).unwrap().to_string();
+                                        .locals
+                                        .ident_ids
+                                        .get_name(ident_id)
+                                        .unwrap()
+                                        .to_string();
                                     let def_annotation = def.annotation.clone().unwrap();
                                     let annotation = crate::annotation::Annotation {
                                         typ: def_annotation.signature,

compiler/can/src/scope.rs

@@ -12,9 +12,7 @@ use bitvec::vec::BitVec;
 
 #[derive(Clone, Debug)]
 pub struct Scope {
-    idents: IdentStore,
+    pub locals: ScopedIdentIds,
-
-    locals: ScopedIdentIds,
 
     /// The type aliases currently in scope
     pub aliases: VecMap<Symbol, Alias>,
@@ -26,8 +24,6 @@ pub struct Scope {
     /// unqualified idents into Symbols.
     home: ModuleId,
 
-    pub ident_ids: IdentIds,
-
     /// The first `exposed_ident_count` identifiers are exposed
     exposed_ident_count: usize,
 
@@ -84,9 +80,7 @@ impl Scope {
         Scope {
             home,
             exposed_ident_count: initial_ident_ids.len(),
-            locals: ScopedIdentIds::from_ident_ids(home, initial_ident_ids.clone()),
+            locals: ScopedIdentIds::from_ident_ids(home, initial_ident_ids),
-            ident_ids: initial_ident_ids,
-            idents: IdentStore::new(),
             aliases: VecMap::default(),
             // TODO(abilities): default abilities in scope
             abilities_store: AbilitiesStore::default(),
@@ -107,9 +101,7 @@ impl Scope {
         Scope {
             home,
             exposed_ident_count: initial_ident_ids.len(),
-            locals: ScopedIdentIds::from_ident_ids(home, initial_ident_ids.clone()),
+            locals: ScopedIdentIds::from_ident_ids(home, initial_ident_ids),
-            ident_ids: initial_ident_ids,
-            idents: IdentStore::new(),
             aliases: add_aliases(var_store),
             // TODO(abilities): default abilities in scope
             abilities_store: AbilitiesStore::default(),
@@ -120,14 +112,14 @@ impl Scope {
     pub fn lookup(&self, ident: &Ident, region: Region) -> Result<Symbol, RuntimeError> {
         // match self.idents.get_symbol(ident) {
 
-        let a = self.idents.get_symbol(ident);
+        // let a = self.idents.get_symbol(ident);
         let b = self.locals.get_symbol(ident);
 
         // dbg!(ident);
 
         // assert_eq!(a, b, "{:?} != {:?} | {:?}", a, b, ident);
 
-        match a {
+        match b {
             Some(symbol) => Ok(symbol),
             None => {
                 for (import, symbol, _) in self.imports.iter() {
@@ -216,8 +208,16 @@ impl Scope {
         opt_defined_alias: Option<Region>,
     ) -> RuntimeError {
         let opaques_in_scope = self
-            .idents
+            .locals
-            .iter_idents_symbols()
+            .ident_ids
+            .ident_strs()
+            .filter_map(|(ident_id, string)| {
+                if string.is_empty() {
+                    None
+                } else {
+                    Some((string, Symbol::new(self.home, ident_id)))
+                }
+            })
             .filter(|(_, sym)| {
                 self.aliases
                     .get(sym)
@@ -225,7 +225,7 @@ impl Scope {
                     .unwrap_or(AliasKind::Structural)
                     == AliasKind::Opaque
             })
-            .map(|(v, _)| v.as_ref().into())
+            .map(|(v, _)| v.into())
             .collect();
 
         RuntimeError::OpaqueNotDefined {
@@ -252,8 +252,7 @@ impl Scope {
         match self.introduce_without_shadow_symbol(&ident, region) {
             Ok(symbol) => Ok(symbol),
             Err((original_region, shadow)) => {
-                let ident_id = self.ident_ids.add_ident(&ident);
+                let symbol = self.locals.scopeless_symbol(&ident, region);
-                let symbol = Symbol::new(self.home, ident_id);
 
                 Err((original_region, shadow, symbol))
             }
@@ -304,23 +303,22 @@ impl Scope {
         ident: Ident,
         region: Region,
     ) -> Result<(Symbol, Option<Symbol>), (Region, Loc<Ident>, Symbol)> {
-        match self.idents.get_index(&ident) {
+        match self.locals.has_in_scope(&ident) {
-            Some(index) => {
+            Some((ident_id, original_region)) => {
-                let original_symbol = self.idents.symbols[index];
+                let index = ident_id.index();
-                let original_region = self.idents.regions[index];
 
-                let shadow_ident_id = self.ident_ids.add_ident(&ident);
+                let original_symbol = Symbol::new(self.home, ident_id);
-                let shadow_symbol = Symbol::new(self.home, shadow_ident_id);
+                let shadow_symbol = self.locals.scopeless_symbol(&ident, region);
 
                 if self.abilities_store.is_ability_member_name(original_symbol) {
                     self.abilities_store
                         .register_specializing_symbol(shadow_symbol, original_symbol);
 
-                    // Add a symbol for the shadow, but don't re-associate the member name.
+                    //                    // Add a symbol for the shadow, but don't re-associate the member name.
-                    let dummy = Ident::default();
+                    //                    let dummy = Ident::default();
-                    self.idents.insert_unchecked(&dummy, shadow_symbol, region);
+                    //                    self.idents.insert_unchecked(&dummy, shadow_symbol, region);
-
+                    //
-                    let _ = self.locals.scopeless(&ident, region);
+                    //                    let _ = self.locals.scopeless(&ident, region);
 
                     Ok((shadow_symbol, Some(original_symbol)))
                 } else {
@@ -342,27 +340,18 @@ impl Scope {
 
     fn commit_introduction(&mut self, ident: &Ident, region: Region) -> Symbol {
         // if the identifier is exposed, use the IdentId we already have for it
-        match self.ident_ids.get_id(ident) {
+        match self.locals.ident_ids.get_id(ident) {
             Some(ident_id) if ident_id.index() < self.exposed_ident_count => {
                 let symbol = Symbol::new(self.home, ident_id);
 
-                self.idents.insert_unchecked(ident, symbol, region);
-
                 self.locals.in_scope.set(ident_id.index(), true);
                 self.locals.regions[ident_id.index()] = region;
 
                 symbol
             }
             _ => {
-                let ident_id = self.ident_ids.add_ident(ident);
+                let ident_id = self.locals.introduce_into_scope(ident, region);
-
+                Symbol::new(self.home, ident_id)
-                let symbol = Symbol::new(self.home, ident_id);
-
-                self.idents.insert_unchecked(ident, symbol, region);
-
-                self.locals.introduce_into_scope(ident, region);
-
-                symbol
             }
         }
     }
@@ -371,9 +360,7 @@ impl Scope {
     ///
     /// Used for record guards like { x: Just _ }
     pub fn ignore(&mut self, ident: &Ident) -> Symbol {
-        let ident_id = self.ident_ids.add_ident(ident);
+        self.locals.scopeless_symbol(&ident, Region::zero())
-        let _ = self.locals.scopeless(ident, Region::zero());
-        Symbol::new(self.home, ident_id)
     }
 
     /// Import a Symbol from another module into this module's top-level scope.
@@ -424,13 +411,11 @@ impl Scope {
         // - idents: we have to clone for now
         // - aliases: stored in a VecMap, we just discard anything added in an inner scope
         // - exposed_ident_count: unchanged
-        let idents = self.idents.clone();
         let aliases_count = self.aliases.len();
         let locals_snapshot = self.locals.snapshot();
 
         let result = f(self);
 
-        self.idents = idents;
         self.aliases.truncate(aliases_count);
         self.locals.revert(locals_snapshot);
 
@@ -438,7 +423,7 @@ impl Scope {
     }
 
     pub fn register_debug_idents(&self) {
-        self.home.register_debug_idents(&self.ident_ids)
+        self.home.register_debug_idents(&self.locals.ident_ids)
     }
 
     /// Generates a unique, new symbol like "$1" or "$5",
@@ -447,7 +432,7 @@ impl Scope {
     /// This is used, for example, during canonicalization of an Expr::Closure
     /// to generate a unique symbol to refer to that closure.
     pub fn gen_unique_symbol(&mut self) -> Symbol {
-        let ident_id = self.ident_ids.gen_unique();
+        let ident_id = self.locals.gen_unique();
 
         Symbol::new(self.home, ident_id)
     }
@@ -499,74 +484,8 @@ pub fn create_alias(
 }
 
 #[derive(Clone, Debug)]
-struct IdentStore {
+pub struct ScopedIdentIds {
-    interner: SmallStringInterner,
+    pub ident_ids: IdentIds,
-
-    /// A Symbol for each Ident
-    symbols: Vec<Symbol>,
-
-    /// A Region for each Ident
-    regions: Vec<Region>,
-}
-
-impl IdentStore {
-    fn new() -> Self {
-        let capacity = 64;
-
-        Self {
-            interner: SmallStringInterner::with_capacity(capacity),
-            symbols: Vec::with_capacity(capacity),
-            regions: Vec::with_capacity(capacity),
-        }
-    }
-
-    fn iter_idents_symbols(&self) -> impl Iterator<Item = (Ident, Symbol)> + '_ {
-        self.interner
-            .iter()
-            .zip(self.symbols.iter())
-            .filter_map(move |(string, symbol)| {
-                // empty slice is used when ability members are shadowed
-                if string.is_empty() {
-                    None
-                } else {
-                    Some((Ident::from(string), *symbol))
-                }
-            })
-    }
-
-    fn get_index(&self, ident: &Ident) -> Option<usize> {
-        let ident_str = ident.as_inline_str().as_str();
-
-        self.interner.find_index(ident_str)
-    }
-
-    fn get_symbol(&self, ident: &Ident) -> Option<Symbol> {
-        Some(self.symbols[self.get_index(ident)?])
-    }
-
-    fn get_symbol_and_region(&self, ident: &Ident) -> Option<(Symbol, Region)> {
-        let index = self.get_index(ident)?;
-
-        Some((self.symbols[index], self.regions[index]))
-    }
-
-    /// Does not check that the ident is unique
-    fn insert_unchecked(&mut self, ident: &Ident, symbol: Symbol, region: Region) {
-        let ident_str = ident.as_inline_str().as_str();
-
-        let index = self.interner.insert(ident_str);
-
-        debug_assert_eq!(index, self.symbols.len());
-        debug_assert_eq!(index, self.regions.len());
-
-        self.symbols.push(symbol);
-        self.regions.push(region);
-    }
-}
-
-#[derive(Clone, Debug)]
-struct ScopedIdentIds {
-    ident_ids: IdentIds,
     in_scope: BitVec,
     regions: Vec<Region>,
     home: ModuleId,
@@ -584,14 +503,6 @@ impl ScopedIdentIds {
         }
     }
 
-    fn len(&self) -> usize {
-        self.in_scope.count_ones()
-    }
-
-    fn is_empty(&self) -> bool {
-        self.len() == 0
-    }
-
     fn snapshot(&self) -> usize {
         debug_assert_eq!(self.ident_ids.len(), self.in_scope.len());
 
@@ -655,6 +566,10 @@ impl ScopedIdentIds {
         id
     }
 
+    pub fn scopeless_symbol(&mut self, ident_name: &Ident, region: Region) -> Symbol {
+        Symbol::new(self.home, self.scopeless(ident_name, region))
+    }
+
     fn scopeless(&mut self, ident_name: &Ident, region: Region) -> IdentId {
         let id = self.ident_ids.add_ident(ident_name);
 
@@ -666,6 +581,18 @@ impl ScopedIdentIds {
 
         id
     }
+
+    fn gen_unique(&mut self) -> IdentId {
+        let id = self.ident_ids.gen_unique();
+
+        debug_assert_eq!(id.index(), self.in_scope.len());
+        debug_assert_eq!(id.index(), self.regions.len());
+
+        self.in_scope.push(false);
+        self.regions.push(Region::zero());
+
+        id
+    }
 }
 
 #[cfg(test)]

compiler/load_internal/src/docs.rs

@@ -95,7 +95,12 @@ pub fn generate_module_docs<'a>(
         parsed_defs
             .iter()
             .fold((vec![], None), |(acc, maybe_comments_after), def| {
-                generate_entry_doc(&scope.ident_ids, acc, maybe_comments_after, &def.value)
+                generate_entry_doc(
+                    &scope.locals.ident_ids,
+                    acc,
+                    maybe_comments_after,
+                    &def.value,
+                )
             });
 
     ModuleDocumentation {

compiler/load_internal/src/file.rs

@@ -3907,7 +3907,7 @@ fn canonicalize_and_constrain<'a>(
                 var_store,
                 constraints,
                 constraint,
-                ident_ids: module_output.scope.ident_ids,
+                ident_ids: module_output.scope.locals.ident_ids,
                 dep_idents,
                 module_timing,
             };

reporting/tests/helpers/mod.rs

@@ -184,7 +184,7 @@ pub fn can_expr_with<'a>(
         introduce_builtin_imports(&mut constraints, imports, constraint, &mut var_store);
 
     let mut all_ident_ids = IdentIds::exposed_builtins(1);
-    all_ident_ids.insert(home, scope.ident_ids);
+    all_ident_ids.insert(home, scope.locals.ident_ids);
 
     let interns = Interns {
         module_ids: env.module_ids.clone(),