fix(experimental elaborator): Only add_generics once for each item

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -84,10 +84,10 @@ impl<'context> Elaborator<'context> {
                     expr_type: inner_expr_type.clone(),
                     expr_span: span,
                 });
+            }
 
-                if i + 1 == statements.len() {
-                    block_type = stmt_type;
-                }
+            if i + 1 == statements.len() {
+                block_type = stmt_type;
             }
         }
 

compiler/noirc_frontend/src/elaborator/traits.rs

@@ -26,10 +26,6 @@ use super::Elaborator;
 
 impl<'context> Elaborator<'context> {
     pub fn collect_traits(&mut self, traits: BTreeMap<TraitId, UnresolvedTrait>) {
-        for (trait_id, unresolved_trait) in &traits {
-            self.interner.push_empty_trait(*trait_id, unresolved_trait);
-        }
-
         for (trait_id, unresolved_trait) in traits {
             let generics = vecmap(&unresolved_trait.trait_def.generics, |_| {
                 TypeVariable::unbound(self.interner.next_type_variable_id())
@@ -187,7 +183,9 @@ impl<'context> Elaborator<'context> {
             return_visibility: Visibility::Private,
         };
 
-        self.elaborate_function(NoirFunction { kind, def }, func_id);
+        let mut function = NoirFunction { kind, def };
+        self.define_function_meta(&mut function, func_id);
+        self.elaborate_function(function, func_id);
         let _ = self.scopes.end_function();
         // Don't check the scope tree for unused variables, they can't be used in a declaration anyway.
         self.trait_bounds.clear();

compiler/noirc_frontend/src/elaborator/types.rs

@@ -1204,6 +1204,10 @@ impl<'context> Elaborator<'context> {
             other => match self.interner.lookup_primitive_method(&other, method_name) {
                 Some(method_id) => Some(HirMethodReference::FuncId(method_id)),
                 None => {
+                    panic!(
+                        "Unresolved method call on other/primitive! {:?}::{}",
+                        object_type, method_name
+                    );
                     self.push_err(TypeCheckError::UnresolvedMethodCall {
                         method_name: method_name.to_string(),
                         object_type: object_type.clone(),

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

@@ -5,6 +5,7 @@ use crate::graph::CrateId;
 use crate::hir::comptime::{Interpreter, InterpreterError};
 use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
 use crate::hir::resolution::errors::ResolverError;
+use crate::{Type, TypeVariable};
 
 use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution};
 use crate::hir::resolution::{
@@ -18,7 +19,9 @@ use crate::hir::type_check::{
 use crate::hir::Context;
 
 use crate::macros_api::{MacroError, MacroProcessor};
-use crate::node_interner::{FuncId, GlobalId, NodeInterner, StructId, TraitId, TypeAliasId};
+use crate::node_interner::{
+    FuncId, GlobalId, NodeInterner, StructId, TraitId, TraitImplId, TypeAliasId,
+};
 
 use crate::ast::{
     ExpressionKind, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait,
@@ -30,6 +33,7 @@ use iter_extended::vecmap;
 use noirc_errors::{CustomDiagnostic, Span};
 use std::collections::{BTreeMap, HashMap};
 
+use std::rc::Rc;
 use std::vec;
 
 #[derive(Default)]
@@ -47,6 +51,9 @@ pub struct UnresolvedFunctions {
     pub file_id: FileId,
     pub functions: Vec<(LocalModuleId, FuncId, NoirFunction)>,
     pub trait_id: Option<TraitId>,
+
+    // The object type this set of functions was declared on, if there is one.
+    pub self_type: Option<Type>,
 }
 
 impl UnresolvedFunctions {
@@ -107,13 +114,18 @@ pub struct UnresolvedTrait {
 pub struct UnresolvedTraitImpl {
     pub file_id: FileId,
     pub module_id: LocalModuleId,
-    pub trait_id: Option<TraitId>,
     pub trait_generics: Vec<UnresolvedType>,
     pub trait_path: Path,
     pub object_type: UnresolvedType,
     pub methods: UnresolvedFunctions,
     pub generics: UnresolvedGenerics,
     pub where_clause: Vec<UnresolvedTraitConstraint>,
+
+    // These fields are filled in later
+    pub trait_id: Option<TraitId>,
+    pub impl_id: Option<TraitImplId>,
+    pub resolved_object_type: Option<Type>,
+    pub resolved_generics: Vec<(Rc<String>, TypeVariable, Span)>,
 }
 
 #[derive(Clone)]
@@ -337,7 +349,7 @@ impl DefCollector {
 
         if use_elaborator {
             let mut more_errors = Elaborator::elaborate(context, crate_id, def_collector.items);
-            more_errors.append(&mut errors);
+            errors.append(&mut more_errors);
             return errors;
         }
 

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

@@ -144,6 +144,7 @@
                 file_id: self.file_id,
                 functions: Vec::new(),
                 trait_id: None,
+                self_type: None,
             };
 
             for (method, _) in r#impl.methods {
@@ -187,8 +188,13 @@
                 object_type: trait_impl.object_type,
                 generics: trait_impl.impl_generics,
                 where_clause: trait_impl.where_clause,
-                trait_id: None, // will be filled later
                 trait_generics: trait_impl.trait_generics,
+
+                // These last fields are filled later on
+                trait_id: None,
+                impl_id: None,
+                resolved_object_type: None,
+                resolved_generics: Vec::new(),
             };
 
             self.def_collector.items.trait_impls.push(unresolved_trait_impl);
@@ -201,8 +207,12 @@
         trait_impl: &NoirTraitImpl,
         krate: CrateId,
     ) -> UnresolvedFunctions {
-        let mut unresolved_functions =
-            UnresolvedFunctions { file_id: self.file_id, functions: Vec::new(), trait_id: None };
+        let mut unresolved_functions = UnresolvedFunctions {
+            file_id: self.file_id,
+            functions: Vec::new(),
+            trait_id: None,
+            self_type: None,
+        };
 
         let module = ModuleId { krate, local_id: self.module_id };
 
@@ -224,8 +234,12 @@
         functions: Vec<NoirFunction>,
         krate: CrateId,
     ) -> Vec<(CompilationError, FileId)> {
-        let mut unresolved_functions =
-            UnresolvedFunctions { file_id: self.file_id, functions: Vec::new(), trait_id: None };
+        let mut unresolved_functions = UnresolvedFunctions {
+            file_id: self.file_id,
+            functions: Vec::new(),
+            trait_id: None,
+            self_type: None,
+        };
         let mut errors = vec![];
 
         let module = ModuleId { krate, local_id: self.module_id };
@@ -398,6 +412,7 @@
                 file_id: self.file_id,
                 functions: Vec::new(),
                 trait_id: None,
+                self_type: None,
             };
 
             let mut method_ids = HashMap::new();
@@ -482,7 +497,7 @@
                         }
                     }
                     TraitItem::Type { name } => {
                         // TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id()
                         if let Err((first_def, second_def)) = self.def_collector.def_map.modules
                             [trait_id.0.local_id.0]
                             .declare_type_alias(name.clone(), TypeAliasId::dummy_id())
@@ -507,6 +522,7 @@
                 method_ids,
                 fns_with_default_impl: unresolved_functions,
             };
+            context.def_interner.push_empty_trait(trait_id, &unresolved);
             self.def_collector.items.traits.insert(trait_id, unresolved);
         }
         errors

compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -1104,6 +1104,9 @@ impl<'a> Resolver<'a> {
             trait_constraints: self.resolve_trait_constraints(&func.def.where_clause),
             is_entry_point: self.is_entry_point_function(func),
             has_inline_attribute,
+
+            // This is only used by the elaborator
+            all_generics: Vec::new(),
         }
     }
 

compiler/noirc_frontend/src/hir_def/function.rs

@@ -109,6 +109,12 @@ pub struct FuncMeta {
     /// such as a trait's `Self` type variable.
     pub direct_generics: Vec<(Rc<String>, TypeVariable)>,
 
+    /// All the generics used by this function, which includes any implicit generics or generics
+    /// from outer scopes, such as those introduced by an impl.
+    /// This is stored when the FuncMeta is first created to later be used to set the current
+    /// generics when the function's body is later resolved.
+    pub all_generics: Vec<(Rc<String>, TypeVariable, Span)>,
+
     pub location: Location,
 
     // This flag is needed for the attribute check pass

compiler/noirc_frontend/src/node_interner.rs

@@ -111,7 +111,9 @@ pub struct NodeInterner {
     // The purpose for this hashmap is to detect duplication of trait implementations ( if any )
     //
     // Indexed by TraitImplIds
-    pub(crate) trait_implementations: Vec<Shared<TraitImpl>>,
+    pub(crate) trait_implementations: HashMap<TraitImplId, Shared<TraitImpl>>,
+
+    next_trait_implementation_id: usize,
 
     /// Trait implementations on each type. This is expected to always have the same length as
     /// `self.trait_implementations`.
@@ -485,7 +487,8 @@ impl Default for NodeInterner {
             struct_attributes: HashMap::new(),
             type_aliases: Vec::new(),
             traits: HashMap::new(),
-            trait_implementations: Vec::new(),
+            trait_implementations: HashMap::new(),
+            next_trait_implementation_id: 0,
             trait_implementation_map: HashMap::new(),
             selected_trait_implementations: HashMap::new(),
             operator_traits: HashMap::new(),
@@ -1143,7 +1146,7 @@ impl NodeInterner {
     }
 
     pub fn get_trait_implementation(&self, id: TraitImplId) -> Shared<TraitImpl> {
-        self.trait_implementations[id.0].clone()
+        self.trait_implementations[&id].clone()
     }
 
     /// Given a `ObjectType: TraitId` pair, try to find an existing impl that satisfies the
@@ -1378,9 +1381,7 @@ impl NodeInterner {
         impl_generics: Generics,
         trait_impl: Shared<TraitImpl>,
     ) -> Result<(), (Span, FileId)> {
-        assert_eq!(impl_id.0, self.trait_implementations.len(), "trait impl defined out of order");
-
-        self.trait_implementations.push(trait_impl.clone());
+        self.trait_implementations.insert(impl_id, trait_impl.clone());
 
         // Replace each generic with a fresh type variable
         let substitutions = impl_generics
@@ -1483,10 +1484,10 @@ impl NodeInterner {
     }
 
     /// Returns what the next trait impl id is expected to be.
-    /// Note that this does not actually reserve the slot so care should
-    /// be taken that the next trait impl added matches this ID.
-    pub fn next_trait_impl_id(&self) -> TraitImplId {
-        TraitImplId(self.trait_implementations.len())
+    pub fn next_trait_impl_id(&mut self) -> TraitImplId {
+        let next_id = self.next_trait_implementation_id;
+        self.next_trait_implementation_id += 1;
+        TraitImplId(next_id)
     }
 
     /// Removes all TraitImplKind::Assumed from the list of known impls for the given trait

compiler/noirc_frontend/src/resolve_locations.rs

@@ -157,11 +157,11 @@ impl NodeInterner {
         self.trait_implementations
             .iter()
             .find(|shared_trait_impl| {
-                let trait_impl = shared_trait_impl.borrow();
+                let trait_impl = shared_trait_impl.1.borrow();
                 trait_impl.file == location.file && trait_impl.ident.span().contains(&location.span)
             })
             .and_then(|shared_trait_impl| {
-                let trait_impl = shared_trait_impl.borrow();
+                let trait_impl = shared_trait_impl.1.borrow();
                 self.traits.get(&trait_impl.trait_id).map(|trait_| trait_.location)
             })
     }

tooling/nargo_cli/src/cli/info_cmd.rs

@@ -102,7 +102,8 @@ pub(crate) fn run(args: InfoCommand, config: NargoConfig) -> Result<(), CliError
     } else {
         // Otherwise print human-readable table.
         if !info_report.programs.is_empty() {
-            let mut program_table = table!([Fm->"Package", Fm->"Function", Fm->"Expression Width", Fm->"ACIR Opcodes"]);
+            let mut program_table =
+                table!([Fm->"Package", Fm->"Function", Fm->"Expression Width", Fm->"ACIR Opcodes"]);
 
             for program_info in info_report.programs {
                 let program_rows: Vec<Row> = program_info.into();