diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index ae2cf224cbd..431bed3b604 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1722,8 +1722,18 @@ impl NodeInterner { let instantiated_object_type = object_type.substitute(&substitutions); let trait_generics = &trait_impl.borrow().trait_generics; + + // Replace any associated types with fresh type variables so that we match + // any existing impl regardless of associated types if one already exists. + // E.g. if we already have an `impl Foo for Baz`, we should + // reject `impl Foo for Baz` if it were to be added. let associated_types = self.get_associated_types_for_impl(impl_id); + let associated_types = vecmap(associated_types, |named| { + let typ = self.next_type_variable(); + NamedType { name: named.name.clone(), typ } + }); + // Ignoring overlapping `TraitImplKind::Assumed` impls here is perfectly fine. // It should never happen since impls are defined at global scope, but even // if they were, we should never prevent defining a new impl because a 'where' @@ -1732,7 +1742,7 @@ impl NodeInterner { &instantiated_object_type, trait_id, trait_generics, - associated_types, + &associated_types, ) { let existing_impl = self.get_trait_implementation(existing); let existing_impl = existing_impl.borrow(); diff --git a/compiler/noirc_frontend/src/tests/traits.rs b/compiler/noirc_frontend/src/tests/traits.rs index 8d3a63f65cf..11cdb95391f 100644 --- a/compiler/noirc_frontend/src/tests/traits.rs +++ b/compiler/noirc_frontend/src/tests/traits.rs @@ -1,4 +1,5 @@ use crate::hir::def_collector::dc_crate::CompilationError; +use crate::hir::def_collector::errors::DefCollectorErrorKind; use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::import::PathResolutionError; use crate::hir::type_check::TypeCheckError; @@ -1237,6 +1238,64 @@ fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_on assert_eq!(trait_name, "private_mod::Foo"); } +#[test] +fn error_on_duplicate_impl_with_associated_type() { + let src = r#" + trait Foo { + type Bar; + } + + impl Foo for i32 { + type Bar = u32; + } + + impl Foo for i32 { + type Bar = u8; + } + + fn main() {} + "#; + + // Expect "Impl for type `i32` overlaps with existing impl" + // and "Previous impl defined here" + let errors = get_program_errors(src); + assert_eq!(errors.len(), 2); + + use CompilationError::DefinitionError; + use DefCollectorErrorKind::*; + assert!(matches!(&errors[0].0, DefinitionError(OverlappingImpl { .. }))); + assert!(matches!(&errors[1].0, DefinitionError(OverlappingImplNote { .. }))); +} + +#[test] +fn error_on_duplicate_impl_with_associated_constant() { + let src = r#" + trait Foo { + let Bar: u32; + } + + impl Foo for i32 { + let Bar = 5; + } + + impl Foo for i32 { + let Bar = 6; + } + + fn main() {} + "#; + + // Expect "Impl for type `i32` overlaps with existing impl" + // and "Previous impl defined here" + let errors = get_program_errors(src); + assert_eq!(errors.len(), 2); + + use CompilationError::DefinitionError; + use DefCollectorErrorKind::*; + assert!(matches!(&errors[0].0, DefinitionError(OverlappingImpl { .. }))); + assert!(matches!(&errors[1].0, DefinitionError(OverlappingImplNote { .. }))); +} + // See https://github.com/noir-lang/noir/issues/6530 #[test] fn regression_6530() {