feat: Check where clauses when searching for trait impls (#3407)

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

@@ -13,13 +13,15 @@ use crate::hir::resolution::{
 use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
 use crate::hir::Context;
 use crate::hir_def::traits::{Trait, TraitConstant, TraitFunction, TraitImpl, TraitType};
-use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId};
+use crate::node_interner::{
+    FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplId, TypeAliasId,
+};
 
 use crate::parser::{ParserError, SortedModule};
 use crate::{
     ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait,
     NoirTypeAlias, Path, Shared, StructType, TraitItem, Type, TypeBinding, TypeVariableKind,
-    UnresolvedGenerics, UnresolvedType,
+    UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType,
 };
 use fm::FileId;
 use iter_extended::vecmap;
@@ -91,6 +93,7 @@ pub struct UnresolvedTraitImpl {
     pub object_type: UnresolvedType,
     pub methods: UnresolvedFunctions,
     pub generics: UnresolvedGenerics,
+    pub where_clause: Vec<UnresolvedTraitConstraint>,
 }
 
 #[derive(Clone)]
@@ -919,6 +922,7 @@ fn resolve_impls(
                 def_maps,
                 functions,
                 Some(self_type.clone()),
+                None,
                 generics,
                 errors,
             );
@@ -968,13 +972,16 @@ fn resolve_trait_impls(
         let self_type = resolver.resolve_type(unresolved_type.clone());
         let generics = resolver.get_generics().to_vec();
 
+        let impl_id = interner.next_trait_impl_id();
+
         let mut impl_methods = resolve_function_set(
             interner,
             crate_id,
             &context.def_maps,
             trait_impl.methods.clone(),
             Some(self_type.clone()),
-            generics,
+            Some(impl_id),
+            generics.clone(),
             errors,
         );
 
@@ -993,6 +1000,8 @@ fn resolve_trait_impls(
 
         let mut new_resolver =
             Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id);
+
+        new_resolver.set_generics(generics);
         new_resolver.set_self_type(Some(self_type.clone()));
 
         if let Some(trait_id) = maybe_trait_id {
@@ -1004,17 +1013,27 @@ fn resolve_trait_impls(
                 errors,
             );
 
+            let where_clause = trait_impl
+                .where_clause
+                .into_iter()
+                .flat_map(|item| new_resolver.resolve_trait_constraint(item))
+                .collect();
+
             let resolved_trait_impl = Shared::new(TraitImpl {
                 ident: trait_impl.trait_path.last_segment().clone(),
                 typ: self_type.clone(),
                 trait_id,
                 file: trait_impl.file_id,
+                where_clause,
                 methods: vecmap(&impl_methods, |(_, func_id)| *func_id),
             });
 
-            if let Some((prev_span, prev_file)) =
-                interner.add_trait_implementation(self_type.clone(), trait_id, resolved_trait_impl)
-            {
+            if let Err((prev_span, prev_file)) = interner.add_trait_implementation(
+                self_type.clone(),
+                trait_id,
+                impl_id,
+                resolved_trait_impl,
+            ) {
                 let error = DefCollectorErrorKind::OverlappingImpl {
                     typ: self_type.clone(),
                     span: self_type_span.unwrap_or_else(|| trait_impl.trait_path.span()),
@@ -1147,19 +1166,22 @@ fn resolve_free_functions(
                 def_maps,
                 unresolved_functions,
                 self_type.clone(),
+                None,
                 vec![], // no impl generics
                 errors,
             )
         })
         .collect()
 }
 
+#[allow(clippy::too_many_arguments)]
 fn resolve_function_set(
     interner: &mut NodeInterner,
     crate_id: CrateId,
     def_maps: &BTreeMap<CrateId, CrateDefMap>,
     mut unresolved_functions: UnresolvedFunctions,
     self_type: Option<Type>,
+    trait_impl_id: Option<TraitImplId>,
     impl_generics: Vec<(Rc<String>, Shared<TypeBinding>, Span)>,
     errors: &mut Vec<(CompilationError, FileId)>,
 ) -> Vec<(FileId, FuncId)> {
@@ -1180,6 +1202,7 @@ fn resolve_function_set(
         resolver.set_generics(impl_generics.clone());
         resolver.set_self_type(self_type.clone());
         resolver.set_trait_id(unresolved_functions.trait_id);
+        resolver.set_trait_impl_id(trait_impl_id);
 
         // Without this, impl methods can accidentally be placed in contracts. See #3254
         if self_type.is_some() {

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

@@ -162,6 +162,7 @@ impl<'a> ModCollector<'a> {
                 methods: unresolved_functions,
                 object_type: trait_impl.object_type,
                 generics: trait_impl.impl_generics,
+                where_clause: trait_impl.where_clause,
                 trait_id: None, // will be filled later
             };
 

compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -29,6 +29,7 @@ use crate::hir::def_map::{LocalModuleId, ModuleDefId, TryFromModuleDefId, MAIN_F
 use crate::hir_def::stmt::{HirAssignStatement, HirForStatement, HirLValue, HirPattern};
 use crate::node_interner::{
     DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, StructId, TraitId,
+    TraitImplId,
 };
 use crate::{
     hir::{def_map::CrateDefMap, resolution::path_resolver::PathResolver},
@@ -88,6 +89,10 @@ pub struct Resolver<'a> {
     /// Set to the current type if we're resolving an impl
     self_type: Option<Type>,
 
+    /// If we're currently resolving methods within a trait impl, this will be set
+    /// to the corresponding trait impl ID.
+    current_trait_impl: Option<TraitImplId>,
+
     /// True if the current module is a contract.
     /// This is usually determined by self.path_resolver.module_id(), but it can
     /// be overriden for impls. Impls are an odd case since the methods within resolve
@@ -142,6 +147,7 @@ impl<'a> Resolver<'a> {
             generics: Vec::new(),
             errors: Vec::new(),
             lambda_stack: Vec::new(),
+            current_trait_impl: None,
             file,
             in_contract,
         }
@@ -155,6 +161,10 @@ impl<'a> Resolver<'a> {
         self.trait_id = trait_id;
     }
 
+    pub fn set_trait_impl_id(&mut self, impl_id: Option<TraitImplId>) {
+        self.current_trait_impl = impl_id;
+    }
+
     pub fn get_self_type(&mut self) -> Option<&Type> {
         self.self_type.as_ref()
     }
@@ -357,6 +367,15 @@ impl<'a> Resolver<'a> {
         (hir_func, func_meta)
     }
 
+    pub fn resolve_trait_constraint(
+        &mut self,
+        constraint: UnresolvedTraitConstraint,
+    ) -> Option<TraitConstraint> {
+        let typ = self.resolve_type(constraint.typ);
+        let trait_id = self.lookup_trait_or_error(constraint.trait_bound.trait_path)?.id;
+        Some(TraitConstraint { typ, trait_id })
+    }
+
     /// Translates an UnresolvedType into a Type and appends any
     /// freshly created TypeVariables created to new_variables.
     fn resolve_type_inner(&mut self, typ: UnresolvedType, new_variables: &mut Generics) -> Type {
@@ -809,6 +828,7 @@ impl<'a> Resolver<'a> {
             kind: func.kind,
             location,
             typ,
+            trait_impl: self.current_trait_impl,
             parameters: parameters.into(),
             return_type: func.def.return_type.clone(),
             return_visibility: func.def.return_visibility,

compiler/noirc_frontend/src/hir/type_check/errors.rs

@@ -111,6 +111,8 @@ pub enum TypeCheckError {
         parameter_span: Span,
         parameter_index: usize,
     },
+    #[error("No matching impl found")]
+    NoMatchingImplFound { constraints: Vec<(Type, String)>, span: Span },
 }
 
 impl TypeCheckError {
@@ -250,11 +252,22 @@ impl From<TypeCheckError> for Diagnostic {
                 Diagnostic::simple_warning(primary_message, secondary_message, span)
             }
             TypeCheckError::UnusedResultError { expr_type, expr_span } => {
-                Diagnostic::simple_warning(
-                    format!("Unused expression result of type {expr_type}"),
-                    String::new(),
-                    expr_span,
-                )
+                let msg = format!("Unused expression result of type {expr_type}");
+                Diagnostic::simple_warning(msg, String::new(), expr_span)
+            }
+            TypeCheckError::NoMatchingImplFound { constraints, span } => {
+                assert!(!constraints.is_empty());
+                let msg = format!("No matching impl found for `{}: {}`", constraints[0].0, constraints[0].1);
+                let mut diagnostic = Diagnostic::from_message(&msg);
+
+                diagnostic.add_secondary(format!("No impl for `{}: {}`", constraints[0].0, constraints[0].1), span);
+
+                // These must be notes since secondaries are unordered
+                for (typ, trait_name) in &constraints[1..] {
+                    diagnostic.add_note(format!("Required by `{typ}: {trait_name}`"));
+                }
+
+                diagnostic
             }
         }
     }

compiler/noirc_frontend/src/hir/type_check/expr.rs

@@ -146,7 +146,7 @@ impl<'interner> TypeChecker<'interner> {
                 match self.lookup_method(&object_type, method_name, expr_id) {
                     Some(method_ref) => {
                         let mut args = vec![(
-                            object_type,
+                            object_type.clone(),
                             method_call.object,
                             self.interner.expr_span(&method_call.object),
                         )];
@@ -160,11 +160,14 @@ impl<'interner> TypeChecker<'interner> {
                         // so that the backend doesn't need to worry about methods
                         let location = method_call.location;
 
-                        if let HirMethodReference::FuncId(func_id) = method_ref {
+                        let mut func_id = None;
+                        if let HirMethodReference::FuncId(id) = method_ref {
+                            func_id = Some(id);
+
                             // Automatically add `&mut` if the method expects a mutable reference and
                             // the object is not already one.
-                            if func_id != FuncId::dummy_id() {
-                                let func_meta = self.interner.function_meta(&func_id);
+                            if id != FuncId::dummy_id() {
+                                let func_meta = self.interner.function_meta(&id);
                                 self.try_add_mutable_reference_to_object(
                                     &mut method_call,
                                     &func_meta.typ,
@@ -182,6 +185,31 @@ impl<'interner> TypeChecker<'interner> {
                         let span = self.interner.expr_span(expr_id);
                         let ret = self.check_method_call(&function_id, method_ref, args, span);
 
+                        if let Some(func_id) = func_id {
+                            let meta = self.interner.function_meta(&func_id);
+
+                            if let Some(impl_id) = meta.trait_impl {
+                                let trait_impl = self.interner.get_trait_implementation(impl_id);
+
+                                let result = self.interner.lookup_trait_implementation(
+                                    &object_type,
+                                    trait_impl.borrow().trait_id,
+                                );
+
+                                if let Err(erroring_constraints) = result {
+                                    let constraints = vecmap(erroring_constraints, |constraint| {
+                                        let r#trait = self.interner.get_trait(constraint.trait_id);
+                                        (constraint.typ, r#trait.name.to_string())
+                                    });
+
+                                    self.errors.push(TypeCheckError::NoMatchingImplFound {
+                                        constraints,
+                                        span,
+                                    });
+                                }
+                            }
+                        }
+
                         self.interner.replace_expr(expr_id, function_call);
                         ret
                     }

compiler/noirc_frontend/src/hir/type_check/mod.rs

@@ -65,13 +65,10 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type
         let (expr_span, empty_function) = function_info(interner, function_body_id);
         let func_span = interner.expr_span(function_body_id); // XXX: We could be more specific and return the span of the last stmt, however stmts do not have spans yet
         if let Type::TraitAsType(t) = &declared_return_type {
-            if interner
-                .lookup_trait_implementation(function_last_type.follow_bindings(), t.id)
-                .is_none()
-            {
+            if interner.lookup_trait_implementation(&function_last_type, t.id).is_err() {
                 let error = TypeCheckError::TypeMismatchWithSource {
                     expected: declared_return_type.clone(),
-                    actual: function_last_type.clone(),
+                    actual: function_last_type,
                     span: func_span,
                     source: Source::Return(meta.return_type, expr_span),
                 };
@@ -287,6 +284,7 @@ mod test {
             return_visibility: Visibility::Private,
             return_distinctness: Distinctness::DuplicationAllowed,
             has_body: true,
+            trait_impl: None,
             return_type: FunctionReturnType::Default(Span::default()),
             trait_constraints: Vec::new(),
         };

compiler/noirc_frontend/src/hir_def/function.rs

@@ -4,7 +4,7 @@ use noirc_errors::{Location, Span};
 use super::expr::{HirBlockExpression, HirExpression, HirIdent};
 use super::stmt::HirPattern;
 use super::traits::TraitConstraint;
-use crate::node_interner::{ExprId, NodeInterner};
+use crate::node_interner::{ExprId, NodeInterner, TraitImplId};
 use crate::FunctionKind;
 use crate::{Distinctness, FunctionReturnType, Type, Visibility};
 
@@ -114,6 +114,9 @@ pub struct FuncMeta {
     pub has_body: bool,
 
     pub trait_constraints: Vec<TraitConstraint>,
+
+    /// The trait impl this function belongs to, if any
+    pub trait_impl: Option<TraitImplId>,
 }
 
 impl FuncMeta {

compiler/noirc_frontend/src/hir_def/traits.rs

@@ -67,6 +67,12 @@ pub struct TraitImpl {
     pub trait_id: TraitId,
     pub file: FileId,
     pub methods: Vec<FuncId>, // methods[i] is the implementation of trait.methods[i] for Type typ
+
+    /// The where clause, if present, contains each trait requirement which must
+    /// be satisfied for this impl to be selected. E.g. in `impl Eq for [T] where T: Eq`,
+    /// `where_clause` would contain the one `T: Eq` constraint. If there is no where clause,
+    /// this Vec is empty.
+    pub where_clause: Vec<TraitConstraint>,
 }
 
 #[derive(Debug, Clone)]
@@ -76,6 +82,12 @@ pub struct TraitConstraint {
     // pub trait_generics: Generics, TODO
 }
 
+impl TraitConstraint {
+    pub fn new(typ: Type, trait_id: TraitId) -> Self {
+        Self { typ, trait_id }
+    }
+}
+
 impl std::hash::Hash for Trait {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         self.id.hash(state);

compiler/noirc_frontend/src/hir_def/types.rs

@@ -1085,10 +1085,13 @@ impl Type {
     }
 
     /// Replace each NamedGeneric (and TypeVariable) in this type with a fresh type variable
-    pub(crate) fn instantiate_named_generics(&self, interner: &NodeInterner) -> Type {
+    pub(crate) fn instantiate_named_generics(
+        &self,
+        interner: &NodeInterner,
+    ) -> (Type, TypeBindings) {
         let mut substitutions = HashMap::new();
         self.find_all_unbound_type_variables(interner, &mut substitutions);
-        self.substitute(&substitutions)
+        (self.substitute(&substitutions), substitutions)
     }
 
     /// For each unbound type variable in the current type, add a type binding to the given list

compiler/noirc_frontend/src/monomorphization/mod.rs

@@ -820,7 +820,7 @@ impl<'interner> Monomorphizer<'interner> {
 
         let trait_impl = self
             .interner
-            .lookup_trait_implementation(self_type.follow_bindings(), method.trait_id)
+            .lookup_trait_implementation(&self_type, method.trait_id)
             .expect("ICE: missing trait impl - should be caught during type checking");
 
         let hir_func_id = trait_impl.borrow().methods[method.method_index];