From 57afc7ddd424220106af7b9c6e0715007f6ea8b8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 13 Sep 2024 15:55:31 -0300 Subject: [PATCH] fix: error on `&mut x` when `x` is not mutable (#6037) # Description ## Problem Resolves #5721 ## Summary Today I bumped into this bug so I thought about trying to fix it. ## Additional Context ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/elaborator/expressions.rs | 28 +++++++++- .../src/hir/type_check/errors.rs | 5 +- compiler/noirc_frontend/src/tests.rs | 52 +++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 0ab9a252f09..56e5bbb2da7 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -5,7 +5,7 @@ use rustc_hash::FxHashSet as HashSet; use crate::{ ast::{ - ArrayLiteral, ConstructorExpression, IfExpression, InfixExpression, Lambda, + ArrayLiteral, ConstructorExpression, IfExpression, InfixExpression, Lambda, UnaryOp, UnresolvedTypeData, UnresolvedTypeExpression, }, hir::{ @@ -248,10 +248,17 @@ impl<'context> Elaborator<'context> { } fn elaborate_prefix(&mut self, prefix: PrefixExpression, span: Span) -> (ExprId, Type) { + let rhs_span = prefix.rhs.span; + let (rhs, rhs_type) = self.elaborate_expression(prefix.rhs); let trait_id = self.interner.get_prefix_operator_trait_method(&prefix.operator); let operator = prefix.operator; + + if let UnaryOp::MutableReference = operator { + self.check_can_mutate(rhs, rhs_span); + } + let expr = HirExpression::Prefix(HirPrefixExpression { operator, rhs, trait_method_id: trait_id }); let expr_id = self.interner.push_expr(expr); @@ -264,6 +271,25 @@ impl<'context> Elaborator<'context> { (expr_id, typ) } + fn check_can_mutate(&mut self, expr_id: ExprId, span: Span) { + let expr = self.interner.expression(&expr_id); + match expr { + HirExpression::Ident(hir_ident, _) => { + let definition = self.interner.definition(hir_ident.id); + if !definition.mutable { + self.push_err(TypeCheckError::CannotMutateImmutableVariable { + name: definition.name.clone(), + span, + }); + } + } + HirExpression::MemberAccess(member_access) => { + self.check_can_mutate(member_access.lhs, span); + } + _ => (), + } + } + fn elaborate_index(&mut self, index_expr: IndexExpression) -> (HirExpression, Type) { let span = index_expr.index.span; let (index, index_type) = self.elaborate_expression(index_expr.index); diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 5eb7a40dcd7..626084ce1b6 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -65,8 +65,10 @@ pub enum TypeCheckError { UnsupportedCast { span: Span }, #[error("Index {index} is out of bounds for this tuple {lhs_type} of length {length}")] TupleIndexOutOfBounds { index: usize, lhs_type: Type, length: usize, span: Span }, - #[error("Variable {name} must be mutable to be assigned to")] + #[error("Variable `{name}` must be mutable to be assigned to")] VariableMustBeMutable { name: String, span: Span }, + #[error("Cannot mutate immutable variable `{name}`")] + CannotMutateImmutableVariable { name: String, span: Span }, #[error("No method named '{method_name}' found for type '{object_type}'")] UnresolvedMethodCall { method_name: String, object_type: Type, span: Span }, #[error("Integers must have the same signedness LHS is {sign_x:?}, RHS is {sign_y:?}")] @@ -268,6 +270,7 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { | TypeCheckError::UnsupportedCast { span } | TypeCheckError::TupleIndexOutOfBounds { span, .. } | TypeCheckError::VariableMustBeMutable { span, .. } + | TypeCheckError::CannotMutateImmutableVariable { span, .. } | TypeCheckError::UnresolvedMethodCall { span, .. } | TypeCheckError::IntegerSignedness { span, .. } | TypeCheckError::IntegerBitWidth { span, .. } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 414bfa5fa5b..9692f4da5e2 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3462,3 +3462,55 @@ fn comptime_type_in_runtime_code() { CompilationError::ResolverError(ResolverError::ComptimeTypeInRuntimeCode { .. }) )); } + +#[test] +fn cannot_mutate_immutable_variable() { + let src = r#" + fn main() { + let array = [1]; + mutate(&mut array); + } + + fn mutate(_: &mut [Field; 1]) {} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::CannotMutateImmutableVariable { name, .. }) = + &errors[0].0 + else { + panic!("Expected a CannotMutateImmutableVariable error"); + }; + + assert_eq!(name, "array"); +} + +#[test] +fn cannot_mutate_immutable_variable_on_member_access() { + let src = r#" + struct Foo { + x: Field + } + + fn main() { + let foo = Foo { x: 0 }; + mutate(&mut foo.x); + } + + fn mutate(foo: &mut Field) { + *foo = 1; + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::CannotMutateImmutableVariable { name, .. }) = + &errors[0].0 + else { + panic!("Expected a CannotMutateImmutableVariable error"); + }; + + assert_eq!(name, "foo"); +}