Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(traits): allow multiple traits to share the same associated function name and to be implemented for the same type #3126

Merged
merged 9 commits into from
Oct 12, 2023
10 changes: 6 additions & 4 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -550,6 +551,7 @@ fn collect_trait_impl(
struct_type,
*func_id,
ast.name_ident(),
trait_id,
) {
Ok(()) => {}
Err(err) => {
Expand Down
106 changes: 88 additions & 18 deletions compiler/noirc_frontend/src/hir/def_map/item_scope.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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>,
}
Expand All @@ -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(())
}
Expand All @@ -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),
Expand All @@ -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
}

Expand Down
38 changes: 26 additions & 12 deletions compiler/noirc_frontend/src/hir/def_map/module_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "trait_associated_member_names_clashes"
type = "bin"
authors = [""]
compiler_version = "0.16.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -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);
}
Loading