Skip to content

Commit

Permalink
fix: error on &mut x when x is not mutable (#6037)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
asterite authored Sep 13, 2024
1 parent 76dea7b commit 57afc7d
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 2 deletions.
28 changes: 27 additions & 1 deletion compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}")]
Expand Down Expand Up @@ -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, .. }
Expand Down
52 changes: 52 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

0 comments on commit 57afc7d

Please sign in to comment.