feat: error if generic type parameter in impl is not mentioned in self type

compiler/noirc_frontend/src/ast/visitor.rs

@@ -17,7 +17,7 @@
     },
     parser::{Item, ItemKind, ParsedSubModule},
     token::{CustomAttribute, SecondaryAttribute, Tokens},
-    ParsedModule, QuotedType,
+    BinaryTypeOperator, ParsedModule, QuotedType,
 };
 
 use super::{
@@ -397,7 +397,9 @@
         true
     }
 
-    fn visit_expression_type(&mut self, _: &UnresolvedTypeExpression, _: Span) {}
+    fn visit_expression_type(&mut self, _: &UnresolvedTypeExpression, _: Span) -> bool {
+        true
+    }
 
     fn visit_format_string_type(
         &mut self,
@@ -408,7 +410,9 @@
         true
     }
 
-    fn visit_string_type(&mut self, _: &UnresolvedTypeExpression, _: Span) {}
+    fn visit_string_type(&mut self, _: &UnresolvedTypeExpression, _: Span) -> bool {
+        true
+    }
 
     fn visit_unspecified_type(&mut self, _: Span) {}
 
@@ -446,6 +450,30 @@
         true
     }
 
+    fn visit_unresolved_type_expression(&mut self, _: &UnresolvedTypeExpression) -> bool {
+        true
+    }
+
+    fn visit_variable_type_expression(&mut self, _: &Path) -> bool {
+        true
+    }
+
+    fn visit_constant_type_expression(&mut self, _value: FieldElement, _span: Span) {}
+
+    fn visit_binary_type_expression(
+        &mut self,
+        _lhs: &UnresolvedTypeExpression,
+        _op: BinaryTypeOperator,
+        _rhs: &UnresolvedTypeExpression,
+        _span: Span,
+    ) -> bool {
+        true
+    }
+
+    fn visit_as_trait_path_type_expression(&mut self, _as_trait_path: &AsTraitPath) -> bool {
+        true
+    }
+
     fn visit_pattern(&mut self, _: &Pattern) -> bool {
         true
     }
@@ -1261,6 +1289,7 @@
             UnresolvedTypeData::Array(unresolved_type_expression, unresolved_type) => {
                 if visitor.visit_array_type(unresolved_type_expression, unresolved_type, self.span)
                 {
+                    unresolved_type_expression.accept(visitor);
                     unresolved_type.accept(visitor);
                 }
             }
@@ -1308,18 +1337,27 @@
                     as_trait_path.accept(self.span, visitor);
                 }
             }
-            UnresolvedTypeData::Expression(expr) => visitor.visit_expression_type(expr, self.span),
+            UnresolvedTypeData::Expression(expr) => {
+                if visitor.visit_expression_type(expr, self.span) {
+                    expr.accept(visitor);
+                }
+            }
             UnresolvedTypeData::FormatString(expr, typ) => {
                 if visitor.visit_format_string_type(expr, typ, self.span) {
+                    expr.accept(visitor);
                     typ.accept(visitor);
                 }
             }
-            UnresolvedTypeData::String(expr) => visitor.visit_string_type(expr, self.span),
+            UnresolvedTypeData::String(expr) => {
+                if visitor.visit_string_type(expr, self.span) {
+                    expr.accept(visitor);
+                }
+            }
             UnresolvedTypeData::Unspecified => visitor.visit_unspecified_type(self.span),
             UnresolvedTypeData::Quoted(typ) => visitor.visit_quoted_type(typ, self.span),
             UnresolvedTypeData::FieldElement => visitor.visit_field_element_type(self.span),
             UnresolvedTypeData::Integer(signdness, size) => {
                 visitor.visit_integer_type(*signdness, *size, self.span);
             }
             UnresolvedTypeData::Bool => visitor.visit_bool_type(self.span),
             UnresolvedTypeData::Unit => visitor.visit_unit_type(self.span),
@@ -1394,6 +1432,38 @@
     }
 }
 
+impl UnresolvedTypeExpression {
+    pub fn accept(&self, visitor: &mut impl Visitor) {
+        if visitor.visit_unresolved_type_expression(self) {
+            self.accept_children(visitor);
+        }
+    }
+
+    pub fn accept_children(&self, visitor: &mut impl Visitor) {
+        match self {
+            UnresolvedTypeExpression::Variable(path) => {
+                if visitor.visit_variable_type_expression(path) {
+                    path.accept(visitor);
+                }
+            }
+            UnresolvedTypeExpression::Constant(field_element, span) => {
+                visitor.visit_constant_type_expression(*field_element, *span);
+            }
+            UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, span) => {
+                if visitor.visit_binary_type_expression(lhs, *op, rhs, *span) {
+                    lhs.accept(visitor);
+                    rhs.accept(visitor);
+                }
+            }
+            UnresolvedTypeExpression::AsTraitPath(as_trait_path) => {
+                if visitor.visit_as_trait_path_type_expression(as_trait_path) {
+                    as_trait_path.accept(self.span(), visitor);
+                }
+            }
+        }
+    }
+}
+
 impl Pattern {
     pub fn accept(&self, visitor: &mut impl Visitor) {
         if visitor.visit_pattern(self) {

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -3,35 +3,32 @@ use std::{
     rc::Rc,
 };
 
-use crate::{
-    ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, StructField, StructType, TypeBindings,
-};
 use crate::{
     ast::{
         BlockExpression, FunctionKind, GenericTypeArgs, Ident, NoirFunction, NoirStruct, Param,
         Path, Pattern, TraitBound, UnresolvedGeneric, UnresolvedGenerics,
-        UnresolvedTraitConstraint, UnresolvedTypeData, UnsupportedNumericGenericType,
+        UnresolvedTraitConstraint, UnresolvedTypeData, UnsupportedNumericGenericType, Visitor,
     },
     graph::CrateId,
     hir::{
-        def_collector::dc_crate::{
-            filter_literal_globals, CompilationError, ImplMap, UnresolvedFunctions,
-            UnresolvedGlobal, UnresolvedStruct, UnresolvedTraitImpl, UnresolvedTypeAlias,
+        def_collector::{
+            dc_crate::{
+                filter_literal_globals, CollectedItems, CompilationError, ImplMap,
+                UnresolvedFunctions, UnresolvedGlobal, UnresolvedStruct, UnresolvedTraitImpl,
+                UnresolvedTypeAlias,
+            },
+            errors::DefCollectorErrorKind,
         },
-        def_collector::{dc_crate::CollectedItems, errors::DefCollectorErrorKind},
-        def_map::{DefMaps, ModuleData},
-        def_map::{LocalModuleId, ModuleDefId, ModuleId, MAIN_FUNCTION},
-        resolution::errors::ResolverError,
-        resolution::import::PathResolution,
+        def_map::{DefMaps, LocalModuleId, ModuleData, ModuleDefId, ModuleId, MAIN_FUNCTION},
+        resolution::{errors::ResolverError, import::PathResolution},
         scope::ScopeForest as GenericScopeForest,
         type_check::{generics::TraitGenerics, TypeCheckError},
         Context,
     },
-    hir_def::traits::TraitImpl,
     hir_def::{
         expr::{HirCapturedVar, HirIdent},
         function::{FuncMeta, FunctionBody, HirFunction},
-        traits::TraitConstraint,
+        traits::{TraitConstraint, TraitImpl},
         types::{Generics, Kind, ResolvedGeneric},
     },
     node_interner::{
@@ -41,6 +38,11 @@ use crate::{
     token::{CustomAttribute, SecondaryAttribute},
     Shared, Type, TypeVariable,
 };
+use crate::{
+    ast::{ItemVisibility, UnresolvedType},
+    hir_def::traits::ResolvedTraitBound,
+    StructField, StructType, TypeBindings,
+};
 
 mod comptime;
 mod expressions;
@@ -54,6 +56,7 @@ pub mod types;
 mod unquote;
 
 use fm::FileId;
+use im::HashSet;
 use iter_extended::vecmap;
 use noirc_errors::{Location, Span, Spanned};
 use types::bind_ordered_generics;
@@ -277,8 +280,8 @@ impl<'context> Elaborator<'context> {
         // 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.
-        for ((_self_type, module), impls) in &mut items.impls {
-            self.collect_impls(*module, impls);
+        for ((self_type, module), impls) in &mut items.impls {
+            self.collect_impls(*module, impls, self_type);
         }
 
         // Bind trait impls to their trait. Collect trait functions, that have a
@@ -1222,10 +1225,13 @@ impl<'context> Elaborator<'context> {
         &mut self,
         module: LocalModuleId,
         impls: &mut [(UnresolvedGenerics, Span, UnresolvedFunctions)],
+        self_type: &UnresolvedType,
     ) {
         self.local_module = module;
 
         for (generics, span, unresolved) in impls {
+            self.check_generics_appear_in_type(generics, self_type);
+
             self.file = unresolved.file_id;
             let old_generic_count = self.generics.len();
             self.add_generics(generics);
@@ -1713,6 +1719,7 @@ impl<'context> Elaborator<'context> {
                 self.file = function_set.file_id;
                 self.add_generics(generics);
                 let self_type = self.resolve_type(self_type.clone());
+
                 function_set.self_type = Some(self_type.clone());
                 self.self_type = Some(self_type);
                 self.define_function_metas_for_functions(function_set);
@@ -1798,4 +1805,56 @@ impl<'context> Elaborator<'context> {
                 _ => true,
             })
     }
+
+    /// Check that all the generics show up in `self_type` (if they don't, we produce an error)
+    fn check_generics_appear_in_type(
+        &mut self,
+        generics: &[UnresolvedGeneric],
+        self_type: &UnresolvedType,
+    ) {
+        if generics.is_empty() {
+            return;
+        }
+
+        // Turn each generic into an Ident
+        let mut idents = HashSet::new();
+        for generic in generics {
+            match generic {
+                UnresolvedGeneric::Variable(ident) => {
+                    idents.insert(ident.clone());
+                }
+                UnresolvedGeneric::Numeric { ident, typ: _ } => {
+                    idents.insert(ident.clone());
+                }
+                UnresolvedGeneric::Resolved(quoted_type_id, span) => {
+                    if let Type::NamedGeneric(_type_variable, name) =
+                        self.interner.get_quoted_type(*quoted_type_id).follow_bindings()
+                    {
+                        idents.insert(Ident::new(name.to_string(), *span));
+                    }
+                }
+            }
+        }
+
+        // Remove the ones that show up in `self_type`
+        let mut visitor = RemoveGenericsAppearingInTypeVisitor { idents: &mut idents };
+        self_type.accept(&mut visitor);
+
+        // The ones that remain are not mentioned in the impl: it's an error.
+        for ident in idents {
+            self.push_err(ResolverError::UnconstrainedTypeParameter { ident });
+        }
+    }
+}
+
+struct RemoveGenericsAppearingInTypeVisitor<'a> {
+    idents: &'a mut HashSet<Ident>,
+}
+
+impl<'a> Visitor for RemoveGenericsAppearingInTypeVisitor<'a> {
+    fn visit_path(&mut self, path: &Path) {
+        if let Some(ident) = path.as_ident() {
+            self.idents.remove(ident);
+        }
+    }
 }

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

@@ -160,6 +160,8 @@ pub enum ResolverError {
         span: Span,
         missing_trait_location: Location,
     },
+    #[error("The type parameter `{ident}` is not constrained by the impl trait, self type, or predicates")]
+    UnconstrainedTypeParameter { ident: Ident },
 }
 
 impl ResolverError {
@@ -606,6 +608,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                 diagnostic.add_secondary_with_file(format!("required by this bound in `{impl_trait}"), missing_trait_location.span, missing_trait_location.file);
                 diagnostic
             },
+            ResolverError::UnconstrainedTypeParameter { ident} => {
+                Diagnostic::simple_error(
+                    format!("The type parameter `{ident}` is not constrained by the impl trait, self type, or predicates"),
+                    format!("Hint: remove the `{ident}` type parameter"),
+                    ident.span(),
+                )
+            },
         }
     }
 }

compiler/noirc_frontend/src/tests.rs

@@ -1976,7 +1976,7 @@
 }
 
 // TODO(https://github.com/noir-lang/noir/issues/6238):
 // The EvaluatedGlobalIsntU32 warning is a stopgap
 // (originally from https://github.com/noir-lang/noir/issues/6125)
 #[test]
 fn numeric_generic_field_larger_than_u32() {
@@ -1993,7 +1993,7 @@
     assert_eq!(errors.len(), 2);
     assert!(matches!(
         errors[0].0,
         CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),
     ));
     assert!(matches!(
         errors[1].0,
@@ -2002,7 +2002,7 @@
 }
 
 // TODO(https://github.com/noir-lang/noir/issues/6238):
 // The EvaluatedGlobalIsntU32 warning is a stopgap
 // (originally from https://github.com/noir-lang/noir/issues/6126)
 #[test]
 fn numeric_generic_field_arithmetic_larger_than_u32() {
@@ -2031,7 +2031,7 @@
 
     assert!(matches!(
         errors[0].0,
         CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),
     ));
 
     assert!(matches!(
@@ -2167,7 +2167,7 @@
     assert_eq!(errors.len(), 3);
 
     // TODO(https://github.com/noir-lang/noir/issues/6238):
     // The EvaluatedGlobalIsntU32 warning is a stopgap
     assert!(matches!(
         errors[0].0,
         CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),
@@ -3190,7 +3190,7 @@
 }
 
 #[test]
 fn arithmetic_generics_canonicalization_deduplication_regression() {
     let source = r#"
         struct ArrData<let N: u32> {
             a: [Field; N],
@@ -3397,7 +3397,7 @@
 
 #[test]
 fn unconditional_recursion_fail() {
     let srcs = vec![
         r#"
         fn main() {
             main()
@@ -3459,7 +3459,7 @@
         "#,
     ];
 
     for src in srcs {
         let errors = get_program_errors(src);
         assert!(
             !errors.is_empty(),
@@ -3562,3 +3562,28 @@
     "#;
     assert_no_errors(src);
 }
+
+#[test]
+fn unconstrained_type_parameter_in_impl() {
+    let src = r#"
+        pub struct Foo<T> {}
+
+        impl<T, U> Foo<T> {}
+
+        fn main() {
+            let _ = Foo {};
+        }
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::ResolverError(ResolverError::UnconstrainedTypeParameter {
+        ident, ..
+    }) = &errors[0].0
+    else {
+        panic!("Expected an UnconstrainedTypeParameter error, got {:?}", &errors[0].0);
+    };
+
+    assert_eq!(ident.to_string(), "U");
+}