Add fix for #2502

crates/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -268,10 +268,10 @@ fn collect_impls(
 
             if let Some(struct_type) = get_struct_type(&typ) {
                 let struct_type = struct_type.borrow();
-                let type_module = struct_type.id.0.local_id;
+                let type_module = struct_type.id.local_module_id();
 
                 // `impl`s are only allowed on types defined within the current crate
-                if struct_type.id.0.krate != crate_id {
+                if struct_type.id.krate() != crate_id {
                     let span = *span;
                     let type_name = struct_type.name.to_string();
                     let error = DefCollectorErrorKind::ForeignImpl { span, type_name };
@@ -383,14 +383,8 @@ fn resolve_structs(
     crate_id: CrateId,
     errors: &mut Vec<FileDiagnostic>,
 ) {
-    // We must first go through the struct list once to ensure all IDs are pushed to
-    // the def_interner map. This lets structs refer to each other regardless of declaration order
-    // without resolve_struct_fields non-deterministically unwrapping a value
-    // that isn't in the BTreeMap.
-    for (type_id, typ) in &structs {
-        context.def_interner.push_empty_struct(*type_id, typ);
-    }
-
+    // Resolve each field in each struct.
+    // Each struct should already be present in the NodeInterner after def collection.
     for (type_id, typ) in structs {
         let (generics, fields) = resolve_struct_fields(context, crate_id, typ, errors);
         context.def_interner.update_struct(type_id, |struct_def| {

crates/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -4,7 +4,7 @@ use noirc_errors::{FileDiagnostic, Location};
 use crate::{
     graph::CrateId,
     hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
-    node_interner::{StructId, TraitId},
+    node_interner::TraitId,
     parser::SubModule,
     FunctionDefinition, FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct,
     NoirTrait, NoirTypeAlias, ParsedModule, TraitImpl, TraitImplItem, TraitItem, TypeImpl,
@@ -60,7 +60,7 @@ pub fn collect_defs(
 
     collector.collect_traits(ast.traits, crate_id, errors);
 
-    collector.collect_structs(ast.types, crate_id, errors);
+    collector.collect_structs(context, ast.types, crate_id, errors);
 
     collector.collect_type_aliases(context, ast.type_aliases, errors);
 
@@ -377,16 +377,23 @@ impl<'a> ModCollector<'a> {
     /// Returns a vector of errors if any structs were already defined.
     fn collect_structs(
         &mut self,
+        context: &mut Context,
         types: Vec<NoirStruct>,
         krate: CrateId,
         errors: &mut Vec<FileDiagnostic>,
     ) {
         for struct_definition in types {
             let name = struct_definition.name.clone();
 
+            let unresolved = UnresolvedStruct {
+                file_id: self.file_id,
+                module_id: self.module_id,
+                struct_def: struct_definition,
+            };
+
             // Create the corresponding module for the struct namespace
             let id = match self.push_child_module(&name, self.file_id, false, false, errors) {
-                Some(local_id) => StructId(ModuleId { krate, local_id }),
+                Some(local_id) => context.def_interner.new_struct(&unresolved, krate, local_id),
                 None => continue,
             };
 
@@ -404,11 +411,6 @@ impl<'a> ModCollector<'a> {
             }
 
             // And store the TypeId -> StructType mapping somewhere it is reachable
-            let unresolved = UnresolvedStruct {
-                file_id: self.file_id,
-                module_id: self.module_id,
-                struct_def: struct_definition,
-            };
             self.def_collector.collected_types.insert(id, unresolved);
         }
     }

crates/noirc_frontend/src/hir/mod.rs

@@ -98,7 +98,7 @@ impl Context {
     /// For example, if you project contains a `main.nr` and `foo.nr` and you provide the `main_crate_id` and the
     /// `bar_struct_id` where the `Bar` struct is inside `foo.nr`, this function would return `foo::Bar` as a [String].
     pub fn fully_qualified_struct_path(&self, crate_id: &CrateId, id: StructId) -> String {
-        let module_id = id.0;
+        let module_id = id.module_id();
         let child_id = module_id.local_id.0;
         let def_map =
             self.def_map(&module_id.krate).expect("The local crate should be analyzed already");

crates/noirc_frontend/src/hir/resolution/import.rs

@@ -151,7 +151,7 @@ fn resolve_name_in_module(
             ModuleDefId::ModuleId(id) => id,
             ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"),
             // TODO: If impls are ever implemented, types can be used in a path
-            ModuleDefId::TypeId(id) => id.0,
+            ModuleDefId::TypeId(id) => id.module_id(),
             ModuleDefId::TypeAliasId(_) => panic!("type aliases cannot be used in type namespace"),
             ModuleDefId::TraitId(id) => id.0,
             ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"),

crates/noirc_frontend/src/node_interner.rs

@@ -140,7 +140,7 @@ impl FuncId {
 }
 
 #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)]
-pub struct StructId(pub ModuleId);
+pub struct StructId(ModuleId);
 
 impl StructId {
     //dummy id for error reporting
@@ -149,6 +149,18 @@ impl StructId {
     pub fn dummy_id() -> StructId {
         StructId(ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() })
     }
+
+    pub fn module_id(self) -> ModuleId {
+        self.0
+    }
+
+    pub fn krate(self) -> CrateId {
+        self.0.krate
+    }
+
+    pub fn local_module_id(self) -> LocalModuleId {
+        self.0.local_id
+    }
 }
 
 #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)]
@@ -347,24 +359,29 @@ impl NodeInterner {
         );
     }
 
-    pub fn push_empty_struct(&mut self, type_id: StructId, typ: &UnresolvedStruct) {
-        self.structs.insert(
-            type_id,
-            Shared::new(StructType::new(
-                type_id,
-                typ.struct_def.name.clone(),
-                typ.struct_def.span,
-                Vec::new(),
-                vecmap(&typ.struct_def.generics, |_| {
-                    // Temporary type variable ids before the struct is resolved to its actual ids.
-                    // This lets us record how many arguments the type expects so that other types
-                    // can refer to it with generic arguments before the generic parameters themselves
-                    // are resolved.
-                    let id = TypeVariableId(0);
-                    (id, Shared::new(TypeBinding::Unbound(id)))
-                }),
-            )),
-        );
+    pub fn new_struct(
+        &mut self,
+        typ: &UnresolvedStruct,
+        krate: CrateId,
+        local_id: LocalModuleId,
+    ) -> StructId {
+        let struct_id = StructId(ModuleId { krate, local_id });
+        let name = typ.struct_def.name.clone();
+
+        // Fields will be filled in later
+        let no_fields = Vec::new();
+        let generics = vecmap(&typ.struct_def.generics, |_| {
+            // Temporary type variable ids before the struct is resolved to its actual ids.
+            // This lets us record how many arguments the type expects so that other types
+            // can refer to it with generic arguments before the generic parameters themselves
+            // are resolved.
+            let id = TypeVariableId(0);
+            (id, Shared::new(TypeBinding::Unbound(id)))
+        });
+
+        let new_struct = StructType::new(struct_id, name, typ.struct_def.span, no_fields, generics);
+        self.structs.insert(struct_id, Shared::new(new_struct));
+        struct_id
     }
 
     pub fn push_type_alias(&mut self, typ: &UnresolvedTypeAlias) -> TypeAliasId {