diff --git a/compiler/noirc_frontend/src/ownership/mod.rs b/compiler/noirc_frontend/src/ownership/mod.rs index e38ff712dbc..66cb688be31 100644 --- a/compiler/noirc_frontend/src/ownership/mod.rs +++ b/compiler/noirc_frontend/src/ownership/mod.rs @@ -134,9 +134,9 @@ impl Context { /// /// # Returns /// A boolean representing whether or not the expression was borrowed by reference (false) or moved (true). - fn handle_reference_expression(&mut self, expr: &mut Expression) -> bool { + fn handle_reference_expression(&mut self, expr: &mut Expression) { match expr { - Expression::Ident(_) => false, + Expression::Ident(_) => (), Expression::Block(exprs) => { let len_minus_one = exprs.len().saturating_sub(1); for expr in exprs.iter_mut().take(len_minus_one) { @@ -144,28 +144,22 @@ impl Context { self.handle_expression(expr); } if let Some(expr) = exprs.last_mut() { - self.handle_reference_expression(expr) - } else { - true + self.handle_reference_expression(expr); } } Expression::Unary(Unary { rhs, operator: UnaryOp::Dereference { .. }, .. }) => { - self.handle_reference_expression(rhs) + self.handle_reference_expression(rhs); } Expression::ExtractTupleField(tuple, _index) => self.handle_reference_expression(tuple), Expression::Index(index) => { - let is_moved = self.handle_reference_expression(&mut index.collection); + self.handle_reference_expression(&mut index.collection); self.handle_expression(&mut index.index); - is_moved } // If we have something like `f(arg)` then we want to treat those variables normally // rather than avoid cloning them. So we shouldn't recur in `handle_reference_expression`. - other => { - self.handle_expression(other); - true - } + other => self.handle_expression(other), } } @@ -286,11 +280,11 @@ impl Context { }; // Don't clone the collection, cloning only the resulting element is cheaper. - let is_moved = self.handle_reference_expression(&mut index.collection); + self.handle_reference_expression(&mut index.collection); self.handle_expression(&mut index.index); // If the index collection is being borrowed we need to clone the result. - if !is_moved && contains_array_or_str_type(&index.element_type) { + if contains_array_or_str_type(&index.element_type) { clone_expr(index_expr); } } diff --git a/compiler/noirc_frontend/src/ownership/tests.rs b/compiler/noirc_frontend/src/ownership/tests.rs index 0baef4e8523..615185feeab 100644 --- a/compiler/noirc_frontend/src/ownership/tests.rs +++ b/compiler/noirc_frontend/src/ownership/tests.rs @@ -146,7 +146,7 @@ fn borrows_on_nested_index() { } #[test] -fn moves_call_array_result() { +fn clone_call_array_result() { let src = " unconstrained fn main(i: u32) -> pub u32 { let _a = foo()[1][0][1]; @@ -162,8 +162,8 @@ fn moves_call_array_result() { // We expect no clones insta::assert_snapshot!(program, @r" unconstrained fn main$f0(i$l0: u32) -> pub u32 { - let _a$l1 = foo$f1()[1][0][1]; - let _s$l2 = foo$f1()[1][0][1]; + let _a$l1 = foo$f1()[1][0][1].clone(); + let _s$l2 = foo$f1()[1][0][1].clone(); i$l0 } unconstrained fn foo$f1() -> [[[[u128; 0]; 2]; 1]; 2] { @@ -295,3 +295,44 @@ fn clone_global_nested_array_used_as_call_arg() { } "); } + +// Regression for issue https://github.com/noir-lang/noir/issues/9907 +#[test] +fn regression_9907() { + let src = " + unconstrained fn main() -> pub [[Field; 1]; 1] { + foo([[0xcafebabe]]) + } + unconstrained fn foo(mut a: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + let mut b = bar(a)[0]; + + let mut x = 0; + while (x != 0) {} + + b[0] = 0xdeadbeef; + a + } + unconstrained fn bar(mut a: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + a + } + "; + + let program = get_monomorphized_no_emit_test(src).unwrap(); + // There are clones on both bar input and output + insta::assert_snapshot!(program, @r" + unconstrained fn main$f0() -> pub [[Field; 1]; 1] { + foo$f1([[3405691582]]) + } + unconstrained fn foo$f1(mut a$l0: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + let mut b$l1 = bar$f2(a$l0.clone())[0].clone(); + let mut x$l2 = 0; + while (x$l2 != 0) { + }; + b$l1[0] = 3735928559; + a$l0 + } + unconstrained fn bar$f2(mut a$l3: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + a$l3 + } + "); +} diff --git a/test_programs/execution_success/regression_9907/Nargo.toml b/test_programs/execution_success/regression_9907/Nargo.toml new file mode 100644 index 00000000000..aac375eb212 --- /dev/null +++ b/test_programs/execution_success/regression_9907/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_9907" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_9907/Prover.toml b/test_programs/execution_success/regression_9907/Prover.toml new file mode 100644 index 00000000000..7c804afe041 --- /dev/null +++ b/test_programs/execution_success/regression_9907/Prover.toml @@ -0,0 +1 @@ +return = [[0xcafebabe]] \ No newline at end of file diff --git a/test_programs/execution_success/regression_9907/src/main.nr b/test_programs/execution_success/regression_9907/src/main.nr new file mode 100644 index 00000000000..aae1ee4b4df --- /dev/null +++ b/test_programs/execution_success/regression_9907/src/main.nr @@ -0,0 +1,19 @@ +fn main() -> pub [[Field; 1]; 1] { + let a = unsafe { foo([[0xcafebabe]]) }; + assert(a[0][0] == 0xcafebabe); + a +} + +unconstrained fn foo(mut a: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + let mut b = bar(a)[0]; + + let mut x = 0; + while (x != 0) {} + + b[0] = 0xdeadbeef; + a +} + +fn bar(mut a: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + a +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9907/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9907/execute__tests__expanded.snap new file mode 100644 index 00000000000..e9cbac5f806 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9907/execute__tests__expanded.snap @@ -0,0 +1,22 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main() -> pub [[Field; 1]; 1] { + // Safety: comment added by `nargo expand` + let a: [[Field; 1]; 1] = unsafe { foo([[3405691582_Field]]) }; + assert(a[0_u32][0_u32] == 3405691582_Field); + a +} + +unconstrained fn foo(mut a: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + let mut b: [Field; 1] = bar(a)[0_u32]; + let mut x: Field = 0_Field; + while x != 0_Field {} + b[0_u32] = 3735928559_Field; + a +} + +fn bar(mut a: [[Field; 1]; 1]) -> [[Field; 1]; 1] { + a +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9907/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9907/execute__tests__stdout.snap new file mode 100644 index 00000000000..6c22ff185ba --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9907/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- +[regression_9907] Circuit output: [[0xcafebabe]]