feat(traits): allow multiple traits to share the same associated func…

…tion name and to be implemented for the same type (#3126)

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

@@ -499,17 +499,18 @@ fn add_method_to_struct_namespace(
     struct_type: &Shared<StructType>,
     func_id: FuncId,
     name_ident: &Ident,
+    trait_id: TraitId,
 ) -> 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 {
+    module.declare_trait_function(name_ident.clone(), func_id, trait_id).map_err(
+        |(first_def, second_def)| DefCollectorErrorKind::Duplicate {
             typ: DuplicateType::TraitImplementation,
             first_def,
             second_def,
-        }
-    })
+        },
+    )
 }
 
 fn collect_trait_impl(
@@ -550,6 +551,7 @@ fn collect_trait_impl(
                     struct_type,
                     *func_id,
                     ast.name_ident(),
+                    trait_id,
                 ) {
                     Ok(()) => {}
                     Err(err) => {

compiler/noirc_frontend/src/hir/def_map/item_scope.rs

@@ -1,5 +1,8 @@
 use super::{namespace::PerNs, ModuleDefId, ModuleId};
-use crate::{node_interner::FuncId, Ident};
+use crate::{
+    node_interner::{FuncId, TraitId},
+    Ident,
+};
 use std::collections::{hash_map::Entry, HashMap};
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
@@ -9,8 +12,8 @@ pub enum Visibility {
 
 #[derive(Default, Debug, PartialEq, Eq)]
 pub struct ItemScope {
-    types: HashMap<Ident, (ModuleDefId, Visibility)>,
-    values: HashMap<Ident, (ModuleDefId, Visibility)>,
+    types: HashMap<Ident, HashMap<Option<TraitId>, (ModuleDefId, Visibility)>>,
+    values: HashMap<Ident, HashMap<Option<TraitId>, (ModuleDefId, Visibility)>>,
 
     defs: Vec<ModuleDefId>,
 }
@@ -20,8 +23,9 @@ impl ItemScope {
         &mut self,
         name: Ident,
         mod_def: ModuleDefId,
+        trait_id: Option<TraitId>,
     ) -> Result<(), (Ident, Ident)> {
-        self.add_item_to_namespace(name, mod_def)?;
+        self.add_item_to_namespace(name, mod_def, trait_id)?;
         self.defs.push(mod_def);
         Ok(())
     }
@@ -33,16 +37,26 @@ impl ItemScope {
         &mut self,
         name: Ident,
         mod_def: ModuleDefId,
+        trait_id: Option<TraitId>,
     ) -> Result<(), (Ident, Ident)> {
-        let add_item = |map: &mut HashMap<Ident, (ModuleDefId, Visibility)>| {
-            if let Entry::Occupied(o) = map.entry(name.clone()) {
-                let old_ident = o.key();
-                Err((old_ident.clone(), name))
-            } else {
-                map.insert(name, (mod_def, Visibility::Public));
-                Ok(())
-            }
-        };
+        let add_item =
+            |map: &mut HashMap<Ident, HashMap<Option<TraitId>, (ModuleDefId, Visibility)>>| {
+                if let Entry::Occupied(mut o) = map.entry(name.clone()) {
+                    let trait_hashmap = o.get_mut();
+                    if let Entry::Occupied(_) = trait_hashmap.entry(trait_id) {
+                        let old_ident = o.key();
+                        Err((old_ident.clone(), name))
+                    } else {
+                        trait_hashmap.insert(trait_id, (mod_def, Visibility::Public));
+                        Ok(())
+                    }
+                } else {
+                    let mut trait_hashmap = HashMap::new();
+                    trait_hashmap.insert(trait_id, (mod_def, Visibility::Public));
+                    map.insert(name, trait_hashmap);
+                    Ok(())
+                }
+            };
 
         match mod_def {
             ModuleDefId::ModuleId(_) => add_item(&mut self.types),
@@ -55,34 +69,90 @@ impl ItemScope {
     }
 
     pub fn find_module_with_name(&self, mod_name: &Ident) -> Option<&ModuleId> {
-        let (module_def, _) = self.types.get(mod_name)?;
+        let (module_def, _) = self.types.get(mod_name)?.get(&None)?;
         match module_def {
             ModuleDefId::ModuleId(id) => Some(id),
             _ => None,
         }
     }
 
     pub fn find_func_with_name(&self, func_name: &Ident) -> Option<FuncId> {
-        let (module_def, _) = self.values.get(func_name)?;
+        let trait_hashmap = self.values.get(func_name)?;
+        // methods introduced without trait take priority and hide methods with the same name that come from a trait
+        let a = trait_hashmap.get(&None);
+        match a {
+            Some((module_def, _)) => match module_def {
+                ModuleDefId::FunctionId(id) => Some(*id),
+                _ => None,
+            },
+            None => {
+                if trait_hashmap.len() == 1 {
+                    let (module_def, _) = trait_hashmap.get(trait_hashmap.keys().last()?)?;
+                    match module_def {
+                        ModuleDefId::FunctionId(id) => Some(*id),
+                        _ => None,
+                    }
+                } else {
+                    // ambiguous name (multiple traits, containing the same function name)
+                    None
+                }
+            }
+        }
+    }
+
+    pub fn find_func_with_name_and_trait_id(
+        &self,
+        func_name: &Ident,
+        trait_id: &Option<TraitId>,
+    ) -> Option<FuncId> {
+        let (module_def, _) = self.values.get(func_name)?.get(trait_id)?;
         match module_def {
             ModuleDefId::FunctionId(id) => Some(*id),
             _ => None,
         }
     }
 
     pub fn find_name(&self, name: &Ident) -> PerNs {
-        PerNs { types: self.types.get(name).cloned(), values: self.values.get(name).cloned() }
+        // Names, not associated with traits are searched first. If not found, we search for name, coming from a trait.
+        // If we find only one name from trait, we return it. If there are multiple traits, providing the same name, we return None.
+        let find_name_in =
+            |a: &HashMap<Ident, HashMap<Option<TraitId>, (ModuleDefId, Visibility)>>| {
+                if let Some(t) = a.get(name) {
+                    if let Some(tt) = t.get(&None) {
+                        Some(*tt)
+                    } else if t.len() == 1 {
+                        t.values().last().cloned()
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            };
+
+        PerNs { types: find_name_in(&self.types), values: find_name_in(&self.values) }
+    }
+
+    pub fn find_name_for_trait_id(&self, name: &Ident, trait_id: &Option<TraitId>) -> PerNs {
+        PerNs {
+            types: if let Some(t) = self.types.get(name) { t.get(trait_id).cloned() } else { None },
+            values: if let Some(v) = self.values.get(name) {
+                v.get(trait_id).cloned()
+            } else {
+                None
+            },
+        }
     }
 
     pub fn definitions(&self) -> Vec<ModuleDefId> {
         self.defs.clone()
     }
 
-    pub fn types(&self) -> &HashMap<Ident, (ModuleDefId, Visibility)> {
+    pub fn types(&self) -> &HashMap<Ident, HashMap<Option<TraitId>, (ModuleDefId, Visibility)>> {
         &self.types
     }
 
-    pub fn values(&self) -> &HashMap<Ident, (ModuleDefId, Visibility)> {
+    pub fn values(&self) -> &HashMap<Ident, HashMap<Option<TraitId>, (ModuleDefId, Visibility)>> {
         &self.values
     }
 

compiler/noirc_frontend/src/hir/def_map/module_data.rs

@@ -41,16 +41,30 @@ impl ModuleData {
         }
     }
 
-    fn declare(&mut self, name: Ident, item_id: ModuleDefId) -> Result<(), (Ident, Ident)> {
-        self.scope.add_definition(name.clone(), item_id)?;
+    fn declare(
+        &mut self,
+        name: Ident,
+        item_id: ModuleDefId,
+        trait_id: Option<TraitId>,
+    ) -> Result<(), (Ident, Ident)> {
+        self.scope.add_definition(name.clone(), item_id, trait_id)?;
 
         // definitions is a subset of self.scope so it is expected if self.scope.define_func_def
         // returns without error, so will self.definitions.define_func_def.
-        self.definitions.add_definition(name, item_id)
+        self.definitions.add_definition(name, item_id, trait_id)
     }
 
     pub fn declare_function(&mut self, name: Ident, id: FuncId) -> Result<(), (Ident, Ident)> {
-        self.declare(name, id.into())
+        self.declare(name, id.into(), None)
+    }
+
+    pub fn declare_trait_function(
+        &mut self,
+        name: Ident,
+        id: FuncId,
+        trait_id: TraitId,
+    ) -> Result<(), (Ident, Ident)> {
+        self.declare(name, id.into(), Some(trait_id))
     }
 
     pub fn remove_function(&mut self, name: &Ident) {
@@ -59,52 +73,52 @@ impl ModuleData {
     }
 
     pub fn declare_global(&mut self, name: Ident, id: StmtId) -> Result<(), (Ident, Ident)> {
-        self.declare(name, id.into())
+        self.declare(name, id.into(), None)
     }
 
     pub fn declare_struct(&mut self, name: Ident, id: StructId) -> Result<(), (Ident, Ident)> {
-        self.declare(name, ModuleDefId::TypeId(id))
+        self.declare(name, ModuleDefId::TypeId(id), None)
     }
 
     pub fn declare_type_alias(
         &mut self,
         name: Ident,
         id: TypeAliasId,
     ) -> Result<(), (Ident, Ident)> {
-        self.declare(name, id.into())
+        self.declare(name, id.into(), None)
     }
 
     pub fn declare_trait(&mut self, name: Ident, id: TraitId) -> Result<(), (Ident, Ident)> {
-        self.declare(name, ModuleDefId::TraitId(id))
+        self.declare(name, ModuleDefId::TraitId(id), None)
     }
 
     pub fn declare_child_module(
         &mut self,
         name: Ident,
         child_id: ModuleId,
     ) -> Result<(), (Ident, Ident)> {
-        self.declare(name, child_id.into())
+        self.declare(name, child_id.into(), None)
     }
 
     pub fn find_func_with_name(&self, name: &Ident) -> Option<FuncId> {
         self.scope.find_func_with_name(name)
     }
 
     pub fn import(&mut self, name: Ident, id: ModuleDefId) -> Result<(), (Ident, Ident)> {
-        self.scope.add_item_to_namespace(name, id)
+        self.scope.add_item_to_namespace(name, id, None)
     }
 
     pub fn find_name(&self, name: &Ident) -> PerNs {
         self.scope.find_name(name)
     }
 
     pub fn type_definitions(&self) -> impl Iterator<Item = ModuleDefId> + '_ {
-        self.definitions.types().values().map(|(id, _)| *id)
+        self.definitions.types().values().flat_map(|a| a.values().map(|(id, _)| *id))
     }
 
     /// Return an iterator over all definitions defined within this module,
     /// excluding any type definitions.
     pub fn value_definitions(&self) -> impl Iterator<Item = ModuleDefId> + '_ {
-        self.definitions.values().values().map(|(id, _)| *id)
+        self.definitions.values().values().flat_map(|a| a.values().map(|(id, _)| *id))
     }
 }

tooling/nargo_cli/tests/execution_success/trait_associated_member_names_clashes/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "trait_associated_member_names_clashes"
+type = "bin"
+authors = [""]
+compiler_version = "0.16.0"
+
+[dependencies]

tooling/nargo_cli/tests/execution_success/trait_associated_member_names_clashes/src/main.nr

@@ -0,0 +1,36 @@
+trait Trait1 {
+    fn tralala() -> Field;
+}
+
+trait Trait2 {
+    fn tralala() -> Field;
+}
+
+struct Struct1 {
+}
+
+impl Struct1 {
+    fn tralala() -> Field {
+        123456
+    }
+}
+
+impl Trait1 for Struct1 {
+    fn tralala() -> Field {
+        111111
+    }
+}
+
+impl Trait2 for Struct1 {
+    fn tralala() -> Field {
+        222222
+    }
+}
+
+fn main() {
+    // the struct impl takes priority over trait methods
+    assert(Struct1::tralala() == 123456);
+    // TODO: uncomment these, once we support the <Type as Trait>:: syntax
+    //assert(<Struct1 as Trait1>::tralala() == 111111);
+    //assert(<Struct1 as Trait2>::tralala() == 222222);
+}