Remove the reverse index, only look up by global ID

compiler/noirc_frontend/src/elaborator/patterns.rs

@@ -331,8 +331,8 @@ impl<'context> Elaborator<'context> {
         warn_if_unused: bool,
         definition: DefinitionKind,
     ) -> HirIdent {
-        if definition.is_global() {
-            return self.add_global_variable_decl(name, definition);
+        if let DefinitionKind::Global(global_id) = definition {
+            return self.add_global_variable_decl(name, global_id);
         }
 
         let location = Location::new(name.span(), self.file);
@@ -377,44 +377,19 @@ impl<'context> Elaborator<'context> {
         }
     }
 
-    pub fn add_global_variable_decl(
-        &mut self,
-        name: Ident,
-        definition: DefinitionKind,
-    ) -> HirIdent {
-        let comptime = self.in_comptime_context();
+    pub fn add_global_variable_decl(&mut self, name: Ident, global_id: GlobalId) -> HirIdent {
         let scope = self.scopes.get_mut_scope();
-
-        // This check is necessary to maintain the same definition ids in the interner. Currently, each function uses a new resolver that has its own ScopeForest and thus global scope.
-        // We must first check whether an existing definition ID has been inserted as otherwise there will be multiple definitions for the same global statement.
-        // This leads to an error in evaluation where the wrong definition ID is selected when evaluating a statement using the global. The check below prevents this error.
-        let global_id =
-            self.interner.find_global_by_local_and_ident(self.local_module, name.clone());
-
-        let (ident, resolver_meta) = if let Some(id) = global_id {
-            // Sanity check that we're referring to the same global.
-            assert_eq!(definition, DefinitionKind::Global(id));
-            let global = self.interner.get_global(id);
-            let hir_ident = HirIdent::non_trait_method(global.definition_id, global.location);
-            let ident = hir_ident.clone();
-            let resolver_meta = ResolverMeta { num_times_used: 0, ident, warn_if_unused: true };
-            (hir_ident, resolver_meta)
-        } else {
-            let location = Location::new(name.span(), self.file);
-            let name = name.0.contents.clone();
-            let id = self.interner.push_definition(name, false, comptime, definition, location);
-            let ident = HirIdent::non_trait_method(id, location);
-            let resolver_meta =
-                ResolverMeta { num_times_used: 0, ident: ident.clone(), warn_if_unused: true };
-            (ident, resolver_meta)
-        };
+        let global = self.interner.get_global(global_id);
+        let ident = HirIdent::non_trait_method(global.definition_id, global.location);
+        let resolver_meta =
+            ResolverMeta { num_times_used: 0, ident: ident.clone(), warn_if_unused: true };
 
         let old_global_value = scope.add_key_value(name.0.contents.clone(), resolver_meta);
         if let Some(old_global_value) = old_global_value {
             self.push_err(ResolverError::DuplicateDefinition {
-                name: name.0.contents.clone(),
-                first_span: old_global_value.ident.location.span,
                 second_span: name.span(),
+                name: name.0.contents,
+                first_span: old_global_value.ident.location.span,
             });
         }
         ident

compiler/noirc_frontend/src/node_interner.rs

@@ -181,7 +181,6 @@ pub struct NodeInterner {
     // NOTE: currently only used for checking repeat globals and restricting their scope to a module
     globals: Vec<GlobalInfo>,
     global_attributes: HashMap<GlobalId, Vec<SecondaryAttribute>>,
-    globals_by_local_and_ident: HashMap<(LocalModuleId, Ident), GlobalId>,
 
     next_type_variable_id: std::cell::Cell<usize>,
 
@@ -661,7 +660,6 @@ impl Default for NodeInterner {
             next_type_variable_id: std::cell::Cell::new(0),
             globals: Vec::new(),
             global_attributes: HashMap::default(),
-            globals_by_local_and_ident: HashMap::default(),
             struct_methods: HashMap::default(),
             primitive_methods: HashMap::default(),
             type_alias_ref: Vec::new(),
@@ -854,10 +852,6 @@ impl NodeInterner {
         let definition_id =
             self.push_definition(name, mutable, comptime, DefinitionKind::Global(id), location);
 
-        // To guarantee equivalence with the former method of looping through all globals
-        // and returning the first match, do not overwrite an existing entry.
-        self.globals_by_local_and_ident.entry((local_id, ident.clone())).or_insert(id);
-
         self.globals.push(GlobalInfo {
             id,
             definition_id,
@@ -1272,14 +1266,6 @@ impl NodeInterner {
         &self.globals
     }
 
-    pub fn find_global_by_local_and_ident(
-        &self,
-        local_id: LocalModuleId,
-        ident: Ident,
-    ) -> Option<GlobalId> {
-        self.globals_by_local_and_ident.get(&(local_id, ident)).cloned()
-    }
-
     /// Returns the type of an item stored in the Interner or Error if it was not found.
     pub fn id_type(&self, index: impl Into<Index>) -> Type {
         self.id_to_type.get(&index.into()).cloned().unwrap_or(Type::Error)