From 78cff10dce4ded304a49de818e7bf00d5de2f66f Mon Sep 17 00:00:00 2001
From: Nikolay Nikolov <nickysn@gmail.com>
Date: Wed, 20 Sep 2023 09:57:00 +0300
Subject: [PATCH 01/17] feat(traits): multi module support for traits

This adds initial module support for traits. It's now allowed for
traits, impl blocks & the type definitions to reside in different
modules or crates.

Co-authored-by: Alex Vitkov <alexvitkov98@gmail.com>
---
 compiler/noirc_frontend/src/ast/traits.rs     |  10 +-
 .../src/hir/def_collector/dc_crate.rs         | 286 +++++++++++++++---
 .../src/hir/def_collector/dc_mod.rs           | 227 ++++----------
 .../src/hir/def_collector/errors.rs           |  31 +-
 .../src/hir/def_map/item_scope.rs             |  21 +-
 .../src/hir/def_map/module_data.rs            |   6 +-
 .../src/hir/def_map/module_def.rs             |   6 +
 .../src/hir/resolution/resolver.rs            |   5 +
 compiler/noirc_frontend/src/hir_def/traits.rs |  10 +-
 compiler/noirc_frontend/src/node_interner.rs  |   1 +
 compiler/noirc_frontend/src/parser/parser.rs  |   6 +-
 .../dup_trait_implementation_4/Nargo.toml     |   7 +
 .../dup_trait_implementation_4/Prover.toml    |   0
 .../dup_trait_implementation_4/src/main.nr    |   6 +
 .../dup_trait_implementation_4/src/module1.nr |   2 +
 .../dup_trait_implementation_4/src/module2.nr |   2 +
 .../dup_trait_implementation_4/src/module3.nr |   7 +
 .../dup_trait_implementation_5/Nargo.toml     |   7 +
 .../dup_trait_implementation_5/Prover.toml    |   0
 .../dup_trait_implementation_5/src/main.nr    |   7 +
 .../dup_trait_implementation_5/src/module1.nr |   2 +
 .../dup_trait_implementation_5/src/module2.nr |   2 +
 .../dup_trait_implementation_5/src/module3.nr |   4 +
 .../dup_trait_implementation_5/src/module4.nr |   3 +
 .../orphaned_trait_impl/Nargo.toml            |   9 +
 .../orphaned_trait_impl/Prover.toml           |   2 +
 .../orphaned_trait_impl/crate1/Nargo.toml     |   7 +
 .../orphaned_trait_impl/crate1/src/lib.nr     |   2 +
 .../orphaned_trait_impl/crate2/Nargo.toml     |   7 +
 .../orphaned_trait_impl/crate2/src/lib.nr     |   2 +
 .../orphaned_trait_impl/src/main.nr           |   6 +
 .../trait_multi_module_test/Nargo.toml        |   7 +
 .../trait_multi_module_test/Prover.toml       |   0
 .../trait_multi_module_test/src/main.nr       |   9 +
 .../trait_multi_module_test/src/module1.nr    |   2 +
 .../trait_multi_module_test/src/module2.nr    |   2 +
 .../trait_multi_module_test/src/module3.nr    |   5 +
 .../trait_multi_module_test/src/module4.nr    |   2 +
 .../trait_multi_module_test/src/module5.nr    |   2 +
 .../trait_multi_module_test/src/module6.nr    |   2 +
 .../trait_where_clause/src/main.nr            |   8 +-
 .../trait_where_clause/src/the_trait.nr       |   3 +
 .../traits_in_crates_1/Nargo.toml             |   9 +
 .../traits_in_crates_1/Prover.toml            |   2 +
 .../traits_in_crates_1/crate1/Nargo.toml      |   8 +
 .../traits_in_crates_1/crate1/src/lib.nr      |   9 +
 .../traits_in_crates_1/crate2/Nargo.toml      |   7 +
 .../traits_in_crates_1/crate2/src/lib.nr      |   3 +
 .../traits_in_crates_1/src/main.nr            |   5 +
 .../traits_in_crates_2/Nargo.toml             |   9 +
 .../traits_in_crates_2/Prover.toml            |   2 +
 .../traits_in_crates_2/crate1/Nargo.toml      |   7 +
 .../traits_in_crates_2/crate1/src/lib.nr      |   3 +
 .../traits_in_crates_2/crate2/Nargo.toml      |   8 +
 .../traits_in_crates_2/crate2/src/lib.nr      |   9 +
 .../traits_in_crates_2/src/main.nr            |   5 +
 56 files changed, 564 insertions(+), 257 deletions(-)
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/Prover.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/main.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module1.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module2.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module3.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/Prover.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/main.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module1.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module2.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module3.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module4.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Prover.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/src/lib.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/src/lib.nr
 create mode 100644 tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/src/main.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/Prover.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/main.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module1.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module2.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module3.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module4.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module5.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module6.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/trait_where_clause/src/the_trait.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Prover.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/src/lib.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/src/lib.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_1/src/main.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Prover.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/src/lib.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/Nargo.toml
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/src/lib.nr
 create mode 100644 tooling/nargo_cli/tests/execution_success/traits_in_crates_2/src/main.nr

diff --git a/compiler/noirc_frontend/src/ast/traits.rs b/compiler/noirc_frontend/src/ast/traits.rs
index 4078ea72cbe..915134e725d 100644
--- a/compiler/noirc_frontend/src/ast/traits.rs
+++ b/compiler/noirc_frontend/src/ast/traits.rs
@@ -5,7 +5,7 @@ use noirc_errors::Span;
 
 use crate::{
     node_interner::TraitId, BlockExpression, Expression, FunctionReturnType, Ident, NoirFunction,
-    UnresolvedGenerics, UnresolvedType,
+    Path, UnresolvedGenerics, UnresolvedType,
 };
 
 /// AST node for trait definitions:
@@ -57,7 +57,7 @@ pub struct TypeImpl {
 pub struct NoirTraitImpl {
     pub impl_generics: UnresolvedGenerics,
 
-    pub trait_name: Ident,
+    pub trait_name: Path,
     pub trait_generics: Vec<UnresolvedType>,
 
     pub object_type: UnresolvedType,
@@ -83,7 +83,7 @@ pub struct UnresolvedTraitConstraint {
 /// Represents a single trait bound, such as `TraitX` or `TraitY<U, V>`
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TraitBound {
-    pub trait_name: Ident,
+    pub trait_path: Path,
     pub trait_id: Option<TraitId>, // initially None, gets assigned during DC
     pub trait_generics: Vec<UnresolvedType>,
 }
@@ -178,9 +178,9 @@ impl Display for TraitBound {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let generics = vecmap(&self.trait_generics, |generic| generic.to_string());
         if !generics.is_empty() {
-            write!(f, "{}<{}>", self.trait_name, generics.join(", "))
+            write!(f, "{}<{}>", self.trait_path, generics.join(", "))
         } else {
-            write!(f, "{}", self.trait_name)
+            write!(f, "{}", self.trait_path)
         }
     }
 }
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 0e4c1710ce8..72a41180093 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -1,9 +1,10 @@
 use super::dc_mod::collect_defs;
 use super::errors::{DefCollectorErrorKind, DuplicateType};
 use crate::graph::CrateId;
-use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
+use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId};
 use crate::hir::resolution::errors::ResolverError;
 use crate::hir::resolution::import::PathResolutionError;
+use crate::hir::resolution::path_resolver::PathResolver;
 use crate::hir::resolution::resolver::Resolver;
 use crate::hir::resolution::{
     import::{resolve_imports, ImportDirective},
@@ -13,18 +14,19 @@ use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
 use crate::hir::Context;
 use crate::hir_def::traits::{TraitConstant, TraitFunction, TraitImpl, TraitType};
 use crate::node_interner::{
-    FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
+    FuncId, FunctionModifiers, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
 };
+use crate::token::Attributes;
 use crate::{
     ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait,
-    NoirTypeAlias, ParsedModule, Shared, StructType, TraitItem, Type, TypeBinding,
+    NoirTypeAlias, ParsedModule, Path, Shared, StructType, TraitItem, Type, TypeBinding,
     TypeVariableKind, UnresolvedGenerics, UnresolvedType,
 };
 use fm::FileId;
 use iter_extended::vecmap;
 use noirc_errors::Span;
 use noirc_errors::{CustomDiagnostic, FileDiagnostic};
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, HashMap, HashSet};
 use std::rc::Rc;
 use std::vec;
 
@@ -39,6 +41,32 @@ impl UnresolvedFunctions {
     pub fn push_fn(&mut self, mod_id: LocalModuleId, func_id: FuncId, func: NoirFunction) {
         self.functions.push((mod_id, func_id, func));
     }
+
+    pub fn resolve_trait_bounds_trait_ids(
+        &mut self,
+        def_maps: &BTreeMap<CrateId, CrateDefMap>,
+        crate_id: CrateId,
+    ) -> Vec<DefCollectorErrorKind> {
+        let mut errors = Vec::new();
+
+        for (local_id, _, func) in &mut self.functions {
+            let module = ModuleId { krate: crate_id, local_id: *local_id };
+
+            for bound in &mut func.def.where_clause {
+                match resolve_trait_by_path(def_maps, module, bound.trait_bound.trait_path.clone())
+                {
+                    Ok(trait_id) => {
+                        bound.trait_bound.trait_id = Some(trait_id);
+                    }
+                    Err(err) => {
+                        errors.push(err);
+                    }
+                }
+            }
+        }
+
+        errors
+    }
 }
 
 pub struct UnresolvedStruct {
@@ -51,15 +79,20 @@ pub struct UnresolvedStruct {
 pub struct UnresolvedTrait {
     pub file_id: FileId,
     pub module_id: LocalModuleId,
+    pub crate_id: CrateId,
     pub trait_def: NoirTrait,
+    pub fns_with_default_impl: UnresolvedFunctions,
 }
 
 pub struct UnresolvedTraitImpl {
     pub file_id: FileId,
     pub module_id: LocalModuleId,
-    pub the_trait: UnresolvedTrait,
+    pub the_trait: Option<TraitId>,
+    pub trait_path: Path,
     pub methods: UnresolvedFunctions,
-    pub trait_impl_ident: Ident, // for error reporting
+    pub methods_using_default_impl_in_trait: Option<UnresolvedFunctions>,
+    pub object_type: UnresolvedType,
+    pub object_type_span: Span,
 }
 
 #[derive(Clone)]
@@ -87,7 +120,7 @@ pub struct DefCollector {
     pub(crate) collected_traits: BTreeMap<TraitId, UnresolvedTrait>,
     pub(crate) collected_globals: Vec<UnresolvedGlobal>,
     pub(crate) collected_impls: ImplMap,
-    pub(crate) collected_traits_impls: TraitImplMap,
+    pub(crate) collected_traits_impls: Vec<UnresolvedTraitImpl>,
 }
 
 /// Maps the type and the module id in which the impl is defined to the functions contained in that
@@ -100,8 +133,6 @@ pub struct DefCollector {
 type ImplMap =
     HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>;
 
-type TraitImplMap = HashMap<(UnresolvedType, LocalModuleId, TraitId), UnresolvedTraitImpl>;
-
 impl DefCollector {
     fn new(def_map: CrateDefMap) -> DefCollector {
         DefCollector {
@@ -113,7 +144,7 @@ impl DefCollector {
             collected_traits: BTreeMap::new(),
             collected_impls: HashMap::new(),
             collected_globals: vec![],
-            collected_traits_impls: HashMap::new(),
+            collected_traits_impls: vec![],
         }
     }
 
@@ -220,7 +251,9 @@ impl DefCollector {
         // impl since that determines the module we should collect into.
         collect_impls(context, crate_id, &def_collector.collected_impls, errors);
 
-        collect_trait_impls(context, crate_id, &def_collector.collected_traits_impls, errors);
+        // Bind trait impls to their trait. Collect trait functions, that have a
+        // default implementation, which hasn't been overriden.
+        collect_trait_impls(context, crate_id, &mut def_collector.collected_traits_impls, errors);
 
         // Lower each function in the crate. This is now possible since imports have been resolved
         let file_func_ids = resolve_free_functions(
@@ -319,22 +352,141 @@ fn collect_impls(
 fn collect_trait_impls(
     context: &mut Context,
     crate_id: CrateId,
-    collected_impls: &TraitImplMap,
+    collected_impls: &mut [UnresolvedTraitImpl],
     errors: &mut Vec<FileDiagnostic>,
 ) {
     let interner = &mut context.def_interner;
     let def_maps = &mut context.def_maps;
 
-    // TODO: To follow the semantics of Rust, we must allow the impl if either
-    //     1. The type is a struct and it's defined in the current crate
-    //     2. The trait is defined in the current crate
-    for ((unresolved_type, module_id, _), trait_impl) in collected_impls {
-        let path_resolver =
-            StandardPathResolver::new(ModuleId { local_id: *module_id, krate: crate_id });
+    for trait_impl in collected_impls.iter_mut() {
+        let module_id = trait_impl.module_id;
+        let unresolved_type = trait_impl.object_type.clone();
+
+        let module = ModuleId { local_id: module_id, krate: crate_id };
+        let resolve_result = resolve_trait_by_path(def_maps, module, trait_impl.trait_path.clone());
+
+        let maybe_trait_id = match resolve_result {
+            Ok(trait_id) => Some(trait_id),
+            Err(error) => {
+                errors.push(error.into_file_diagnostic(trait_impl.file_id));
+                None
+            }
+        };
+        trait_impl.the_trait = maybe_trait_id;
+
+        if let Some(trait_id) = maybe_trait_id {
+            let the_trait = interner.get_trait(trait_id);
+            let the_trait = the_trait.borrow();
+
+            // check whether the trait implementation is in the same crate as either the trait or the type
+            {
+                let file = def_maps[&crate_id].file_id(module_id);
+                let path_resolver = StandardPathResolver::new(module);
+                let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
+                let typ = resolver.resolve_type(unresolved_type.clone());
+                let crate_of_object = if let Some(struct_type) = get_struct_type(&typ) {
+                    let struct_type = struct_type.borrow();
+                    struct_type.id.krate()
+                } else {
+                    CrateId::Dummy
+                };
+
+                if crate_id != the_trait.crate_id && crate_id != crate_of_object {
+                    let error = DefCollectorErrorKind::TraitImplOrphaned {
+                        span: trait_impl.object_type_span,
+                    };
+                    errors.push(error.into_file_diagnostic(trait_impl.file_id));
+                }
+            }
+
+            // set of function ids that have a corresponding method in the trait
+            let mut func_ids_in_trait = HashSet::new();
+
+            let mut unresolved_functions =
+                UnresolvedFunctions { file_id: FileId::dummy(), functions: Vec::new() };
+
+            for method in &the_trait.methods {
+                let overrides: Vec<_> = trait_impl
+                    .methods
+                    .functions
+                    .iter()
+                    .filter(|(_, _, f)| f.name() == method.name.0.contents)
+                    .collect();
+                if overrides.is_empty() {
+                    unresolved_functions.file_id = method.default_impl_file_id;
+                    if let Some(default_impl) = &method.default_impl {
+                        let method_name = default_impl.name();
+                        let func_id = interner.push_empty_fn();
+
+                        let modifiers = FunctionModifiers {
+                            name: method_name.to_owned(),
+                            visibility: crate::Visibility::Public,
+                            attributes: Attributes::empty(),
+                            is_unconstrained: false,
+                            contract_function_type: None,
+                            is_internal: None,
+                        };
+                        interner.push_function_definition(
+                            method_name.to_string(),
+                            func_id,
+                            modifiers,
+                            module,
+                        );
+                        func_ids_in_trait.insert(func_id);
+                        unresolved_functions.push_fn(
+                            method.default_impl_module_id,
+                            func_id,
+                            *default_impl.clone(),
+                        );
+                    } else {
+                        let error = DefCollectorErrorKind::TraitMissingMethod {
+                            trait_name: the_trait.name.clone(),
+                            method_name: method.name.clone(),
+                            trait_impl_span: trait_impl.object_type_span,
+                        };
+                        errors.push(error.into_file_diagnostic(trait_impl.file_id));
+                    }
+                } else {
+                    for (_, func_id, _) in &overrides {
+                        func_ids_in_trait.insert(*func_id);
+                    }
+                }
+            }
+
+            for (_, func_id, noir_function) in &unresolved_functions.functions {
+                let name = noir_function.name().to_owned();
+
+                let modifiers = FunctionModifiers {
+                    name: name.clone(),
+                    visibility: crate::Visibility::Public,
+                    attributes: Attributes::empty(),
+                    is_unconstrained: false,
+                    contract_function_type: None,
+                    is_internal: None,
+                };
+                interner.push_function_definition(name, *func_id, modifiers, module);
+            }
+
+            // Emit MethodNotInTrait error for methods in the impl block that
+            // don't have a corresponding method signature defined in the trait
+            for (_, func_id, func) in &trait_impl.methods.functions {
+                if !func_ids_in_trait.contains(func_id) {
+                    let error = DefCollectorErrorKind::MethodNotInTrait {
+                        trait_name: the_trait.name.clone(),
+                        impl_method: func.name_ident().clone(),
+                    };
+                    errors.push(error.into_file_diagnostic(trait_impl.file_id));
+                }
+            }
+
+            //trait_impl.methods_using_default_impl_in_trait = Some(unresolved_functions);
+            trait_impl.methods.functions.append(&mut unresolved_functions.functions);
+        }
 
         for (_, func_id, ast) in &trait_impl.methods.functions {
-            let file = def_maps[&crate_id].file_id(*module_id);
+            let file = def_maps[&crate_id].file_id(module_id);
 
+            let path_resolver = StandardPathResolver::new(module);
             let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
             resolver.add_generics(&ast.def.generics);
             let typ = resolver.resolve_type(unresolved_type.clone());
@@ -359,15 +511,30 @@ fn collect_trait_impls(
                     errors.push(err.into_file_diagnostic(trait_impl.file_id));
                 }
             } else {
-                let span = trait_impl.trait_impl_ident.span();
-                let trait_ident = trait_impl.the_trait.trait_def.name.clone();
-                let error = DefCollectorErrorKind::NonStructTraitImpl { trait_ident, span };
+                let error = DefCollectorErrorKind::NonStructTraitImpl {
+                    trait_path: trait_impl.trait_path.clone(),
+                    span: trait_impl.trait_path.span(),
+                };
                 errors.push(error.into_file_diagnostic(trait_impl.file_id));
             }
         }
     }
 }
 
+fn resolve_trait_by_path(
+    def_maps: &BTreeMap<CrateId, CrateDefMap>,
+    module: ModuleId,
+    path: Path,
+) -> Result<TraitId, DefCollectorErrorKind> {
+    let path_resolver = StandardPathResolver::new(module);
+
+    match path_resolver.resolve(def_maps, path.clone()) {
+        Ok(ModuleDefId::TraitId(trait_id)) => Ok(trait_id),
+        Ok(_) => Err(DefCollectorErrorKind::NotATrait { not_a_trait_name: path }),
+        Err(_) => Err(DefCollectorErrorKind::TraitNotFound { trait_path: path }),
+    }
+}
+
 fn get_struct_type(typ: &Type) -> Option<&Shared<StructType>> {
     match typ {
         Type::Struct(definition, _) => Some(definition),
@@ -519,12 +686,30 @@ fn resolve_trait_methods(
             // TODO
             let generics: Generics = vec![];
             let span: Span = name.span();
+            let default_impl_list: Vec<_> = unresolved_trait
+                .fns_with_default_impl
+                .functions
+                .iter()
+                .filter(|(_, _, q)| q.name() == name.0.contents)
+                .collect();
+            let default_impl = if !default_impl_list.is_empty() {
+                if default_impl_list.len() > 1 {
+                    panic!("Too many functions with the same name!");
+                }
+                Some(Box::new(default_impl_list[0].2.clone()))
+            } else {
+                None
+            };
+
             let f = TraitFunction {
                 name,
                 generics,
                 arguments,
                 return_type: resolved_return_type,
                 span,
+                default_impl,
+                default_impl_file_id: unresolved_trait.file_id,
+                default_impl_module_id: unresolved_trait.module_id,
             };
             res.push(f);
             let new_errors = take_errors_filter_self_not_resolved(resolver);
@@ -670,17 +855,19 @@ fn resolve_impls(
 
 fn resolve_trait_impls(
     context: &mut Context,
-    traits: TraitImplMap,
+    traits: Vec<UnresolvedTraitImpl>,
     crate_id: CrateId,
     errors: &mut Vec<FileDiagnostic>,
 ) -> Vec<(FileId, FuncId)> {
     let interner = &mut context.def_interner;
     let mut methods = Vec::<(FileId, FuncId)>::new();
 
-    for ((unresolved_type, _, trait_id), trait_impl) in traits {
+    for trait_impl in traits {
+        let unresolved_type = trait_impl.object_type;
         let local_mod_id = trait_impl.module_id;
         let module_id = ModuleId { krate: crate_id, local_id: local_mod_id };
         let path_resolver = StandardPathResolver::new(module_id);
+        let trait_definition_ident = trait_impl.trait_path.last_segment();
 
         let self_type = {
             let mut resolver =
@@ -688,6 +875,8 @@ fn resolve_trait_impls(
             resolver.resolve_type(unresolved_type.clone())
         };
 
+        let maybe_trait_id = trait_impl.the_trait;
+
         let mut impl_methods = resolve_function_set(
             interner,
             crate_id,
@@ -698,34 +887,33 @@ fn resolve_trait_impls(
             errors,
         );
 
-        let resolved_trait_impl = Shared::new(TraitImpl {
-            ident: trait_impl.trait_impl_ident.clone(),
-            typ: self_type.clone(),
-            trait_id,
-            methods: vecmap(&impl_methods, |(_, func_id)| *func_id),
-        });
-
         let mut new_resolver =
             Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id);
         new_resolver.set_self_type(Some(self_type.clone()));
 
-        check_methods_signatures(&mut new_resolver, &impl_methods, trait_id, errors);
-
-        let trait_definition_ident = &trait_impl.trait_impl_ident;
-        let key = TraitImplKey { typ: self_type.clone(), trait_id };
+        if let Some(trait_id) = maybe_trait_id {
+            check_methods_signatures(&mut new_resolver, &impl_methods, trait_id, errors);
+
+            let key = TraitImplKey { typ: self_type.clone(), trait_id };
+            if let Some(prev_trait_impl_ident) = interner.get_trait_implementation(&key) {
+                let err = DefCollectorErrorKind::Duplicate {
+                    typ: DuplicateType::TraitImplementation,
+                    first_def: prev_trait_impl_ident.borrow().ident.clone(),
+                    second_def: trait_definition_ident.clone(),
+                };
+                errors.push(err.into_file_diagnostic(trait_impl.methods.file_id));
+            } else {
+                let resolved_trait_impl = Shared::new(TraitImpl {
+                    ident: trait_impl.trait_path.last_segment().clone(),
+                    typ: self_type.clone(),
+                    trait_id,
+                    methods: vecmap(&impl_methods, |(_, func_id)| *func_id),
+                });
+                interner.add_trait_implementation(&key, resolved_trait_impl.clone());
+            }
 
-        if let Some(prev_trait_impl_ident) = interner.get_trait_implementation(&key) {
-            let err = DefCollectorErrorKind::Duplicate {
-                typ: DuplicateType::TraitImplementation,
-                first_def: prev_trait_impl_ident.borrow().ident.clone(),
-                second_def: trait_definition_ident.clone(),
-            };
-            errors.push(err.into_file_diagnostic(trait_impl.methods.file_id));
-        } else {
-            interner.add_trait_implementation(&key, resolved_trait_impl);
+            methods.append(&mut impl_methods);
         }
-
-        methods.append(&mut impl_methods);
     }
 
     methods
@@ -841,13 +1029,17 @@ fn resolve_function_set(
     interner: &mut NodeInterner,
     crate_id: CrateId,
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
-    unresolved_functions: UnresolvedFunctions,
+    mut unresolved_functions: UnresolvedFunctions,
     self_type: Option<Type>,
     impl_generics: Vec<(Rc<String>, Shared<TypeBinding>, Span)>,
     errors: &mut Vec<FileDiagnostic>,
 ) -> Vec<(FileId, FuncId)> {
     let file_id = unresolved_functions.file_id;
 
+    let where_clause_errors =
+        unresolved_functions.resolve_trait_bounds_trait_ids(def_maps, crate_id);
+    extend_errors(errors, file_id, where_clause_errors);
+
     vecmap(unresolved_functions.functions, |(mod_id, func_id, func)| {
         let module_id = ModuleId { krate: crate_id, local_id: mod_id };
         let path_resolver = StandardPathResolver::new(module_id);
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index 53c8947d046..014972cddbd 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -1,14 +1,9 @@
-use std::collections::HashSet;
-
 use fm::FileId;
 use noirc_errors::{CustomDiagnostic, FileDiagnostic, Location};
 
 use crate::{
     graph::CrateId,
-    hir::{
-        def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
-        def_map::ScopeResolveError,
-    },
+    hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
     node_interner::{FunctionModifiers, TraitId},
     parser::SubModule,
     token::Attributes,
@@ -66,7 +61,7 @@ pub fn collect_defs(
 
     collector.collect_globals(context, ast.globals, errors);
 
-    collector.collect_traits(ast.traits, crate_id, errors);
+    collector.collect_traits(context, ast.traits, crate_id, errors);
 
     collector.collect_structs(context, ast.types, crate_id, errors);
 
@@ -74,7 +69,7 @@ pub fn collect_defs(
 
     collector.collect_functions(context, ast.functions, crate_id, errors);
 
-    collector.collect_trait_impls(context, ast.trait_impls, crate_id, errors);
+    collector.collect_trait_impls(context, ast.trait_impls, crate_id);
 
     collector.collect_impls(context, ast.impls, crate_id);
 }
@@ -139,75 +134,52 @@ impl<'a> ModCollector<'a> {
         context: &mut Context,
         impls: Vec<NoirTraitImpl>,
         krate: CrateId,
-        errors: &mut Vec<FileDiagnostic>,
     ) {
-        let module_id = ModuleId { krate, local_id: self.module_id };
-
         for trait_impl in impls {
-            let trait_name = &trait_impl.trait_name;
-            let module = &self.def_collector.def_map.modules[self.module_id.0];
-
-            if let Some(trait_id) = self.find_trait_or_emit_error(module, trait_name, errors) {
-                let collected_trait =
-                    self.def_collector.collected_traits.get(&trait_id).cloned().unwrap();
+            let trait_name = trait_impl.trait_name.clone();
 
-                let unresolved_functions = self.collect_trait_implementations(
-                    context,
-                    &trait_impl,
-                    &collected_trait.trait_def,
-                    krate,
-                    errors,
-                );
+            let unresolved_functions =
+                self.collect_trait_impl_function_overrides(context, &trait_impl, krate);
 
-                for (_, func_id, noir_function) in &unresolved_functions.functions {
-                    let function = &noir_function.def;
-                    context.def_interner.push_function(*func_id, function, module_id);
-                }
+            for (_, func_id, noir_function) in &unresolved_functions.functions {
+                let name = noir_function.name().to_owned();
 
-                let unresolved_trait_impl = UnresolvedTraitImpl {
-                    file_id: self.file_id,
-                    module_id: self.module_id,
-                    the_trait: collected_trait,
-                    methods: unresolved_functions,
-                    trait_impl_ident: trait_impl.trait_name.clone(),
+                let modifiers = FunctionModifiers {
+                    name: name.clone(),
+                    visibility: crate::Visibility::Public,
+                    attributes: Attributes::empty(),
+                    is_unconstrained: false,
+                    contract_function_type: None,
+                    is_internal: None,
                 };
-
-                let key = (trait_impl.object_type, self.module_id, trait_id);
-                self.def_collector.collected_traits_impls.insert(key, unresolved_trait_impl);
+                context.def_interner.push_function_definition(
+                    name,
+                    *func_id,
+                    modifiers,
+                    ModuleId::dummy_id(),
+                );
             }
-        }
-    }
 
-    fn find_trait_or_emit_error(
-        &self,
-        module: &ModuleData,
-        trait_name: &Ident,
-        errors: &mut Vec<FileDiagnostic>,
-    ) -> Option<TraitId> {
-        match module.find_trait_with_name(trait_name) {
-            Ok(trait_id) => Some(trait_id),
-            Err(ScopeResolveError::WrongKind) => {
-                let error =
-                    DefCollectorErrorKind::NotATrait { not_a_trait_name: trait_name.clone() };
-                errors.push(error.into_file_diagnostic(self.file_id));
-                None
-            }
-            Err(ScopeResolveError::NotFound) => {
-                let error =
-                    DefCollectorErrorKind::TraitNotFound { trait_ident: trait_name.clone() };
-                errors.push(error.into_file_diagnostic(self.file_id));
-                None
-            }
+            let unresolved_trait_impl = UnresolvedTraitImpl {
+                file_id: self.file_id,
+                module_id: self.module_id,
+                trait_path: trait_name,
+                methods: unresolved_functions,
+                object_type: trait_impl.object_type,
+                object_type_span: trait_impl.object_type_span,
+                the_trait: None,                           // will be filled later
+                methods_using_default_impl_in_trait: None, // will be filled later
+            };
+
+            self.def_collector.collected_traits_impls.push(unresolved_trait_impl);
         }
     }
 
-    fn collect_trait_implementations(
+    fn collect_trait_impl_function_overrides(
         &mut self,
         context: &mut Context,
         trait_impl: &NoirTraitImpl,
-        trait_def: &NoirTrait,
         krate: CrateId,
-        errors: &mut Vec<FileDiagnostic>,
     ) -> UnresolvedFunctions {
         let mut unresolved_functions =
             UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() };
@@ -222,91 +194,6 @@ impl<'a> ModCollector<'a> {
             }
         }
 
-        // set of function ids that have a corresponding method in the trait
-        let mut func_ids_in_trait = HashSet::new();
-
-        for item in &trait_def.items {
-            // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
-            if let TraitItem::Function {
-                name,
-                generics,
-                parameters,
-                return_type,
-                where_clause,
-                body,
-            } = item
-            {
-                // List of functions in the impl block with the same name as the method
-                //  `matching_fns.len() == 0`  => missing method impl
-                //  `matching_fns.len() > 1`   => duplicate definition (collect_functions will throw a Duplicate error)
-                let matching_fns: Vec<_> = unresolved_functions
-                    .functions
-                    .iter()
-                    .filter(|(_, _, func_impl)| func_impl.name() == name.0.contents)
-                    .collect();
-
-                if matching_fns.is_empty() {
-                    match body {
-                        Some(body) => {
-                            // if there's a default implementation for the method, use it
-                            let method_name = name.0.contents.clone();
-                            let func_id = context.def_interner.push_empty_fn();
-                            let modifiers = FunctionModifiers {
-                                name: name.0.contents.clone(),
-                                // trait functions are always public
-                                visibility: crate::Visibility::Public,
-                                attributes: Attributes::empty(),
-                                is_unconstrained: false,
-                                contract_function_type: None,
-                                is_internal: None,
-                            };
-
-                            context.def_interner.push_function_definition(
-                                method_name,
-                                func_id,
-                                modifiers,
-                                module,
-                            );
-                            let impl_method = NoirFunction::normal(FunctionDefinition::normal(
-                                name,
-                                generics,
-                                parameters,
-                                body,
-                                where_clause,
-                                return_type,
-                            ));
-                            func_ids_in_trait.insert(func_id);
-                            unresolved_functions.push_fn(self.module_id, func_id, impl_method);
-                        }
-                        None => {
-                            let error = DefCollectorErrorKind::TraitMissingMethod {
-                                trait_name: trait_def.name.clone(),
-                                method_name: name.clone(),
-                                trait_impl_span: trait_impl.object_type_span,
-                            };
-                            errors.push(error.into_file_diagnostic(self.file_id));
-                        }
-                    }
-                } else {
-                    for (_, func_id, _) in &matching_fns {
-                        func_ids_in_trait.insert(*func_id);
-                    }
-                }
-            }
-        }
-
-        // Emit MethodNotInTrait error for methods in the impl block that
-        // don't have a corresponding method signature defined in the trait
-        for (_, func_id, func) in &unresolved_functions.functions {
-            if !func_ids_in_trait.contains(func_id) {
-                let error = DefCollectorErrorKind::MethodNotInTrait {
-                    trait_name: trait_def.name.clone(),
-                    impl_method: func.name_ident().clone(),
-                };
-                errors.push(error.into_file_diagnostic(self.file_id));
-            }
-        }
-
         unresolved_functions
     }
 
@@ -322,7 +209,7 @@ impl<'a> ModCollector<'a> {
 
         let module = ModuleId { krate, local_id: self.module_id };
 
-        for mut function in functions {
+        for function in functions {
             let name = function.name_ident().clone();
             let func_id = context.def_interner.push_empty_fn();
 
@@ -330,19 +217,6 @@ impl<'a> ModCollector<'a> {
             // So that we can get a FuncId
             context.def_interner.push_function(func_id, &function.def, module);
 
-            // Then go over the where clause and assign trait_ids to the constraints
-            for constraint in &mut function.def.where_clause {
-                let module = &self.def_collector.def_map.modules[self.module_id.0];
-
-                if let Some(trait_id) = self.find_trait_or_emit_error(
-                    module,
-                    &constraint.trait_bound.trait_name,
-                    errors,
-                ) {
-                    constraint.trait_bound.trait_id = Some(trait_id);
-                }
-            }
-
             // Now link this func_id to a crate level map with the noir function and the module id
             // Encountering a NoirFunction, we retrieve it's module_data to get the namespace
             // Once we have lowered it to a HirFunction, we retrieve it's Id from the DefInterner
@@ -451,6 +325,7 @@ impl<'a> ModCollector<'a> {
     /// Returns a vector of errors if any traits were already defined.
     fn collect_traits(
         &mut self,
+        context: &mut Context,
         traits: Vec<NoirTrait>,
         krate: CrateId,
         errors: &mut Vec<FileDiagnostic>,
@@ -477,11 +352,41 @@ impl<'a> ModCollector<'a> {
                 errors.push(err.into_file_diagnostic(self.file_id));
             }
 
+            // Add all functions that have a default implementation in the trait
+            let mut unresolved_functions =
+                UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() };
+            for trait_item in &trait_definition.items {
+                // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
+                if let TraitItem::Function {
+                    name,
+                    generics,
+                    parameters,
+                    return_type,
+                    where_clause,
+                    body: Some(body),
+                } = trait_item
+                {
+                    let func_id = context.def_interner.push_empty_fn();
+
+                    let impl_method = NoirFunction::normal(FunctionDefinition::normal(
+                        name,
+                        generics,
+                        parameters,
+                        body,
+                        where_clause,
+                        return_type,
+                    ));
+                    unresolved_functions.push_fn(self.module_id, func_id, impl_method);
+                }
+            }
+
             // And store the TraitId -> TraitType mapping somewhere it is reachable
             let unresolved = UnresolvedTrait {
                 file_id: self.file_id,
                 module_id: self.module_id,
+                crate_id: krate,
                 trait_def: trait_definition,
+                fns_with_default_impl: unresolved_functions,
             };
             self.def_collector.collected_traits.insert(id, unresolved);
         }
diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs
index 1e3b36844f9..73e1d8c6acd 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs
@@ -1,5 +1,6 @@
 use crate::hir::resolution::import::PathResolutionError;
 use crate::Ident;
+use crate::Path;
 
 use noirc_errors::CustomDiagnostic as Diagnostic;
 use noirc_errors::FileDiagnostic;
@@ -30,7 +31,7 @@ pub enum DefCollectorErrorKind {
     #[error("Non-struct type used in impl")]
     NonStructTypeInImpl { span: Span },
     #[error("Non-struct type used in trait impl")]
-    NonStructTraitImpl { trait_ident: Ident, span: Span },
+    NonStructTraitImpl { trait_path: Path, span: Span },
     #[error("Cannot `impl` a type defined outside the current crate")]
     ForeignImpl { span: Span, type_name: String },
     #[error("Mismatch number of parameters in of trait implementation")]
@@ -44,11 +45,15 @@ pub enum DefCollectorErrorKind {
     #[error("Method is not defined in trait")]
     MethodNotInTrait { trait_name: Ident, impl_method: Ident },
     #[error("Only traits can be implemented")]
-    NotATrait { not_a_trait_name: Ident },
+    NotATrait { not_a_trait_name: Path },
     #[error("Trait not found")]
-    TraitNotFound { trait_ident: Ident },
+    TraitNotFound { trait_path: Path },
     #[error("Missing Trait method implementation")]
     TraitMissingMethod { trait_name: Ident, method_name: Ident, trait_impl_span: Span },
+    #[error(
+        "Either the type or the trait must be from the same crate as the trait implementation"
+    )]
+    TraitImplOrphaned { span: Span },
 }
 
 impl DefCollectorErrorKind {
@@ -107,9 +112,9 @@ impl From<DefCollectorErrorKind> for Diagnostic {
                 "Only struct types may have implementation methods".into(),
                 span,
             ),
-            DefCollectorErrorKind::NonStructTraitImpl { trait_ident, span } => {
+            DefCollectorErrorKind::NonStructTraitImpl { trait_path, span } => {
                 Diagnostic::simple_error(
-                    format!("Only struct types may implement trait `{trait_ident}`"),
+                    format!("Only struct types may implement trait `{trait_path}`"),
                     "Only struct types may implement traits".into(),
                     span,
                 )
@@ -119,10 +124,10 @@ impl From<DefCollectorErrorKind> for Diagnostic {
                 format!("{type_name} was defined outside the current crate"),
                 span,
             ),
-            DefCollectorErrorKind::TraitNotFound { trait_ident } => Diagnostic::simple_error(
-                format!("Trait {trait_ident} not found"),
+            DefCollectorErrorKind::TraitNotFound { trait_path } => Diagnostic::simple_error(
+                format!("Trait {trait_path} not found"),
                 "".to_string(),
-                trait_ident.span(),
+                trait_path.span(),
             ),
             DefCollectorErrorKind::MismatchTraitImplementationNumParameters {
                 expected_num_parameters,
@@ -159,14 +164,18 @@ impl From<DefCollectorErrorKind> for Diagnostic {
                 )
             }
             DefCollectorErrorKind::NotATrait { not_a_trait_name } => {
-                let span = not_a_trait_name.0.span();
-                let name = &not_a_trait_name.0.contents;
+                let span = not_a_trait_name.span();
                 Diagnostic::simple_error(
-                    format!("{name} is not a trait, therefore it can't be implemented"),
+                    format!("{not_a_trait_name} is not a trait, therefore it can't be implemented"),
                     String::new(),
                     span,
                 )
             }
+            DefCollectorErrorKind::TraitImplOrphaned { span } => Diagnostic::simple_error(
+                "Orphaned trait implementation".into(),
+                "Either the type or the trait must be from the same crate as the trait implementation".into(),
+                span,
+            ),
         }
     }
 }
diff --git a/compiler/noirc_frontend/src/hir/def_map/item_scope.rs b/compiler/noirc_frontend/src/hir/def_map/item_scope.rs
index 490e046a9d9..7dcc5051a0c 100644
--- a/compiler/noirc_frontend/src/hir/def_map/item_scope.rs
+++ b/compiler/noirc_frontend/src/hir/def_map/item_scope.rs
@@ -1,8 +1,5 @@
 use super::{namespace::PerNs, ModuleDefId, ModuleId};
-use crate::{
-    node_interner::{FuncId, TraitId},
-    Ident,
-};
+use crate::{node_interner::FuncId, Ident};
 use std::collections::{hash_map::Entry, HashMap};
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
@@ -18,14 +15,6 @@ pub struct ItemScope {
     defs: Vec<ModuleDefId>,
 }
 
-pub enum ScopeResolveError {
-    /// the ident we attempted to resolve isn't declared
-    NotFound,
-
-    /// The ident we attempted to resolve is declared, but is the wrong kind - e.g. we want a trait, but it's a function
-    WrongKind,
-}
-
 impl ItemScope {
     pub fn add_definition(
         &mut self,
@@ -80,14 +69,6 @@ impl ItemScope {
             _ => None,
         }
     }
-    pub fn find_trait_with_name(&self, trait_name: &Ident) -> Result<TraitId, ScopeResolveError> {
-        let (module_def, _) = self.types.get(trait_name).ok_or(ScopeResolveError::NotFound)?;
-
-        match module_def {
-            ModuleDefId::TraitId(id) => Ok(*id),
-            _ => Err(ScopeResolveError::WrongKind),
-        }
-    }
 
     pub fn find_name(&self, name: &Ident) -> PerNs {
         PerNs { types: self.types.get(name).cloned(), values: self.values.get(name).cloned() }
diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs
index 1f1fa44108d..1a5b1692208 100644
--- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs
+++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs
@@ -7,7 +7,7 @@ use crate::{
     Ident,
 };
 
-use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs, ScopeResolveError};
+use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs};
 
 /// Contains the actual contents of a module: its parent (if one exists),
 /// children, and scope with all definitions defined within the scope.
@@ -85,10 +85,6 @@ impl ModuleData {
         self.scope.find_func_with_name(name)
     }
 
-    pub fn find_trait_with_name(&self, name: &Ident) -> Result<TraitId, ScopeResolveError> {
-        self.scope.find_trait_with_name(name)
-    }
-
     pub fn import(&mut self, name: Ident, id: ModuleDefId) -> Result<(), (Ident, Ident)> {
         self.scope.add_item_to_namespace(name, id)
     }
diff --git a/compiler/noirc_frontend/src/hir/def_map/module_def.rs b/compiler/noirc_frontend/src/hir/def_map/module_def.rs
index ade0fcaf7aa..659d7712ab4 100644
--- a/compiler/noirc_frontend/src/hir/def_map/module_def.rs
+++ b/compiler/noirc_frontend/src/hir/def_map/module_def.rs
@@ -87,6 +87,12 @@ impl From<StmtId> for ModuleDefId {
     }
 }
 
+impl From<TraitId> for ModuleDefId {
+    fn from(trait_id: TraitId) -> Self {
+        ModuleDefId::TraitId(trait_id)
+    }
+}
+
 pub trait TryFromModuleDefId: Sized {
     fn try_from(id: ModuleDefId) -> Option<Self>;
     fn dummy_id() -> Self;
diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs
index 36f50525d00..dff29555b92 100644
--- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs
+++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs
@@ -105,6 +105,11 @@ struct ResolverMeta {
     warn_if_unused: bool,
 }
 
+pub enum ResolvePathError {
+    WrongKind,
+    NotFound,
+}
+
 impl<'a> Resolver<'a> {
     pub fn new(
         interner: &'a mut NodeInterner,
diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs
index 5e9b8723dbe..b3585297a97 100644
--- a/compiler/noirc_frontend/src/hir_def/traits.rs
+++ b/compiler/noirc_frontend/src/hir_def/traits.rs
@@ -1,6 +1,7 @@
 use crate::{
+    graph::CrateId,
     node_interner::{FuncId, TraitId},
-    Generics, Ident, Type, TypeVariable, TypeVariableId,
+    Generics, Ident, NoirFunction, Type, TypeVariable, TypeVariableId,
 };
 use noirc_errors::Span;
 
@@ -11,6 +12,9 @@ pub struct TraitFunction {
     pub arguments: Vec<Type>,
     pub return_type: Type,
     pub span: Span,
+    pub default_impl: Option<Box<NoirFunction>>,
+    pub default_impl_file_id: fm::FileId,
+    pub default_impl_module_id: crate::hir::def_map::LocalModuleId,
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -36,6 +40,8 @@ pub struct Trait {
     /// struct traits are equal.
     pub id: TraitId,
 
+    pub crate_id: CrateId,
+
     pub methods: Vec<TraitFunction>,
     pub constants: Vec<TraitConstant>,
     pub types: Vec<TraitType>,
@@ -82,6 +88,7 @@ impl Trait {
     pub fn new(
         id: TraitId,
         name: Ident,
+        crate_id: CrateId,
         span: Span,
         generics: Generics,
         self_type_typevar_id: TypeVariableId,
@@ -90,6 +97,7 @@ impl Trait {
         Trait {
             id,
             name,
+            crate_id,
             span,
             methods: Vec::new(),
             constants: Vec::new(),
diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs
index 01633c29178..54d6d9d8163 100644
--- a/compiler/noirc_frontend/src/node_interner.rs
+++ b/compiler/noirc_frontend/src/node_interner.rs
@@ -419,6 +419,7 @@ impl NodeInterner {
             Shared::new(Trait::new(
                 type_id,
                 typ.trait_def.name.clone(),
+                typ.crate_id,
                 typ.trait_def.span,
                 vecmap(&typ.trait_def.generics, |_| {
                     // Temporary type variable ids before the trait is resolved to its actual ids.
diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs
index 3da674f6552..37a0abb5581 100644
--- a/compiler/noirc_frontend/src/parser/parser.rs
+++ b/compiler/noirc_frontend/src/parser/parser.rs
@@ -556,7 +556,7 @@ fn implementation() -> impl NoirParser<TopLevelStatement> {
 fn trait_implementation() -> impl NoirParser<TopLevelStatement> {
     keyword(Keyword::Impl)
         .ignore_then(generics())
-        .then(ident())
+        .then(path())
         .then(generic_type_args(parse_type()))
         .then_ignore(keyword(Keyword::For))
         .then(parse_type().map_with_span(|typ, span| (typ, span)))
@@ -631,8 +631,8 @@ fn trait_bounds() -> impl NoirParser<Vec<TraitBound>> {
 }
 
 fn trait_bound() -> impl NoirParser<TraitBound> {
-    ident().then(generic_type_args(parse_type())).map(|(trait_name, trait_generics)| TraitBound {
-        trait_name,
+    path().then(generic_type_args(parse_type())).map(|(trait_path, trait_generics)| TraitBound {
+        trait_path,
         trait_generics,
         trait_id: None,
     })
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/Nargo.toml
new file mode 100644
index 00000000000..7c2c50884fe
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_implementation_4"
+type = "bin"
+authors = [""]
+compiler_version = "0.11.1"
+
+[dependencies]
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/Prover.toml
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/main.nr
new file mode 100644
index 00000000000..b9f712ceff0
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/main.nr
@@ -0,0 +1,6 @@
+mod module1;
+mod module2;
+mod module3;
+
+fn main() {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module1.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module1.nr
new file mode 100644
index 00000000000..4d41ff2909a
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module1.nr
@@ -0,0 +1,2 @@
+trait MyTrait {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module2.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module2.nr
new file mode 100644
index 00000000000..3cadb6d78cb
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module2.nr
@@ -0,0 +1,2 @@
+struct MyStruct {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module3.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module3.nr
new file mode 100644
index 00000000000..a7612345cf1
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_4/src/module3.nr
@@ -0,0 +1,7 @@
+use crate::module1::MyTrait;
+use crate::module2::MyStruct;
+
+// those are not the same 'Path', but they refer to the same trait & impl
+// so a Duplicate error should be thrown
+impl MyTrait for MyStruct {}
+impl crate::module1::MyTrait for crate::module2::MyStruct { }
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/Nargo.toml
new file mode 100644
index 00000000000..b5bacc433f3
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "dup_trait_implementation_5"
+type = "bin"
+authors = [""]
+compiler_version = "0.11.1"
+
+[dependencies]
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/Prover.toml b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/Prover.toml
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/main.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/main.nr
new file mode 100644
index 00000000000..78e3867e4a1
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/main.nr
@@ -0,0 +1,7 @@
+mod module1;
+mod module2;
+mod module3;
+mod module4;
+
+fn main() {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module1.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module1.nr
new file mode 100644
index 00000000000..4d41ff2909a
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module1.nr
@@ -0,0 +1,2 @@
+trait MyTrait {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module2.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module2.nr
new file mode 100644
index 00000000000..3cadb6d78cb
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module2.nr
@@ -0,0 +1,2 @@
+struct MyStruct {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module3.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module3.nr
new file mode 100644
index 00000000000..ac886d441cd
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module3.nr
@@ -0,0 +1,4 @@
+use crate::module1::MyTrait;
+use crate::module2::MyStruct;
+
+impl MyTrait for MyStruct {}
diff --git a/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module4.nr b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module4.nr
new file mode 100644
index 00000000000..dfa660f80b1
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/dup_trait_implementation_5/src/module4.nr
@@ -0,0 +1,3 @@
+// another module in the crate implements the same trait + struct
+// a Duplicate error should be thrown
+impl crate::module1::MyTrait for crate::module2::MyStruct { }
diff --git a/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Nargo.toml
new file mode 100644
index 00000000000..47df960cc33
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Nargo.toml
@@ -0,0 +1,9 @@
+[package]
+name = "orphaned_trait_impl"
+type = "bin"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
+crate1 = { path = "crate1" }
+crate2 = { path = "crate2" }
diff --git a/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Prover.toml b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Prover.toml
new file mode 100644
index 00000000000..2c1854573a4
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/Prover.toml
@@ -0,0 +1,2 @@
+x = 1
+y = 2
diff --git a/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/Nargo.toml
new file mode 100644
index 00000000000..b28e0e840c4
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "crate1"
+type = "lib"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/src/lib.nr b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/src/lib.nr
new file mode 100644
index 00000000000..4d41ff2909a
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate1/src/lib.nr
@@ -0,0 +1,2 @@
+trait MyTrait {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/Nargo.toml
new file mode 100644
index 00000000000..a90a5dcceea
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "crate2"
+type = "lib"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/src/lib.nr b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/src/lib.nr
new file mode 100644
index 00000000000..3cadb6d78cb
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/crate2/src/lib.nr
@@ -0,0 +1,2 @@
+struct MyStruct {
+}
diff --git a/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/src/main.nr b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/src/main.nr
new file mode 100644
index 00000000000..d245bd68ea1
--- /dev/null
+++ b/tooling/nargo_cli/tests/compile_failure/orphaned_trait_impl/src/main.nr
@@ -0,0 +1,6 @@
+impl dep::crate1::MyTrait for dep::crate2::MyStruct {
+}
+
+fn main(x : Field, y : pub Field) {
+    assert(x != y);
+}
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/Nargo.toml b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/Nargo.toml
new file mode 100644
index 00000000000..89d83e18f66
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "trait_multi_module_test"
+type = "bin"
+authors = [""]
+compiler_version = "0.11.1"
+
+[dependencies]
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/Prover.toml b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/Prover.toml
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/main.nr
new file mode 100644
index 00000000000..63b6f08ed52
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/main.nr
@@ -0,0 +1,9 @@
+mod module1;
+mod module2;
+mod module3;
+mod module4;
+mod module5;
+mod module6;
+
+fn main() {
+}
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module1.nr b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module1.nr
new file mode 100644
index 00000000000..4d41ff2909a
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module1.nr
@@ -0,0 +1,2 @@
+trait MyTrait {
+}
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module2.nr b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module2.nr
new file mode 100644
index 00000000000..3cadb6d78cb
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module2.nr
@@ -0,0 +1,2 @@
+struct MyStruct {
+}
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module3.nr b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module3.nr
new file mode 100644
index 00000000000..4b2fbc9bfcc
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module3.nr
@@ -0,0 +1,5 @@
+use crate::module1::MyTrait;
+use crate::module2::MyStruct;
+
+// ensure we can implement traits that are imported with the `use` syntax
+impl MyTrait for MyStruct {}
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module4.nr b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module4.nr
new file mode 100644
index 00000000000..f9458e83c4a
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module4.nr
@@ -0,0 +1,2 @@
+trait MyTrait4 {
+}
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module5.nr b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module5.nr
new file mode 100644
index 00000000000..cd9b7f0bf39
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module5.nr
@@ -0,0 +1,2 @@
+struct MyStruct5 {
+}
diff --git a/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module6.nr b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module6.nr
new file mode 100644
index 00000000000..35f5ce3a183
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_multi_module_test/src/module6.nr
@@ -0,0 +1,2 @@
+// ensure we can implement traits using the Path syntax
+impl crate::module4::MyTrait4 for crate::module5::MyStruct5 { }
diff --git a/tooling/nargo_cli/tests/execution_success/trait_where_clause/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_where_clause/src/main.nr
index 2d6d71d8df0..aac2362c1d9 100644
--- a/tooling/nargo_cli/tests/execution_success/trait_where_clause/src/main.nr
+++ b/tooling/nargo_cli/tests/execution_success/trait_where_clause/src/main.nr
@@ -5,9 +5,9 @@
 //      - trait impl blocks (impl<T,U> Foo for Bar where T...)
 //      - structs (struct Foo<T> where T: ...)
 
-trait Asd {
-    fn asd(self) -> Field;
-}
+// import the trait from another module to ensure the where clauses are ok with that
+mod the_trait;
+use crate::the_trait::Asd;
 
 struct Add10  { x: Field, }
 struct Add20  { x: Field, }
@@ -24,7 +24,7 @@ impl Asd for AddXY {
     }
 }
 
-fn assert_asd_eq_100<T>(t: T) where T: Asd {
+fn assert_asd_eq_100<T>(t: T) where T: crate::the_trait::Asd {
     assert(t.asd() == 100);
 }
 
diff --git a/tooling/nargo_cli/tests/execution_success/trait_where_clause/src/the_trait.nr b/tooling/nargo_cli/tests/execution_success/trait_where_clause/src/the_trait.nr
new file mode 100644
index 00000000000..1b8803fddfd
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/trait_where_clause/src/the_trait.nr
@@ -0,0 +1,3 @@
+trait Asd {
+    fn asd(self) -> Field;
+}
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Nargo.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Nargo.toml
new file mode 100644
index 00000000000..a13ed98b632
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Nargo.toml
@@ -0,0 +1,9 @@
+[package]
+name = "traits_in_crates_1"
+type = "bin"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
+crate1 = { path = "crate1" }
+crate2 = { path = "crate2" }
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Prover.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Prover.toml
new file mode 100644
index 00000000000..c2005d59807
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/Prover.toml
@@ -0,0 +1,2 @@
+x = 1
+y = 11
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/Nargo.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/Nargo.toml
new file mode 100644
index 00000000000..0a94742b76a
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/Nargo.toml
@@ -0,0 +1,8 @@
+[package]
+name = "crate1"
+type = "lib"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
+crate2 = { path = "../crate2" }
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/src/lib.nr b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/src/lib.nr
new file mode 100644
index 00000000000..62dd5a2c111
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate1/src/lib.nr
@@ -0,0 +1,9 @@
+trait MyTrait {
+    fn Add10(&mut self);
+}
+
+impl MyTrait for dep::crate2::MyStruct {
+    fn Add10(&mut self) {
+        self.Q += 10;
+    }
+}
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/Nargo.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/Nargo.toml
new file mode 100644
index 00000000000..a90a5dcceea
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "crate2"
+type = "lib"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/src/lib.nr b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/src/lib.nr
new file mode 100644
index 00000000000..c59bf0387c1
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/crate2/src/lib.nr
@@ -0,0 +1,3 @@
+struct MyStruct {
+    Q: Field,
+}
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/src/main.nr b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/src/main.nr
new file mode 100644
index 00000000000..cb6416f7732
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_1/src/main.nr
@@ -0,0 +1,5 @@
+fn main(x : Field, y : pub Field) {
+    let mut V = dep::crate2::MyStruct { Q: x };
+    V.Add10();
+    assert(V.Q == y);
+}
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Nargo.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Nargo.toml
new file mode 100644
index 00000000000..8b5bc49e0bb
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Nargo.toml
@@ -0,0 +1,9 @@
+[package]
+name = "traits_in_crates_2"
+type = "bin"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
+crate1 = { path = "crate1" }
+crate2 = { path = "crate2" }
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Prover.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Prover.toml
new file mode 100644
index 00000000000..c2005d59807
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/Prover.toml
@@ -0,0 +1,2 @@
+x = 1
+y = 11
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/Nargo.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/Nargo.toml
new file mode 100644
index 00000000000..b28e0e840c4
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "crate1"
+type = "lib"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
\ No newline at end of file
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/src/lib.nr b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/src/lib.nr
new file mode 100644
index 00000000000..59a28a50c79
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate1/src/lib.nr
@@ -0,0 +1,3 @@
+trait MyTrait {
+    fn Add10(&mut self);
+}
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/Nargo.toml b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/Nargo.toml
new file mode 100644
index 00000000000..51c201372ee
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/Nargo.toml
@@ -0,0 +1,8 @@
+[package]
+name = "crate2"
+type = "lib"
+authors = [""]
+compiler_version = "0.12.0"
+
+[dependencies]
+crate1 = { path = "../crate1" }
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/src/lib.nr b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/src/lib.nr
new file mode 100644
index 00000000000..38870489131
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/crate2/src/lib.nr
@@ -0,0 +1,9 @@
+struct MyStruct {
+    Q: Field,
+}
+
+impl dep::crate1::MyTrait for MyStruct {
+    fn Add10(&mut self) {
+        self.Q += 10;
+    }
+}
diff --git a/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/src/main.nr b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/src/main.nr
new file mode 100644
index 00000000000..cb6416f7732
--- /dev/null
+++ b/tooling/nargo_cli/tests/execution_success/traits_in_crates_2/src/main.nr
@@ -0,0 +1,5 @@
+fn main(x : Field, y : pub Field) {
+    let mut V = dep::crate2::MyStruct { Q: x };
+    V.Add10();
+    assert(V.Q == y);
+}

From 10e42c523ce60f1a258ddc6523f6f60b97925b99 Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Tue, 26 Sep 2023 09:57:01 +0300
Subject: [PATCH 02/17] fix(traits): cleanup, fmt

---
 .../src/hir/def_collector/dc_crate.rs         | 51 ++++++++++---------
 .../src/hir/resolution/import.rs              | 15 ++++++
 2 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 72a41180093..0c6a734fea0 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -3,8 +3,8 @@ use super::errors::{DefCollectorErrorKind, DuplicateType};
 use crate::graph::CrateId;
 use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId};
 use crate::hir::resolution::errors::ResolverError;
-use crate::hir::resolution::import::PathResolutionError;
-use crate::hir::resolution::path_resolver::PathResolver;
+use crate::hir::resolution::import::{get_path_crate, PathResolutionError};
+use crate::hir::resolution::path_resolver::{resolve_path, PathResolver};
 use crate::hir::resolution::resolver::Resolver;
 use crate::hir::resolution::{
     import::{resolve_imports, ImportDirective},
@@ -365,39 +365,21 @@ fn collect_trait_impls(
         let module = ModuleId { local_id: module_id, krate: crate_id };
         let resolve_result = resolve_trait_by_path(def_maps, module, trait_impl.trait_path.clone());
 
-        let maybe_trait_id = match resolve_result {
+        trait_impl.the_trait = match resolve_result {
             Ok(trait_id) => Some(trait_id),
             Err(error) => {
                 errors.push(error.into_file_diagnostic(trait_impl.file_id));
                 None
             }
         };
-        trait_impl.the_trait = maybe_trait_id;
 
-        if let Some(trait_id) = maybe_trait_id {
+        if let Some(trait_id) = trait_impl.the_trait {
             let the_trait = interner.get_trait(trait_id);
             let the_trait = the_trait.borrow();
 
             // check whether the trait implementation is in the same crate as either the trait or the type
-            {
-                let file = def_maps[&crate_id].file_id(module_id);
-                let path_resolver = StandardPathResolver::new(module);
-                let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
-                let typ = resolver.resolve_type(unresolved_type.clone());
-                let crate_of_object = if let Some(struct_type) = get_struct_type(&typ) {
-                    let struct_type = struct_type.borrow();
-                    struct_type.id.krate()
-                } else {
-                    CrateId::Dummy
-                };
-
-                if crate_id != the_trait.crate_id && crate_id != crate_of_object {
-                    let error = DefCollectorErrorKind::TraitImplOrphaned {
-                        span: trait_impl.object_type_span,
-                    };
-                    errors.push(error.into_file_diagnostic(trait_impl.file_id));
-                }
-            }
+            let current_def_map = def_maps.get_mut(&crate_id).unwrap();
+            check_trait_impl_crate_coherence(trait_impl, crate_id, current_def_map, errors);
 
             // set of function ids that have a corresponding method in the trait
             let mut func_ids_in_trait = HashSet::new();
@@ -521,6 +503,27 @@ fn collect_trait_impls(
     }
 }
 
+fn check_trait_impl_crate_coherence(
+    trait_impl: &UnresolvedTraitImpl,
+    current_crate: CrateId,
+    current_def_map: &CrateDefMap,
+    errors: &mut Vec<FileDiagnostic>,
+) {
+    let trait_crate = get_path_crate(current_crate, current_def_map, &trait_impl.trait_path);
+
+    let object_crate = match &trait_impl.object_type.typ {
+        crate::UnresolvedTypeData::Named(path, _) => {
+            get_path_crate(current_crate, current_def_map, path)
+        }
+        _ => None,
+    };
+
+    if Some(current_crate) != trait_crate && Some(current_crate) != object_crate {
+        let error = DefCollectorErrorKind::TraitImplOrphaned { span: trait_impl.object_type_span };
+        errors.push(error.into_file_diagnostic(trait_impl.file_id));
+    }
+}
+
 fn resolve_trait_by_path(
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
     module: ModuleId,
diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs
index 6f3140a65d4..7b57da1c687 100644
--- a/compiler/noirc_frontend/src/hir/resolution/import.rs
+++ b/compiler/noirc_frontend/src/hir/resolution/import.rs
@@ -210,3 +210,18 @@ fn resolve_external_dep(
 
     resolve_path_to_ns(&dep_directive, dep_def_map, def_maps, allow_contracts)
 }
+
+pub fn get_path_crate(
+    current_crate: CrateId,
+    current_def_map: &CrateDefMap,
+    path: &Path,
+) -> Option<CrateId> {
+    match path.kind {
+        PathKind::Dep => {
+            let crate_name = &path.segments.first().unwrap();
+            current_def_map.extern_prelude.get(&crate_name.0.contents).map(|module| module.krate)
+        }
+        PathKind::Crate => Some(current_crate),
+        PathKind::Plain => Some(current_crate),
+    }
+}

From 3c6c4a49f5ae6027d4ac95defb302482a6f996f8 Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Tue, 26 Sep 2023 10:04:02 +0300
Subject: [PATCH 03/17] fix(traits): remove redundant code

---
 .../src/hir/def_collector/dc_crate.rs             | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 0c6a734fea0..23c1be3771f 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -394,6 +394,7 @@ fn collect_trait_impls(
                     .iter()
                     .filter(|(_, _, f)| f.name() == method.name.0.contents)
                     .collect();
+
                 if overrides.is_empty() {
                     unresolved_functions.file_id = method.default_impl_file_id;
                     if let Some(default_impl) = &method.default_impl {
@@ -435,20 +436,6 @@ fn collect_trait_impls(
                 }
             }
 
-            for (_, func_id, noir_function) in &unresolved_functions.functions {
-                let name = noir_function.name().to_owned();
-
-                let modifiers = FunctionModifiers {
-                    name: name.clone(),
-                    visibility: crate::Visibility::Public,
-                    attributes: Attributes::empty(),
-                    is_unconstrained: false,
-                    contract_function_type: None,
-                    is_internal: None,
-                };
-                interner.push_function_definition(name, *func_id, modifiers, module);
-            }
-
             // Emit MethodNotInTrait error for methods in the impl block that
             // don't have a corresponding method signature defined in the trait
             for (_, func_id, func) in &trait_impl.methods.functions {

From aa77ec60e2d210c7bf7ef20218adc07e408f8621 Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Tue, 26 Sep 2023 10:12:01 +0300
Subject: [PATCH 04/17] fix(traits): remove object_type_span

---
 compiler/noirc_frontend/src/ast/traits.rs                 | 1 -
 compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 8 +++-----
 compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs   | 4 +---
 compiler/noirc_frontend/src/parser/parser.rs              | 5 ++---
 4 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/compiler/noirc_frontend/src/ast/traits.rs b/compiler/noirc_frontend/src/ast/traits.rs
index 915134e725d..feb627df60b 100644
--- a/compiler/noirc_frontend/src/ast/traits.rs
+++ b/compiler/noirc_frontend/src/ast/traits.rs
@@ -61,7 +61,6 @@ pub struct NoirTraitImpl {
     pub trait_generics: Vec<UnresolvedType>,
 
     pub object_type: UnresolvedType,
-    pub object_type_span: Span,
 
     pub where_clause: Vec<UnresolvedTraitConstraint>,
 
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 23c1be3771f..e82d0c3945c 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -4,7 +4,7 @@ use crate::graph::CrateId;
 use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId};
 use crate::hir::resolution::errors::ResolverError;
 use crate::hir::resolution::import::{get_path_crate, PathResolutionError};
-use crate::hir::resolution::path_resolver::{resolve_path, PathResolver};
+use crate::hir::resolution::path_resolver::PathResolver;
 use crate::hir::resolution::resolver::Resolver;
 use crate::hir::resolution::{
     import::{resolve_imports, ImportDirective},
@@ -90,9 +90,7 @@ pub struct UnresolvedTraitImpl {
     pub the_trait: Option<TraitId>,
     pub trait_path: Path,
     pub methods: UnresolvedFunctions,
-    pub methods_using_default_impl_in_trait: Option<UnresolvedFunctions>,
     pub object_type: UnresolvedType,
-    pub object_type_span: Span,
 }
 
 #[derive(Clone)]
@@ -425,7 +423,7 @@ fn collect_trait_impls(
                         let error = DefCollectorErrorKind::TraitMissingMethod {
                             trait_name: the_trait.name.clone(),
                             method_name: method.name.clone(),
-                            trait_impl_span: trait_impl.object_type_span,
+                            trait_impl_span: trait_impl.object_type.span.expect("type must have a span"),
                         };
                         errors.push(error.into_file_diagnostic(trait_impl.file_id));
                     }
@@ -506,7 +504,7 @@ fn check_trait_impl_crate_coherence(
     };
 
     if Some(current_crate) != trait_crate && Some(current_crate) != object_crate {
-        let error = DefCollectorErrorKind::TraitImplOrphaned { span: trait_impl.object_type_span };
+        let error = DefCollectorErrorKind::TraitImplOrphaned { span: trait_impl.object_type.span.expect("object type must have a span") };
         errors.push(error.into_file_diagnostic(trait_impl.file_id));
     }
 }
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index 014972cddbd..50f6285fc98 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -166,9 +166,7 @@ impl<'a> ModCollector<'a> {
                 trait_path: trait_name,
                 methods: unresolved_functions,
                 object_type: trait_impl.object_type,
-                object_type_span: trait_impl.object_type_span,
-                the_trait: None,                           // will be filled later
-                methods_using_default_impl_in_trait: None, // will be filled later
+                the_trait: None, // will be filled later
             };
 
             self.def_collector.collected_traits_impls.push(unresolved_trait_impl);
diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs
index 37a0abb5581..ccdd25f4cbf 100644
--- a/compiler/noirc_frontend/src/parser/parser.rs
+++ b/compiler/noirc_frontend/src/parser/parser.rs
@@ -559,14 +559,14 @@ fn trait_implementation() -> impl NoirParser<TopLevelStatement> {
         .then(path())
         .then(generic_type_args(parse_type()))
         .then_ignore(keyword(Keyword::For))
-        .then(parse_type().map_with_span(|typ, span| (typ, span)))
+        .then(parse_type())
         .then(where_clause())
         .then_ignore(just(Token::LeftBrace))
         .then(trait_implementation_body())
         .then_ignore(just(Token::RightBrace))
         .validate(|args, span, emit| {
             let ((other_args, where_clause), items) = args;
-            let (((impl_generics, trait_name), trait_generics), (object_type, object_type_span)) =
+            let (((impl_generics, trait_name), trait_generics), object_type) =
                 other_args;
 
             emit(ParserError::with_reason(ParserErrorReason::ExperimentalFeature("Traits"), span));
@@ -575,7 +575,6 @@ fn trait_implementation() -> impl NoirParser<TopLevelStatement> {
                 trait_name,
                 trait_generics,
                 object_type,
-                object_type_span,
                 items,
                 where_clause,
             })

From 76d086e327aa317044c5f83768b5f7790319f948 Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Tue, 26 Sep 2023 10:50:22 +0300
Subject: [PATCH 05/17] fix(traits): correct order of
 UnresolvedTraitImpl::methods

---
 .../src/hir/def_collector/dc_crate.rs         | 43 +++++++++++++------
 .../src/hir/def_collector/dc_mod.rs           |  3 +-
 compiler/noirc_frontend/src/parser/parser.rs  |  3 +-
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index e82d0c3945c..c7e4d41955f 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -89,8 +89,16 @@ pub struct UnresolvedTraitImpl {
     pub module_id: LocalModuleId,
     pub the_trait: Option<TraitId>,
     pub trait_path: Path,
-    pub methods: UnresolvedFunctions,
     pub object_type: UnresolvedType,
+
+    /// List of methods in the impl block, in the order the user defined them. There can be missing methods,
+    /// duplicates, methods not corresponding to a trait method, etc.
+    pub defined_methods: UnresolvedFunctions,
+
+    /// The final list of methods in the impl block. They are in the same order as the methods in the trait,
+    /// e.g. final_methods[2] is the implementation of the_trait.methods[2]. If there is a missing method in
+    /// `defined_methods`, a default implementation will be added here in the correct slot.
+    pub methods: UnresolvedFunctions,
 }
 
 #[derive(Clone)]
@@ -382,19 +390,15 @@ fn collect_trait_impls(
             // set of function ids that have a corresponding method in the trait
             let mut func_ids_in_trait = HashSet::new();
 
-            let mut unresolved_functions =
-                UnresolvedFunctions { file_id: FileId::dummy(), functions: Vec::new() };
-
             for method in &the_trait.methods {
                 let overrides: Vec<_> = trait_impl
-                    .methods
+                    .defined_methods
                     .functions
                     .iter()
                     .filter(|(_, _, f)| f.name() == method.name.0.contents)
                     .collect();
 
                 if overrides.is_empty() {
-                    unresolved_functions.file_id = method.default_impl_file_id;
                     if let Some(default_impl) = &method.default_impl {
                         let method_name = default_impl.name();
                         let func_id = interner.push_empty_fn();
@@ -414,7 +418,7 @@ fn collect_trait_impls(
                             module,
                         );
                         func_ids_in_trait.insert(func_id);
-                        unresolved_functions.push_fn(
+                        trait_impl.methods.push_fn(
                             method.default_impl_module_id,
                             func_id,
                             *default_impl.clone(),
@@ -423,7 +427,10 @@ fn collect_trait_impls(
                         let error = DefCollectorErrorKind::TraitMissingMethod {
                             trait_name: the_trait.name.clone(),
                             method_name: method.name.clone(),
-                            trait_impl_span: trait_impl.object_type.span.expect("type must have a span"),
+                            trait_impl_span: trait_impl
+                                .object_type
+                                .span
+                                .expect("type must have a span"),
                         };
                         errors.push(error.into_file_diagnostic(trait_impl.file_id));
                     }
@@ -431,12 +438,23 @@ fn collect_trait_impls(
                     for (_, func_id, _) in &overrides {
                         func_ids_in_trait.insert(*func_id);
                     }
+
+                    if overrides.len() > 1 {
+                        let error = DefCollectorErrorKind::Duplicate {
+                            typ: DuplicateType::Function,
+                            first_def: overrides[0].2.name_ident().clone(),
+                            second_def: overrides[1].2.name_ident().clone(),
+                        };
+                        errors.push(error.into_file_diagnostic(trait_impl.file_id));
+                    }
+
+                    trait_impl.methods.functions.push(overrides[0].clone());
                 }
             }
 
             // Emit MethodNotInTrait error for methods in the impl block that
             // don't have a corresponding method signature defined in the trait
-            for (_, func_id, func) in &trait_impl.methods.functions {
+            for (_, func_id, func) in &trait_impl.defined_methods.functions {
                 if !func_ids_in_trait.contains(func_id) {
                     let error = DefCollectorErrorKind::MethodNotInTrait {
                         trait_name: the_trait.name.clone(),
@@ -445,9 +463,6 @@ fn collect_trait_impls(
                     errors.push(error.into_file_diagnostic(trait_impl.file_id));
                 }
             }
-
-            //trait_impl.methods_using_default_impl_in_trait = Some(unresolved_functions);
-            trait_impl.methods.functions.append(&mut unresolved_functions.functions);
         }
 
         for (_, func_id, ast) in &trait_impl.methods.functions {
@@ -504,7 +519,9 @@ fn check_trait_impl_crate_coherence(
     };
 
     if Some(current_crate) != trait_crate && Some(current_crate) != object_crate {
-        let error = DefCollectorErrorKind::TraitImplOrphaned { span: trait_impl.object_type.span.expect("object type must have a span") };
+        let error = DefCollectorErrorKind::TraitImplOrphaned {
+            span: trait_impl.object_type.span.expect("object type must have a span"),
+        };
         errors.push(error.into_file_diagnostic(trait_impl.file_id));
     }
 }
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index 50f6285fc98..57e76ee300c 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -164,7 +164,8 @@ impl<'a> ModCollector<'a> {
                 file_id: self.file_id,
                 module_id: self.module_id,
                 trait_path: trait_name,
-                methods: unresolved_functions,
+                defined_methods: unresolved_functions,
+                methods: UnresolvedFunctions { file_id: self.file_id, functions: vec![] },
                 object_type: trait_impl.object_type,
                 the_trait: None, // will be filled later
             };
diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs
index ccdd25f4cbf..be5e89fb2e9 100644
--- a/compiler/noirc_frontend/src/parser/parser.rs
+++ b/compiler/noirc_frontend/src/parser/parser.rs
@@ -566,8 +566,7 @@ fn trait_implementation() -> impl NoirParser<TopLevelStatement> {
         .then_ignore(just(Token::RightBrace))
         .validate(|args, span, emit| {
             let ((other_args, where_clause), items) = args;
-            let (((impl_generics, trait_name), trait_generics), object_type) =
-                other_args;
+            let (((impl_generics, trait_name), trait_generics), object_type) = other_args;
 
             emit(ParserError::with_reason(ParserErrorReason::ExperimentalFeature("Traits"), span));
             TopLevelStatement::TraitImpl(NoirTraitImpl {

From 37db4cda964652f67d1d058b3eec1852fa0e43a0 Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Tue, 26 Sep 2023 11:57:50 +0300
Subject: [PATCH 06/17] test(traits): add test for trait with many methods

---
 .../trait_override_implementation/src/main.nr | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/tooling/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr
index 92e19f97d50..0ca6171ad9e 100644
--- a/tooling/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr
+++ b/tooling/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr
@@ -24,7 +24,30 @@ impl Default for Foo {
     }
 }
 
+trait F {
+    fn f1(self) -> Field;
+    fn f2(self) -> Field { 2 };
+    fn f3(self) -> Field { 3 };
+    fn f4(self) -> Field { 4 };
+    fn f5(self) -> Field { 5 };
+}
+
+struct Bar {}
+
+impl F for Bar {
+    fn f5(self) -> Field { 50 }
+    fn f1(self) -> Field { 10 }
+    fn f3(self) -> Field { 30 }
+}
+
 fn main(x: Field) {
     let first = Foo::method2(x);
     assert(first == 3 * x);
-}
+
+    let bar = Bar{};
+    assert(bar.f1() == 10);
+    assert(bar.f2() == 2);
+    assert(bar.f3() == 30);
+    assert(bar.f4() == 4);
+    assert(bar.f5() == 50);
+}
\ No newline at end of file

From d6569270d365a3b9c82c5d60ecdcabbf33338617 Mon Sep 17 00:00:00 2001
From: Nikolay Nikolov <nickysn@gmail.com>
Date: Tue, 26 Sep 2023 13:01:49 +0300
Subject: [PATCH 07/17] chore(traits): added comment with TODO. No functional
 changes.

---
 compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index c7e4d41955f..7ae28564561 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -699,6 +699,7 @@ fn resolve_trait_methods(
                 .collect();
             let default_impl = if !default_impl_list.is_empty() {
                 if default_impl_list.len() > 1 {
+                    // TODO(nickysn): Add check for method duplicates in the trait and emit proper error messages. This is planned in a future PR.
                     panic!("Too many functions with the same name!");
                 }
                 Some(Box::new(default_impl_list[0].2.clone()))

From cb2f0f485b0d840b8a9c673244a8bc5c04ea8e73 Mon Sep 17 00:00:00 2001
From: Nikolay Nikolov <nickysn@gmail.com>
Date: Tue, 26 Sep 2023 20:33:43 +0300
Subject: [PATCH 08/17] style(traits): fix cargo fmt error

---
 compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index a769312c395..179b083d135 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -576,7 +576,7 @@ fn check_trait_impl_crate_coherence(
     trait_impl: &UnresolvedTraitImpl,
     current_crate: CrateId,
     current_def_map: &CrateDefMap,
-) -> Vec<(CompilationError, FileId)>  {
+) -> Vec<(CompilationError, FileId)> {
     let mut errors: Vec<(CompilationError, FileId)> = vec![];
     let trait_crate = get_path_crate(current_crate, current_def_map, &trait_impl.trait_path);
 

From 67654771d5ffd1b2f344567e836d18dc8abe4104 Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Wed, 27 Sep 2023 09:16:54 +0300
Subject: [PATCH 09/17] chore(traits): store Trait instead of Shared<Trait>

---
 .../noirc_frontend/src/hir/def_collector/dc_crate.rs | 10 +++-------
 .../noirc_frontend/src/hir/resolution/resolver.rs    |  4 ++--
 compiler/noirc_frontend/src/hir/type_check/expr.rs   |  2 --
 compiler/noirc_frontend/src/hir_def/traits.rs        |  8 ++++----
 compiler/noirc_frontend/src/monomorphization/mod.rs  |  1 -
 compiler/noirc_frontend/src/node_interner.rs         | 12 ++++++------
 6 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 179b083d135..43b743c8d66 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -447,7 +447,6 @@ fn collect_trait_impls(
 
         if let Some(trait_id) = trait_impl.the_trait {
             let the_trait = interner.get_trait(trait_id);
-            let the_trait = the_trait.borrow();
 
             // check whether the trait implementation is in the same crate as either the trait or the type
             let current_def_map = def_maps.get_mut(&crate_id).unwrap();
@@ -773,10 +772,8 @@ fn resolve_trait_methods(
         } = item
         {
             let the_trait = interner.get_trait(trait_id);
-            let self_type = Type::TypeVariable(
-                the_trait.borrow().self_type_typevar.clone(),
-                TypeVariableKind::Normal,
-            );
+            let self_type =
+                Type::TypeVariable(the_trait.self_type_typevar.clone(), TypeVariableKind::Normal);
 
             let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
             resolver.set_self_type(Some(self_type));
@@ -1028,8 +1025,7 @@ fn check_methods_signatures(
     trait_id: TraitId,
     errors: &mut Vec<(CompilationError, FileId)>,
 ) {
-    let the_trait_shared = resolver.interner.get_trait(trait_id);
-    let the_trait = the_trait_shared.borrow();
+    let the_trait = resolver.interner.get_trait(trait_id);
 
     let self_type = resolver.get_self_type().expect("trait impl must have a Self type");
 
diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs
index 6add92fb86d..3ddb3b9efa9 100644
--- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs
+++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs
@@ -1375,8 +1375,8 @@ impl<'a> Resolver<'a> {
         self.interner.get_struct(type_id)
     }
 
-    pub fn get_trait(&self, type_id: TraitId) -> Shared<Trait> {
-        self.interner.get_trait(type_id)
+    pub fn get_trait(&self, trait_id: TraitId) -> Trait {
+        self.interner.get_trait(trait_id)
     }
 
     fn lookup<T: TryFromModuleDefId>(&mut self, path: Path) -> Result<T, ResolverError> {
diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs
index 920f2071586..16e8f2d4200 100644
--- a/compiler/noirc_frontend/src/hir/type_check/expr.rs
+++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs
@@ -500,7 +500,6 @@ impl<'interner> TypeChecker<'interner> {
             }
             HirMethodReference::TraitMethodId(method) => {
                 let the_trait = self.interner.get_trait(method.trait_id);
-                let the_trait = the_trait.borrow();
                 let method = &the_trait.methods[method.method_index];
 
                 (method.get_type(), method.arguments.len())
@@ -859,7 +858,6 @@ impl<'interner> TypeChecker<'interner> {
                 for constraint in func_meta.trait_constraints {
                     if *object_type == constraint.typ {
                         let the_trait = self.interner.get_trait(constraint.trait_id);
-                        let the_trait = the_trait.borrow();
 
                         for (method_index, method) in the_trait.methods.iter().enumerate() {
                             if method.name.0.contents == method_name {
diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs
index b3585297a97..b51405122cd 100644
--- a/compiler/noirc_frontend/src/hir_def/traits.rs
+++ b/compiler/noirc_frontend/src/hir_def/traits.rs
@@ -5,7 +5,7 @@ use crate::{
 };
 use noirc_errors::Span;
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TraitFunction {
     pub name: Ident,
     pub generics: Generics,
@@ -17,14 +17,14 @@ pub struct TraitFunction {
     pub default_impl_module_id: crate::hir::def_map::LocalModuleId,
 }
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TraitConstant {
     pub name: Ident,
     pub ty: Type,
     pub span: Span,
 }
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TraitType {
     pub name: Ident,
     pub ty: Type,
@@ -34,7 +34,7 @@ pub struct TraitType {
 /// Represents a trait in the type system. Each instance of this struct
 /// will be shared across all Type::Trait variants that represent
 /// the same trait.
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub struct Trait {
     /// A unique id representing this trait type. Used to check if two
     /// struct traits are equal.
diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs
index 07b60ae48e9..0a62b71f105 100644
--- a/compiler/noirc_frontend/src/monomorphization/mod.rs
+++ b/compiler/noirc_frontend/src/monomorphization/mod.rs
@@ -819,7 +819,6 @@ impl<'interner> Monomorphizer<'interner> {
         };
 
         let the_trait = self.interner.get_trait(method.trait_id);
-        let the_trait = the_trait.borrow();
 
         ast::Expression::Ident(ast::Ident {
             definition: Definition::Function(func_id),
diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs
index 54d6d9d8163..d28892711e0 100644
--- a/compiler/noirc_frontend/src/node_interner.rs
+++ b/compiler/noirc_frontend/src/node_interner.rs
@@ -87,7 +87,7 @@ pub struct NodeInterner {
     //
     // TODO: We may be able to remove the Shared wrapper once traits are no longer types.
     // We'd just lookup their methods as needed through the NodeInterner.
-    traits: HashMap<TraitId, Shared<Trait>>,
+    traits: HashMap<TraitId, Trait>,
 
     // Trait implementation map
     // For each type that implements a given Trait ( corresponding TraitId), there should be an entry here
@@ -416,7 +416,7 @@ impl NodeInterner {
 
         self.traits.insert(
             type_id,
-            Shared::new(Trait::new(
+            Trait::new(
                 type_id,
                 typ.trait_def.name.clone(),
                 typ.crate_id,
@@ -431,7 +431,7 @@ impl NodeInterner {
                 }),
                 self_type_typevar_id,
                 self_type_typevar,
-            )),
+            ),
         );
     }
 
@@ -483,8 +483,8 @@ impl NodeInterner {
     }
 
     pub fn update_trait(&mut self, trait_id: TraitId, f: impl FnOnce(&mut Trait)) {
-        let mut value = self.traits.get_mut(&trait_id).unwrap().borrow_mut();
-        f(&mut value);
+        let value = self.traits.get_mut(&trait_id).unwrap();
+        f(value);
     }
 
     pub fn set_type_alias(&mut self, type_id: TypeAliasId, typ: Type, generics: Generics) {
@@ -752,7 +752,7 @@ impl NodeInterner {
         self.structs[&id].clone()
     }
 
-    pub fn get_trait(&self, id: TraitId) -> Shared<Trait> {
+    pub fn get_trait(&self, id: TraitId) -> Trait {
         self.traits[&id].clone()
     }
 

From 58d545f24edc913ee44631c5c705d6d4cc4a6667 Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Wed, 27 Sep 2023 11:13:54 +0300
Subject: [PATCH 10/17] chore(traits): remove defined_functions

---
 .../src/hir/def_collector/dc_crate.rs         | 24 +++++++++----------
 .../src/hir/def_collector/dc_mod.rs           |  3 +--
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 43b743c8d66..7cc7f07f17a 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -92,14 +92,6 @@ pub struct UnresolvedTraitImpl {
     pub the_trait: Option<TraitId>,
     pub trait_path: Path,
     pub object_type: UnresolvedType,
-
-    /// List of methods in the impl block, in the order the user defined them. There can be missing methods,
-    /// duplicates, methods not corresponding to a trait method, etc.
-    pub defined_methods: UnresolvedFunctions,
-
-    /// The final list of methods in the impl block. They are in the same order as the methods in the trait,
-    /// e.g. final_methods[2] is the implementation of the_trait.methods[2]. If there is a missing method in
-    /// `defined_methods`, a default implementation will be added here in the correct slot.
     pub methods: UnresolvedFunctions,
 }
 
@@ -445,6 +437,10 @@ fn collect_trait_impls(
             }
         };
 
+        // In this Vec methods[i] corresponds to trait.methods[i]. If the impl has no implementation
+        // for a particular method, the default implementation will be added at that slot.
+        let mut ordered_methods = Vec::new();
+
         if let Some(trait_id) = trait_impl.the_trait {
             let the_trait = interner.get_trait(trait_id);
 
@@ -457,7 +453,7 @@ fn collect_trait_impls(
 
             for method in &the_trait.methods {
                 let overrides: Vec<_> = trait_impl
-                    .defined_methods
+                    .methods
                     .functions
                     .iter()
                     .filter(|(_, _, f)| f.name() == method.name.0.contents)
@@ -483,11 +479,11 @@ fn collect_trait_impls(
                             module,
                         );
                         func_ids_in_trait.insert(func_id);
-                        trait_impl.methods.push_fn(
+                        ordered_methods.push((
                             method.default_impl_module_id,
                             func_id,
                             *default_impl.clone(),
-                        );
+                        ));
                     } else {
                         let error = DefCollectorErrorKind::TraitMissingMethod {
                             trait_name: the_trait.name.clone(),
@@ -513,13 +509,13 @@ fn collect_trait_impls(
                         errors.push((error.into(), trait_impl.file_id));
                     }
 
-                    trait_impl.methods.functions.push(overrides[0].clone());
+                    ordered_methods.push(overrides[0].clone());
                 }
             }
 
             // Emit MethodNotInTrait error for methods in the impl block that
             // don't have a corresponding method signature defined in the trait
-            for (_, func_id, func) in &trait_impl.defined_methods.functions {
+            for (_, func_id, func) in &trait_impl.methods.functions {
                 if !func_ids_in_trait.contains(func_id) {
                     let error = DefCollectorErrorKind::MethodNotInTrait {
                         trait_name: the_trait.name.clone(),
@@ -530,6 +526,8 @@ fn collect_trait_impls(
             }
         }
 
+        trait_impl.methods.functions = ordered_methods;
+
         for (_, func_id, ast) in &trait_impl.methods.functions {
             let file = def_maps[&crate_id].file_id(module_id);
 
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index 2451cdc40b4..fb53cb8114e 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -168,8 +168,7 @@ impl<'a> ModCollector<'a> {
                 file_id: self.file_id,
                 module_id: self.module_id,
                 trait_path: trait_name,
-                defined_methods: unresolved_functions,
-                methods: UnresolvedFunctions { file_id: self.file_id, functions: vec![] },
+                methods: unresolved_functions,
                 object_type: trait_impl.object_type,
                 the_trait: None, // will be filled later
             };

From bee0f58263ffb99609048b65d41c901786053e8f Mon Sep 17 00:00:00 2001
From: Nikolay Nikolov <nickysn@gmail.com>
Date: Wed, 27 Sep 2023 13:43:35 +0300
Subject: [PATCH 11/17] fix(traits): removed duplicated code, added
 accidentally during merge conflict resolution

---
 .../src/hir/def_collector/dc_mod.rs           | 28 -------------------
 1 file changed, 28 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index fb53cb8114e..577a4c1ac6b 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -392,34 +392,6 @@ impl<'a> ModCollector<'a> {
                 }
             }
 
-            // Add all functions that have a default implementation in the trait
-            let mut unresolved_functions =
-                UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() };
-            for trait_item in &trait_definition.items {
-                // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
-                if let TraitItem::Function {
-                    name,
-                    generics,
-                    parameters,
-                    return_type,
-                    where_clause,
-                    body: Some(body),
-                } = trait_item
-                {
-                    let func_id = context.def_interner.push_empty_fn();
-
-                    let impl_method = NoirFunction::normal(FunctionDefinition::normal(
-                        name,
-                        generics,
-                        parameters,
-                        body,
-                        where_clause,
-                        return_type,
-                    ));
-                    unresolved_functions.push_fn(self.module_id, func_id, impl_method);
-                }
-            }
-
             // And store the TraitId -> TraitType mapping somewhere it is reachable
             let unresolved = UnresolvedTrait {
                 file_id: self.file_id,

From 8e1d8ddeaf746cbc3505d2b4b1d2ae6da40eada5 Mon Sep 17 00:00:00 2001
From: Nikolay Nikolov <nickysn@gmail.com>
Date: Wed, 27 Sep 2023 14:56:45 +0300
Subject: [PATCH 12/17] refactor(traits): extracted method collect_trait_impl.
 No functional changes.

---
 .../src/hir/def_collector/dc_crate.rs         | 251 +++++++++---------
 1 file changed, 131 insertions(+), 120 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 7cc7f07f17a..194d5ca5944 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -413,159 +413,170 @@ fn collect_impls(
     errors
 }
 
-fn collect_trait_impls(
+fn collect_trait_impl(
     context: &mut Context,
     crate_id: CrateId,
-    collected_impls: &mut [UnresolvedTraitImpl],
+    trait_impl: &mut UnresolvedTraitImpl,
 ) -> Vec<(CompilationError, FileId)> {
     let interner = &mut context.def_interner;
     let def_maps = &mut context.def_maps;
     let mut errors: Vec<(CompilationError, FileId)> = vec![];
 
-    for trait_impl in collected_impls.iter_mut() {
-        let module_id = trait_impl.module_id;
-        let unresolved_type = trait_impl.object_type.clone();
+    let module_id = trait_impl.module_id;
+    let unresolved_type = trait_impl.object_type.clone();
 
-        let module = ModuleId { local_id: module_id, krate: crate_id };
-        let resolve_result = resolve_trait_by_path(def_maps, module, trait_impl.trait_path.clone());
+    let module = ModuleId { local_id: module_id, krate: crate_id };
+    let resolve_result = resolve_trait_by_path(def_maps, module, trait_impl.trait_path.clone());
 
-        trait_impl.the_trait = match resolve_result {
-            Ok(trait_id) => Some(trait_id),
-            Err(error) => {
-                errors.push((error.into(), trait_impl.file_id));
-                None
-            }
-        };
+    trait_impl.the_trait = match resolve_result {
+        Ok(trait_id) => Some(trait_id),
+        Err(error) => {
+            errors.push((error.into(), trait_impl.file_id));
+            None
+        }
+    };
 
-        // In this Vec methods[i] corresponds to trait.methods[i]. If the impl has no implementation
-        // for a particular method, the default implementation will be added at that slot.
-        let mut ordered_methods = Vec::new();
+    // In this Vec methods[i] corresponds to trait.methods[i]. If the impl has no implementation
+    // for a particular method, the default implementation will be added at that slot.
+    let mut ordered_methods = Vec::new();
 
-        if let Some(trait_id) = trait_impl.the_trait {
-            let the_trait = interner.get_trait(trait_id);
+    if let Some(trait_id) = trait_impl.the_trait {
+        let the_trait = interner.get_trait(trait_id);
 
-            // check whether the trait implementation is in the same crate as either the trait or the type
-            let current_def_map = def_maps.get_mut(&crate_id).unwrap();
-            errors.extend(check_trait_impl_crate_coherence(trait_impl, crate_id, current_def_map));
-
-            // set of function ids that have a corresponding method in the trait
-            let mut func_ids_in_trait = HashSet::new();
-
-            for method in &the_trait.methods {
-                let overrides: Vec<_> = trait_impl
-                    .methods
-                    .functions
-                    .iter()
-                    .filter(|(_, _, f)| f.name() == method.name.0.contents)
-                    .collect();
-
-                if overrides.is_empty() {
-                    if let Some(default_impl) = &method.default_impl {
-                        let method_name = default_impl.name();
-                        let func_id = interner.push_empty_fn();
-
-                        let modifiers = FunctionModifiers {
-                            name: method_name.to_owned(),
-                            visibility: crate::Visibility::Public,
-                            attributes: Attributes::empty(),
-                            is_unconstrained: false,
-                            contract_function_type: None,
-                            is_internal: None,
-                        };
-                        interner.push_function_definition(
-                            method_name.to_string(),
-                            func_id,
-                            modifiers,
-                            module,
-                        );
-                        func_ids_in_trait.insert(func_id);
-                        ordered_methods.push((
-                            method.default_impl_module_id,
-                            func_id,
-                            *default_impl.clone(),
-                        ));
-                    } else {
-                        let error = DefCollectorErrorKind::TraitMissingMethod {
-                            trait_name: the_trait.name.clone(),
-                            method_name: method.name.clone(),
-                            trait_impl_span: trait_impl
-                                .object_type
-                                .span
-                                .expect("type must have a span"),
-                        };
-                        errors.push((error.into(), trait_impl.file_id));
-                    }
-                } else {
-                    for (_, func_id, _) in &overrides {
-                        func_ids_in_trait.insert(*func_id);
-                    }
+        // check whether the trait implementation is in the same crate as either the trait or the type
+        let current_def_map = def_maps.get_mut(&crate_id).unwrap();
+        errors.extend(check_trait_impl_crate_coherence(trait_impl, crate_id, current_def_map));
 
-                    if overrides.len() > 1 {
-                        let error = DefCollectorErrorKind::Duplicate {
-                            typ: DuplicateType::Function,
-                            first_def: overrides[0].2.name_ident().clone(),
-                            second_def: overrides[1].2.name_ident().clone(),
-                        };
-                        errors.push((error.into(), trait_impl.file_id));
-                    }
+        // set of function ids that have a corresponding method in the trait
+        let mut func_ids_in_trait = HashSet::new();
 
-                    ordered_methods.push(overrides[0].clone());
-                }
-            }
+        for method in &the_trait.methods {
+            let overrides: Vec<_> = trait_impl
+                .methods
+                .functions
+                .iter()
+                .filter(|(_, _, f)| f.name() == method.name.0.contents)
+                .collect();
 
-            // Emit MethodNotInTrait error for methods in the impl block that
-            // don't have a corresponding method signature defined in the trait
-            for (_, func_id, func) in &trait_impl.methods.functions {
-                if !func_ids_in_trait.contains(func_id) {
-                    let error = DefCollectorErrorKind::MethodNotInTrait {
+            if overrides.is_empty() {
+                if let Some(default_impl) = &method.default_impl {
+                    let method_name = default_impl.name();
+                    let func_id = interner.push_empty_fn();
+
+                    let modifiers = FunctionModifiers {
+                        name: method_name.to_owned(),
+                        visibility: crate::Visibility::Public,
+                        attributes: Attributes::empty(),
+                        is_unconstrained: false,
+                        contract_function_type: None,
+                        is_internal: None,
+                    };
+                    interner.push_function_definition(
+                        method_name.to_string(),
+                        func_id,
+                        modifiers,
+                        module,
+                    );
+                    func_ids_in_trait.insert(func_id);
+                    ordered_methods.push((
+                        method.default_impl_module_id,
+                        func_id,
+                        *default_impl.clone(),
+                    ));
+                } else {
+                    let error = DefCollectorErrorKind::TraitMissingMethod {
                         trait_name: the_trait.name.clone(),
-                        impl_method: func.name_ident().clone(),
+                        method_name: method.name.clone(),
+                        trait_impl_span: trait_impl
+                            .object_type
+                            .span
+                            .expect("type must have a span"),
+                    };
+                    errors.push((error.into(), trait_impl.file_id));
+                }
+            } else {
+                for (_, func_id, _) in &overrides {
+                    func_ids_in_trait.insert(*func_id);
+                }
+
+                if overrides.len() > 1 {
+                    let error = DefCollectorErrorKind::Duplicate {
+                        typ: DuplicateType::Function,
+                        first_def: overrides[0].2.name_ident().clone(),
+                        second_def: overrides[1].2.name_ident().clone(),
                     };
                     errors.push((error.into(), trait_impl.file_id));
                 }
+
+                ordered_methods.push(overrides[0].clone());
             }
         }
 
-        trait_impl.methods.functions = ordered_methods;
+        // Emit MethodNotInTrait error for methods in the impl block that
+        // don't have a corresponding method signature defined in the trait
+        for (_, func_id, func) in &trait_impl.methods.functions {
+            if !func_ids_in_trait.contains(func_id) {
+                let error = DefCollectorErrorKind::MethodNotInTrait {
+                    trait_name: the_trait.name.clone(),
+                    impl_method: func.name_ident().clone(),
+                };
+                errors.push((error.into(), trait_impl.file_id));
+            }
+        }
+    }
 
-        for (_, func_id, ast) in &trait_impl.methods.functions {
-            let file = def_maps[&crate_id].file_id(module_id);
+    trait_impl.methods.functions = ordered_methods;
 
-            let path_resolver = StandardPathResolver::new(module);
-            let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
-            resolver.add_generics(&ast.def.generics);
-            let typ = resolver.resolve_type(unresolved_type.clone());
+    for (_, func_id, ast) in &trait_impl.methods.functions {
+        let file = def_maps[&crate_id].file_id(module_id);
 
-            // Add the method to the struct's namespace
-            if let Some(struct_type) = get_struct_type(&typ) {
-                errors.extend(
-                    resolver.take_errors().iter().cloned().map(|e| (e.into(), trait_impl.file_id)),
-                );
+        let path_resolver = StandardPathResolver::new(module);
+        let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
+        resolver.add_generics(&ast.def.generics);
+        let typ = resolver.resolve_type(unresolved_type.clone());
 
-                let struct_type = struct_type.borrow();
-                let type_module = struct_type.id.local_module_id();
+        // Add the method to the struct's namespace
+        if let Some(struct_type) = get_struct_type(&typ) {
+            errors.extend(
+                resolver.take_errors().iter().cloned().map(|e| (e.into(), trait_impl.file_id)),
+            );
 
-                let module = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0];
+            let struct_type = struct_type.borrow();
+            let type_module = struct_type.id.local_module_id();
 
-                let result = module.declare_function(ast.name_ident().clone(), *func_id);
+            let module = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0];
 
-                if let Err((first_def, second_def)) = result {
-                    let err = DefCollectorErrorKind::Duplicate {
-                        typ: DuplicateType::Function,
-                        first_def,
-                        second_def,
-                    };
-                    errors.push((err.into(), trait_impl.file_id));
-                }
-            } else {
-                let error = DefCollectorErrorKind::NonStructTraitImpl {
-                    trait_path: trait_impl.trait_path.clone(),
-                    span: trait_impl.trait_path.span(),
+            let result = module.declare_function(ast.name_ident().clone(), *func_id);
+
+            if let Err((first_def, second_def)) = result {
+                let err = DefCollectorErrorKind::Duplicate {
+                    typ: DuplicateType::Function,
+                    first_def,
+                    second_def,
                 };
-                errors.push((error.into(), trait_impl.file_id));
+                errors.push((err.into(), trait_impl.file_id));
             }
+        } else {
+            let error = DefCollectorErrorKind::NonStructTraitImpl {
+                trait_path: trait_impl.trait_path.clone(),
+                span: trait_impl.trait_path.span(),
+            };
+            errors.push((error.into(), trait_impl.file_id));
         }
     }
+
+    errors
+}
+
+fn collect_trait_impls(
+    context: &mut Context,
+    crate_id: CrateId,
+    collected_impls: &mut [UnresolvedTraitImpl],
+) -> Vec<(CompilationError, FileId)> {
+    let mut errors: Vec<(CompilationError, FileId)> = vec![];
+    for trait_impl in collected_impls.iter_mut() {
+        errors.append(&mut collect_trait_impl(context, crate_id, trait_impl));
+    }
     errors
 }
 

From dcffa0711a98c51613fad2c5dbeba74a50ac6ae2 Mon Sep 17 00:00:00 2001
From: Yordan Madzhunkov <ymadzhunkov@gmail.com>
Date: Wed, 27 Sep 2023 17:13:19 +0300
Subject: [PATCH 13/17] Fix PR commit with simplification

---
 .../src/hir/def_collector/dc_mod.rs           | 22 ++++---------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index 577a4c1ac6b..7e6c9c4c60b 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -6,9 +6,8 @@ use noirc_errors::Location;
 use crate::{
     graph::CrateId,
     hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
-    node_interner::{FunctionModifiers, TraitId},
+    node_interner::TraitId,
     parser::SubModule,
-    token::Attributes,
     FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl,
     NoirTypeAlias, ParsedModule, TraitImplItem, TraitItem, TypeImpl,
 };
@@ -145,23 +144,10 @@ impl<'a> ModCollector<'a> {
             let unresolved_functions =
                 self.collect_trait_impl_function_overrides(context, &trait_impl, krate);
 
+            let module = ModuleId { krate, local_id: self.module_id };
+
             for (_, func_id, noir_function) in &unresolved_functions.functions {
-                let name = noir_function.name().to_owned();
-
-                let modifiers = FunctionModifiers {
-                    name: name.clone(),
-                    visibility: crate::Visibility::Public,
-                    attributes: Attributes::empty(),
-                    is_unconstrained: false,
-                    contract_function_type: None,
-                    is_internal: None,
-                };
-                context.def_interner.push_function_definition(
-                    name,
-                    *func_id,
-                    modifiers,
-                    ModuleId::dummy_id(),
-                );
+                context.def_interner.push_function(*func_id, &noir_function.def, module);
             }
 
             let unresolved_trait_impl = UnresolvedTraitImpl {

From db7909a16e2861d43a704a9f5c4766f295c2d982 Mon Sep 17 00:00:00 2001
From: Yordan Madzhunkov <ymadzhunkov@gmail.com>
Date: Thu, 28 Sep 2023 12:07:23 +0300
Subject: [PATCH 14/17] Try to simplify collect_trait_impl

---
 .../src/hir/def_collector/dc_crate.rs         | 283 +++++++++---------
 .../src/hir/def_collector/dc_mod.rs           |   2 +-
 2 files changed, 147 insertions(+), 138 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 194d5ca5944..c19a5bd0cba 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -14,12 +14,11 @@ use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
 use crate::hir::Context;
 use crate::hir_def::traits::{TraitConstant, TraitFunction, TraitImpl, TraitType};
 use crate::node_interner::{
-    FuncId, FunctionModifiers, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
+    FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
 };
 
 use crate::parser::ParserError;
 
-use crate::token::Attributes;
 use crate::{
     ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait,
     NoirTypeAlias, ParsedModule, Path, Shared, StructType, TraitItem, Type, TypeBinding,
@@ -89,7 +88,7 @@ pub struct UnresolvedTrait {
 pub struct UnresolvedTraitImpl {
     pub file_id: FileId,
     pub module_id: LocalModuleId,
-    pub the_trait: Option<TraitId>,
+    pub trait_id: Option<TraitId>,
     pub trait_path: Path,
     pub object_type: UnresolvedType,
     pub methods: UnresolvedFunctions,
@@ -413,156 +412,167 @@ fn collect_impls(
     errors
 }
 
-fn collect_trait_impl(
-    context: &mut Context,
-    crate_id: CrateId,
+fn collect_trait_impl_methods(
+    //context: &mut Context,
+    //crate_id: CrateId,
+    interner: &mut NodeInterner,
+    current_def_map: &mut CrateDefMap,
+    trait_id: TraitId,
     trait_impl: &mut UnresolvedTraitImpl,
 ) -> Vec<(CompilationError, FileId)> {
-    let interner = &mut context.def_interner;
-    let def_maps = &mut context.def_maps;
-    let mut errors: Vec<(CompilationError, FileId)> = vec![];
-
-    let module_id = trait_impl.module_id;
-    let unresolved_type = trait_impl.object_type.clone();
-
-    let module = ModuleId { local_id: module_id, krate: crate_id };
-    let resolve_result = resolve_trait_by_path(def_maps, module, trait_impl.trait_path.clone());
-
-    trait_impl.the_trait = match resolve_result {
-        Ok(trait_id) => Some(trait_id),
-        Err(error) => {
-            errors.push((error.into(), trait_impl.file_id));
-            None
-        }
-    };
+    let crate_id = current_def_map.krate();
 
     // In this Vec methods[i] corresponds to trait.methods[i]. If the impl has no implementation
     // for a particular method, the default implementation will be added at that slot.
     let mut ordered_methods = Vec::new();
 
-    if let Some(trait_id) = trait_impl.the_trait {
-        let the_trait = interner.get_trait(trait_id);
-
-        // check whether the trait implementation is in the same crate as either the trait or the type
-        let current_def_map = def_maps.get_mut(&crate_id).unwrap();
-        errors.extend(check_trait_impl_crate_coherence(trait_impl, crate_id, current_def_map));
-
-        // set of function ids that have a corresponding method in the trait
-        let mut func_ids_in_trait = HashSet::new();
-
-        for method in &the_trait.methods {
-            let overrides: Vec<_> = trait_impl
-                .methods
-                .functions
-                .iter()
-                .filter(|(_, _, f)| f.name() == method.name.0.contents)
-                .collect();
-
-            if overrides.is_empty() {
-                if let Some(default_impl) = &method.default_impl {
-                    let method_name = default_impl.name();
-                    let func_id = interner.push_empty_fn();
-
-                    let modifiers = FunctionModifiers {
-                        name: method_name.to_owned(),
-                        visibility: crate::Visibility::Public,
-                        attributes: Attributes::empty(),
-                        is_unconstrained: false,
-                        contract_function_type: None,
-                        is_internal: None,
-                    };
-                    interner.push_function_definition(
-                        method_name.to_string(),
-                        func_id,
-                        modifiers,
-                        module,
-                    );
-                    func_ids_in_trait.insert(func_id);
-                    ordered_methods.push((
-                        method.default_impl_module_id,
-                        func_id,
-                        *default_impl.clone(),
-                    ));
-                } else {
-                    let error = DefCollectorErrorKind::TraitMissingMethod {
-                        trait_name: the_trait.name.clone(),
-                        method_name: method.name.clone(),
-                        trait_impl_span: trait_impl
-                            .object_type
-                            .span
-                            .expect("type must have a span"),
-                    };
-                    errors.push((error.into(), trait_impl.file_id));
-                }
+    let the_trait = interner.get_trait(trait_id);
+
+    // check whether the trait implementation is in the same crate as either the trait or the type
+    let mut errors = check_trait_impl_crate_coherence(trait_impl, crate_id, current_def_map);
+
+    // set of function ids that have a corresponding method in the trait
+    let mut func_ids_in_trait = HashSet::new();
+
+    for method in &the_trait.methods {
+        let overrides: Vec<_> = trait_impl
+            .methods
+            .functions
+            .iter()
+            .filter(|(_, _, f)| f.name() == method.name.0.contents)
+            .collect();
+
+        if overrides.is_empty() {
+            if let Some(default_impl) = &method.default_impl {
+                let func_id = interner.push_empty_fn();
+                let module = ModuleId { local_id: trait_impl.module_id, krate: crate_id };
+                interner.push_function(func_id, &default_impl.def, module);
+                func_ids_in_trait.insert(func_id);
+                ordered_methods.push((
+                    method.default_impl_module_id,
+                    func_id,
+                    *default_impl.clone(),
+                ));
             } else {
-                for (_, func_id, _) in &overrides {
-                    func_ids_in_trait.insert(*func_id);
-                }
-
-                if overrides.len() > 1 {
-                    let error = DefCollectorErrorKind::Duplicate {
-                        typ: DuplicateType::Function,
-                        first_def: overrides[0].2.name_ident().clone(),
-                        second_def: overrides[1].2.name_ident().clone(),
-                    };
-                    errors.push((error.into(), trait_impl.file_id));
-                }
-
-                ordered_methods.push(overrides[0].clone());
+                let error = DefCollectorErrorKind::TraitMissingMethod {
+                    trait_name: the_trait.name.clone(),
+                    method_name: method.name.clone(),
+                    trait_impl_span: trait_impl.object_type.span.expect("type must have a span"),
+                };
+                errors.push((error.into(), trait_impl.file_id));
+            }
+        } else {
+            for (_, func_id, _) in &overrides {
+                func_ids_in_trait.insert(*func_id);
             }
-        }
 
-        // Emit MethodNotInTrait error for methods in the impl block that
-        // don't have a corresponding method signature defined in the trait
-        for (_, func_id, func) in &trait_impl.methods.functions {
-            if !func_ids_in_trait.contains(func_id) {
-                let error = DefCollectorErrorKind::MethodNotInTrait {
-                    trait_name: the_trait.name.clone(),
-                    impl_method: func.name_ident().clone(),
+            if overrides.len() > 1 {
+                let error = DefCollectorErrorKind::Duplicate {
+                    typ: DuplicateType::Function,
+                    first_def: overrides[0].2.name_ident().clone(),
+                    second_def: overrides[1].2.name_ident().clone(),
                 };
                 errors.push((error.into(), trait_impl.file_id));
             }
+
+            ordered_methods.push(overrides[0].clone());
         }
     }
 
+    // Emit MethodNotInTrait error for methods in the impl block that
+    // don't have a corresponding method signature defined in the trait
+    for (_, func_id, func) in &trait_impl.methods.functions {
+        if !func_ids_in_trait.contains(func_id) {
+            let error = DefCollectorErrorKind::MethodNotInTrait {
+                trait_name: the_trait.name.clone(),
+                impl_method: func.name_ident().clone(),
+            };
+            errors.push((error.into(), trait_impl.file_id));
+        }
+    }
     trait_impl.methods.functions = ordered_methods;
+    errors
+}
 
-    for (_, func_id, ast) in &trait_impl.methods.functions {
-        let file = def_maps[&crate_id].file_id(module_id);
-
-        let path_resolver = StandardPathResolver::new(module);
-        let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
-        resolver.add_generics(&ast.def.generics);
-        let typ = resolver.resolve_type(unresolved_type.clone());
-
-        // Add the method to the struct's namespace
-        if let Some(struct_type) = get_struct_type(&typ) {
-            errors.extend(
-                resolver.take_errors().iter().cloned().map(|e| (e.into(), trait_impl.file_id)),
-            );
-
-            let struct_type = struct_type.borrow();
-            let type_module = struct_type.id.local_module_id();
-
-            let module = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0];
+fn add_method_to_struct_namespace(
+    current_def_map: &mut CrateDefMap,
+    struct_type: &Shared<StructType>,
+    func_id: FuncId,
+    name_ident: &Ident,
+) -> Result<(), DefCollectorErrorKind> {
+    let struct_type = struct_type.borrow();
+    let type_module = struct_type.id.local_module_id();
+    let module = &mut current_def_map.modules[type_module.0];
+    module.declare_function(name_ident.clone(), func_id).map_err(|(first_def, second_def)| {
+        DefCollectorErrorKind::Duplicate { typ: DuplicateType::Function, first_def, second_def }
+    })
+}
 
-            let result = module.declare_function(ast.name_ident().clone(), *func_id);
+fn collect_trait_impl(
+    context: &mut Context,
+    crate_id: CrateId,
+    trait_impl: &mut UnresolvedTraitImpl,
+) -> Vec<(CompilationError, FileId)> {
+    let interner = &mut context.def_interner;
+    let def_maps = &mut context.def_maps;
+    let mut errors: Vec<(CompilationError, FileId)> = vec![];
+    let unresolved_type = trait_impl.object_type.clone();
+    let module = ModuleId { local_id: trait_impl.module_id, krate: crate_id };
+    trait_impl.trait_id =
+        match resolve_trait_by_path(def_maps, module, trait_impl.trait_path.clone()) {
+            Ok(trait_id) => Some(trait_id),
+            Err(error) => {
+                errors.push((error.into(), trait_impl.file_id));
+                None
+            }
+        };
 
-            if let Err((first_def, second_def)) = result {
-                let err = DefCollectorErrorKind::Duplicate {
-                    typ: DuplicateType::Function,
-                    first_def,
-                    second_def,
-                };
-                errors.push((err.into(), trait_impl.file_id));
+    match trait_impl.trait_id {
+        Some(trait_id) => {
+            errors.extend(collect_trait_impl_methods(
+                interner,
+                def_maps.get_mut(&crate_id).unwrap(),
+                trait_id,
+                trait_impl,
+            ));
+            for (_, func_id, ast) in &trait_impl.methods.functions {
+                let file = def_maps[&crate_id].file_id(trait_impl.module_id);
+
+                let path_resolver = StandardPathResolver::new(module);
+                let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
+                resolver.add_generics(&ast.def.generics);
+                let typ = resolver.resolve_type(unresolved_type.clone());
+                let resolver_errors: Vec<_> = resolver
+                    .take_errors()
+                    .iter()
+                    .cloned()
+                    .map(|e| (e.into(), trait_impl.file_id))
+                    .collect();
+
+                if let Some(struct_type) = get_struct_type(&typ) {
+                    errors.extend(resolver_errors);
+                    let current_def_map = def_maps.get_mut(&crate_id).unwrap();
+                    match add_method_to_struct_namespace(
+                        current_def_map,
+                        struct_type,
+                        *func_id,
+                        ast.name_ident(),
+                    ) {
+                        Ok(()) => {}
+                        Err(err) => {
+                            errors.push((err.into(), trait_impl.file_id));
+                        }
+                    }
+                } else {
+                    let error = DefCollectorErrorKind::NonStructTraitImpl {
+                        trait_path: trait_impl.trait_path.clone(),
+                        span: trait_impl.trait_path.span(),
+                    };
+                    errors.push((error.into(), trait_impl.file_id));
+                }
             }
-        } else {
-            let error = DefCollectorErrorKind::NonStructTraitImpl {
-                trait_path: trait_impl.trait_path.clone(),
-                span: trait_impl.trait_path.span(),
-            };
-            errors.push((error.into(), trait_impl.file_id));
         }
+        None => {}
     }
 
     errors
@@ -573,11 +583,10 @@ fn collect_trait_impls(
     crate_id: CrateId,
     collected_impls: &mut [UnresolvedTraitImpl],
 ) -> Vec<(CompilationError, FileId)> {
-    let mut errors: Vec<(CompilationError, FileId)> = vec![];
-    for trait_impl in collected_impls.iter_mut() {
-        errors.append(&mut collect_trait_impl(context, crate_id, trait_impl));
-    }
-    errors
+    collected_impls
+        .iter_mut()
+        .flat_map(|trait_impl| collect_trait_impl(context, crate_id, trait_impl))
+        .collect()
 }
 
 fn check_trait_impl_crate_coherence(
@@ -983,7 +992,7 @@ fn resolve_trait_impls(
             resolver.resolve_type(unresolved_type.clone())
         };
 
-        let maybe_trait_id = trait_impl.the_trait;
+        let maybe_trait_id = trait_impl.trait_id;
 
         let mut impl_methods = resolve_function_set(
             interner,
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
index 7e6c9c4c60b..189cfaa1569 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -156,7 +156,7 @@ impl<'a> ModCollector<'a> {
                 trait_path: trait_name,
                 methods: unresolved_functions,
                 object_type: trait_impl.object_type,
-                the_trait: None, // will be filled later
+                trait_id: None, // will be filled later
             };
 
             self.def_collector.collected_traits_impls.push(unresolved_trait_impl);

From 78262494be05d6df3d6b16b93bdb16cc453d274d Mon Sep 17 00:00:00 2001
From: Yordan Madzhunkov <ymadzhunkov@gmail.com>
Date: Thu, 28 Sep 2023 12:18:59 +0300
Subject: [PATCH 15/17] More simplification

---
 .../src/hir/def_collector/dc_crate.rs         | 20 +++++++------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index c19a5bd0cba..5ca65a3d3ed 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -367,9 +367,7 @@ fn collect_impls(
             resolver.add_generics(generics);
             let typ = resolver.resolve_type(unresolved_type.clone());
 
-            errors.extend(
-                resolver.take_errors().iter().cloned().map(|e| (e.into(), unresolved.file_id)),
-            );
+            errors.extend(take_errors(unresolved.file_id, resolver));
 
             if let Some(struct_type) = get_struct_type(&typ) {
                 let struct_type = struct_type.borrow();
@@ -413,8 +411,6 @@ fn collect_impls(
 }
 
 fn collect_trait_impl_methods(
-    //context: &mut Context,
-    //crate_id: CrateId,
     interner: &mut NodeInterner,
     current_def_map: &mut CrateDefMap,
     trait_id: TraitId,
@@ -542,15 +538,9 @@ fn collect_trait_impl(
                 let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
                 resolver.add_generics(&ast.def.generics);
                 let typ = resolver.resolve_type(unresolved_type.clone());
-                let resolver_errors: Vec<_> = resolver
-                    .take_errors()
-                    .iter()
-                    .cloned()
-                    .map(|e| (e.into(), trait_impl.file_id))
-                    .collect();
 
                 if let Some(struct_type) = get_struct_type(&typ) {
-                    errors.extend(resolver_errors);
+                    errors.extend(take_errors(trait_impl.file_id, resolver));
                     let current_def_map = def_maps.get_mut(&crate_id).unwrap();
                     match add_method_to_struct_namespace(
                         current_def_map,
@@ -680,7 +670,7 @@ fn resolve_globals(
         let name = global.stmt_def.pattern.name_ident().clone();
 
         let hir_stmt = resolver.resolve_global_let(global.stmt_def);
-        errors.extend(resolver.take_errors().iter().cloned().map(|e| (e.into(), global.file_id)));
+        errors.extend(take_errors(global.file_id, resolver));
 
         context.def_interner.update_global(global.stmt_id, hir_stmt);
 
@@ -854,6 +844,10 @@ fn take_errors_filter_self_not_resolved(
         .collect()
 }
 
+fn take_errors(file_id: FileId, resolver: Resolver<'_>) -> Vec<(CompilationError, FileId)> {
+    resolver.take_errors().iter().cloned().map(|e| (e.into(), file_id)).collect()
+}
+
 /// Create the mappings from TypeId -> TraitType
 /// so that expressions can access the elements of traits
 fn resolve_traits(

From b8b9a81cbe399848d12fd68e811458088e8e242d Mon Sep 17 00:00:00 2001
From: Alex Vitkov <alexvitkov98@gmail.com>
Date: Thu, 28 Sep 2023 10:27:48 +0300
Subject: [PATCH 16/17] fix(traits): dont use paths to check for crate

---
 .../src/hir/def_collector/dc_crate.rs         | 47 +++++++++----------
 .../src/hir/resolution/import.rs              | 15 ------
 2 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 5ca65a3d3ed..9799ca9007f 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -3,7 +3,7 @@ use super::errors::{DefCollectorErrorKind, DuplicateType};
 use crate::graph::CrateId;
 use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId};
 use crate::hir::resolution::errors::ResolverError;
-use crate::hir::resolution::import::{get_path_crate, PathResolutionError};
+use crate::hir::resolution::import::PathResolutionError;
 use crate::hir::resolution::path_resolver::PathResolver;
 use crate::hir::resolution::resolver::Resolver;
 use crate::hir::resolution::{
@@ -12,7 +12,7 @@ use crate::hir::resolution::{
 };
 use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
 use crate::hir::Context;
-use crate::hir_def::traits::{TraitConstant, TraitFunction, TraitImpl, TraitType};
+use crate::hir_def::traits::{Trait, TraitConstant, TraitFunction, TraitImpl, TraitType};
 use crate::node_interner::{
     FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
 };
@@ -412,12 +412,11 @@ fn collect_impls(
 
 fn collect_trait_impl_methods(
     interner: &mut NodeInterner,
-    current_def_map: &mut CrateDefMap,
+    def_maps: &mut BTreeMap<CrateId, CrateDefMap>,
+    crate_id: CrateId,
     trait_id: TraitId,
     trait_impl: &mut UnresolvedTraitImpl,
 ) -> Vec<(CompilationError, FileId)> {
-    let crate_id = current_def_map.krate();
-
     // In this Vec methods[i] corresponds to trait.methods[i]. If the impl has no implementation
     // for a particular method, the default implementation will be added at that slot.
     let mut ordered_methods = Vec::new();
@@ -425,8 +424,8 @@ fn collect_trait_impl_methods(
     let the_trait = interner.get_trait(trait_id);
 
     // check whether the trait implementation is in the same crate as either the trait or the type
-    let mut errors = check_trait_impl_crate_coherence(trait_impl, crate_id, current_def_map);
-
+    let mut errors =
+        check_trait_impl_crate_coherence(interner, &the_trait, trait_impl, crate_id, def_maps);
     // set of function ids that have a corresponding method in the trait
     let mut func_ids_in_trait = HashSet::new();
 
@@ -523,13 +522,10 @@ fn collect_trait_impl(
             }
         };
 
-    match trait_impl.trait_id {
-        Some(trait_id) => {
+    
+    if let Some(trait_id) = trait_impl.trait_id {
             errors.extend(collect_trait_impl_methods(
-                interner,
-                def_maps.get_mut(&crate_id).unwrap(),
-                trait_id,
-                trait_impl,
+                interner, def_maps, crate_id, trait_id, trait_impl,
             ));
             for (_, func_id, ast) in &trait_impl.methods.functions {
                 let file = def_maps[&crate_id].file_id(trait_impl.module_id);
@@ -548,7 +544,7 @@ fn collect_trait_impl(
                         *func_id,
                         ast.name_ident(),
                     ) {
-                        Ok(()) => {}
+                        Ok(()) => {},
                         Err(err) => {
                             errors.push((err.into(), trait_impl.file_id));
                         }
@@ -562,9 +558,6 @@ fn collect_trait_impl(
                 }
             }
         }
-        None => {}
-    }
-
     errors
 }
 
@@ -580,21 +573,25 @@ fn collect_trait_impls(
 }
 
 fn check_trait_impl_crate_coherence(
+    interner: &mut NodeInterner,
+    the_trait: &Trait,
     trait_impl: &UnresolvedTraitImpl,
     current_crate: CrateId,
-    current_def_map: &CrateDefMap,
+    def_maps: &BTreeMap<CrateId, CrateDefMap>,
 ) -> Vec<(CompilationError, FileId)> {
     let mut errors: Vec<(CompilationError, FileId)> = vec![];
-    let trait_crate = get_path_crate(current_crate, current_def_map, &trait_impl.trait_path);
 
-    let object_crate = match &trait_impl.object_type.typ {
-        crate::UnresolvedTypeData::Named(path, _) => {
-            get_path_crate(current_crate, current_def_map, path)
-        }
-        _ => None,
+    let module = ModuleId { krate: current_crate, local_id: trait_impl.module_id };
+    let file = def_maps[&current_crate].file_id(trait_impl.module_id);
+    let path_resolver = StandardPathResolver::new(module);
+    let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
+
+    let object_crate = match resolver.resolve_type(trait_impl.object_type.clone()) {
+        Type::Struct(struct_type, _) => struct_type.borrow().id.krate(),
+        _ => CrateId::Dummy,
     };
 
-    if Some(current_crate) != trait_crate && Some(current_crate) != object_crate {
+    if current_crate != the_trait.crate_id && current_crate != object_crate {
         let error = DefCollectorErrorKind::TraitImplOrphaned {
             span: trait_impl.object_type.span.expect("object type must have a span"),
         };
diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs
index 7b57da1c687..6f3140a65d4 100644
--- a/compiler/noirc_frontend/src/hir/resolution/import.rs
+++ b/compiler/noirc_frontend/src/hir/resolution/import.rs
@@ -210,18 +210,3 @@ fn resolve_external_dep(
 
     resolve_path_to_ns(&dep_directive, dep_def_map, def_maps, allow_contracts)
 }
-
-pub fn get_path_crate(
-    current_crate: CrateId,
-    current_def_map: &CrateDefMap,
-    path: &Path,
-) -> Option<CrateId> {
-    match path.kind {
-        PathKind::Dep => {
-            let crate_name = &path.segments.first().unwrap();
-            current_def_map.extern_prelude.get(&crate_name.0.contents).map(|module| module.krate)
-        }
-        PathKind::Crate => Some(current_crate),
-        PathKind::Plain => Some(current_crate),
-    }
-}

From 0231aae6e73392abbad1bc42cdb32f533b809d45 Mon Sep 17 00:00:00 2001
From: Yordan Madzhunkov <ymadzhunkov@gmail.com>
Date: Thu, 28 Sep 2023 14:39:11 +0300
Subject: [PATCH 17/17] Apply cargo fmt --all

---
 .../src/hir/def_collector/dc_crate.rs         | 60 +++++++++----------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
index 9799ca9007f..86369a66758 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
@@ -522,42 +522,40 @@ fn collect_trait_impl(
             }
         };
 
-    
     if let Some(trait_id) = trait_impl.trait_id {
-            errors.extend(collect_trait_impl_methods(
-                interner, def_maps, crate_id, trait_id, trait_impl,
-            ));
-            for (_, func_id, ast) in &trait_impl.methods.functions {
-                let file = def_maps[&crate_id].file_id(trait_impl.module_id);
-
-                let path_resolver = StandardPathResolver::new(module);
-                let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
-                resolver.add_generics(&ast.def.generics);
-                let typ = resolver.resolve_type(unresolved_type.clone());
-
-                if let Some(struct_type) = get_struct_type(&typ) {
-                    errors.extend(take_errors(trait_impl.file_id, resolver));
-                    let current_def_map = def_maps.get_mut(&crate_id).unwrap();
-                    match add_method_to_struct_namespace(
-                        current_def_map,
-                        struct_type,
-                        *func_id,
-                        ast.name_ident(),
-                    ) {
-                        Ok(()) => {},
-                        Err(err) => {
-                            errors.push((err.into(), trait_impl.file_id));
-                        }
+        errors
+            .extend(collect_trait_impl_methods(interner, def_maps, crate_id, trait_id, trait_impl));
+        for (_, func_id, ast) in &trait_impl.methods.functions {
+            let file = def_maps[&crate_id].file_id(trait_impl.module_id);
+
+            let path_resolver = StandardPathResolver::new(module);
+            let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
+            resolver.add_generics(&ast.def.generics);
+            let typ = resolver.resolve_type(unresolved_type.clone());
+
+            if let Some(struct_type) = get_struct_type(&typ) {
+                errors.extend(take_errors(trait_impl.file_id, resolver));
+                let current_def_map = def_maps.get_mut(&crate_id).unwrap();
+                match add_method_to_struct_namespace(
+                    current_def_map,
+                    struct_type,
+                    *func_id,
+                    ast.name_ident(),
+                ) {
+                    Ok(()) => {}
+                    Err(err) => {
+                        errors.push((err.into(), trait_impl.file_id));
                     }
-                } else {
-                    let error = DefCollectorErrorKind::NonStructTraitImpl {
-                        trait_path: trait_impl.trait_path.clone(),
-                        span: trait_impl.trait_path.span(),
-                    };
-                    errors.push((error.into(), trait_impl.file_id));
                 }
+            } else {
+                let error = DefCollectorErrorKind::NonStructTraitImpl {
+                    trait_path: trait_impl.trait_path.clone(),
+                    span: trait_impl.trait_path.span(),
+                };
+                errors.push((error.into(), trait_impl.file_id));
             }
         }
+    }
     errors
 }