diff --git a/compiler/noirc_frontend/src/ownership/last_uses.rs b/compiler/noirc_frontend/src/ownership/last_uses.rs index 045d79d7a2b..93b1458c45f 100644 --- a/compiler/noirc_frontend/src/ownership/last_uses.rs +++ b/compiler/noirc_frontend/src/ownership/last_uses.rs @@ -428,8 +428,8 @@ impl LastUseContext { } fn track_variables_in_assign(&mut self, assign: &ast::Assign) { - self.track_variables_in_lvalue(&assign.lvalue); self.track_variables_in_expression(&assign.expression); + self.track_variables_in_lvalue(&assign.lvalue, false /* nested */); } /// A variable in an lvalue position is never moved (otherwise you wouldn't @@ -441,19 +441,27 @@ impl LastUseContext { /// if the last use of one was just before it is assigned, it can actually be /// moved before it is assigned. This should be fine because we move out of the /// binding, and the binding isn't used until it is set to a new value. - fn track_variables_in_lvalue(&mut self, lvalue: &ast::LValue) { + /// + /// The `nested` parameter indicates whether this l-value is nested inside another l-value. + /// For top-level identifiers there's nothing to track, but for an identifier happening + /// as part of an index (`ident[index] = ...`) we do want to consider `ident` as moved. + fn track_variables_in_lvalue(&mut self, lvalue: &ast::LValue, nested: bool) { match lvalue { // All identifiers in lvalues are implicitly `&mut ident` and thus aren't moved - ast::LValue::Ident(_) => (), + ast::LValue::Ident(ident) => { + if nested { + self.track_variables_in_ident(ident); + } + } ast::LValue::Index { array, index, element_type: _, location: _ } => { self.track_variables_in_expression(index); - self.track_variables_in_lvalue(array); + self.track_variables_in_lvalue(array, true); } ast::LValue::MemberAccess { object, field_index: _ } => { - self.track_variables_in_lvalue(object); + self.track_variables_in_lvalue(object, true); } ast::LValue::Dereference { reference, element_type: _ } => { - self.track_variables_in_lvalue(reference); + self.track_variables_in_lvalue(reference, true); } ast::LValue::Clone(_) => todo!(), } diff --git a/compiler/noirc_frontend/src/ownership/mod.rs b/compiler/noirc_frontend/src/ownership/mod.rs index ea26274be59..e38ff712dbc 100644 --- a/compiler/noirc_frontend/src/ownership/mod.rs +++ b/compiler/noirc_frontend/src/ownership/mod.rs @@ -372,8 +372,8 @@ impl Context { } fn handle_assign(&mut self, assign: &mut crate::monomorphization::ast::Assign) { - self.handle_lvalue(&mut assign.lvalue); self.handle_expression(&mut assign.expression); + self.handle_lvalue(&mut assign.lvalue); } fn handle_lvalue(&mut self, lvalue: &mut LValue) { diff --git a/compiler/noirc_frontend/src/ownership/tests.rs b/compiler/noirc_frontend/src/ownership/tests.rs index 842628ab807..8b69975c4c0 100644 --- a/compiler/noirc_frontend/src/ownership/tests.rs +++ b/compiler/noirc_frontend/src/ownership/tests.rs @@ -171,3 +171,55 @@ fn moves_call_array_result() { } "); } + +#[test] +fn considers_lvalue_index_identifier_in_last_use() { + let src = " + unconstrained fn main() { + let mut b = [true]; + let mut c = [false]; + c = b; + b[0] = !c[0]; + assert_eq(c[0], true); + } + "; + + let program = get_monomorphized_no_emit_test(src).unwrap(); + insta::assert_snapshot!(program, @r" + unconstrained fn main$f0() -> () { + let mut b$l0 = [true]; + let mut c$l1 = [false]; + c$l1 = b$l0.clone(); + b$l0[0] = (!c$l1[0]); + assert((c$l1[0] == true)); + } + "); +} + +#[test] +fn analyzes_expression_before_lvalue_in_assignment() { + let src = " + unconstrained fn main() { + let mut b = [true]; + let mut c = [false]; + b[0] = { + c = b; + !c[0] + }; + assert_eq(c[0], true); + } + "; + + let program = get_monomorphized_no_emit_test(src).unwrap(); + insta::assert_snapshot!(program, @r" + unconstrained fn main$f0() -> () { + let mut b$l0 = [true]; + let mut c$l1 = [false]; + b$l0[0] = { + c$l1 = b$l0.clone(); + (!c$l1[0]) + }; + assert((c$l1[0] == true)); + } + "); +} diff --git a/test_programs/execution_success/regression_9725_1/Nargo.toml b/test_programs/execution_success/regression_9725_1/Nargo.toml new file mode 100644 index 00000000000..d59f1c35cac --- /dev/null +++ b/test_programs/execution_success/regression_9725_1/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_9725_1" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_9725_1/src/main.nr b/test_programs/execution_success/regression_9725_1/src/main.nr new file mode 100644 index 00000000000..a5022f7f62a --- /dev/null +++ b/test_programs/execution_success/regression_9725_1/src/main.nr @@ -0,0 +1,7 @@ +unconstrained fn main() { + let mut b = [true]; + let mut c = [false]; + c = b; + b[0] = !c[0]; + assert_eq(c[0], true); +} diff --git a/test_programs/execution_success/regression_9725_2/Nargo.toml b/test_programs/execution_success/regression_9725_2/Nargo.toml new file mode 100644 index 00000000000..3018ebadd56 --- /dev/null +++ b/test_programs/execution_success/regression_9725_2/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_9725_2" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_9725_2/src/main.nr b/test_programs/execution_success/regression_9725_2/src/main.nr new file mode 100644 index 00000000000..00431d6386f --- /dev/null +++ b/test_programs/execution_success/regression_9725_2/src/main.nr @@ -0,0 +1,15 @@ +fn main() { + // Safety: + let result = unsafe { func_1([true], [false]) }; + assert_eq(result[0], true); +} +unconstrained fn func_1(mut b: [bool; 1], mut c: [bool; 1]) -> [bool; 1] { + b[0] = { + c = if (c[0]) { + b[0] = true; + b + } else { b }; + !c[0] + }; + c +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_1/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_1/execute__tests__expanded.snap new file mode 100644 index 00000000000..48bd474f2e8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_1/execute__tests__expanded.snap @@ -0,0 +1,11 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +unconstrained fn main() { + let mut b: [bool; 1] = [true]; + let mut c: [bool; 1] = [false]; + c = b; + b[0_u32] = !c[0_u32]; + assert(c[0_u32] == true); +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_1/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_1/execute__tests__stdout.snap new file mode 100644 index 00000000000..e86e3de90e1 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_1/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- + diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_2/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_2/execute__tests__expanded.snap new file mode 100644 index 00000000000..da250cc3e72 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_2/execute__tests__expanded.snap @@ -0,0 +1,20 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main() { + // Safety: comment added by `nargo expand` + let result: [bool; 1] = unsafe { func_1([true], [false]) }; + assert(result[0_u32] == true); +} + +unconstrained fn func_1(mut b: [bool; 1], mut c: [bool; 1]) -> [bool; 1] { + b[0_u32] = { + c = if c[0_u32] { + b[0_u32] = true; + b + } else { b }; + !c[0_u32] + }; + c +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_2/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_2/execute__tests__stdout.snap new file mode 100644 index 00000000000..e86e3de90e1 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9725_2/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- +