From 1d1d4421c0861419c6298bcfc7ddb34087b17003 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 26 Jul 2023 13:20:56 -0500 Subject: [PATCH 1/2] Fix 2054 --- .../references/src/main.nr | 11 ++++++ .../src/ssa_refactor/ssa_gen/mod.rs | 35 ++++++++++++++++--- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/references/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/references/src/main.nr index d2c0b7f1244..d2af8d876e1 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/references/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/references/src/main.nr @@ -30,6 +30,17 @@ fn main(mut x: Field) { }; *c.bar.array = [3, 4]; assert(*c.bar.array == [3, 4]); + + regression_2054(); +} + +// 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); } fn add1(x: &mut Field) { diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs index 13e67f26cc5..c57f4e6c10a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -96,8 +96,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), @@ -111,6 +116,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) => { @@ -165,20 +175,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_lhs(&unary.rhs).map(|rhs| { match rhs { value::Value::Normal(value) => { let alloc = self.builder.insert_allocate(); @@ -191,7 +202,21 @@ impl<'a> FunctionContext<'a> { } }) } - noirc_frontend::UnaryOp::Dereference => self.dereference(&rhs, &unary.result_type), + noirc_frontend::UnaryOp::Dereference => { + let rhs = self.codegen_expression(&unary.rhs); + self.dereference(&rhs, &unary.result_type) + } + } + } + + fn codegen_reference_lhs(&mut self, expr: &Expression) -> Values { + match expr { + Expression::Ident(ident) => self.codegen_ident_reference(ident), + Expression::ExtractTupleField(tuple, index) => { + let tuple = self.codegen_reference_lhs(tuple); + Self::get_field(tuple, *index) + } + other => self.codegen_expression(other), } } From 2c010e0bea9a93a58327de916742d495fb1cf187 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 28 Jul 2023 08:33:02 -0500 Subject: [PATCH 2/2] Rename function --- crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs index c57f4e6c10a..6b21d7c4a41 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -189,7 +189,7 @@ impl<'a> FunctionContext<'a> { self.builder.insert_binary(zero, BinaryOp::Sub, rhs).into() } noirc_frontend::UnaryOp::MutableReference => { - self.codegen_reference_lhs(&unary.rhs).map(|rhs| { + self.codegen_reference(&unary.rhs).map(|rhs| { match rhs { value::Value::Normal(value) => { let alloc = self.builder.insert_allocate(); @@ -209,11 +209,11 @@ impl<'a> FunctionContext<'a> { } } - fn codegen_reference_lhs(&mut self, expr: &Expression) -> Values { + 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_lhs(tuple); + let tuple = self.codegen_reference(tuple); Self::get_field(tuple, *index) } other => self.codegen_expression(other),