fix: Apply trait constraints from method calls

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

@@ -1450,6 +1450,8 @@ impl<'a> Resolver<'a> {
                 HirExpression::MemberAccess(HirMemberAccess {
                     lhs: self.resolve_expression(access.lhs),
                     rhs: access.rhs,
+                    // This is only used when lhs is a reference and we want to return a reference to rhs
+                    is_offset: false,
                 })
             }
             ExpressionKind::Error => HirExpression::Error,

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

@@ -206,24 +206,22 @@ impl<'interner> TypeChecker<'interner> {
                 let object_ref = &mut object;
                 let mutable_ref = &mut mutable;
 
+                let dereference_lhs = move |_: &mut Self, _, element_type| {
+                    // We must create a temporary value first to move out of object_ref before
+                    // we eventually reassign to it.
+                    let id = DefinitionId::dummy_id();
+                    let location = Location::new(span, fm::FileId::dummy());
+                    let ident = HirIdent::non_trait_method(id, location);
+                    let tmp_value = HirLValue::Ident(ident, Type::Error);
+
+                    let lvalue = std::mem::replace(object_ref, Box::new(tmp_value));
+                    *object_ref = Box::new(HirLValue::Dereference { lvalue, element_type });
+                    *mutable_ref = true;
+                };
+
+                let name = &field_name.0.contents;
                 let (object_type, field_index) = self
-                    .check_field_access(
-                        &lhs_type,
-                        &field_name.0.contents,
-                        span,
-                        move |_, _, element_type| {
-                            // We must create a temporary value first to move out of object_ref before
-                            // we eventually reassign to it.
-                            let id = DefinitionId::dummy_id();
-                            let location = Location::new(span, fm::FileId::dummy());
-                            let ident = HirIdent::non_trait_method(id, location);
-                            let tmp_value = HirLValue::Ident(ident, Type::Error);
-
-                            let lvalue = std::mem::replace(object_ref, Box::new(tmp_value));
-                            *object_ref = Box::new(HirLValue::Dereference { lvalue, element_type });
-                            *mutable_ref = true;
-                        },
-                    )
+                    .check_field_access(&lhs_type, name, span, Some(dereference_lhs))
                     .unwrap_or((Type::Error, 0));
 
                 let field_index = Some(field_index);
@@ -325,6 +323,7 @@ impl<'interner> TypeChecker<'interner> {
             // Now check if LHS is the same type as the RHS
             // Importantly, we do not coerce any types implicitly
             let expr_span = self.interner.expr_span(&rhs_expr);
+
             self.unify_with_coercions(&expr_type, &annotated_type, rhs_expr, || {
                 TypeCheckError::TypeMismatch {
                     expected_typ: annotated_type.to_string(),
@@ -335,10 +334,8 @@ impl<'interner> TypeChecker<'interner> {
             if annotated_type.is_unsigned() {
                 self.lint_overflowing_uint(&rhs_expr, &annotated_type);
             }
-            annotated_type
-        } else {
-            expr_type
         }
+        expr_type
     }
 
     /// Check if an assignment is overflowing with respect to `annotated_type`

compiler/noirc_frontend/src/hir_def/expr.rs

@@ -152,6 +152,12 @@ pub struct HirMemberAccess {
     // This field is not an IdentId since the rhs of a field
     // access has no corresponding definition
     pub rhs: Ident,
+
+    /// True if we should return an offset of the field rather than the field itself.
+    /// For most cases this is false, corresponding to `foo.bar` in source code.
+    /// This is true when calling methods or when we have an lvalue we want to preserve such
+    /// that if `foo : &mut Foo` has a field `bar : Bar`, we can return an `&mut Bar`.
+    pub is_offset: bool,
 }
 
 #[derive(Debug, Clone)]
@@ -201,13 +207,14 @@ pub enum HirMethodReference {
 }
 
 impl HirMethodCallExpression {
+    /// Converts a method call into a function call
     pub fn into_function_call(
         mut self,
         method: &HirMethodReference,
         object_type: Type,
         location: Location,
         interner: &mut NodeInterner,
-    ) -> (ExprId, HirExpression) {
+    ) -> HirExpression {
         let mut arguments = vec![self.object];
         arguments.append(&mut self.arguments);
 
@@ -225,9 +232,10 @@ impl HirMethodCallExpression {
                 (id, ImplKind::TraitMethod(*method_id, constraint, false))
             }
         };
-        let expr = HirExpression::Ident(HirIdent { location, id, impl_kind });
-        let func = interner.push_expr(expr);
-        (func, HirExpression::Call(HirCallExpression { func, arguments, location }))
+        let func = HirExpression::Ident(HirIdent { location, id, impl_kind });
+        let func = interner.push_expr(func);
+        interner.push_expr_location(func, location.span, location.file);
+        HirExpression::Call(HirCallExpression { func, arguments, location })
     }
 }
 

compiler/noirc_frontend/src/monomorphization/printer.rs

@@ -30,7 +30,9 @@ impl AstPrinter {
 
     pub fn print_expr(&mut self, expr: &Expression, f: &mut Formatter) -> std::fmt::Result {
         match expr {
-            Expression::Ident(ident) => write!(f, "{}${}", ident.name, ident.definition),
+            Expression::Ident(ident) => {
+                write!(f, "{}${}", ident.name, ident.definition)
+            }
             Expression::Literal(literal) => self.print_literal(literal, f),
             Expression::Block(exprs) => self.print_block(exprs, f),
             Expression::Unary(unary) => self.print_unary(unary, f),

test_programs/execution_success/regression_4124/Nargo.toml

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

test_programs/execution_success/regression_4124/Prover.toml

@@ -0,0 +1 @@
+value = 0

test_programs/execution_success/regression_4124/src/main.nr

@@ -0,0 +1,39 @@
+use dep::std::option::Option;
+
+trait MyDeserialize<N> {
+    fn deserialize(fields: [Field; N]) -> Self;
+}
+
+impl MyDeserialize<1> for Field {
+    fn deserialize(fields: [Field; 1]) -> Self {
+        fields[0]
+    }
+}
+
+pub fn storage_read<N>() -> [Field; N] {
+    dep::std::unsafe::zeroed()
+}
+
+struct PublicState<T> {
+    storage_slot: Field,
+}
+
+impl<T> PublicState<T> {
+    pub fn new(storage_slot: Field) -> Self {
+        assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
+        PublicState { storage_slot }
+    }
+
+    pub fn read<T_SERIALIZED_LEN>(_self: Self) -> T where T: MyDeserialize<T_SERIALIZED_LEN> {
+        // storage_read returns slice here
+        let fields: [Field; T_SERIALIZED_LEN] = storage_read();
+        T::deserialize(fields)
+    }
+}
+
+fn main(value: Field) {
+    let ps: PublicState<Field> = PublicState::new(27);
+
+    // error here
+    assert(ps.read() == value);
+}