Skip to content

Commit

Permalink
fix: Mutating a variable no longer mutates its copy (#2057)
Browse files Browse the repository at this point in the history
* Fix 2054

* Rename function
  • Loading branch information
jfecher authored Aug 1, 2023
1 parent ab61e3a commit e85e485
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
10 changes: 10 additions & 0 deletions crates/nargo_cli/tests/test_data/references/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ fn main(mut x: Field) {
assert(*c.bar.array == [3, 4]);

regression_1887();
regression_2054();
}

fn add1(x: &mut Field) {
Expand Down Expand Up @@ -77,3 +78,12 @@ impl Bar {
self.x = 32;
}
}

// Ensure that mutating a variable does not also mutate its copy
fn regression_2054() {
let mut x = 2;
let z = x;

x += 1;
assert(z == 2);
}
31 changes: 27 additions & 4 deletions crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,13 @@ impl<'a> FunctionContext<'a> {
self.codegen_expression(expr).into_leaf().eval(self)
}

/// Codegen for identifiers
fn codegen_ident(&mut self, ident: &ast::Ident) -> Values {
/// Codegen a reference to an ident.
/// The only difference between this and codegen_ident is that if the variable is mutable
/// as in `let mut var = ...;` the `Value::Mutable` will be returned directly instead of
/// being automatically loaded from. This is needed when taking the reference of a variable
/// to reassign to it. Note that mutable references `let x = &mut ...;` do not require this
/// since they are not automatically loaded from and must be explicitly dereferenced.
fn codegen_ident_reference(&mut self, ident: &ast::Ident) -> Values {
match &ident.definition {
ast::Definition::Local(id) => self.lookup(*id),
ast::Definition::Function(id) => self.get_or_queue_function(*id),
Expand All @@ -104,6 +109,11 @@ impl<'a> FunctionContext<'a> {
}
}

/// Codegen an identifier, automatically loading its value if it is mutable.
fn codegen_ident(&mut self, ident: &ast::Ident) -> Values {
self.codegen_ident_reference(ident).map(|value| value.eval(self).into())
}

fn codegen_literal(&mut self, literal: &ast::Literal) -> Values {
match literal {
ast::Literal::Array(array) => {
Expand Down Expand Up @@ -159,20 +169,21 @@ impl<'a> FunctionContext<'a> {
}

fn codegen_unary(&mut self, unary: &ast::Unary) -> Values {
let rhs = self.codegen_expression(&unary.rhs);
match unary.operator {
noirc_frontend::UnaryOp::Not => {
let rhs = self.codegen_expression(&unary.rhs);
let rhs = rhs.into_leaf().eval(self);
self.builder.insert_not(rhs).into()
}
noirc_frontend::UnaryOp::Minus => {
let rhs = self.codegen_expression(&unary.rhs);
let rhs = rhs.into_leaf().eval(self);
let typ = self.builder.type_of_value(rhs);
let zero = self.builder.numeric_constant(0u128, typ);
self.builder.insert_binary(zero, BinaryOp::Sub, rhs).into()
}
noirc_frontend::UnaryOp::MutableReference => {
rhs.map(|rhs| {
self.codegen_reference(&unary.rhs).map(|rhs| {
match rhs {
value::Value::Normal(value) => {
let alloc = self.builder.insert_allocate();
Expand All @@ -186,11 +197,23 @@ impl<'a> FunctionContext<'a> {
})
}
noirc_frontend::UnaryOp::Dereference { .. } => {
let rhs = self.codegen_expression(&unary.rhs);
self.dereference(&rhs, &unary.result_type)
}
}
}

fn codegen_reference(&mut self, expr: &Expression) -> Values {
match expr {
Expression::Ident(ident) => self.codegen_ident_reference(ident),
Expression::ExtractTupleField(tuple, index) => {
let tuple = self.codegen_reference(tuple);
Self::get_field(tuple, *index)
}
other => self.codegen_expression(other),
}
}

fn codegen_binary(&mut self, binary: &ast::Binary) -> Values {
let lhs = self.codegen_non_tuple_expression(&binary.lhs);
let rhs = self.codegen_non_tuple_expression(&binary.rhs);
Expand Down

0 comments on commit e85e485

Please sign in to comment.