fix(frontend): Call trait method with mut self from generic definition (

#5041)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

Not sure if there is another issue for this I will have to look again,
but I found this bug when trying to implement
#4514

## Summary\*

When type checking a `MethodCall` from the HIR now we also add a mutable
reference to the object type when we have a
`HirMethodReference::TraitMethodId`. We were previously only doing this
for a `HirMethodReference::FuncId`.

For example that meant this program would fail with the following errors
laid out in the comments:
```rust
fn hash_simple_array<H>(input: [Field; 2]) -> Field where H: Hasher + Default {
    // Check that we can a trait method to instead a trait implementation
    let mut hasher: H = H::default();
    // Regression that the object is converted to a mutable reference type `&mut _`.
    // Otherwise will see `Expected type &mut _, found type H`.
    // Then we need to make sure to also auto dereference later in the type checking process
    // when searching for a matching impl or else we will get `No matching impl found for `&mut H: Hasher`
    hasher.write(input[0]);
    hasher.write(input[1]);
    hasher.finish()
}
```
I also had to add auto dereferencing when verifying the trait
constraints later inside of `type_check_func`. So first we add a mutable
reference to the method call's object type to avoid a mismatched type
error, and then we later dereference it to find the appropriate trait
implementation.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Jake Fecher <jake@aztecprotocol.com>
Co-authored-by: Jake Fecher <jfecher11@gmail.com>
Co-authored-by: Tom French <tom@tomfren.ch>

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

@@ -236,17 +236,27 @@ impl<'interner> TypeChecker<'interner> {
 
                         // Automatically add `&mut` if the method expects a mutable reference and
                         // the object is not already one.
-                        if let HirMethodReference::FuncId(func_id) = &method_ref {
-                            if *func_id != FuncId::dummy_id() {
-                                let function_type =
-                                    self.interner.function_meta(func_id).typ.clone();
-
-                                self.try_add_mutable_reference_to_object(
-                                    &mut method_call,
-                                    &function_type,
-                                    &mut object_type,
-                                );
+                        let func_id = match &method_ref {
+                            HirMethodReference::FuncId(func_id) => *func_id,
+                            HirMethodReference::TraitMethodId(method_id, _) => {
+                                let id = self.interner.trait_method_id(*method_id);
+                                let definition = self.interner.definition(id);
+                                let DefinitionKind::Function(func_id) = definition.kind else {
+                                    unreachable!(
+                                        "Expected trait function to be a DefinitionKind::Function"
+                                    )
+                                };
+                                func_id
                             }
+                        };
+
+                        if func_id != FuncId::dummy_id() {
+                            let function_type = self.interner.function_meta(&func_id).typ.clone();
+                            self.try_add_mutable_reference_to_object(
+                                &mut method_call,
+                                &function_type,
+                                &mut object_type,
+                            );
                         }
 
                         // TODO: update object_type here?
@@ -567,7 +577,7 @@ impl<'interner> TypeChecker<'interner> {
 
     /// Insert as many dereference operations as necessary to automatically dereference a method
     /// call object to its base value type T.
-    fn insert_auto_dereferences(&mut self, object: ExprId, typ: Type) -> (ExprId, Type) {
+    pub(crate) fn insert_auto_dereferences(&mut self, object: ExprId, typ: Type) -> (ExprId, Type) {
         if let Type::MutableReference(element) = typ {
             let location = self.interner.id_location(object);
 

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

@@ -141,8 +141,15 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type
     }
 
     // Verify any remaining trait constraints arising from the function body
-    for (constraint, expr_id) in std::mem::take(&mut type_checker.trait_constraints) {
+    for (mut constraint, expr_id) in std::mem::take(&mut type_checker.trait_constraints) {
         let span = type_checker.interner.expr_span(&expr_id);
+
+        if matches!(&constraint.typ, Type::MutableReference(_)) {
+            let (_, dereferenced_typ) =
+                type_checker.insert_auto_dereferences(expr_id, constraint.typ.clone());
+            constraint.typ = dereferenced_typ;
+        }
+
         type_checker.verify_trait_constraint(
             &constraint.typ,
             constraint.trait_id,

noir_stdlib/src/lib.nr

@@ -69,4 +69,4 @@ pub fn wrapping_mul<T>(x: T, y: T) -> T {
 }
 
 #[builtin(as_witness)]
-pub fn as_witness(x: Field) {}
+pub fn as_witness(x: Field) {}

test_programs/execution_success/trait_method_mut_self/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "trait_method_mut_self"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.29.0"
+
+[dependencies]

test_programs/execution_success/trait_method_mut_self/Prover.toml

@@ -0,0 +1,2 @@
+x = "5"
+y = "10"

test_programs/execution_success/trait_method_mut_self/src/main.nr

@@ -0,0 +1,74 @@
+use dep::std::hash::Hasher;
+use dep::std::hash::poseidon2::Poseidon2Hasher;
+
+fn main(x: Field, y: pub Field) {
+    let mut a_mut_ref = AType { x };
+
+    pass_trait_by_value(a_mut_ref, y);
+    assert(a_mut_ref.x == x);
+
+    pass_trait_by_value_impl_param(a_mut_ref, y);
+    assert(a_mut_ref.x == x);
+
+    pass_trait_by_mut_ref(&mut a_mut_ref, y);
+    assert(a_mut_ref.x == y);
+
+    let mut hasher = Poseidon2Hasher::default();
+    hasher.write(x);
+    hasher.write(y);
+    let expected_hash = hasher.finish();
+    // Check that we get the same result when using the hasher in a 
+    // method that purely uses trait methods without a supplied implementation.
+    assert(hash_simple_array::<Poseidon2Hasher>([x, y]) == expected_hash);
+}
+
+trait SomeTrait {
+    fn set_value(&mut self, new_value: Field) -> ();
+
+    fn get_value(self) -> Field;
+}
+
+struct AType {
+    x: Field
+}
+
+impl SomeTrait for AType {
+    fn set_value(&mut self, new_value: Field) -> () {
+        self.x = new_value;
+    }
+
+    fn get_value(self) -> Field {
+        self.x
+    }
+}
+
+fn pass_trait_by_value_impl_param(mut a_mut_ref: impl SomeTrait, value: Field) {
+    // We auto add a mutable reference to the object type if the method call expects a mutable self
+    a_mut_ref.set_value(value);
+    assert(a_mut_ref.get_value() == value);
+}
+
+fn pass_trait_by_value<T>(mut a_mut_ref: T, value: Field) where T: SomeTrait {
+    // We auto add a mutable reference to the object type if the method call expects a mutable self
+    a_mut_ref.set_value(value);
+    assert(a_mut_ref.get_value() == value);
+}
+
+fn pass_trait_by_mut_ref<T>(a_mut_ref: &mut T, value: Field) where T: SomeTrait {
+    // We auto add a mutable reference to the object type if the method call expects a mutable self
+    a_mut_ref.set_value(value);
+}
+
+fn hash_simple_array<H>(input: [Field; 2]) -> Field where H: Hasher + Default {
+    // Check that we can call a trait method instead of a trait implementation
+    // TODO: Need to remove the need for this type annotation
+    // TODO: Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher`
+    let mut hasher: H = H::default();
+    // Regression that the object is converted to a mutable reference type `&mut _`.
+    // Otherwise will see `Expected type &mut _, found type H`.
+    // Then we need to make sure to also auto dereference later in the type checking process
+    // when searching for a matching impl or else we will get `No matching impl found for `&mut H: Hasher`
+    hasher.write(input[0]);
+    hasher.write(input[1]);
+    hasher.finish()
+}