From 8b23b349ae15afa48f6cbe8962586bbe79e79890 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 30 Nov 2023 19:52:14 +0000 Subject: [PATCH] feat: Complex slice inputs for dynamic slice builtins (#3617) # Description ## Problem\* Resolves #3364 ## Summary\* This PR heavily changes the way slice intrinsics are implemented in ACIR gen. Following the `fill_internal_slices` pass nested slices now have dummy data automatically attached to them to enable known sizes to be used when fetching from an array with nested slices. Most of the slice intrinsics were written for non-complex inputs and also did not account for the addition of already included dummy data. With dummy data already included we cannot simply push back or push front elements for example, we need to make sure we do not alter the size of the internal slice we are using. The slice intrinsic implementations essentially treat the input already as a dynamic array and perform the appropriate memory operations upon them. I considered alternatives and went with what I think is the most efficient way to implement these intrinsics, however, there is definitely room for cleanup and in the name of getting a first iteration working, we can consider how to optimize these intrinsics in another PR. I have added comments in the ACIR gen itself to help with some of the logic for the implementation, but do let me know if something is not clear and I can document it further. The `fill_internal_slices` pass now also accounts for slice constants passed as inputs to slice intrinsics as these must also be filled with dummy data when they are being attached to a nested slice. ## Additional Context A bug was found in the `fill_internal_slices` pass that was necessary to resolve this issue. Essentially the children fetched from a slice were not being accurately tracked when an `array_get` occurs. This fix is included in this PR as it did not come up until more complex operations with nested slices were attempted. SliceInsert and SliceRemove now use a dynamic index. I only tested using a constant index in this PR and will have a followup PR that shows this tested out as this PR is already large enough. ## Documentation\* Check one: - [ ] No documentation needed. - [X] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Tom French Co-authored-by: jfecher --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 2 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 629 ++++++++++++++---- compiler/noirc_evaluator/src/ssa/ir/types.rs | 8 + .../src/ssa/opt/fill_internal_slices.rs | 94 ++- noir_stdlib/src/slice.nr | 16 +- .../slice_dynamic_index/src/main.nr | 8 +- .../slice_struct_field/src/main.nr | 319 ++++++++- 7 files changed, 911 insertions(+), 165 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 999ae88da99..723bc1b65a0 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -937,7 +937,7 @@ impl AcirContext { /// Returns an `AcirVar` which will be `1` if lhs >= rhs /// and `0` otherwise. - fn more_than_eq_var( + pub(crate) fn more_than_eq_var( &mut self, lhs: AcirVar, rhs: AcirVar, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 32d2e709ddf..eea2ce4de17 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -133,6 +133,16 @@ impl AcirValue { } } + fn borrow_var(&self) -> Result { + match self { + AcirValue::Var(var, _) => Ok(*var), + AcirValue::DynamicArray(_) | AcirValue::Array(_) => Err(InternalError::General { + message: "Called AcirValue::borrow_var on an array".to_string(), + call_stack: CallStack::new(), + }), + } + } + fn flatten(self) -> Vec<(AcirVar, AcirType)> { match self { AcirValue::Var(var, typ) => vec![(var, typ)], @@ -896,7 +906,7 @@ impl Context { // The first max size is going to be the length of the parent slice // As we are fetching from the parent slice we just want its internal - // slize sizes. + // slice sizes. let slice_sizes = slice_sizes[1..].to_vec(); let value = self.array_get_value(&res_typ, block_id, &mut var_index, &slice_sizes)?; @@ -1021,7 +1031,7 @@ impl Context { self.copy_dynamic_array(block_id, result_block_id, array_len)?; } - self.array_set_value(store_value, result_block_id, &mut var_index)?; + self.array_set_value(&store_value, result_block_id, &mut var_index)?; // Set new resulting array to have the same slice sizes as the instruction input if let Type::Slice(element_types) = &array_typ { @@ -1056,7 +1066,7 @@ impl Context { fn array_set_value( &mut self, - value: AcirValue, + value: &AcirValue, block_id: BlockId, var_index: &mut AcirVar, ) -> Result<(), RuntimeError> { @@ -1064,8 +1074,8 @@ impl Context { match value { AcirValue::Var(store_var, _) => { // Write the new value into the new array at the specified index - self.acir_context.write_to_memory(block_id, var_index, &store_var)?; - // Incremement the var_index in case of a nested array + self.acir_context.write_to_memory(block_id, var_index, store_var)?; + // Increment the var_index in case of a nested array *var_index = self.acir_context.add_var(*var_index, one)?; } AcirValue::Array(values) => { @@ -1074,13 +1084,13 @@ impl Context { } } AcirValue::DynamicArray(AcirDynamicArray { block_id: inner_block_id, len, .. }) => { - let values = try_vecmap(0..len, |i| { + let values = try_vecmap(0..*len, |i| { let index_var = self.acir_context.add_constant(i); - let read = self.acir_context.read_from_memory(inner_block_id, &index_var)?; + let read = self.acir_context.read_from_memory(*inner_block_id, &index_var)?; Ok::(AcirValue::Var(read, AcirType::field())) })?; - self.array_set_value(AcirValue::Array(values.into()), block_id, var_index)?; + self.array_set_value(&AcirValue::Array(values.into()), block_id, var_index)?; } } Ok(()) @@ -1765,39 +1775,67 @@ impl Context { Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())]) } Intrinsic::SlicePushBack => { + // arguments = [slice_length, slice_contents, ...elements_to_push] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - let (array_id, array_typ, _) = + let (slice_contents, slice_typ, _) = self.check_array_is_initialized(arguments[1], dfg)?; - let slice = self.convert_value(arguments[1], dfg); + let slice = self.convert_value(slice_contents, dfg); - // TODO(#3364): make sure that we have handled nested struct inputs - let element = self.convert_value(arguments[2], dfg); - let one = self.acir_context.add_constant(FieldElement::one()); - let new_slice_length = self.acir_context.add_var(slice_length, one)?; + let mut new_elem_size = Self::flattened_value_size(&slice); - // We attach the element no matter what in case len == capacity, as otherwise we - // may get an out of bounds error. let mut new_slice = Vector::new(); - self.slice_intrinsic_input(&mut new_slice, slice.clone())?; - new_slice.push_back(element.clone()); + self.slice_intrinsic_input(&mut new_slice, slice)?; + + let elements_to_push = &arguments[2..]; + // We only fill internal slices for nested slices (a slice inside of a slice). + // So we must directly push back elements for slices which are not a nested slice. + if !slice_typ.is_nested_slice() { + for elem in elements_to_push { + let element = self.convert_value(*elem, dfg); + + new_elem_size += Self::flattened_value_size(&element); + new_slice.push_back(element); + } + } + + // Increase the slice length by one to enable accessing more elements in the slice. + let one = self.acir_context.add_constant(FieldElement::one()); + let new_slice_length = self.acir_context.add_var(slice_length, one)?; - // TODO(#3364): This works for non-nested outputs - let len = Self::flattened_value_size(&slice); - let new_elem_size = Self::flattened_value_size(&element); let new_slice_val = AcirValue::Array(new_slice); let result_block_id = self.block_id(&result_ids[1]); - self.initialize_array( - result_block_id, - len + new_elem_size, - Some(new_slice_val.clone()), - )?; + self.initialize_array(result_block_id, new_elem_size, Some(new_slice_val.clone()))?; + // The previous slice length represents the index we want to write into. let mut var_index = slice_length; - self.array_set_value(element, result_block_id, &mut var_index)?; + // Dynamic arrays are represented as flat memory. We must flatten the user facing index + // to a flattened index that matches the complex slice structure. + if slice_typ.is_nested_slice() { + let element_size = slice_typ.element_size(); + + // Multiply the element size against the var index before fetching the flattened index + // This operation makes sure our user-facing slice index matches the strategy for indexing in SSA, + // which is how `get_flattened_index` expects its index input. + let element_size_var = + self.acir_context.add_constant(FieldElement::from(element_size as u128)); + var_index = self.acir_context.mul_var(slice_length, element_size_var)?; + var_index = + self.get_flattened_index(&slice_typ, slice_contents, var_index, dfg)?; + } - let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) { + // Write the elements we wish to push back directly. + // The slice's underlying array value should already be filled with dummy data + // to enable this write to be within bounds. + // The dummy data is either attached during SSA gen or in this match case for non-nested slices. + // These values can then be accessed due to the increased dynamic slice length. + for elem in elements_to_push { + let element = self.convert_value(*elem, dfg); + self.array_set_value(&element, result_block_id, &mut var_index)?; + } + + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { Some(self.init_element_type_sizes_array( - &array_typ, - array_id, + &slice_typ, + slice_contents, Some(new_slice_val), dfg, )?) @@ -1806,155 +1844,510 @@ impl Context { }; let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, - len: len + new_elem_size, + len: new_elem_size, element_type_sizes, }); Ok(vec![AcirValue::Var(new_slice_length, AcirType::field()), result]) } Intrinsic::SlicePushFront => { + // arguments = [slice_length, slice_contents, ...elements_to_push] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - let slice: AcirValue = self.convert_value(arguments[1], dfg); - // TODO(#3364): make sure that we have handled nested struct inputs - let element = self.convert_value(arguments[2], dfg); + let (slice_contents, slice_typ, _) = + self.check_array_is_initialized(arguments[1], dfg)?; + let slice: AcirValue = self.convert_value(slice_contents, dfg); + + let mut new_slice_size = Self::flattened_value_size(&slice); + + // Increase the slice length by one to enable accessing more elements in the slice. let one = self.acir_context.add_constant(FieldElement::one()); let new_slice_length = self.acir_context.add_var(slice_length, one)?; let mut new_slice = Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; - new_slice.push_front(element); - Ok(vec![ - AcirValue::Var(new_slice_length, AcirType::field()), - AcirValue::Array(new_slice), - ]) + let elements_to_push = &arguments[2..]; + let mut elem_size = 0; + // We only fill internal slices for nested slices (a slice inside of a slice). + // So we must directly push front elements for slices which are not a nested slice. + if !slice_typ.is_nested_slice() { + for elem in elements_to_push.iter().rev() { + let element = self.convert_value(*elem, dfg); + + elem_size += Self::flattened_value_size(&element); + new_slice.push_front(element); + } + new_slice_size += elem_size; + } else { + // We have already filled the appropriate dummy values for nested slice during SSA gen. + // We need to account for that we do not go out of bounds by removing dummy data as we + // push elements to the front of our slice. + // Using this strategy we are able to avoid dynamic writes like we do for a SlicePushBack. + for elem in elements_to_push.iter().rev() { + let element = self.convert_value(*elem, dfg); + + let elem_size = Self::flattened_value_size(&element); + // Have to pop based off of the flattened value size as we read the + // slice intrinsic as a flat list of AcirValue::Var + for _ in 0..elem_size { + new_slice.pop_back(); + } + new_slice.push_front(element); + } + } + + let new_slice_val = AcirValue::Array(new_slice.clone()); + + let result_block_id = self.block_id(&result_ids[1]); + self.initialize_array( + result_block_id, + new_slice_size, + Some(new_slice_val.clone()), + )?; + + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { + Some(self.init_element_type_sizes_array( + &slice_typ, + slice_contents, + Some(new_slice_val), + dfg, + )?) + } else { + None + }; + let result = AcirValue::DynamicArray(AcirDynamicArray { + block_id: result_block_id, + len: new_slice_size, + element_type_sizes, + }); + + Ok(vec![AcirValue::Var(new_slice_length, AcirType::field()), result]) } Intrinsic::SlicePopBack => { + // arguments = [slice_length, slice_contents] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - let slice = self.convert_value(arguments[1], dfg); let one = self.acir_context.add_constant(FieldElement::one()); let new_slice_length = self.acir_context.sub_var(slice_length, one)?; - - let (_, _, block_id) = self.check_array_is_initialized(arguments[1], dfg)?; + // For a pop back operation we want to fetch from the `length - 1` as this is the + // last valid index that can be accessed in a slice. After the pop back operation + // the elements stored at that index will no longer be able to be accessed. let mut var_index = new_slice_length; - let elem = self.array_get_value( - &dfg.type_of_value(result_ids[2]), - block_id, - &mut var_index, - &[], - )?; - // TODO(#3364): make sure that we have handled nested struct inputs + let (slice_contents, slice_typ, block_id) = + self.check_array_is_initialized(arguments[1], dfg)?; + let slice = self.convert_value(slice_contents, dfg); + + let element_size = slice_typ.element_size(); + + let mut popped_elements = Vec::new(); + // Fetch the values we are popping off of the slice. + // In the case of non-nested slice the logic is simple as we do not + // need to account for the internal slice sizes or flattening the index. + // + // The pop back operation results are of the format [slice length, slice contents, popped elements]. + // Thus, we look at the result ids at index 2 and onwards to determine the type of each popped element. + if !slice_typ.is_nested_slice() { + for res in &result_ids[2..] { + let elem = self.array_get_value( + &dfg.type_of_value(*res), + block_id, + &mut var_index, + &[], + )?; + popped_elements.push(elem); + } + } else { + // Fetch the slice sizes of the nested slice. + let slice_sizes = self.slice_sizes.get(&slice_contents); + let mut slice_sizes = + slice_sizes.expect("ICE: should have slice sizes").clone(); + // We want to remove the parent size as we are fetching the child + slice_sizes.remove(0); + + // Multiply the element size against the var index before fetching the flattened index + // This operation makes sure our user-facing slice index matches the strategy for indexing in SSA, + // which is how `get_flattened_index` expects its index input. + let element_size_var = + self.acir_context.add_constant(FieldElement::from(element_size as u128)); + // We want to use an index one less than the slice length + var_index = self.acir_context.mul_var(var_index, element_size_var)?; + var_index = + self.get_flattened_index(&slice_typ, slice_contents, var_index, dfg)?; + + for res in &result_ids[2..] { + let elem = self.array_get_value( + &dfg.type_of_value(*res), + block_id, + &mut var_index, + &slice_sizes, + )?; + popped_elements.push(elem); + } + } + let mut new_slice = Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; - Ok(vec![ + let mut results = vec![ AcirValue::Var(new_slice_length, AcirType::field()), AcirValue::Array(new_slice), - elem, - ]) + ]; + results.append(&mut popped_elements); + + Ok(results) } Intrinsic::SlicePopFront => { + // arguments = [slice_length, slice_contents] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - let slice = self.convert_value(arguments[1], dfg); + + let (slice_contents, slice_typ, block_id) = + self.check_array_is_initialized(arguments[1], dfg)?; + let slice = self.convert_value(slice_contents, dfg); let one = self.acir_context.add_constant(FieldElement::one()); let new_slice_length = self.acir_context.sub_var(slice_length, one)?; let mut new_slice = Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; - // TODO(#3364): make sure that we have handled nested struct inputs - let elem = new_slice - .pop_front() - .expect("There are no elements in this slice to be removed"); - Ok(vec![ - elem, - AcirValue::Var(new_slice_length, AcirType::field()), - AcirValue::Array(new_slice), - ]) + let element_size = slice_typ.element_size(); + + let mut popped_elements: Vec = Vec::new(); + let mut popped_elements_size = 0; + let mut var_index = self.acir_context.add_constant(FieldElement::zero()); + // Fetch the values we are popping off of the slice. + // In the case of non-nested slice the logic is simple as we do not + // need to account for the internal slice sizes or flattening the index. + // + // The pop front operation results are of the format [popped elements, slice length, slice contents]. + // Thus, we look at the result ids up to the element size to determine the type of each popped element. + if !slice_typ.is_nested_slice() { + for res in &result_ids[..element_size] { + let element = self.array_get_value( + &dfg.type_of_value(*res), + block_id, + &mut var_index, + &[], + )?; + let elem_size = Self::flattened_value_size(&element); + popped_elements_size += elem_size; + popped_elements.push(element); + } + } else { + let slice_sizes = self.slice_sizes.get(&slice_contents); + let mut slice_sizes = + slice_sizes.expect("ICE: should have slice sizes").clone(); + // We want to remove the parent size as we are fetching the child + slice_sizes.remove(0); + + for res in &result_ids[..element_size] { + let element = self.array_get_value( + &dfg.type_of_value(*res), + block_id, + &mut var_index, + &slice_sizes, + )?; + let elem_size = Self::flattened_value_size(&element); + popped_elements_size += elem_size; + popped_elements.push(element); + } + } + // It is expected that the `popped_elements_size` is the flattened size of the elements, + // as the input slice should be a dynamic array which is represented by flat memory. + new_slice = new_slice.slice(popped_elements_size..); + + popped_elements.push(AcirValue::Var(new_slice_length, AcirType::field())); + popped_elements.push(AcirValue::Array(new_slice)); + + Ok(popped_elements) } Intrinsic::SliceInsert => { - // Slice insert with a constant index + // arguments = [slice_length, slice_contents, insert_index, ...elements_to_insert] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - let slice = self.convert_value(arguments[1], dfg); - let index = self.convert_value(arguments[2], dfg).into_var()?; - let element = self.convert_value(arguments[3], dfg); + + let (slice_contents, slice_typ, block_id) = + self.check_array_is_initialized(arguments[1], dfg)?; + + let slice = self.convert_value(slice_contents, dfg); + let insert_index = self.convert_value(arguments[2], dfg).into_var()?; let one = self.acir_context.add_constant(FieldElement::one()); let new_slice_length = self.acir_context.add_var(slice_length, one)?; - // TODO(#2462): Slice insert is a little less obvious on how to implement due to the case - // of having a dynamic index - // The slice insert logic will need a more involved codegen - let index = self.acir_context.var_to_expression(index)?.to_const(); - let index = index - .expect("ICE: slice length should be fully tracked and constant by ACIR gen"); - let index = index.to_u128() as usize; + let slice_size = Self::flattened_value_size(&slice); + + // Fetch the flattened index from the user provided index argument. + let element_size = slice_typ.element_size(); + let element_size_var = + self.acir_context.add_constant(FieldElement::from(element_size as u128)); + let flat_insert_index = + self.acir_context.mul_var(insert_index, element_size_var)?; + let flat_user_index = + self.get_flattened_index(&slice_typ, slice_contents, flat_insert_index, dfg)?; + + let elements_to_insert = &arguments[3..]; + // Determine the elements we need to write into our resulting dynamic array. + // We need to a fully flat list of AcirVar's as a dynamic array is represented with flat memory. + let mut inner_elem_size_usize = 0; + let mut flattened_elements = Vec::new(); + for elem in elements_to_insert { + let element = self.convert_value(*elem, dfg); + let elem_size = Self::flattened_value_size(&element); + inner_elem_size_usize += elem_size; + let mut flat_elem = element.flatten().into_iter().map(|(var, _)| var).collect(); + flattened_elements.append(&mut flat_elem); + } + let inner_elem_size = self + .acir_context + .add_constant(FieldElement::from(inner_elem_size_usize as u128)); + // Set the maximum flattened index at which a new element should be inserted. + let max_flat_user_index = + self.acir_context.add_var(flat_user_index, inner_elem_size)?; + + // Go through the entire slice argument and determine what value should be written to the new slice. + // 1. If we are below the starting insertion index we should insert the value that was already + // in the original slice. + // 2. If we are above the starting insertion index but below the max insertion index we should insert + // the flattened element arguments. + // 3. If we are above the max insertion index we should insert the previous value from the original slice, + // as during an insertion we want to shift all elements after the insertion up an index. + let result_block_id = self.block_id(&result_ids[1]); + self.initialize_array(result_block_id, slice_size, None)?; + let mut current_insert_index = 0; + for i in 0..slice_size { + let current_index = + self.acir_context.add_constant(FieldElement::from(i as u128)); + + // Check that we are above the lower bound of the insertion index + let greater_eq_than_idx = self.acir_context.more_than_eq_var( + current_index, + flat_user_index, + 64, + self.current_side_effects_enabled_var, + )?; + // Check that we are below the upper bound of the insertion index + let less_than_idx = self.acir_context.less_than_var( + current_index, + max_flat_user_index, + 64, + self.current_side_effects_enabled_var, + )?; + + // Read from the original slice the value we want to insert into our new slice. + // We need to make sure of two things: + // 1. That we read the previous element for when we have an index greater than insertion index. + // 2. That the index we are reading from is within the array bounds + let shifted_index = if i < inner_elem_size_usize { + current_index + } else { + let index_minus_elem_size = self + .acir_context + .add_constant(FieldElement::from((i - inner_elem_size_usize) as u128)); - let mut new_slice = Vector::new(); - self.slice_intrinsic_input(&mut new_slice, slice)?; + let use_shifted_index_pred = self + .acir_context + .mul_var(index_minus_elem_size, greater_eq_than_idx)?; - // We do not return an index out of bounds error directly here - // as the length of the slice is dynamic, and length of `new_slice` - // represents the capacity of the slice, not the actual length. - // - // Constraints should be generated during SSA gen to tell the user - // they are attempting to insert at too large of an index. - // This check prevents a panic inside of the im::Vector insert method. - if index <= new_slice.len() { - // TODO(#3364): make sure that we have handled nested struct inputs - new_slice.insert(index, element); + let not_pred = self.acir_context.sub_var(one, greater_eq_than_idx)?; + let use_current_index_pred = + self.acir_context.mul_var(not_pred, current_index)?; + + self.acir_context.add_var(use_shifted_index_pred, use_current_index_pred)? + }; + + let value_shifted_index = + self.acir_context.read_from_memory(block_id, &shifted_index)?; + + // Final predicate to determine whether we are within the insertion bounds + let should_insert_value_pred = + self.acir_context.mul_var(greater_eq_than_idx, less_than_idx)?; + let insert_value_pred = self.acir_context.mul_var( + flattened_elements[current_insert_index], + should_insert_value_pred, + )?; + + let not_pred = self.acir_context.sub_var(one, should_insert_value_pred)?; + let shifted_value_pred = + self.acir_context.mul_var(not_pred, value_shifted_index)?; + + let new_value = + self.acir_context.add_var(insert_value_pred, shifted_value_pred)?; + + self.acir_context.write_to_memory( + result_block_id, + ¤t_index, + &new_value, + )?; + + current_insert_index += 1; + if inner_elem_size_usize == current_insert_index { + current_insert_index = 0; + } } - Ok(vec![ - AcirValue::Var(new_slice_length, AcirType::field()), - AcirValue::Array(new_slice), - ]) + // let new_slice_val = AcirValue::Array(new_slice); + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { + Some(self.init_element_type_sizes_array( + &slice_typ, + slice_contents, + Some(slice), + dfg, + )?) + } else { + None + }; + let result = AcirValue::DynamicArray(AcirDynamicArray { + block_id: result_block_id, + len: slice_size, + element_type_sizes, + }); + + Ok(vec![AcirValue::Var(new_slice_length, AcirType::field()), result]) } Intrinsic::SliceRemove => { - // Slice insert with a constant index + // arguments = [slice_length, slice_contents, remove_index] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - let slice = self.convert_value(arguments[1], dfg); - let index = self.convert_value(arguments[2], dfg).into_var()?; + + let (slice_contents, slice_typ, block_id) = + self.check_array_is_initialized(arguments[1], dfg)?; + + let slice = self.convert_value(slice_contents, dfg); + let remove_index = self.convert_value(arguments[2], dfg).into_var()?; let one = self.acir_context.add_constant(FieldElement::one()); let new_slice_length = self.acir_context.sub_var(slice_length, one)?; - // TODO(#2462): allow slice remove with a constant index - // Slice remove is a little less obvious on how to implement due to the case - // of having a dynamic index - // The slice remove logic will need a more involved codegen - let index = self.acir_context.var_to_expression(index)?.to_const(); - let index = index - .expect("ICE: slice length should be fully tracked and constant by ACIR gen"); - let index = index.to_u128() as usize; + let slice_size = Self::flattened_value_size(&slice); let mut new_slice = Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; - // We do not return an index out of bounds error directly here - // as the length of the slice is dynamic, and length of `new_slice` - // represents the capacity of the slice, not the actual length. - // - // Constraints should be generated during SSA gen to tell the user - // they are attempting to remove at too large of an index. - // This check prevents a panic inside of the im::Vector remove method. - let removed_elem = if index < new_slice.len() { - // TODO(#3364): make sure that we have handled nested struct inputs - new_slice.remove(index) + // Compiler sanity check + assert_eq!( + new_slice.len(), + slice_size, + "ICE: The read flattened slice should match the computed size" + ); + + // Fetch the flattened index from the user provided index argument. + let element_size = slice_typ.element_size(); + let element_size_var = + self.acir_context.add_constant(FieldElement::from(element_size as u128)); + let flat_remove_index = + self.acir_context.mul_var(remove_index, element_size_var)?; + let flat_user_index = + self.get_flattened_index(&slice_typ, slice_contents, flat_remove_index, dfg)?; + + // Fetch the values we are remove from the slice. + // In the case of non-nested slice the logic is simple as we do not + // need to account for the internal slice sizes or flattening the index. + // As we fetch the values we can determine the size of the removed values + // which we will later use for writing the correct resulting slice. + let mut popped_elements = Vec::new(); + let mut popped_elements_size = 0; + // Set a temp index just for fetching from the original slice as `array_get_value` mutates + // the index internally. + let mut temp_index = flat_user_index; + if !slice_typ.is_nested_slice() { + for res in &result_ids[2..(2 + element_size)] { + let element = self.array_get_value( + &dfg.type_of_value(*res), + block_id, + &mut temp_index, + &[], + )?; + let elem_size = Self::flattened_value_size(&element); + popped_elements_size += elem_size; + popped_elements.push(element); + } } else { - // This is a dummy value which should never be used if the appropriate - // slice access checks are generated before this slice remove call. - AcirValue::Var(slice_length, AcirType::field()) + let slice_sizes = self.slice_sizes.get(&slice_contents); + let mut slice_sizes = + slice_sizes.expect("ICE: should have slice sizes").clone(); + // We want to remove the parent size as we are fetching the child + slice_sizes.remove(0); + + for res in &result_ids[2..(2 + element_size)] { + let element = self.array_get_value( + &dfg.type_of_value(*res), + block_id, + &mut temp_index, + &slice_sizes, + )?; + let elem_size = Self::flattened_value_size(&element); + popped_elements_size += elem_size; + popped_elements.push(element); + } + } + + // Go through the entire slice argument and determine what value should be written to the new slice. + // 1. If the current index is greater than the removal index we must write the next value + // from the original slice to the current index + // 2. At the end of the slice reading from the next value of the original slice + // can lead to a potential out of bounds error. In this case we just fetch from the original slice + // at the current index. As we are decreasing the slice in length, this is a safe operation. + let result_block_id = self.block_id(&result_ids[1]); + self.initialize_array(result_block_id, slice_size, None)?; + for i in 0..slice_size { + let current_index = + self.acir_context.add_constant(FieldElement::from(i as u128)); + + let shifted_index = if (i + popped_elements_size) >= slice_size { + current_index + } else { + self.acir_context + .add_constant(FieldElement::from((i + popped_elements_size) as u128)) + }; + + let value_shifted_index = + self.acir_context.read_from_memory(block_id, &shifted_index)?; + let value_current_index = new_slice[i].borrow_var()?; + + let use_shifted_value = self.acir_context.more_than_eq_var( + current_index, + flat_user_index, + 64, + self.current_side_effects_enabled_var, + )?; + + let shifted_value_pred = + self.acir_context.mul_var(value_shifted_index, use_shifted_value)?; + let not_pred = self.acir_context.sub_var(one, use_shifted_value)?; + let current_value_pred = + self.acir_context.mul_var(not_pred, value_current_index)?; + + let new_value = + self.acir_context.add_var(shifted_value_pred, current_value_pred)?; + + self.acir_context.write_to_memory( + result_block_id, + ¤t_index, + &new_value, + )?; + } + + let new_slice_val = AcirValue::Array(new_slice); + let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) { + Some(self.init_element_type_sizes_array( + &slice_typ, + slice_contents, + Some(new_slice_val), + dfg, + )?) + } else { + None }; + let result = AcirValue::DynamicArray(AcirDynamicArray { + block_id: result_block_id, + len: slice_size, + element_type_sizes, + }); - Ok(vec![ - AcirValue::Var(new_slice_length, AcirType::field()), - AcirValue::Array(new_slice), - removed_elem, - ]) + let mut result = vec![AcirValue::Var(new_slice_length, AcirType::field()), result]; + result.append(&mut popped_elements); + + Ok(result) } _ => todo!("expected a black box function"), } diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index 8f2fe2d236b..fbc95a16387 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -104,6 +104,14 @@ impl Type { } } + pub(crate) fn is_nested_slice(&self) -> bool { + if let Type::Slice(element_types) = self { + element_types.as_ref().iter().any(|typ| typ.contains_slice_element()) + } else { + false + } + } + /// True if this type is an array (or slice) or internally contains an array (or slice) pub(crate) fn contains_an_array(&self) -> bool { match self { diff --git a/compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs b/compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs index d40ef1f996e..fede0e540e2 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs @@ -174,6 +174,9 @@ impl<'f> Context<'f> { panic!("ICE: should have inner slice set for {slice_value}") }); slice_sizes.insert(results[0], inner_slice.clone()); + if slice_value != results[0] { + self.mapped_slice_values.insert(slice_value, results[0]); + } } } } @@ -198,17 +201,11 @@ impl<'f> Context<'f> { let inner_sizes = slice_sizes.get_mut(array).expect("ICE expected slice sizes"); inner_sizes.1.push(*value); - - let value_parent = self.resolve_slice_parent(*value); - if slice_values.contains(&value_parent) { - // Map the value parent to the current array in case nested slices - // from the current array are set to larger values later in the program - self.mapped_slice_values.insert(value_parent, *array); - } } if let Some(inner_sizes) = slice_sizes.get_mut(array) { let inner_sizes = inner_sizes.clone(); + slice_sizes.insert(results[0], inner_sizes); self.mapped_slice_values.insert(*array, results[0]); @@ -224,14 +221,27 @@ impl<'f> Context<'f> { | Intrinsic::SlicePopBack | Intrinsic::SliceInsert | Intrinsic::SliceRemove => (1, 1), - Intrinsic::SlicePopFront => (1, 2), + // `pop_front` returns the popped element, and then the respective slice. + // This means in the case of a slice with structs, the result index of the popped slice + // will change depending on the number of elements in the struct. + // For example, a slice with four elements will look as such in SSA: + // v3, v4, v5, v6, v7, v8 = call slice_pop_front(v1, v2) + // where v7 is the slice length and v8 is the popped slice itself. + Intrinsic::SlicePopFront => (1, results.len() - 1), _ => return, }; + let slice_contents = arguments[argument_index]; match intrinsic { Intrinsic::SlicePushBack | Intrinsic::SlicePushFront | Intrinsic::SliceInsert => { - let slice_contents = arguments[argument_index]; + for arg in &arguments[(argument_index + 1)..] { + let element_typ = self.inserter.function.dfg.type_of_value(*arg); + if element_typ.contains_slice_element() { + slice_values.push(*arg); + self.compute_slice_sizes(*arg, slice_sizes); + } + } if let Some(inner_sizes) = slice_sizes.get_mut(&slice_contents) { inner_sizes.0 += 1; @@ -240,12 +250,12 @@ impl<'f> Context<'f> { self.mapped_slice_values .insert(slice_contents, results[result_index]); + self.slice_parents.insert(results[result_index], slice_contents); } } Intrinsic::SlicePopBack - | Intrinsic::SlicePopFront - | Intrinsic::SliceRemove => { - let slice_contents = arguments[argument_index]; + | Intrinsic::SliceRemove + | Intrinsic::SlicePopFront => { // We do not decrement the size on intrinsics that could remove values from a slice. // This is because we could potentially go back to the smaller slice and not fill in dummies. // This pass should be tracking the potential max that a slice ***could be*** @@ -255,6 +265,7 @@ impl<'f> Context<'f> { self.mapped_slice_values .insert(slice_contents, results[result_index]); + self.slice_parents.insert(results[result_index], slice_contents); } } _ => {} @@ -277,7 +288,6 @@ impl<'f> Context<'f> { if slice_values.contains(array) { let (new_array_op_instr, call_stack) = self.get_updated_array_op_instr(*array, slice_sizes, instruction); - self.inserter.push_instruction_value( new_array_op_instr, instruction, @@ -288,6 +298,55 @@ impl<'f> Context<'f> { self.inserter.push_instruction(instruction, block); } } + Instruction::Call { func: _, arguments } => { + let mut args_to_replace = Vec::new(); + for (i, arg) in arguments.iter().enumerate() { + let element_typ = self.inserter.function.dfg.type_of_value(*arg); + if slice_values.contains(arg) && element_typ.contains_slice_element() { + args_to_replace.push((i, *arg)); + } + } + if args_to_replace.is_empty() { + self.inserter.push_instruction(instruction, block); + } else { + // Using the original slice is ok to do as during collection of slice information + // we guarantee that only the arguments to slice intrinsic calls can be replaced. + let slice_contents = arguments[1]; + + let element_typ = self.inserter.function.dfg.type_of_value(arguments[1]); + let elem_depth = Self::compute_nested_slice_depth(&element_typ); + + let mut max_sizes = Vec::new(); + max_sizes.resize(elem_depth, 0); + // We want the max for the parent of the argument + let parent = self.resolve_slice_parent(slice_contents); + self.compute_slice_max_sizes(parent, slice_sizes, &mut max_sizes, 0); + + for (index, arg) in args_to_replace { + let element_typ = self.inserter.function.dfg.type_of_value(arg); + max_sizes.remove(0); + let new_array = + self.attach_slice_dummies(&element_typ, Some(arg), false, &max_sizes); + + let instruction_id = instruction; + let (instruction, call_stack) = + self.inserter.map_instruction(instruction_id); + let new_call_instr = match instruction { + Instruction::Call { func, mut arguments } => { + arguments[index] = new_array; + Instruction::Call { func, arguments } + } + _ => panic!("Expected call instruction"), + }; + self.inserter.push_instruction_value( + new_call_instr, + instruction_id, + block, + call_stack, + ); + } + } + } _ => { self.inserter.push_instruction(instruction, block); } @@ -314,6 +373,7 @@ impl<'f> Context<'f> { let typ = self.inserter.function.dfg.type_of_value(array_id); let depth = Self::compute_nested_slice_depth(&typ); max_sizes.resize(depth, 0); + max_sizes[0] = *current_size; self.compute_slice_max_sizes(array_id, slice_sizes, &mut max_sizes, 1); @@ -370,9 +430,12 @@ impl<'f> Context<'f> { if let Some(value) = value { let mut slice = im::Vector::new(); - let array = match self.inserter.function.dfg[value].clone() { + let value = self.inserter.function.dfg[value].clone(); + let array = match value { Value::Array { array, .. } => array, - _ => panic!("Expected an array value"), + _ => { + panic!("Expected an array value"); + } }; if is_parent_slice { @@ -487,7 +550,6 @@ impl<'f> Context<'f> { self.compute_slice_max_sizes(*inner_slice, slice_sizes, max_sizes, depth + 1); } - max_sizes[depth] = max; if max > max_sizes[depth] { max_sizes[depth] = max; } diff --git a/noir_stdlib/src/slice.nr b/noir_stdlib/src/slice.nr index ca06e2aae44..a5a9a38ed53 100644 --- a/noir_stdlib/src/slice.nr +++ b/noir_stdlib/src/slice.nr @@ -21,28 +21,16 @@ impl [T] { #[builtin(slice_pop_front)] pub fn pop_front(_self: Self) -> (T, Self) { } - pub fn insert(self, _index: Field, _elem: T) -> Self { - // TODO(#2462): Slice insert with a dynamic index - crate::assert_constant(_index); - self.__slice_insert(_index, _elem) - } - /// Insert an element at a specified index, shifting all elements /// after it to the right #[builtin(slice_insert)] - fn __slice_insert(_self: Self, _index: Field, _elem: T) -> Self { } - - pub fn remove(self, _index: Field) -> (Self, T) { - // TODO(#2462): Slice remove with a dynamic index - crate::assert_constant(_index); - self.__slice_remove(_index) - } + pub fn insert(_self: Self, _index: Field, _elem: T) -> Self { } /// Remove an element at a specified index, shifting all elements /// after it to the left, returning the altered slice and /// the removed element #[builtin(slice_remove)] - fn __slice_remove(_self: Self, _index: Field) -> (Self, T) { } + pub fn remove(_self: Self, _index: Field) -> (Self, T) { } // Append each element of the `other` slice to the end of `self`. // This returns a new slice and leaves both input slices unchanged. diff --git a/test_programs/execution_success/slice_dynamic_index/src/main.nr b/test_programs/execution_success/slice_dynamic_index/src/main.nr index 2e5c0122dfb..374d2ba4c26 100644 --- a/test_programs/execution_success/slice_dynamic_index/src/main.nr +++ b/test_programs/execution_success/slice_dynamic_index/src/main.nr @@ -123,13 +123,13 @@ fn dynamic_slice_merge_if(mut slice: [Field], x: Field) { let (first_elem, rest_of_slice) = popped_slice.pop_front(); assert(first_elem == 12); assert(rest_of_slice.len() == 6); - // TODO(#2462): SliceInsert and SliceRemove with a dynamic index are not yet implemented in ACIR gen - slice = rest_of_slice.insert(2, 20); + + slice = rest_of_slice.insert(x - 2, 20); assert(slice[2] == 20); assert(slice[6] == 30); assert(slice.len() == 7); - // TODO(#2462): SliceInsert and SliceRemove with a dynamic index are not yet implemented in ACIR gen - let (removed_slice, removed_elem) = slice.remove(3); + + let (removed_slice, removed_elem) = slice.remove(x - 1); // The deconstructed tuple assigns to the slice but is not seen outside of the if statement // without a direct assignment slice = removed_slice; diff --git a/test_programs/execution_success/slice_struct_field/src/main.nr b/test_programs/execution_success/slice_struct_field/src/main.nr index c00fdf85180..a5b971ada4b 100644 --- a/test_programs/execution_success/slice_struct_field/src/main.nr +++ b/test_programs/execution_success/slice_struct_field/src/main.nr @@ -23,12 +23,12 @@ fn main(y: pub Field) { let foo_two = Foo { a: 4, b: b_two, bar: Bar { inner: [103, 104, 105] } }; let foo_three = Foo { a: 7, b: [8, 9, 22], bar: Bar { inner: [106, 107, 108] } }; - let foo_four = Foo { a: 10, b: [11, 12, 23], bar: Bar { inner: [109, 110, 111] } }; + let mut foo_four = Foo { a: 10, b: [11, 12, 23], bar: Bar { inner: [109, 110, 111] } }; let mut x = [foo_one, foo_two]; x = x.push_back(foo_three); x = x.push_back(foo_four); - + assert(x[y - 3].a == 1); let struct_slice = x[y - 3].b; for i in 0..4 { @@ -60,6 +60,14 @@ fn main(y: pub Field) { assert(x[y].bar.inner == [109, 110, 111]); // Check that switching the lhs and rhs is still valid assert([109, 110, 111] == x[y].bar.inner); + + assert(x[y - 3].bar.inner == [100, 101, 102]); + assert(x[y - 2].bar.inner == [103, 104, 105]); + assert(x[y - 1].bar.inner == [106, 107, 108]); + assert(x[y].bar.inner == [109, 110, 111]); + // Check that switching the lhs and rhs is still valid + assert([109, 110, 111] == x[y].bar.inner); + // TODO: Enable merging nested slices // if y != 2 { // x[y].a = 50; @@ -75,12 +83,13 @@ fn main(y: pub Field) { // assert(x[2].b[0] == 100); // assert(x[2].b[1] == 101); // assert(x[2].b[2] == 102); + let q = x.push_back(foo_four); let foo_parent_one = FooParent { parent_arr: [0, 1, 2], foos: x }; let foo_parent_two = FooParent { parent_arr: [3, 4, 5], foos: q }; let mut foo_parents = [foo_parent_one]; foo_parents = foo_parents.push_back(foo_parent_two); - // TODO: make a separate test for compile time + // TODO: make a separate test for entirely compile time // foo_parents[1].foos.push_back(foo_four); // TODO: Merging nested slices is broken // if y == 3 { @@ -88,6 +97,7 @@ fn main(y: pub Field) { // } else { // foo_parents[y - 2].foos[y - 1].b[y - 1] = 1000; // } + assert(foo_parents[y - 2].foos[y - 2].b[y - 1] == 21); foo_parents[y - 2].foos[y - 2].b[y - 1] = 5000; assert(foo_parents[y - 2].foos[y - 2].b[y - 1] == 5000); @@ -108,11 +118,15 @@ fn main(y: pub Field) { assert(foo_parents[y - 2].foos[y - 1].a == 7); foo_parents[y - 2].foos[y - 1].a = 50; + assert(foo_parents[y - 2].foos[y - 1].a == 50); let b_array = foo_parents[y - 2].foos[y - 1].b; + assert(b_array[0] == 8); + assert(b_array[1] == 9); assert(b_array[2] == 22); assert(b_array.len() == 3); - // Test setting a nested array with non-dynamic + + // // Test setting a nested array with non-dynamic let x = [5, 6, 5000, 21, 100, 101].as_slice(); foo_parents[y - 2].foos[y - 1].b = x; @@ -120,16 +134,52 @@ fn main(y: pub Field) { assert(foo_parents[y - 2].foos[y - 1].b[4] == 100); assert(foo_parents[y - 2].foos[y - 1].b[5] == 101); + // Need to account for that foo_parents is not modified outside of this function test_basic_intrinsics_nested_slices(foo_parents, y); - // TODO(#3364): still have to enable slice intrinsics on dynamic nested slices - // assert(foo_parents[y - 2].foos.len() == 5); - // foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_back(foo_four); - // assert(foo_parents[y - 2].foos.len() == 6); + test_complex_intrinsic_nested_slices(foo_parents, y); + + foo_parents[y - 2].foos[y - 1].b = foo_parents[y - 2].foos[y - 1].b.push_back(500); + assert(foo_parents[y - 2].foos[y - 1].b.len() == 7); + assert(foo_parents[y - 2].foos[y - 1].b[6] == 500); + + let (popped_slice, last_elem) = foo_parents[y - 2].foos[y - 1].b.pop_back(); + foo_parents[y - 2].foos[y - 1].b = popped_slice; + assert(foo_parents[y - 2].foos[y - 1].b.len() == 6); + assert(last_elem == 500); + + foo_parents[y - 2].foos[y - 1].b = foo_parents[y - 2].foos[y - 1].b.push_front(11); + assert(foo_parents[y - 2].foos[y - 1].b.len() == 7); + assert(foo_parents[y - 2].foos[y - 1].b[0] == 11); + + assert(foo_parents[y - 2].foos.len() == 5); + foo_four.a = 40; + foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_back(foo_four); + assert(foo_parents[y - 2].foos.len() == 6); + assert(foo_parents[y - 2].foos[y + 2].bar.inner == [109, 110, 111]); + + foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_back(foo_four); + assert(foo_parents[y - 2].foos.len() == 7); + assert(foo_parents[y - 2].foos[6].a == 40); + assert(foo_parents[y - 2].foos[5].bar.inner == [109, 110, 111]); + assert(foo_parents[y - 2].foos[6].bar.inner == [109, 110, 111]); + + foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_back(foo_four); + assert(foo_parents[y - 2].foos.len() == 8); + assert(foo_parents[y - 2].foos[6].a == 40); + assert(foo_parents[y - 2].foos[5].bar.inner == [109, 110, 111]); + assert(foo_parents[y - 2].foos[6].bar.inner == [109, 110, 111]); + + foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_back(foo_four); + assert(foo_parents[y - 2].foos.len() == 9); + + foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_back(foo_four); + assert(foo_parents[y - 2].foos.len() == 10); + let b_array = foo_parents[y - 2].foos[y - 1].b; - assert(b_array[0] == 5); - assert(b_array[1] == 6); - assert(b_array[2] == 5000); - assert(b_array[3] == 21); + assert(b_array[0] == 11); + assert(b_array[1] == 5); + assert(b_array[2] == 6); + assert(b_array[3] == 5000); let b_array = foo_parents[y - 2].foos[y].b; assert(foo_parents[y - 2].foos[y].a == 10); @@ -175,3 +225,248 @@ fn test_basic_intrinsics_nested_slices(mut foo_parents: [FooParent], y: Field) { assert(foo_parents[y - 2].foos[y - 1].b[2] == 20); assert(foo_parents[y - 2].foos[y - 1].b[3] == 21); } + +// This method test intrinsics on nested slices with complex inputs such as +// pushing a `Foo` struct onto a slice in `FooParents`. +fn test_complex_intrinsic_nested_slices(mut foo_parents: [FooParent], y: Field) { + let mut foo = Foo { a: 13, b: [14, 15, 16], bar: Bar { inner: [109, 110, 111] } }; + assert(foo_parents[y - 2].foos.len() == 5); + foo.a = 40; + foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_back(foo); + assert(foo_parents[1].foos.len() == 6); + assert(foo_parents[1].foos[5].a == 40); + assert(foo_parents[1].foos[5].b[0] == 14); + assert(foo_parents[1].foos[5].b[2] == 16); + assert(foo_parents[1].foos[5].b.len() == 3); + assert(foo_parents[1].foos[5].bar.inner == [109, 110, 111]); + + foo_parents[y - 2].foos[y - 1].b = foo_parents[y - 2].foos[y - 1].b.push_back(500); + assert(foo_parents[1].foos[2].b.len() == 7); + assert(foo_parents[1].foos[2].b[6] == 500); + assert(foo_parents[1].foos[2].bar.inner == [106, 107, 108]); + assert(foo_parents[1].foos[5].a == 40); + assert(foo_parents[1].foos[5].b[0] == 14); + assert(foo_parents[1].foos[5].b[2] == 16); + assert(foo_parents[1].foos[5].b.len() == 3); + assert(foo_parents[1].foos[5].bar.inner == [109, 110, 111]); + + let (popped_slice, last_foo) = foo_parents[y - 2].foos.pop_back(); + foo_parents[y - 2].foos = popped_slice; + assert(foo_parents[y - 2].foos.len() == 5); + assert(last_foo.a == 40); + assert(last_foo.b[0] == 14); + assert(last_foo.b[1] == 15); + assert(last_foo.b[2] == 16); + assert(last_foo.bar.inner == [109, 110, 111]); + + foo_parents[y - 2].foos = foo_parents[y - 2].foos.push_front(foo); + assert(foo_parents[1].foos.len() == 6); + assert(foo_parents[1].foos[0].a == 40); + assert(foo_parents[1].foos[0].b[0] == 14); + assert(foo_parents[1].foos[0].b[1] == 15); + assert(foo_parents[1].foos[0].b[2] == 16); + assert(foo_parents[1].foos[5].a == 10); + assert(foo_parents[1].foos[5].b.len() == 3); + assert(foo_parents[1].foos[5].b[0] == 11); + assert(foo_parents[1].foos[5].b[2] == 23); + assert(foo_parents[1].foos[5].bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[1].a == 1); + assert(foo_parents[1].foos[1].bar.inner == [100, 101, 102]); + + let (first_foo, rest_of_slice) = foo_parents[y - 2].foos.pop_front(); + + foo_parents[y - 2].foos = rest_of_slice; + assert(first_foo.a == 40); + assert(first_foo.b[0] == 14); + assert(first_foo.b[1] == 15); + assert(first_foo.b[2] == 16); + assert(first_foo.bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[0].a == 1); + assert(foo_parents[1].foos[0].b[0] == 2); + assert(foo_parents[1].foos[0].b[1] == 3); + assert(foo_parents[1].foos[0].b[2] == 20); + assert(foo_parents[1].foos[0].b[3] == 20); + assert(foo_parents[1].foos[0].bar.inner == [100, 101, 102]); + + test_insert_remove_const_index(foo_parents, y, foo); + + // Check values before insertion + assert(foo_parents[1].foos[1].a == 4); + assert(foo_parents[1].foos[1].b[0] == 5); + assert(foo_parents[1].foos[1].b[1] == 6); + assert(foo_parents[1].foos[1].b[2] == 5000); + assert(foo_parents[1].foos[1].b[3] == 21); + assert(foo_parents[1].foos[1].bar.inner == [103, 104, 105]); + + assert(foo_parents[1].foos.len() == 5); + assert(foo_parents[1].foos[2].a == 50); + assert(foo_parents[1].foos[2].b[0] == 5); + assert(foo_parents[1].foos[2].b[2] == 5000); + assert(foo_parents[1].foos[2].bar.inner == [106, 107, 108]); + + assert(foo_parents[1].foos[3].a == 10); + assert(foo_parents[1].foos[3].b[0] == 11); + assert(foo_parents[1].foos[3].b[2] == 23); + assert(foo_parents[1].foos[3].bar.inner == [109, 110, 111]); + + foo_parents[y - 2].foos = foo_parents[y - 2].foos.insert(y - 1, foo); + assert(foo_parents[1].foos.len() == 6); + + // Check values correctly moved after insertion + assert(foo_parents[1].foos[0].a == 1); + assert(foo_parents[1].foos[0].b[0] == 2); + assert(foo_parents[1].foos[0].b[1] == 3); + assert(foo_parents[1].foos[0].b[2] == 20); + assert(foo_parents[1].foos[0].b[3] == 20); + assert(foo_parents[1].foos[0].bar.inner == [100, 101, 102]); + + assert(foo_parents[1].foos[1].a == 4); + assert(foo_parents[1].foos[1].b[0] == 5); + assert(foo_parents[1].foos[1].b[1] == 6); + assert(foo_parents[1].foos[1].b[2] == 5000); + assert(foo_parents[1].foos[1].b[3] == 21); + assert(foo_parents[1].foos[1].bar.inner == [103, 104, 105]); + + assert(foo_parents[1].foos[2].a == 40); + assert(foo_parents[1].foos[2].b[0] == 14); + assert(foo_parents[1].foos[2].b[2] == 16); + assert(foo_parents[1].foos[2].bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[3].a == 50); + assert(foo_parents[1].foos[3].b[0] == 5); + assert(foo_parents[1].foos[3].b[2] == 5000); + assert(foo_parents[1].foos[3].bar.inner == [106, 107, 108]); + + assert(foo_parents[1].foos[4].a == 10); + assert(foo_parents[1].foos[4].b[0] == 11); + assert(foo_parents[1].foos[4].b[2] == 23); + assert(foo_parents[1].foos[4].bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[5].a == 10); + assert(foo_parents[1].foos[5].b[0] == 11); + assert(foo_parents[1].foos[5].b[2] == 23); + assert(foo_parents[1].foos[5].bar.inner == [109, 110, 111]); + + let (rest_of_slice, removed_elem) = foo_parents[y - 2].foos.remove(y - 1); + foo_parents[1].foos = rest_of_slice; + + // Check that the accurate element was removed + assert(removed_elem.a == 40); + assert(removed_elem.b[0] == 14); + assert(removed_elem.b[2] == 16); + assert(removed_elem.bar.inner == [109, 110, 111]); + + // Check that we have altered our slice accurately following a removal + assert(foo_parents[1].foos[1].a == 4); + assert(foo_parents[1].foos[1].b[0] == 5); + assert(foo_parents[1].foos[1].b[1] == 6); + assert(foo_parents[1].foos[1].b[2] == 5000); + assert(foo_parents[1].foos[1].b[3] == 21); + assert(foo_parents[1].foos[1].bar.inner == [103, 104, 105]); + + assert(foo_parents[1].foos[2].a == 50); + assert(foo_parents[1].foos[2].b[0] == 5); + assert(foo_parents[1].foos[2].b[2] == 5000); + assert(foo_parents[1].foos[2].bar.inner == [106, 107, 108]); + + assert(foo_parents[1].foos[3].a == 10); + assert(foo_parents[1].foos[3].b[0] == 11); + assert(foo_parents[1].foos[3].b[2] == 23); + assert(foo_parents[1].foos[3].bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[4].b[0] == 11); + assert(foo_parents[1].foos[4].b[2] == 23); + assert(foo_parents[1].foos[4].bar.inner == [109, 110, 111]); +} + +fn test_insert_remove_const_index(mut foo_parents: [FooParent], y: Field, foo: Foo) { + // Check values before insertion + assert(foo_parents[1].foos[1].a == 4); + assert(foo_parents[1].foos[1].b[0] == 5); + assert(foo_parents[1].foos[1].b[1] == 6); + assert(foo_parents[1].foos[1].b[2] == 5000); + assert(foo_parents[1].foos[1].b[3] == 21); + assert(foo_parents[1].foos[1].bar.inner == [103, 104, 105]); + + assert(foo_parents[1].foos.len() == 5); + assert(foo_parents[1].foos[2].a == 50); + assert(foo_parents[1].foos[2].b[0] == 5); + assert(foo_parents[1].foos[2].b[2] == 5000); + assert(foo_parents[1].foos[2].bar.inner == [106, 107, 108]); + + assert(foo_parents[1].foos[3].a == 10); + assert(foo_parents[1].foos[3].b[0] == 11); + assert(foo_parents[1].foos[3].b[2] == 23); + assert(foo_parents[1].foos[3].bar.inner == [109, 110, 111]); + + foo_parents[y - 2].foos = foo_parents[y - 2].foos.insert(2, foo); + assert(foo_parents[1].foos.len() == 6); + + // Check values correctly moved after insertion + assert(foo_parents[1].foos[0].a == 1); + assert(foo_parents[1].foos[0].b[0] == 2); + assert(foo_parents[1].foos[0].b[1] == 3); + assert(foo_parents[1].foos[0].b[2] == 20); + assert(foo_parents[1].foos[0].b[3] == 20); + assert(foo_parents[1].foos[0].bar.inner == [100, 101, 102]); + + assert(foo_parents[1].foos[1].a == 4); + assert(foo_parents[1].foos[1].b[0] == 5); + assert(foo_parents[1].foos[1].b[1] == 6); + assert(foo_parents[1].foos[1].b[2] == 5000); + assert(foo_parents[1].foos[1].b[3] == 21); + assert(foo_parents[1].foos[1].bar.inner == [103, 104, 105]); + + assert(foo_parents[1].foos[2].a == 40); + assert(foo_parents[1].foos[2].b[0] == 14); + assert(foo_parents[1].foos[2].b[2] == 16); + assert(foo_parents[1].foos[2].bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[3].a == 50); + assert(foo_parents[1].foos[3].b[0] == 5); + assert(foo_parents[1].foos[3].b[2] == 5000); + assert(foo_parents[1].foos[3].bar.inner == [106, 107, 108]); + + assert(foo_parents[1].foos[4].a == 10); + assert(foo_parents[1].foos[4].b[0] == 11); + assert(foo_parents[1].foos[4].b[2] == 23); + assert(foo_parents[1].foos[4].bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[5].a == 10); + assert(foo_parents[1].foos[5].b[0] == 11); + assert(foo_parents[1].foos[5].b[2] == 23); + assert(foo_parents[1].foos[5].bar.inner == [109, 110, 111]); + + let (rest_of_slice, removed_elem) = foo_parents[y - 2].foos.remove(2); + foo_parents[1].foos = rest_of_slice; + + // Check that the accurate element was removed + assert(removed_elem.a == 40); + assert(removed_elem.b[0] == 14); + assert(removed_elem.b[2] == 16); + assert(removed_elem.bar.inner == [109, 110, 111]); + + // Check that we have altered our slice accurately following a removal + assert(foo_parents[1].foos[1].a == 4); + assert(foo_parents[1].foos[1].b[0] == 5); + assert(foo_parents[1].foos[1].b[1] == 6); + assert(foo_parents[1].foos[1].b[2] == 5000); + assert(foo_parents[1].foos[1].b[3] == 21); + assert(foo_parents[1].foos[1].bar.inner == [103, 104, 105]); + + assert(foo_parents[1].foos[2].a == 50); + assert(foo_parents[1].foos[2].b[0] == 5); + assert(foo_parents[1].foos[2].b[2] == 5000); + assert(foo_parents[1].foos[2].bar.inner == [106, 107, 108]); + + assert(foo_parents[1].foos[3].a == 10); + assert(foo_parents[1].foos[3].b[0] == 11); + assert(foo_parents[1].foos[3].b[2] == 23); + assert(foo_parents[1].foos[3].bar.inner == [109, 110, 111]); + + assert(foo_parents[1].foos[4].b[0] == 11); + assert(foo_parents[1].foos[4].b[2] == 23); + assert(foo_parents[1].foos[4].bar.inner == [109, 110, 111]); +}