fix: type-check trait default methods

compiler/noirc_frontend/src/ast/expression.rs

@@ -815,7 +815,7 @@ impl FunctionDefinition {
         is_unconstrained: bool,
         generics: &UnresolvedGenerics,
         parameters: &[(Ident, UnresolvedType)],
-        body: &BlockExpression,
+        body: BlockExpression,
         where_clause: &[UnresolvedTraitConstraint],
         return_type: &FunctionReturnType,
     ) -> FunctionDefinition {
@@ -837,7 +837,7 @@ impl FunctionDefinition {
             visibility: ItemVisibility::Private,
             generics: generics.clone(),
             parameters: p,
-            body: body.clone(),
+            body,
             span: name.span(),
             where_clause: where_clause.to_vec(),
             return_type: return_type.clone(),

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -328,6 +328,12 @@ impl<'context> Elaborator<'context> {
             self.elaborate_functions(functions);
         }
 
+        for (trait_id, unresolved_trait) in items.traits {
+            self.current_trait = Some(trait_id);
+            self.elaborate_functions(unresolved_trait.fns_with_default_impl);
+        }
+        self.current_trait = None;
+
         for impls in items.impls.into_values() {
             self.elaborate_impls(impls);
         }

compiler/noirc_frontend/src/elaborator/traits.rs

@@ -28,6 +28,11 @@ impl<'context> Elaborator<'context> {
             self.recover_generics(|this| {
                 this.current_trait = Some(*trait_id);
 
+                let the_trait = this.interner.get_trait(*trait_id);
+                let self_typevar = the_trait.self_type_typevar.clone();
+                let self_type = Type::TypeVariable(self_typevar.clone());
+                this.self_type = Some(self_type.clone());
+
                 let resolved_generics = this.interner.get_trait(*trait_id).generics.clone();
                 this.add_existing_generics(
                     &unresolved_trait.trait_def.generics,
@@ -48,12 +53,15 @@ impl<'context> Elaborator<'context> {
                         .add_trait_dependency(DependencyId::Trait(bound.trait_id), *trait_id);
                 }
 
+                this.interner.update_trait(*trait_id, |trait_def| {
+                    trait_def.set_trait_bounds(resolved_trait_bounds);
+                    trait_def.set_where_clause(where_clause);
+                });
+
                 let methods = this.resolve_trait_methods(*trait_id, unresolved_trait);
 
                 this.interner.update_trait(*trait_id, |trait_def| {
                     trait_def.set_methods(methods);
-                    trait_def.set_trait_bounds(resolved_trait_bounds);
-                    trait_def.set_where_clause(where_clause);
                 });
             });
 
@@ -94,16 +102,17 @@ impl<'context> Elaborator<'context> {
                 parameters,
                 return_type,
                 where_clause,
-                body: _,
+                body,
                 is_unconstrained,
                 visibility: _,
                 is_comptime: _,
             } = &item.item
             {
                 self.recover_generics(|this| {
                     let the_trait = this.interner.get_trait(trait_id);
+                    let the_trait_where_clause = the_trait.where_clause.clone();
+                    let the_trait_constraint = the_trait.as_constraint(the_trait.name.span());
                     let self_typevar = the_trait.self_type_typevar.clone();
-                    let self_type = Type::TypeVariable(self_typevar.clone());
                     let name_span = the_trait.name.span();
 
                     this.add_existing_generic(
@@ -115,7 +124,6 @@ impl<'context> Elaborator<'context> {
                             span: name_span,
                         },
                     );
-                    this.self_type = Some(self_type.clone());
 
                     let func_id = unresolved_trait.method_ids[&name.0.contents];
 
@@ -127,6 +135,7 @@ impl<'context> Elaborator<'context> {
                         parameters,
                         return_type,
                         where_clause,
+                        body,
                         func_id,
                     );
 
@@ -135,7 +144,9 @@ impl<'context> Elaborator<'context> {
                         this.interner.set_doc_comments(id, item.doc_comments.clone());
                     }
 
-                    let func_meta = this.interner.function_meta(&func_id);
+                    let func_meta = this.interner.function_meta_mut(&func_id);
+                    func_meta.trait_constraints.push(the_trait_constraint);
+                    func_meta.trait_constraints.extend(the_trait_where_clause);
 
                     let arguments = vecmap(&func_meta.parameters.0, |(_, typ, _)| typ.clone());
                     let return_type = func_meta.return_type().clone();
@@ -189,19 +200,21 @@ impl<'context> Elaborator<'context> {
         parameters: &[(Ident, UnresolvedType)],
         return_type: &FunctionReturnType,
         where_clause: &[UnresolvedTraitConstraint],
+        body: &Option<BlockExpression>,
         func_id: FuncId,
     ) {
-        let old_generic_count = self.generics.len();
-
-        self.scopes.start_function();
+        let body = match body {
+            Some(body) => body.clone(),
+            None => BlockExpression { statements: Vec::new() },
+        };
 
         let kind = FunctionKind::Normal;
         let mut def = FunctionDefinition::normal(
             name,
             is_unconstrained,
             generics,
             parameters,
-            &BlockExpression { statements: Vec::new() },
+            body,
             where_clause,
             return_type,
         );
@@ -210,10 +223,6 @@ impl<'context> Elaborator<'context> {
 
         let mut function = NoirFunction { kind, def };
         self.define_function_meta(&mut function, func_id, Some(trait_id));
-        self.elaborate_function(func_id);
-        let _ = self.scopes.end_function();
-        // Don't check the scope tree for unused variables, they can't be used in a declaration anyway.
-        self.generics.truncate(old_generic_count);
     }
 }
 

compiler/noirc_frontend/src/elaborator/types.rs

@@ -418,7 +418,7 @@
                 // TODO(https://github.com/noir-lang/noir/issues/6238):
                 // support non-u32 generics here
                 if !kind.unifies(&Kind::u32()) {
                     let error = TypeCheckError::EvaluatedGlobalIsntU32 {
                         expected_kind: Kind::u32().to_string(),
                         expr_kind: kind.to_string(),
                         expr_span: path.span(),
@@ -544,12 +544,17 @@
     }
 
     // this resolves Self::some_static_method, inside an impl block (where we don't have a concrete self_type)
+    // or inside a trait default method.
     //
     // Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not
     // E.g. `t.method()` with `where T: Foo<Bar>` in scope will return `(Foo::method, T, vec![Bar])`
     fn resolve_trait_static_method_by_self(&mut self, path: &Path) -> Option<TraitPathResolution> {
-        let trait_impl = self.current_trait_impl?;
-        let trait_id = self.interner.try_get_trait_implementation(trait_impl)?.borrow().trait_id;
+        let trait_id = if let Some(current_trait) = self.current_trait {
+            current_trait
+        } else {
+            let trait_impl = self.current_trait_impl?;
+            self.interner.try_get_trait_implementation(trait_impl)?.borrow().trait_id
+        };
 
         if path.kind == PathKind::Plain && path.segments.len() == 2 {
             let name = &path.segments[0].ident.0.contents;

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

@@ -518,7 +518,7 @@
                                             *is_unconstrained,
                                             generics,
                                             parameters,
-                                            body,
+                                            body.clone(),
                                             where_clause,
                                             return_type,
                                         ));
@@ -869,7 +869,7 @@
     //   if it's an inline module, or the first char of a the file if it's an external module.
     // - `location` will always point to the token "foo" in `mod foo` regardless of whether
     //   it's inline or external.
     // Eventually the location put in `ModuleData` is used for codelenses about `contract`s,
     // so we keep using `location` so that it continues to work as usual.
     let location = Location::new(mod_name.span(), mod_location.file);
     let new_module = ModuleData::new(

compiler/noirc_frontend/src/hir_def/function.rs

@@ -175,12 +175,12 @@ pub enum FunctionBody {
 
 impl FuncMeta {
     /// A stub function does not have a body. This includes Builtin, LowLevel,
-    /// and Oracle functions in addition to method declarations within a trait.
+    /// and Oracle functions.
     ///
     /// We don't check the return type of these functions since it will always have
     /// an empty body, and we don't check for unused parameters.
     pub fn is_stub(&self) -> bool {
-        self.kind.can_ignore_return_type() || self.trait_id.is_some()
+        self.kind.can_ignore_return_type()
     }
 
     pub fn function_signature(&self) -> FunctionSignature {

compiler/noirc_frontend/src/tests.rs

@@ -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/6125)
 #[test]
 fn numeric_generic_field_larger_than_u32() {
@@ -2019,7 +2019,7 @@
     assert_eq!(errors.len(), 2);
     assert!(matches!(
         errors[0].0,
         CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),
     ));
     assert!(matches!(
         errors[1].0,
@@ -2028,7 +2028,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() {
@@ -2057,7 +2057,7 @@
 
     assert!(matches!(
         errors[0].0,
         CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),
     ));
 
     assert!(matches!(
@@ -2193,7 +2193,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 { .. }),
@@ -2988,7 +2988,7 @@
 fn uses_self_type_in_trait_where_clause() {
     let src = r#"
     pub trait Trait {
-        fn trait_func() -> bool;
+        fn trait_func(self) -> bool;
     }
 
     pub trait Foo where Self: Trait {
@@ -3216,7 +3216,7 @@
 }
 
 #[test]
 fn dont_infer_globals_to_u32_from_type_use() {
     let src = r#"
         global ARRAY_LEN = 3;
         global STR_LEN: _ = 2;
@@ -3246,7 +3246,7 @@
 }
 
 #[test]
 fn dont_infer_partial_global_types() {
     let src = r#"
         pub global ARRAY: [Field; _] = [0; 3];
         pub global NESTED_ARRAY: [[Field; _]; 3] = [[]; 3];
@@ -3487,7 +3487,7 @@
 
 #[test]
 fn unconditional_recursion_fail() {
     let srcs = vec![
         r#"
         fn main() {
             main()

compiler/noirc_frontend/src/tests/traits.rs

@@ -592,7 +592,7 @@ fn trait_bounds_which_are_dependent_on_generic_types_are_resolved_correctly() {
     // Regression test for https://github.com/noir-lang/noir/issues/6420
     let src = r#"
         trait Foo {
-            fn foo() -> Field;
+            fn foo(self) -> Field;
         }
 
         trait Bar<T>: Foo {
@@ -613,7 +613,8 @@ fn trait_bounds_which_are_dependent_on_generic_types_are_resolved_correctly() {
         where
             T: MarkerTrait,
         {
-            fn foo() -> Field {
+            fn foo(self) -> Field {
+                let _ = self;
                 42
             }
         }
@@ -652,3 +653,65 @@ fn does_not_crash_on_as_trait_path_with_empty_path() {
     );
     assert!(!errors.is_empty());
 }
+
+#[test]
+fn type_checks_trait_default_method_and_errors() {
+    let src = r#"
+        pub trait Foo {
+            fn foo(self) -> i32 {
+                let _ = self;
+                true
+            }
+        }
+
+        fn main() {}
+    "#;
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::TypeError(TypeCheckError::TypeMismatchWithSource {
+        expected,
+        actual,
+        ..
+    }) = &errors[0].0
+    else {
+        panic!("Expected a type mismatch error, got {:?}", errors[0].0);
+    };
+
+    assert_eq!(expected.to_string(), "i32");
+    assert_eq!(actual.to_string(), "bool");
+}
+
+#[test]
+fn type_checks_trait_default_method_and_does_not_error() {
+    let src = r#"
+        pub trait Foo {
+            fn foo(self) -> i32 {
+                let _ = self;
+                1
+            }
+        }
+
+        fn main() {}
+    "#;
+    assert_no_errors(src);
+}
+
+#[test]
+fn type_checks_trait_default_method_and_does_not_error_using_self() {
+    let src = r#"
+        pub trait Foo {
+            fn foo(self) -> i32 {
+                self.bar()
+            }
+
+            fn bar(self) -> i32 {
+                let _ = self;
+                1
+            }
+        }
+
+        fn main() {}
+    "#;
+    assert_no_errors(src);
+}