fix: Initialize structs during def collection, not name resolution

crates/acvm_backend_barretenberg/src/proof_system.rs

@@ -76,9 +76,8 @@ impl ProofSystemCompiler for Barretenberg {
         let temp_dir_path_str = temp_directory.to_str().unwrap();
 
         // Create a temporary file for the witness
-        let serialized_witnesses: Vec<u8> = witness_values
-            .try_into()
-            .expect("could not serialize witness map");
+        let serialized_witnesses: Vec<u8> =
+            witness_values.try_into().expect("could not serialize witness map");
         let witness_path = temp_directory.join("witness").with_extension("tr");
         write_to_file(&serialized_witnesses, &witness_path);
 
@@ -232,9 +231,8 @@ fn prepend_public_inputs(proof: Vec<u8>, public_inputs: Vec<FieldElement>) -> Ve
         return proof;
     }
 
-    let public_inputs_bytes = public_inputs
-        .into_iter()
-        .flat_map(|assignment| assignment.to_be_bytes());
+    let public_inputs_bytes =
+        public_inputs.into_iter().flat_map(|assignment| assignment.to_be_bytes());
 
     public_inputs_bytes.chain(proof.into_iter()).collect()
 }

crates/nargo_cli/tests/execution_success/type_aliases/src/main.nr

@@ -26,4 +26,11 @@ fn main(x : [Field; 2]) {
         foo: 10
     };
     assert(s.foo == 10);
+
+    let _regression2502: Regression2502Alias = Regression2502 {};
 }
+
+// An ICE was occurring if a type alias referred to a struct before it was initialized
+// during name resolution. The fix was to initialize structs during def collection instead.
+type Regression2502Alias = Regression2502;
+struct Regression2502 {}

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

@@ -264,10 +264,10 @@
 
             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 };
@@ -379,14 +379,8 @@
     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 HashMap.
-    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| {
@@ -497,9 +491,9 @@
     for (trait_id, unresolved_trait) in traits {
         let mut items: Vec<TraitItemType> = vec![];
         // Resolve order
         // 1. Trait Types ( Trait contants can have a trait type, therefore types before constants)
         items.append(&mut resolve_trait_types(context, crate_id, &unresolved_trait, errors));
         // 2. Trait Constants ( Trait's methods can use trait types & constants, threfore they should be after)
         items.append(&mut resolve_trait_constants(context, crate_id, &unresolved_trait, errors));
         // 3. Trait Methods
         items.append(&mut resolve_trait_methods(context, crate_id, &unresolved_trait, errors));

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

@@ -4,7 +4,7 @@
 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 @@
 
     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);
 
@@ -89,7 +89,7 @@
     for (count, (parameter, typ, _abi_vis)) in impl_method.def.parameters.iter().enumerate() {
         let (_expected_name, expected_type) = &expected_parameters[count];
         if typ.typ != expected_type.typ {
             return Err(DefCollectorErrorKind::MismatchTraitImlementationParameter {
                 trait_name: trait_name.to_owned(),
                 expected_type: expected_type.clone(),
                 impl_method: impl_method.name().to_string(),
@@ -377,16 +377,23 @@
     /// 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 @@
             }
 
             // 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)]
-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)]
@@ -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 {