fix(elaborator): Check that trait constraints aren't unifying unbound associated types from different traits

compiler/noirc_errors/src/position.rs

@@ -63,7 +63,7 @@ impl<T> Located<T> {
     }
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
+#[derive(Copy, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
 pub struct Location {
     pub span: Span,
     pub file: FileId,
@@ -103,3 +103,9 @@ impl PartialOrd for Location {
         Some(self.cmp(other))
     }
 }
+
+impl std::fmt::Debug for Location {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}/{}:{}", self.file.as_usize(), self.span.start(), self.span.end())
+    }
+}

compiler/noirc_frontend/src/elaborator/traits.rs

@@ -616,7 +616,7 @@ impl Elaborator<'_> {
                 }
 
                 let parent_trait_bound =
-                    self.instantiate_parent_trait_bound(trait_bound, &parent_trait_bound);
+                    self.instantiate_parent_trait_bound(object, trait_bound, &parent_trait_bound);
                 self.add_trait_bound_to_scope(
                     location,
                     object,

compiler/noirc_frontend/src/elaborator/types.rs

@@ -2295,6 +2295,7 @@ impl Elaborator<'_> {
                 let the_trait = self.interner.get_trait(trait_id);
                 let constraint = the_trait.as_constraint(the_trait.name.location());
                 let mut matches = self.lookup_methods_in_trait(
+                    object_type,
                     the_trait,
                     method_name,
                     &constraint.trait_bound,
@@ -2331,6 +2332,7 @@ impl Elaborator<'_> {
                     self.interner.try_get_trait(constraint.trait_bound.trait_id)
                 {
                     let trait_matches = self.lookup_methods_in_trait(
+                        object_type,
                         the_trait,
                         method_name,
                         &constraint.trait_bound,
@@ -2392,6 +2394,7 @@ impl Elaborator<'_> {
     /// a child and its parent.
     fn lookup_methods_in_trait(
         &self,
+        object: &Type,
         the_trait: &Trait,
         method_name: &str,
         trait_bound: &ResolvedTraitBound,
@@ -2420,8 +2423,9 @@ impl Elaborator<'_> {
                 }
 
                 let parent_trait_bound =
-                    self.instantiate_parent_trait_bound(trait_bound, parent_trait_bound);
+                    self.instantiate_parent_trait_bound(object, trait_bound, parent_trait_bound);
                 let parent_matches = self.lookup_methods_in_trait(
+                    object,
                     the_trait,
                     method_name,
                     &parent_trait_bound,
@@ -2650,13 +2654,17 @@ impl Elaborator<'_> {
         (expr_location, empty_function)
     }
 
+    /// Insert the ordered generics and associated types from the trait bound.
+    /// If the constraint is `assumed`, it also inserts the binding to the `Self`
+    /// type to whatever type the constraint is defined on.
     pub fn bind_generics_from_trait_constraint(
         &self,
+        object: &Type,
         constraint: &TraitConstraint,
         assumed: bool,
         bindings: &mut TypeBindings,
     ) {
-        self.bind_generics_from_trait_bound(&constraint.trait_bound, bindings);
+        self.bind_generics_from_trait_bound(object, &constraint.trait_bound, bindings);
 
         // If the trait impl is already assumed to exist we should add any type bindings for `Self`.
         // Otherwise `self` will be replaced with a fresh type variable, which will require the user
@@ -2669,26 +2677,38 @@ impl Elaborator<'_> {
         }
     }
 
+    /// Insert the ordered generics and associated types from the trait bound.
     pub fn bind_generics_from_trait_bound(
         &self,
+        object: &Type,
         trait_bound: &ResolvedTraitBound,
         bindings: &mut TypeBindings,
     ) {
         let the_trait = self.interner.get_trait(trait_bound.trait_id);
 
         bind_ordered_generics(&the_trait.generics, &trait_bound.trait_generics.ordered, bindings);
 
-        let associated_types = the_trait.associated_types.clone();
-        bind_named_generics(associated_types, &trait_bound.trait_generics.named, bindings);
+        let associated_types = vecmap(&the_trait.associated_types, |typ| {
+            let is_const = the_trait.associated_constant_ids.contains_key(typ.name.as_str());
+            (typ.clone(), is_const)
+        });
+        bind_named_generics(
+            object,
+            &the_trait.name,
+            associated_types,
+            &trait_bound.trait_generics.named,
+            bindings,
+        );
     }
 
     pub fn instantiate_parent_trait_bound(
         &self,
+        object: &Type,
         trait_bound: &ResolvedTraitBound,
         parent_trait_bound: &ResolvedTraitBound,
     ) -> ResolvedTraitBound {
         let mut bindings = TypeBindings::default();
-        self.bind_generics_from_trait_bound(trait_bound, &mut bindings);
+        self.bind_generics_from_trait_bound(object, trait_bound, &mut bindings);
         ResolvedTraitBound {
             trait_generics: parent_trait_bound.trait_generics.map(|typ| typ.substitute(&bindings)),
             ..*parent_trait_bound
@@ -2755,40 +2775,73 @@ impl Elaborator<'_> {
     }
 }
 
+/// Binds the ordered [ResolvedGeneric]s of a trait to the ordered generics in a [ResolvedTraitBound].
+///
+/// Panics if the number of types do not match the ordered generics in the trait.
 pub(super) fn bind_ordered_generics(
     params: &[ResolvedGeneric],
     args: &[Type],
     bindings: &mut TypeBindings,
 ) {
-    assert_eq!(params.len(), args.len());
+    assert_eq!(params.len(), args.len(), "unexpected number of ordered generics");
 
     for (param, arg) in params.iter().zip(args) {
         bind_generic(param, arg, bindings);
     }
 }
 
+/// Binds the associated [ResolvedGeneric]s of a trait to the named generics in a [ResolvedTraitBound].
+///
+/// Panics if the number of types exceeds the named generics in the trait.
+/// Any named parameter that does not appear in the arguments is bound to [Type::Error].
 fn bind_named_generics(
-    mut params: Vec<ResolvedGeneric>,
+    object: &Type,
+    trait_ident: &Ident,
+    mut params: Vec<(ResolvedGeneric, bool)>,
     args: &[NamedType],
     bindings: &mut TypeBindings,
 ) {
-    assert!(args.len() <= params.len());
+    assert!(args.len() <= params.len(), "more named generics than associated types");
+
+    if params.is_empty() {
+        return;
+    }
+    let object_name = object.to_string();
+    let trait_name = trait_ident.as_str();
 
     for arg in args {
         let i = params
             .iter()
-            .position(|typ| *typ.name == arg.name.as_str())
+            .position(|(typ, _)| *typ.name == arg.name.as_str())
             .unwrap_or_else(|| unreachable!("Expected to find associated type named {}", arg.name));
 
-        let param = params.swap_remove(i);
-        bind_generic(&param, &arg.typ, bindings);
+        let (param, is_const) = params.swap_remove(i);
+
+        match &arg.typ {
+            Type::TypeVariable(v) if !is_const && v.borrow().is_unbound() => {
+                // If we allow associated types to bind to a Type::TypeVariable rather than a Type::NamedGeneric,
+                // then they be unified with anything else,
+                // whereas as a NamedGeneric they only unify with the same type variable.
+                let name = Rc::new(arg.name.to_string());
+                let typ = v.clone().into_named_generic(&name, Some((&object_name, trait_name)));
+                bind_generic(&param, &typ, bindings);
+            }
+            _ => {
+                // Other types or associated constants
+                bind_generic(&param, &arg.typ, bindings);
+            }
+        }
     }
 
-    for unbound_param in params {
+    for (unbound_param, _) in params {
         bind_generic(&unbound_param, &Type::Error, bindings);
     }
 }
 
+/// Binds the type variable in a [ResolvedGeneric], e.g. a generic parameter of a trait,
+/// to a [Type], which itself can be an unbound type variable.
+///
+/// If the type varia itself appears in the type, then it does nothing.
 fn bind_generic(param: &ResolvedGeneric, arg: &Type, bindings: &mut TypeBindings) {
     // Avoid binding t = t
     if !arg.occurs(param.type_var.id()) {

compiler/noirc_frontend/src/elaborator/variable.rs

@@ -576,6 +576,7 @@ impl Elaborator<'_> {
         // the type used in the trait constraint (if it exists). See #4088.
         if let ImplKind::TraitItem(method) = &ident.impl_kind {
             self.bind_generics_from_trait_constraint(
+                &method.constraint.typ,
                 &method.constraint,
                 method.assumed,
                 &mut bindings,
@@ -602,7 +603,7 @@ impl Elaborator<'_> {
         // when the constraint below is later solved for when the function is
         // finished. How to link the two?
         let (typ, bindings) =
-            self.instantiate(t, bindings, generics, function_generic_count, location);
+            self.instantiate(t.clone(), bindings, generics, function_generic_count, location);
 
         if let ImplKind::TraitItem(mut method) = ident.impl_kind {
             method.constraint.apply_bindings(&bindings);
@@ -650,6 +651,7 @@ impl Elaborator<'_> {
         if let Some(definition) = self.interner.try_definition(ident.id) {
             if let DefinitionKind::Function(function) = definition.kind {
                 let function = self.interner.function_meta(&function);
+
                 for mut constraint in function.all_trait_constraints().cloned().collect::<Vec<_>>()
                 {
                     constraint.apply_bindings(&bindings);

compiler/noirc_frontend/src/hir_def/types.rs

@@ -1185,9 +1185,9 @@ impl std::fmt::Display for Type {
             Type::Unit => write!(f, "()"),
             Type::Error => write!(f, "error"),
             Type::NamedGeneric(NamedGeneric { type_var, name, .. }) => match &*type_var.borrow() {
-                TypeBinding::Bound(type_var) => type_var.fmt(f),
-                TypeBinding::Unbound(_, _) if name.is_empty() => write!(f, "_"),
-                TypeBinding::Unbound(_, _) => write!(f, "{name}"),
+                TypeBinding::Bound(type_var) if !type_var.is_unbound_type_var() => type_var.fmt(f),
+                _ if name.is_empty() => write!(f, "_"),
+                _ => write!(f, "{name}"),
             },
             Type::CheckedCast { to, .. } => write!(f, "{to}"),
             Type::Constant(x, _kind) => write!(f, "{x}"),
@@ -1895,6 +1895,18 @@ impl Type {
         }
     }
 
+    /// Check if this is a [Type::TypeVariable] with a [TypeBinding::Unbound] binding.
+    fn is_unbound_type_var(&self) -> bool {
+        let Type::TypeVariable(type_var) = self else {
+            return false;
+        };
+        let binding = type_var.borrow();
+        match &*binding {
+            TypeBinding::Unbound(_, _) => true,
+            TypeBinding::Bound(typ) => typ.is_unbound_type_var(),
+        }
+    }
+
     pub(crate) fn contains_reference(&self) -> bool {
         match self {
             Type::Unit

compiler/noirc_frontend/src/tests/traits/trait_associated_items.rs

@@ -567,6 +567,30 @@ fn associated_type_mismatch_across_modules() {
     check_errors(src);
 }
 
+#[test]
+fn associated_type_mismatch_with_inheritance() {
+    let src = r#"
+    pub trait Foo {
+        type Bar;
+        fn foo(x: Self::Bar) -> Self::Bar;
+    }
+
+    pub trait Qux: Foo {
+        type Baz;
+        fn qux(x: Self::Baz) -> Self::Baz {
+                                ^^^^^^^^^ expected type Self::Baz, found type Self::Bar
+                                ~~~~~~~~~ expected Self::Baz because of return type
+            <Self as Foo>::foo(x)
+                               ^ Expected type Self::Bar, found type Self::Baz
+            ~~~~~~~~~~~~~~~~~~~~~ Self::Bar returned here
+        }
+    }
+
+    fn main() {}
+    "#;
+    check_errors(src);
+}
+
 #[test]
 fn associated_type_behind_self_as_trait() {
     let src = r#"