Skip to content
22 changes: 8 additions & 14 deletions compiler/noirc_frontend/src/ownership/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,38 +134,32 @@ 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) {
// In `&{ a; b; ...; z }` we're only taking the reference of `z`.
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),
}
}

Expand Down Expand Up @@ -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);
}
}
Expand Down
47 changes: 44 additions & 3 deletions compiler/noirc_frontend/src/ownership/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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] {
Expand Down Expand Up @@ -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
}
");
}
6 changes: 6 additions & 0 deletions test_programs/execution_success/regression_9907/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "regression_9907"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return = [[0xcafebabe]]
19 changes: 19 additions & 0 deletions test_programs/execution_success/regression_9907/src/main.nr
Original file line number Diff line number Diff line change
@@ -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
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading