diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 37ad74b78b0..aee3b5c5807 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -232,23 +232,20 @@ impl<'context> Elaborator<'context> { // Must resolve structs before we resolve globals. this.collect_struct_definitions(items.types); - // Bind trait impls to their trait. Collect trait functions, that have a - // default implementation, which hasn't been overridden. - for trait_impl in &mut items.trait_impls { - this.collect_trait_impl(trait_impl); - } - // Before we resolve any function symbols we must go through our impls and // re-collect the methods within into their proper module. This cannot be // done during def collection since we need to be able to resolve the type of // the impl since that determines the module we should collect into. - // - // These are resolved after trait impls so that struct methods are chosen - // over trait methods if there are name conflicts. for ((_self_type, module), impls) in &mut items.impls { this.collect_impls(*module, impls); } + // Bind trait impls to their trait. Collect trait functions, that have a + // default implementation, which hasn't been overridden. + for trait_impl in &mut items.trait_impls { + this.collect_trait_impl(trait_impl); + } + // We must wait to resolve non-literal globals until after we resolve structs since struct // globals will need to reference the struct type they're initialized to to ensure they are valid. while let Some((_, global)) = this.unresolved_globals.pop_first() { @@ -860,9 +857,12 @@ impl<'context> Elaborator<'context> { self.self_type = None; } - fn get_module_mut(&mut self, module: ModuleId) -> &mut ModuleData { + fn get_module_mut( + def_maps: &mut BTreeMap, + module: ModuleId, + ) -> &mut ModuleData { let message = "A crate should always be present for a given crate id"; - &mut self.def_maps.get_mut(&module.krate).expect(message).modules[module.local_id.0] + &mut def_maps.get_mut(&module.krate).expect(message).modules[module.local_id.0] } fn declare_methods_on_struct( @@ -890,7 +890,7 @@ impl<'context> Elaborator<'context> { // Grab the module defined by the struct type. Note that impls are a case // where the module the methods are added to is not the same as the module // they are resolved in. - let module = self.get_module_mut(struct_ref.id.module_id()); + let module = Self::get_module_mut(self.def_maps, struct_ref.id.module_id()); for (_, method_id, method) in &functions.functions { // If this method was already declared, remove it from the module so it cannot @@ -899,22 +899,37 @@ impl<'context> Elaborator<'context> { // If not, that is specialization which is allowed. let name = method.name_ident().clone(); if module.declare_function(name, ItemVisibility::Public, *method_id).is_err() { - module.remove_function(method.name_ident()); + let existing = module.find_func_with_name(method.name_ident()).expect( + "declare_function should only error if there is an existing function", + ); + + // Only remove the existing function from scope if it is from a trait impl as + // well. If it is from a non-trait impl that should override trait impl methods + // anyway so that Foo::bar always resolves to the non-trait impl version. + if self.interner.function_meta(&existing).trait_impl.is_some() { + module.remove_function(method.name_ident()); + } } } - self.declare_struct_methods(self_type, &function_ids); + // Trait impl methods are already declared in NodeInterner::add_trait_implementation + if !is_trait_impl { + self.declare_methods(self_type, &function_ids); + } // We can define methods on primitive types only if we're in the stdlib } else if !is_trait_impl && *self_type != Type::Error { if self.crate_id.is_stdlib() { - self.declare_struct_methods(self_type, &function_ids); + // Trait impl methods are already declared in NodeInterner::add_trait_implementation + if !is_trait_impl { + self.declare_methods(self_type, &function_ids); + } } else { self.push_err(DefCollectorErrorKind::NonStructTypeInImpl { span }); } } } - fn declare_struct_methods(&mut self, self_type: &Type, function_ids: &[FuncId]) { + fn declare_methods(&mut self, self_type: &Type, function_ids: &[FuncId]) { for method_id in function_ids { let method_name = self.interner.function_name(method_id).to_owned(); diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index cef49332b00..6c5f9b6bbcb 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1466,6 +1466,7 @@ impl NodeInterner { force_type_check: bool, ) -> Option { let methods = self.struct_methods.get(&(id, method_name.to_owned())); + // If there is only one method, just return it immediately. // It will still be typechecked later. if !force_type_check { diff --git a/test_programs/compile_success_empty/no_duplicate_methods/Nargo.toml b/test_programs/compile_success_empty/no_duplicate_methods/Nargo.toml new file mode 100644 index 00000000000..2125d475530 --- /dev/null +++ b/test_programs/compile_success_empty/no_duplicate_methods/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "no_duplicate_methods" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/compile_success_empty/no_duplicate_methods/Prover.toml b/test_programs/compile_success_empty/no_duplicate_methods/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/compile_success_empty/no_duplicate_methods/src/main.nr b/test_programs/compile_success_empty/no_duplicate_methods/src/main.nr new file mode 100644 index 00000000000..2be1d3fa11e --- /dev/null +++ b/test_programs/compile_success_empty/no_duplicate_methods/src/main.nr @@ -0,0 +1,26 @@ +// Test that declaring several methods & trait methods with the same name +// does not trigger a duplicate method error +trait ToField { + fn to_field(self) -> Field; +} +trait ToField2 { + fn to_field(self) -> Field; +} + +struct Foo { x: Field } + +impl ToField for Foo { + fn to_field(self) -> Field { self.x } +} + +impl ToField2 for Foo { + fn to_field(self) -> Field { self.x } +} + +impl Foo { + fn to_field(self) -> Field { + self.x + } +} + +fn main() {}