From ae28157071640f1a2c6eb4e516337359b3354881 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 24 Jan 2024 11:58:07 -0800 Subject: [PATCH 1/4] Fix bug by removing code duplication --- .../noirc_frontend/src/hir/type_check/expr.rs | 124 +++++------------- .../noirc_frontend/src/hir/type_check/stmt.rs | 5 +- compiler/noirc_frontend/src/hir_def/expr.rs | 10 +- .../src/monomorphization/printer.rs | 4 +- 4 files changed, 42 insertions(+), 101 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 22baa9f0da5..74c0193ba98 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -170,55 +170,47 @@ impl<'interner> TypeChecker<'interner> { // so that the backend doesn't need to worry about methods let location = method_call.location; - let trait_id = match &method_ref { - HirMethodReference::FuncId(func_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 function_type = - self.interner.function_meta(func_id).typ.clone(); - self.try_add_mutable_reference_to_object( - &mut method_call, - &function_type, - &mut args, - ); - } - - let meta = self.interner.function_meta(func_id); - meta.trait_impl.map(|impl_id| { - self.interner - .get_trait_implementation(impl_id) - .borrow() - .trait_id - }) + // 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 args, + ); } - HirMethodReference::TraitMethodId(method, _) => Some(method.trait_id), - }; + } - let (function_id, function_call) = method_call.into_function_call( + let function_call = method_call.into_function_call( &method_ref, object_type.clone(), location, self.interner, ); - let span = self.interner.expr_span(expr_id); - let ret = self.check_method_call(&function_id, method_ref, args, span); - - if let Some(trait_id) = trait_id { - // Assume no trait generics were specified - // TODO: Fill in type variables - self.verify_trait_constraint( - &object_type, - trait_id, - &[], - function_id, - span, - ); - } + // let span = self.interner.expr_span(expr_id); + // let ret = self.check_method_call(&function_id, method_ref, args, span); + + // if let Some(trait_id) = trait_id { + // // Assume no trait generics were specified + // // TODO: Fill in type variables + // self.verify_trait_constraint( + // &object_type, + // trait_id, + // &[], + // function_id, + // span, + // ); + // } self.interner.replace_expr(expr_id, function_call); - ret + + // Type check the new call now that it has been changed from a method call + // to a function call. This way we avoid duplicating code. + self.check_expression(expr_id) } None => Type::Error, } @@ -584,60 +576,6 @@ impl<'interner> TypeChecker<'interner> { } } - // We need a special function to type check method calls since the method - // is not a Expression::Ident it must be manually instantiated here - fn check_method_call( - &mut self, - function_ident_id: &ExprId, - method_ref: HirMethodReference, - arguments: Vec<(Type, ExprId, Span)>, - span: Span, - ) -> Type { - let (fn_typ, param_len, generic_bindings) = match method_ref { - HirMethodReference::FuncId(func_id) => { - if func_id == FuncId::dummy_id() { - return Type::Error; - } - - let func_meta = self.interner.function_meta(&func_id); - let param_len = func_meta.parameters.len(); - (func_meta.typ.clone(), param_len, TypeBindings::new()) - } - HirMethodReference::TraitMethodId(method, generics) => { - let the_trait = self.interner.get_trait(method.trait_id); - let method = &the_trait.methods[method.method_index]; - - // These are any bindings from the trait's generics itself, - // rather than an impl or method's generics. - let generic_bindings = the_trait - .generics - .iter() - .zip(generics) - .map(|(var, arg)| (var.id(), (var.clone(), arg))) - .collect(); - - (method.typ.clone(), method.arguments().len(), generic_bindings) - } - }; - - let arg_len = arguments.len(); - - if param_len != arg_len { - self.errors.push(TypeCheckError::ArityMisMatch { - expected: param_len as u16, - found: arg_len as u16, - span, - }); - } - - let (function_type, instantiation_bindings) = - fn_typ.instantiate_with_bindings(generic_bindings, self.interner); - - self.interner.store_instantiation_bindings(*function_ident_id, instantiation_bindings); - self.interner.push_expr_type(function_ident_id, function_type.clone()); - self.bind_function_type(function_type.clone(), arguments, span) - } - fn check_if_expr(&mut self, if_expr: &expr::HirIfExpression, expr_id: &ExprId) -> Type { let cond_type = self.check_expression(&if_expr.condition); let then_type = self.check_expression(&if_expr.consequence); diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index c4e72f48c50..72d01071337 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -325,6 +325,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 +336,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` diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index fe1cd78b5ed..93cd1068438 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -201,13 +201,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 +226,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 }) } } diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index e79330de6f8..90cc85e7aca 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/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, ident.typ) + } Expression::Literal(literal) => self.print_literal(literal, f), Expression::Block(exprs) => self.print_block(exprs, f), Expression::Unary(unary) => self.print_unary(unary, f), From bc33a09015a2fac1f79d60b81e444d8a6cd8a832 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 24 Jan 2024 12:09:18 -0800 Subject: [PATCH 2/4] Fix adding &mut, and add test --- .../noirc_frontend/src/hir/type_check/expr.rs | 14 +++---- .../regression_4124/Nargo.toml | 7 ++++ .../regression_4124/Prover.toml | 1 + .../regression_4124/src/main.nr | 39 +++++++++++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 test_programs/execution_success/regression_4124/Nargo.toml create mode 100644 test_programs/execution_success/regression_4124/Prover.toml create mode 100644 test_programs/execution_success/regression_4124/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 74c0193ba98..0b1848eee84 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -176,6 +176,7 @@ impl<'interner> TypeChecker<'interner> { 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, @@ -430,17 +431,15 @@ impl<'interner> TypeChecker<'interner> { // If that didn't work, then wrap the whole expression in an `&mut` let location = self.interner.id_location(method_call.object); - method_call.object = + let new_object = self.interner.push_expr(HirExpression::Prefix(HirPrefixExpression { operator: UnaryOp::MutableReference, rhs: method_call.object, })); - self.interner.push_expr_type(&method_call.object, new_type); - self.interner.push_expr_location( - method_call.object, - location.span, - location.file, - ); + method_call.object = new_object; + argument_types[0].1 = new_object; + self.interner.push_expr_type(&new_object, new_type); + self.interner.push_expr_location(new_object, location.span, location.file); } } // Otherwise if the object type is a mutable reference and the method is not, insert as @@ -450,6 +449,7 @@ impl<'interner> TypeChecker<'interner> { self.insert_auto_dereferences(method_call.object, actual_type); method_call.object = object; argument_types[0].0 = new_type; + argument_types[0].1 = object; } } } diff --git a/test_programs/execution_success/regression_4124/Nargo.toml b/test_programs/execution_success/regression_4124/Nargo.toml new file mode 100644 index 00000000000..9b97d1ce087 --- /dev/null +++ b/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] diff --git a/test_programs/execution_success/regression_4124/Prover.toml b/test_programs/execution_success/regression_4124/Prover.toml new file mode 100644 index 00000000000..533d1af92b9 --- /dev/null +++ b/test_programs/execution_success/regression_4124/Prover.toml @@ -0,0 +1 @@ +value = 0 diff --git a/test_programs/execution_success/regression_4124/src/main.nr b/test_programs/execution_success/regression_4124/src/main.nr new file mode 100644 index 00000000000..b47bf28d461 --- /dev/null +++ b/test_programs/execution_success/regression_4124/src/main.nr @@ -0,0 +1,39 @@ +use dep::std::option::Option; + +trait MyDeserialize { + fn deserialize(fields: [Field; N]) -> Self; +} + +impl MyDeserialize<1> for Field { + fn deserialize(fields: [Field; 1]) -> Self { + fields[0] + } +} + +pub fn storage_read() -> [Field; N] { + dep::std::unsafe::zeroed() +} + +struct PublicState { + storage_slot: Field, +} + +impl PublicState { + 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(_self: Self) -> T where T: MyDeserialize { + // storage_read returns slice here + let fields: [Field; T_SERIALIZED_LEN] = storage_read(); + T::deserialize(fields) + } +} + +fn main(value: Field) { + let ps: PublicState = PublicState::new(27); + + // error here + assert(ps.read() == value); +} From 24c2587238639afacc4cec8013ecbe79d45b273d Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 24 Jan 2024 14:28:54 -0800 Subject: [PATCH 3/4] Cleanup --- .../noirc_frontend/src/hir/type_check/expr.rs | 41 +++---------------- .../src/monomorphization/printer.rs | 2 +- 2 files changed, 6 insertions(+), 37 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 0b1848eee84..977fbebb4f0 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -155,17 +155,6 @@ impl<'interner> TypeChecker<'interner> { let method_name = method_call.method.0.contents.as_str(); match self.lookup_method(&object_type, method_name, expr_id) { Some(method_ref) => { - let mut args = vec![( - object_type.clone(), - method_call.object, - self.interner.expr_span(&method_call.object), - )]; - - for arg in &method_call.arguments { - let typ = self.check_expression(arg); - args.push((typ, *arg, self.interner.expr_span(arg))); - } - // Desugar the method call into a normal, resolved function call // so that the backend doesn't need to worry about methods let location = method_call.location; @@ -180,33 +169,18 @@ impl<'interner> TypeChecker<'interner> { self.try_add_mutable_reference_to_object( &mut method_call, &function_type, - &mut args, + &object_type, ); } } let function_call = method_call.into_function_call( &method_ref, - object_type.clone(), + object_type, location, self.interner, ); - // let span = self.interner.expr_span(expr_id); - // let ret = self.check_method_call(&function_id, method_ref, args, span); - - // if let Some(trait_id) = trait_id { - // // Assume no trait generics were specified - // // TODO: Fill in type variables - // self.verify_trait_constraint( - // &object_type, - // trait_id, - // &[], - // function_id, - // span, - // ); - // } - self.interner.replace_expr(expr_id, function_call); // Type check the new call now that it has been changed from a method call @@ -401,7 +375,7 @@ impl<'interner> TypeChecker<'interner> { &mut self, method_call: &mut HirMethodCallExpression, function_type: &Type, - argument_types: &mut [(Type, ExprId, noirc_errors::Span)], + object_type: &Type, ) { let expected_object_type = match function_type { Type::Function(args, _, _) => args.first(), @@ -413,7 +387,7 @@ impl<'interner> TypeChecker<'interner> { }; if let Some(expected_object_type) = expected_object_type { - let actual_type = argument_types[0].0.follow_bindings(); + let actual_type = object_type.follow_bindings(); if matches!(expected_object_type.follow_bindings(), Type::MutableReference(_)) { if !matches!(actual_type, Type::MutableReference(_)) { @@ -423,7 +397,6 @@ impl<'interner> TypeChecker<'interner> { } let new_type = Type::MutableReference(Box::new(actual_type)); - argument_types[0].0 = new_type.clone(); // First try to remove a dereference operator that may have been implicitly // inserted by a field access expression `foo.bar` on a mutable reference `foo`. @@ -437,7 +410,6 @@ impl<'interner> TypeChecker<'interner> { rhs: method_call.object, })); method_call.object = new_object; - argument_types[0].1 = new_object; self.interner.push_expr_type(&new_object, new_type); self.interner.push_expr_location(new_object, location.span, location.file); } @@ -445,11 +417,8 @@ impl<'interner> TypeChecker<'interner> { // Otherwise if the object type is a mutable reference and the method is not, insert as // many dereferences as needed. } else if matches!(actual_type, Type::MutableReference(_)) { - let (object, new_type) = - self.insert_auto_dereferences(method_call.object, actual_type); + let object = self.insert_auto_dereferences(method_call.object, actual_type).0; method_call.object = object; - argument_types[0].0 = new_type; - argument_types[0].1 = object; } } } diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index 90cc85e7aca..7aec2193494 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/compiler/noirc_frontend/src/monomorphization/printer.rs @@ -31,7 +31,7 @@ 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, ident.typ) + write!(f, "{}${}", ident.name, ident.definition) } Expression::Literal(literal) => self.print_literal(literal, f), Expression::Block(exprs) => self.print_block(exprs, f), From 8c4791699c46d041b63bea522b585876dbf8a6f3 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 25 Jan 2024 12:55:14 -0800 Subject: [PATCH 4/4] Add workaround for deref issue --- .../src/hir/resolution/resolver.rs | 2 + .../noirc_frontend/src/hir/type_check/expr.rs | 79 +++++++++++-------- .../noirc_frontend/src/hir/type_check/stmt.rs | 32 ++++---- compiler/noirc_frontend/src/hir_def/expr.rs | 6 ++ 4 files changed, 71 insertions(+), 48 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index f3e3b221036..df533f6a4ae 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/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, diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 977fbebb4f0..9be8832268b 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -151,7 +151,7 @@ impl<'interner> TypeChecker<'interner> { self.bind_function_type(function, args, span) } HirExpression::MethodCall(mut method_call) => { - let object_type = self.check_expression(&method_call.object).follow_bindings(); + let mut object_type = self.check_expression(&method_call.object).follow_bindings(); let method_name = method_call.method.0.contents.as_str(); match self.lookup_method(&object_type, method_name, expr_id) { Some(method_ref) => { @@ -169,11 +169,12 @@ impl<'interner> TypeChecker<'interner> { self.try_add_mutable_reference_to_object( &mut method_call, &function_type, - &object_type, + &mut object_type, ); } } + // TODO: update object_type here? let function_call = method_call.into_function_call( &method_ref, object_type, @@ -375,7 +376,7 @@ impl<'interner> TypeChecker<'interner> { &mut self, method_call: &mut HirMethodCallExpression, function_type: &Type, - object_type: &Type, + object_type: &mut Type, ) { let expected_object_type = match function_type { Type::Function(args, _, _) => args.first(), @@ -397,11 +398,14 @@ impl<'interner> TypeChecker<'interner> { } let new_type = Type::MutableReference(Box::new(actual_type)); + *object_type = new_type.clone(); // First try to remove a dereference operator that may have been implicitly // inserted by a field access expression `foo.bar` on a mutable reference `foo`. - if self.try_remove_implicit_dereference(method_call.object).is_none() { - // If that didn't work, then wrap the whole expression in an `&mut` + let new_object = self.try_remove_implicit_dereference(method_call.object); + + // If that didn't work, then wrap the whole expression in an `&mut` + method_call.object = new_object.unwrap_or_else(|| { let location = self.interner.id_location(method_call.object); let new_object = @@ -409,15 +413,17 @@ impl<'interner> TypeChecker<'interner> { operator: UnaryOp::MutableReference, rhs: method_call.object, })); - method_call.object = new_object; self.interner.push_expr_type(&new_object, new_type); self.interner.push_expr_location(new_object, location.span, location.file); - } + new_object + }); } // Otherwise if the object type is a mutable reference and the method is not, insert as // many dereferences as needed. } else if matches!(actual_type, Type::MutableReference(_)) { - let object = self.insert_auto_dereferences(method_call.object, actual_type).0; + let (object, new_type) = + self.insert_auto_dereferences(method_call.object, actual_type); + *object_type = new_type; method_call.object = object; } } @@ -446,29 +452,22 @@ impl<'interner> TypeChecker<'interner> { /// Given a method object: `(*foo).bar` of a method call `(*foo).bar.baz()`, remove the /// implicitly added dereference operator if one is found. /// - /// Returns Some(()) if a dereference was removed and None otherwise. - fn try_remove_implicit_dereference(&mut self, object: ExprId) -> Option<()> { + /// Returns Some(new_expr_id) if a dereference was removed and None otherwise. + fn try_remove_implicit_dereference(&mut self, object: ExprId) -> Option { match self.interner.expression(&object) { - HirExpression::MemberAccess(access) => { - self.try_remove_implicit_dereference(access.lhs)?; - - // Since we removed a dereference, instead of returning the field directly, - // we expect to be returning a reference to the field, so update the type accordingly. - let current_type = self.interner.id_type(object); - let reference_type = Type::MutableReference(Box::new(current_type)); - self.interner.push_expr_type(&object, reference_type); - Some(()) + HirExpression::MemberAccess(mut access) => { + let new_lhs = self.try_remove_implicit_dereference(access.lhs)?; + access.lhs = new_lhs; + access.is_offset = true; + + // `object` will have a different type now, which will be filled in + // later when type checking the method call as a function call. + self.interner.replace_expr(&object, HirExpression::MemberAccess(access)); + Some(object) } HirExpression::Prefix(prefix) => match prefix.operator { - UnaryOp::Dereference { implicitly_added: true } => { - // Found a dereference we can remove. Now just replace it with its rhs to remove it. - let rhs = self.interner.expression(&prefix.rhs); - self.interner.replace_expr(&object, rhs); - - let rhs_type = self.interner.id_type(prefix.rhs); - self.interner.push_expr_type(&object, rhs_type); - Some(()) - } + // Found a dereference we can remove. Now just replace it with its rhs to remove it. + UnaryOp::Dereference { implicitly_added: true } => Some(prefix.rhs), _ => None, }, _ => None, @@ -643,6 +642,9 @@ impl<'interner> TypeChecker<'interner> { this.interner.push_expr_location(*access_lhs, span, old_location.file); }; + // If this access is just a field offset, we want to avoid dereferencing + let dereference_lhs = (!access.is_offset).then_some(dereference_lhs); + match self.check_field_access(&lhs_type, &access.rhs.0.contents, span, dereference_lhs) { Some((element_type, index)) => { self.interner.set_field_index(expr_id, index); @@ -667,12 +669,16 @@ impl<'interner> TypeChecker<'interner> { /// expression. The second parameter of this function represents the lhs_type (which should /// always be a Type::MutableReference if `dereference_lhs` is called) and the third /// represents the element type. + /// + /// If `dereference_lhs` is None, this will assume we're taking the offset of a struct field + /// rather than dereferencing it. So the result of `foo.bar` with a `foo : &mut Foo` will + /// be a `&mut Bar` rather than just a `Bar`. pub(super) fn check_field_access( &mut self, lhs_type: &Type, field_name: &str, span: Span, - mut dereference_lhs: impl FnMut(&mut Self, Type, Type), + dereference_lhs: Option, ) -> Option<(Type, usize)> { let lhs_type = lhs_type.follow_bindings(); @@ -702,8 +708,19 @@ impl<'interner> TypeChecker<'interner> { // If the lhs is a mutable reference we automatically transform // lhs.field into (*lhs).field Type::MutableReference(element) => { - dereference_lhs(self, lhs_type.clone(), element.as_ref().clone()); - return self.check_field_access(element, field_name, span, dereference_lhs); + if let Some(mut dereference_lhs) = dereference_lhs { + dereference_lhs(self, lhs_type.clone(), element.as_ref().clone()); + return self.check_field_access( + element, + field_name, + span, + Some(dereference_lhs), + ); + } else { + let (element, index) = + self.check_field_access(element, field_name, span, dereference_lhs)?; + return Some((Type::MutableReference(Box::new(element)), index)); + } } _ => (), } diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 72d01071337..fd8ae62d34e 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/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); diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 93cd1068438..75ed68af0f6 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/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)]