diff --git a/crates/nargo_cli/tests/test_data/slices/src/main.nr b/crates/nargo_cli/tests/test_data/slices/src/main.nr index f97078a2143..cda6657b4ff 100644 --- a/crates/nargo_cli/tests/test_data/slices/src/main.nr +++ b/crates/nargo_cli/tests/test_data/slices/src/main.nr @@ -42,5 +42,39 @@ fn main(x : Field, y : pub Field) { assert(removed_elem == 2); assert(remove_slice[3] == 3); assert(remove_slice.len() == 4); + + regression_2083(); } +// Ensure that slices of struct/tuple values work. +fn regression_2083() { + let y = [(1, 2)]; + let y = y.push_back((3, 4)); // [(1, 2), (3, 4)] + let y = y.push_back((5, 6)); // [(1, 2), (3, 4), (5, 6)] + assert(y[2].1 == 6); + + let y = y.push_front((10, 11)); // [(10, 11), (1, 2), (3, 4), (5, 6)] + let y = y.push_front((12, 13)); // [(12, 13), (10, 11), (1, 2), (3, 4), (5, 6)] + + assert(y[1].0 == 10); + + let y = y.insert(1, (55, 56)); // [(12, 13), (55, 56), (10, 11), (1, 2), (3, 4), (5, 6)] + assert(y[0].1 == 13); + assert(y[1].1 == 56); + assert(y[2].0 == 10); + + let (y, x) = y.remove(2); // [(12, 13), (55, 56), (1, 2), (3, 4), (5, 6)] + assert(y[2].0 == 1); + assert(x.0 == 10); + assert(x.1 == 11); + + let (x, y) = y.pop_front(); // [(55, 56), (1, 2), (3, 4), (5, 6)] + assert(y[0].0 == 55); + assert(x.0 == 12); + assert(x.1 == 13); + + let (y, x) = y.pop_back(); // [(55, 56), (1, 2), (3, 4)] + assert(y.len() == 3); + assert(x.0 == 5); + assert(x.1 == 6); +} diff --git a/crates/noirc_evaluator/src/ssa/ir/dfg.rs b/crates/noirc_evaluator/src/ssa/ir/dfg.rs index 29f5156a88c..1dd54499632 100644 --- a/crates/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa/ir/dfg.rs @@ -369,7 +369,7 @@ impl DataFlowGraph { /// Otherwise, this returns None. pub(crate) fn get_array_constant(&self, value: ValueId) -> Option<(im::Vector, Type)> { match &self.values[self.resolve(value)] { - // Vectors are shared, so cloning them is cheap + // Arrays are shared, so cloning them is cheap Value::Array { array, typ } => Some((array.clone(), typ.clone())), _ => None, } diff --git a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs index 2f0c077a1a7..d5925080870 100644 --- a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -1,4 +1,4 @@ -use std::rc::Rc; +use std::{collections::VecDeque, rc::Rc}; use acvm::{acir::BlackBoxFunc, BlackBoxResolutionError, FieldElement}; use iter_extended::vecmap; @@ -53,22 +53,22 @@ pub(super) fn simplify_call( } Intrinsic::ArrayLen => { let slice = dfg.get_array_constant(arguments[0]); - if let Some((slice, _)) = slice { - SimplifyResult::SimplifiedTo( - dfg.make_constant((slice.len() as u128).into(), Type::field()), - ) + if let Some((slice, typ)) = slice { + let length = FieldElement::from((slice.len() / typ.element_size()) as u128); + SimplifyResult::SimplifiedTo(dfg.make_constant(length, Type::field())) } else if let Some(length) = dfg.try_get_array_length(arguments[0]) { - SimplifyResult::SimplifiedTo( - dfg.make_constant((length as u128).into(), Type::field()), - ) + let length = FieldElement::from(length as u128); + SimplifyResult::SimplifiedTo(dfg.make_constant(length, Type::field())) } else { SimplifyResult::None } } Intrinsic::SlicePushBack => { let slice = dfg.get_array_constant(arguments[0]); - if let (Some((mut slice, element_type)), elem) = (slice, arguments[1]) { - slice.push_back(elem); + if let Some((mut slice, element_type)) = slice { + for elem in &arguments[1..] { + slice.push_back(*elem); + } let new_slice = dfg.make_array(slice, element_type); SimplifyResult::SimplifiedTo(new_slice) } else { @@ -77,8 +77,10 @@ pub(super) fn simplify_call( } Intrinsic::SlicePushFront => { let slice = dfg.get_array_constant(arguments[0]); - if let (Some((mut slice, element_type)), elem) = (slice, arguments[1]) { - slice.push_front(elem); + if let Some((mut slice, element_type)) = slice { + for elem in arguments[1..].iter().rev() { + slice.push_front(*elem); + } let new_slice = dfg.make_array(slice, element_type); SimplifyResult::SimplifiedTo(new_slice) } else { @@ -87,22 +89,41 @@ pub(super) fn simplify_call( } Intrinsic::SlicePopBack => { let slice = dfg.get_array_constant(arguments[0]); - if let Some((mut slice, element_type)) = slice { - let elem = - slice.pop_back().expect("There are no elements in this slice to be removed"); - let new_slice = dfg.make_array(slice, element_type); - SimplifyResult::SimplifiedToMultiple(vec![new_slice, elem]) + if let Some((mut slice, typ)) = slice { + let element_count = typ.element_size(); + let mut results = VecDeque::with_capacity(element_count + 1); + + // We must pop multiple elements in the case of a slice of tuples + for _ in 0..element_count { + let elem = slice + .pop_back() + .expect("There are no elements in this slice to be removed"); + results.push_front(elem); + } + + let new_slice = dfg.make_array(slice, typ); + results.push_front(new_slice); + + SimplifyResult::SimplifiedToMultiple(results.into()) } else { SimplifyResult::None } } Intrinsic::SlicePopFront => { let slice = dfg.get_array_constant(arguments[0]); - if let Some((mut slice, element_type)) = slice { - let elem = - slice.pop_front().expect("There are no elements in this slice to be removed"); - let new_slice = dfg.make_array(slice, element_type); - SimplifyResult::SimplifiedToMultiple(vec![elem, new_slice]) + if let Some((mut slice, typ)) = slice { + let element_count = typ.element_size(); + + // We must pop multiple elements in the case of a slice of tuples + let mut results = vecmap(0..element_count, |_| { + slice.pop_front().expect("There are no elements in this slice to be removed") + }); + + let new_slice = dfg.make_array(slice, typ); + + // The slice is the last item returned for pop_front + results.push(new_slice); + SimplifyResult::SimplifiedToMultiple(results) } else { SimplifyResult::None } @@ -110,11 +131,16 @@ pub(super) fn simplify_call( Intrinsic::SliceInsert => { let slice = dfg.get_array_constant(arguments[0]); let index = dfg.get_numeric_constant(arguments[1]); - if let (Some((mut slice, element_type)), Some(index), value) = - (slice, index, arguments[2]) - { - slice.insert(index.to_u128() as usize, value); - let new_slice = dfg.make_array(slice, element_type); + if let (Some((mut slice, typ)), Some(index)) = (slice, index) { + let elements = &arguments[2..]; + let mut index = index.to_u128() as usize * elements.len(); + + for elem in &arguments[2..] { + slice.insert(index, *elem); + index += 1; + } + + let new_slice = dfg.make_array(slice, typ); SimplifyResult::SimplifiedTo(new_slice) } else { SimplifyResult::None @@ -123,10 +149,18 @@ pub(super) fn simplify_call( Intrinsic::SliceRemove => { let slice = dfg.get_array_constant(arguments[0]); let index = dfg.get_numeric_constant(arguments[1]); - if let (Some((mut slice, element_type)), Some(index)) = (slice, index) { - let removed_elem = slice.remove(index.to_u128() as usize); - let new_slice = dfg.make_array(slice, element_type); - SimplifyResult::SimplifiedToMultiple(vec![new_slice, removed_elem]) + if let (Some((mut slice, typ)), Some(index)) = (slice, index) { + let element_count = typ.element_size(); + let mut results = Vec::with_capacity(element_count + 1); + let index = index.to_u128() as usize * element_count; + + for _ in 0..element_count { + results.push(slice.remove(index)); + } + + let new_slice = dfg.make_array(slice, typ); + results.insert(0, new_slice); + SimplifyResult::SimplifiedToMultiple(results) } else { SimplifyResult::None } diff --git a/crates/noirc_evaluator/src/ssa/ir/types.rs b/crates/noirc_evaluator/src/ssa/ir/types.rs index 7e37a72ff83..38dd6125121 100644 --- a/crates/noirc_evaluator/src/ssa/ir/types.rs +++ b/crates/noirc_evaluator/src/ssa/ir/types.rs @@ -61,6 +61,17 @@ impl Type { pub(crate) fn field() -> Type { Type::Numeric(NumericType::NativeField) } + + /// Returns the size of the element type for this array/slice. + /// The size of a type is defined as representing how many Fields are needed + /// to represent the type. This is 1 for every primitive type, and is the number of fields + /// for any flattened tuple type. + pub(crate) fn element_size(&self) -> usize { + match self { + Type::Array(elements, _) | Type::Slice(elements) => elements.len(), + other => panic!("element_size: Expected array or slice, found {other}"), + } + } } /// Composite Types are essentially flattened struct or tuple types. diff --git a/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs b/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs index c3578e5ee7e..6de804a37b8 100644 --- a/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -641,10 +641,8 @@ impl<'a> FunctionContext<'a> { } fn element_size(&self, array: ValueId) -> FieldElement { - match self.builder.type_of_value(array) { - Type::Array(elements, _) | Type::Slice(elements) => (elements.len() as u128).into(), - t => panic!("Uncaught type error: tried to take element size of non-array type {t}"), - } + let size = self.builder.type_of_value(array).element_size(); + FieldElement::from(size as u128) } /// Given an lhs containing only references, create a store instruction to store each value of