Skip to content

Commit

Permalink
only use the slice capacity for merging, not the length field
Browse files Browse the repository at this point in the history
  • Loading branch information
vezenovm committed Sep 7, 2023
1 parent 487f1a0 commit 2728851
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 57 deletions.
123 changes: 75 additions & 48 deletions crates/nargo_cli/tests/execution_success/slices/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,51 +2,51 @@ use dep::std::slice;
use dep::std;

fn main(x : Field, y : pub Field) {
let mut slice = [0; 2];
assert(slice[0] == 0);
assert(slice[0] != 1);
slice[0] = x;
assert(slice[0] == x);

let slice_plus_10 = slice.push_back(y);
assert(slice_plus_10[2] == 10);
assert(slice_plus_10[2] != 8);
assert(slice_plus_10.len() == 3);

let mut new_slice = [];
for i in 0..5 {
new_slice = new_slice.push_back(i);
}
assert(new_slice.len() == 5);

new_slice = new_slice.push_front(20);
assert(new_slice[0] == 20);
assert(new_slice.len() == 6);

let (popped_slice, last_elem) = new_slice.pop_back();
assert(last_elem == 4);
assert(popped_slice.len() == 5);

let (first_elem, rest_of_slice) = popped_slice.pop_front();
assert(first_elem == 20);
assert(rest_of_slice.len() == 4);

new_slice = rest_of_slice.insert(2, 100);
assert(new_slice[2] == 100);
assert(new_slice[4] == 3);
assert(new_slice.len() == 5);

let (remove_slice, removed_elem) = new_slice.remove(3);
assert(removed_elem == 2);
assert(remove_slice[3] == 3);
assert(remove_slice.len() == 4);

let append = [1, 2].append([3, 4, 5]);
assert(append.len() == 5);
assert(append[0] == 1);
assert(append[4] == 5);

regression_2083();
// let mut slice = [0; 2];
// assert(slice[0] == 0);
// assert(slice[0] != 1);
// slice[0] = x;
// assert(slice[0] == x);

// let slice_plus_10 = slice.push_back(y);
// assert(slice_plus_10[2] == 10);
// assert(slice_plus_10[2] != 8);
// assert(slice_plus_10.len() == 3);

// let mut new_slice = [];
// for i in 0..5 {
// new_slice = new_slice.push_back(i);
// }
// assert(new_slice.len() == 5);

// new_slice = new_slice.push_front(20);
// assert(new_slice[0] == 20);
// assert(new_slice.len() == 6);

// let (popped_slice, last_elem) = new_slice.pop_back();
// assert(last_elem == 4);
// assert(popped_slice.len() == 5);

// let (first_elem, rest_of_slice) = popped_slice.pop_front();
// assert(first_elem == 20);
// assert(rest_of_slice.len() == 4);

// new_slice = rest_of_slice.insert(2, 100);
// assert(new_slice[2] == 100);
// assert(new_slice[4] == 3);
// assert(new_slice.len() == 5);

// let (remove_slice, removed_elem) = new_slice.remove(3);
// assert(removed_elem == 2);
// assert(remove_slice[3] == 3);
// assert(remove_slice.len() == 4);

// let append = [1, 2].append([3, 4, 5]);
// assert(append.len() == 5);
// assert(append[0] == 1);
// assert(append[4] == 5);

// regression_2083();
// The parameters to this function must come from witness values (inputs to main)
regression_merge_slices(x, y);
}
Expand Down Expand Up @@ -86,8 +86,14 @@ fn regression_2083() {

// The parameters to this function must come from witness values (inputs to main)
fn regression_merge_slices(x: Field, y: Field) {
merge_slices_if(x, y);
merge_slices_else(x);
// merge_slices_if(x, y);
// merge_slices_else(x);

let slice = merge_slices_mutate_two_ifs(x, y);
assert(slice.len() == 6);
assert(slice[3] == 5);
assert(slice[4] == 15);
assert(slice[5] == 30);
}

fn merge_slices_if(x: Field, y: Field) {
Expand Down Expand Up @@ -156,4 +162,25 @@ fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] {
slice = slice.push_back(x);
}
slice
}
}

fn merge_slices_mutate_two_ifs(x: Field, y: Field) -> [Field] {
let mut slice = [0; 2];
if x != y {
slice = slice.push_back(y);
slice = slice.push_back(x);
} else {
slice = slice.push_back(x);
}
if x == 20 {
slice = slice.push_back(20);
}
else {
slice = slice.push_back(15);
}
// Breaks if the push back happens without the else case
// slice = slice.push_back(15);
slice = slice.push_back(30);

slice
}
20 changes: 11 additions & 9 deletions crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,33 +489,35 @@ impl<'f> Context<'f> {
let value = &self.inserter.function.dfg[value_id];
match value {
Value::Array { array, .. } => array.len(),
Value::NumericConstant { constant, .. } => constant.to_u128() as usize,
Value::Instruction { instruction: instruction_id, .. } => {
let instruction = &self.inserter.function.dfg[*instruction_id];
match instruction {
Instruction::ArraySet { length, .. } => {
let length = length.expect("ICE: array set on a slice must have a length");
self.get_slice_length(length)
Instruction::ArraySet { array, .. } => {
self.get_slice_length(*array)
}
Instruction::Load { address } => {
let context_store = self
let context_store = if let Some(store) = self.store_values.get(address) {
store
} else {
self
.outer_block_stores
.get(address)
.expect("ICE: load in merger should have store from outer block");
.expect("ICE: load in merger should have store from outer block")
};

self.get_slice_length(context_store.new_value)
}
Instruction::Call { func, arguments } => {
let func = &self.inserter.function.dfg[*func];
let length = arguments[0];
let slice_contents = arguments[1];
match func {
Value::Intrinsic(intrinsic) => match intrinsic {
Intrinsic::SlicePushBack
| Intrinsic::SlicePushFront
| Intrinsic::SliceInsert => self.get_slice_length(length) + 1,
| Intrinsic::SliceInsert => self.get_slice_length(slice_contents) + 1,
Intrinsic::SlicePopBack
| Intrinsic::SlicePopFront
| Intrinsic::SliceRemove => self.get_slice_length(length) - 1,
| Intrinsic::SliceRemove => self.get_slice_length(slice_contents) - 1,
_ => {
unreachable!("ICE: Intrinsic not supported, got {intrinsic:?}")
}
Expand Down

0 comments on commit 2728851

Please sign in to comment.