Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Mutating a variable no longer mutates its copy #2057

Merged
merged 4 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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