use interner in Scope + fix shadowing being reported incorrectly

compiler/can/src/def.rs

@@ -107,7 +107,7 @@ enum PendingTypeDef<'a> {
     },
 
     /// An invalid alias, that is ignored in the rest of the pipeline
-    /// e.g. a shadowed alias, or a definition like `MyAlias 1 : Int`
+    /// e.g. a definition like `MyAlias 1 : Int`
     /// with an incorrect pattern
     InvalidAlias {
         #[allow(dead_code)]
@@ -116,6 +116,9 @@ enum PendingTypeDef<'a> {
         region: Region,
     },
 
+    /// An alias with a name that shadows another symbol
+    ShadowedAlias,
+
     /// An invalid ability, that is ignored in the rest of the pipeline.
     /// E.g. a shadowed ability, or with a bad definition.
     InvalidAbility {
@@ -137,6 +140,7 @@ impl PendingTypeDef<'_> {
             }
             PendingTypeDef::Ability { name, .. } => Some((name.value, name.region)),
             PendingTypeDef::InvalidAlias { symbol, region, .. } => Some((*symbol, *region)),
+            PendingTypeDef::ShadowedAlias { .. } => None,
             PendingTypeDef::InvalidAbility { symbol, region } => Some((*symbol, *region)),
             PendingTypeDef::AbilityNotOnToplevel => None,
             PendingTypeDef::AbilityShadows => None,
@@ -302,6 +306,7 @@ pub(crate) fn canonicalize_defs<'a>(
             PendingTypeDef::InvalidAlias { .. }
             | PendingTypeDef::InvalidAbility { .. }
             | PendingTypeDef::AbilityShadows
+            | PendingTypeDef::ShadowedAlias { .. }
             | PendingTypeDef::AbilityNotOnToplevel => { /* ignore */ }
         }
     }
@@ -1365,7 +1370,7 @@ fn to_pending_type_def<'a>(
 
             let region = Region::span_across(&name.region, &ann.region);
 
-            match scope.introduce(
+            match scope.introduce_without_shadow_symbol(
                 name.value.into(),
                 &env.exposed_ident_ids,
                 &mut env.ident_ids,
@@ -1415,18 +1420,14 @@ fn to_pending_type_def<'a>(
                     }
                 }
 
-                Err((original_region, loc_shadowed_symbol, new_symbol)) => {
+                Err((original_region, loc_shadowed_symbol)) => {
                     env.problem(Problem::Shadowing {
                         original_region,
                         shadow: loc_shadowed_symbol,
                         kind: shadow_kind,
                     });
 
-                    PendingTypeDef::InvalidAlias {
+                    PendingTypeDef::ShadowedAlias
-                        kind,
-                        symbol: new_symbol,
-                        region,
-                    }
                 }
             }
         }

compiler/can/src/pattern.rs

@@ -684,7 +684,6 @@ impl<'a> BindingsFromPattern<'a> {
 
                     match &loc_pattern.value {
                         Identifier(symbol)
-                        | Shadowed(_, _, symbol)
                         | AbilityMemberSpecialization {
                             ident: symbol,
                             specializes: _,
@@ -712,6 +711,7 @@ impl<'a> BindingsFromPattern<'a> {
                         | StrLiteral(_)
                         | SingleQuote(_)
                         | Underscore
+                        | Shadowed(_, _, _)
                         | MalformedPattern(_, _)
                         | UnsupportedPattern(_)
                         | OpaqueNotInScope(..) => (),
@@ -745,7 +745,6 @@ impl<'a> Iterator for BindingsFromPattern<'a> {
             BindingsFromPattern::Empty => None,
             BindingsFromPattern::One(loc_pattern) => match &loc_pattern.value {
                 Identifier(symbol)
-                | Shadowed(_, _, symbol)
                 | AbilityMemberSpecialization {
                     ident: symbol,
                     specializes: _,

compiler/can/src/scope.rs

@@ -1,5 +1,5 @@
 use roc_collections::all::{MutSet, SendMap};
-use roc_collections::soa;
+use roc_collections::SmallStringInterner;
 use roc_module::ident::{Ident, Lowercase};
 use roc_module::symbol::{IdentIds, ModuleId, Symbol};
 use roc_problem::can::RuntimeError;
@@ -11,11 +11,7 @@ use crate::abilities::AbilitiesStore;
 
 #[derive(Clone, Debug)]
 struct IdentStore {
-    /// One big byte array that stores all `Ident`s
+    interner: SmallStringInterner,
-    string: Vec<u8>,
-
-    /// Slices into `string` to get an individual `Ident`
-    idents: Vec<soa::Slice<u8>>,
 
     /// A Symbol for each Ident
     symbols: Vec<Symbol>,
@@ -30,8 +26,7 @@ impl IdentStore {
         let capacity = defaults.len();
 
         let mut this = Self {
-            string: Vec::with_capacity(capacity),
+            interner: SmallStringInterner::with_capacity(capacity),
-            idents: Vec::with_capacity(capacity),
             symbols: Vec::with_capacity(capacity),
             regions: Vec::with_capacity(capacity),
         };
@@ -44,46 +39,34 @@ impl IdentStore {
     }
 
     fn iter_idents(&self) -> impl Iterator<Item = Ident> + '_ {
-        self.idents.iter().filter_map(move |slice| {
+        self.interner.iter().filter_map(move |string| {
-            // empty slice is used when ability members are shadowed
+            // empty string is used when ability members are shadowed
-            if slice.is_empty() {
+            if string.is_empty() {
                 None
             } else {
-                let bytes = &self.string[slice.indices()];
-                let string = unsafe { std::str::from_utf8_unchecked(bytes) };
-
                 Some(Ident::from(string))
             }
         })
     }
 
     fn iter_idents_symbols(&self) -> impl Iterator<Item = (Ident, Symbol)> + '_ {
-        self.idents
+        self.interner
             .iter()
             .zip(self.symbols.iter())
-            .filter_map(move |(slice, symbol)| {
+            .filter_map(move |(string, symbol)| {
                 // empty slice is used when ability members are shadowed
-                if slice.is_empty() {
+                if string.is_empty() {
                     None
                 } else {
-                    let bytes = &self.string[slice.indices()];
-                    let string = unsafe { std::str::from_utf8_unchecked(bytes) };
-
                     Some((Ident::from(string), *symbol))
                 }
             })
     }
 
     fn get_index(&self, ident: &Ident) -> Option<usize> {
-        let ident_bytes = ident.as_inline_str().as_str().as_bytes();
+        let ident_str = ident.as_inline_str().as_str();
 
-        for (i, slice) in self.idents.iter().enumerate() {
+        self.interner.find_index(ident_str)
-            if slice.len() == ident_bytes.len() && &self.string[slice.indices()] == ident_bytes {
-                return Some(i);
-            }
-        }
-
-        None
     }
 
     fn get_symbol(&self, ident: &Ident) -> Option<Symbol> {
@@ -98,11 +81,13 @@ impl IdentStore {
 
     /// Does not check that the ident is unique
     fn insert_unchecked(&mut self, ident: Ident, symbol: Symbol, region: Region) {
-        let ident_bytes = ident.as_inline_str().as_str().as_bytes();
+        let ident_str = ident.as_inline_str().as_str();
 
-        let slice = soa::Slice::extend_new(&mut self.string, ident_bytes.iter().copied());
+        let index = self.interner.insert(ident_str);
+
+        debug_assert_eq!(index, self.symbols.len());
+        debug_assert_eq!(index, self.regions.len());
 
-        self.idents.push(slice);
         self.symbols.push(symbol);
         self.regions.push(region);
     }
@@ -305,9 +290,6 @@ impl Scope {
                 let ident_id = all_ident_ids.add_ident(&ident);
                 let symbol = Symbol::new(self.home, ident_id);
 
-                self.idents.symbols[index] = symbol;
-                self.idents.regions[index] = region;
-
                 Err((original_region, shadow, symbol))
             }
             None => Ok(self.commit_introduction(ident, exposed_ident_ids, all_ident_ids, region)),