diff --git a/crates/nargo/src/ops/foreign_calls.rs b/crates/nargo/src/ops/foreign_calls.rs index 4d2f5988e38..e1768d71c44 100644 --- a/crates/nargo/src/ops/foreign_calls.rs +++ b/crates/nargo/src/ops/foreign_calls.rs @@ -1,5 +1,5 @@ use acvm::{ - acir::brillig::{ForeignCallResult, Value}, + acir::brillig::{ForeignCallOutput, ForeignCallResult, Value}, pwg::ForeignCallWaitInfo, }; use iter_extended::vecmap; @@ -54,13 +54,25 @@ impl ForeignCall { } Some(ForeignCall::Sequence) => { let sequence_length: u128 = foreign_call.inputs[0][0].to_field().to_u128(); - - Ok(vecmap(0..sequence_length, Value::from).into()) + let sequence = vecmap(0..sequence_length, Value::from); + + Ok(ForeignCallResult { + values: vec![ + ForeignCallOutput::Single(sequence_length.into()), + ForeignCallOutput::Array(sequence), + ], + }) } Some(ForeignCall::ReverseSequence) => { let sequence_length: u128 = foreign_call.inputs[0][0].to_field().to_u128(); - - Ok(vecmap((0..sequence_length).rev(), Value::from).into()) + let sequence = vecmap((0..sequence_length).rev(), Value::from); + + Ok(ForeignCallResult { + values: vec![ + ForeignCallOutput::Single(sequence_length.into()), + ForeignCallOutput::Array(sequence), + ], + }) } None => panic!("unexpected foreign call {:?}", foreign_call_name), } diff --git a/crates/nargo_cli/tests/compile_failure/slice_access_failure/Nargo.toml b/crates/nargo_cli/tests/compile_failure/slice_access_failure/Nargo.toml new file mode 100644 index 00000000000..fee471dffd7 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/slice_access_failure/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "slice_access_failure" +type = "bin" +authors = [""] +compiler_version = "0.10.2" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/compile_failure/slice_access_failure/Prover.toml b/crates/nargo_cli/tests/compile_failure/slice_access_failure/Prover.toml new file mode 100644 index 00000000000..f28f2f8cc48 --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/slice_access_failure/Prover.toml @@ -0,0 +1,2 @@ +x = "5" +y = "10" diff --git a/crates/nargo_cli/tests/compile_failure/slice_access_failure/src/main.nr b/crates/nargo_cli/tests/compile_failure/slice_access_failure/src/main.nr new file mode 100644 index 00000000000..dc651cd514d --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/slice_access_failure/src/main.nr @@ -0,0 +1,13 @@ +fn main(x : Field, y : pub 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); + } + // This constraint should fail as the slice length is 3 and the index is 3 + // The right hand side AND case ensures that the circuit inputs have not changed + // and we always hit the else case in the if statement above. + assert((slice[3] == 0) & (slice[2] != y)); +} diff --git a/crates/nargo_cli/tests/execution_success/brillig_slices/src/main.nr b/crates/nargo_cli/tests/execution_success/brillig_slices/src/main.nr index 2d871bc4b2f..4455acc02f8 100644 --- a/crates/nargo_cli/tests/execution_success/brillig_slices/src/main.nr +++ b/crates/nargo_cli/tests/execution_success/brillig_slices/src/main.nr @@ -68,9 +68,85 @@ unconstrained fn main(x: Field, y: Field) { let (empty_slice, should_be_x) = slice.remove(0); assert(should_be_x == x); assert(empty_slice.len() == 0); + + regression_merge_slices(x, y); } // Tests slice passing to/from functions unconstrained fn push_front_to_slice(slice: [T], item: T) -> [T] { slice.push_front(item) } + +// The parameters to this function must come from witness values (inputs to main) +unconstrained fn regression_merge_slices(x: Field, y: Field) { + merge_slices_if(x, y); + merge_slices_else(x); +} + +unconstrained fn merge_slices_if(x: Field, y: Field) { + let slice = merge_slices_return(x, y); + assert(slice[2] == 10); + assert(slice.len() == 3); + + let slice = merge_slices_mutate(x, y); + assert(slice[3] == 5); + assert(slice.len() == 4); + + let slice = merge_slices_mutate_in_loop(x, y); + assert(slice[6] == 4); + assert(slice.len() == 7); +} + +unconstrained fn merge_slices_else(x: Field) { + let slice = merge_slices_return(x, 5); + assert(slice[0] == 0); + assert(slice[1] == 0); + assert(slice.len() == 2); + + let slice = merge_slices_mutate(x, 5); + assert(slice[2] == 5); + assert(slice.len() == 3); + + let slice = merge_slices_mutate_in_loop(x, 5); + assert(slice[2] == 5); + assert(slice.len() == 3); +} + +// Test returning a merged slice without a mutation +unconstrained fn merge_slices_return(x: Field, y: Field) -> [Field] { + let slice = [0; 2]; + if x != y { + if x != 20 { + slice.push_back(y) + } else { + slice + } + } else { + slice + } +} + +// Test mutating a slice inside of an if statement +unconstrained fn merge_slices_mutate(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); + } + slice +} + +// Test mutating a slice inside of a loop in an if statement +unconstrained fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { + let mut slice = [0; 2]; + if x != y { + for i in 0..5 { + slice = slice.push_back(i); + } + } else { + slice = slice.push_back(x); + } + slice +} diff --git a/crates/nargo_cli/tests/execution_success/brillig_to_le_bytes/src/main.nr b/crates/nargo_cli/tests/execution_success/brillig_to_le_bytes/src/main.nr index 8817f9c4c0d..a72b13dcdf5 100644 --- a/crates/nargo_cli/tests/execution_success/brillig_to_le_bytes/src/main.nr +++ b/crates/nargo_cli/tests/execution_success/brillig_to_le_bytes/src/main.nr @@ -4,7 +4,6 @@ unconstrained fn main(x : Field) -> pub [u8; 31] { // The result of this byte array will be little-endian let byte_array = x.to_le_bytes(31); assert(byte_array.len() == 31); - let mut bytes = [0; 31]; for i in 0..31 { bytes[i] = byte_array[i]; diff --git a/crates/nargo_cli/tests/execution_success/slices/src/main.nr b/crates/nargo_cli/tests/execution_success/slices/src/main.nr index ca1c4ac2966..8fbf0a19fc5 100644 --- a/crates/nargo_cli/tests/execution_success/slices/src/main.nr +++ b/crates/nargo_cli/tests/execution_success/slices/src/main.nr @@ -1,9 +1,6 @@ use dep::std::slice; use dep::std; fn main(x : Field, y : pub Field) { - /// TODO(#1889): Using slices in if statements where the condition is a witness - /// is not yet supported - let mut slice = [0; 2]; assert(slice[0] == 0); assert(slice[0] != 1); @@ -49,6 +46,8 @@ fn main(x : Field, y : pub Field) { assert(append[4] == 5); regression_2083(); + // The parameters to this function must come from witness values (inputs to main) + regression_merge_slices(x, y); } // Ensure that slices of struct/tuple values work. @@ -83,3 +82,77 @@ fn regression_2083() { assert(x.0 == 5); assert(x.1 == 6); } + +// 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); +} + +fn merge_slices_if(x: Field, y: Field) { + let slice = merge_slices_return(x, y); + assert(slice[2] == 10); + assert(slice.len() == 3); + + let slice = merge_slices_mutate(x, y); + assert(slice[3] == 5); + assert(slice.len() == 4); + + let slice = merge_slices_mutate_in_loop(x, y); + assert(slice[6] == 4); + assert(slice.len() == 7); +} + +fn merge_slices_else(x: Field) { + let slice = merge_slices_return(x, 5); + assert(slice[0] == 0); + assert(slice[1] == 0); + assert(slice.len() == 2); + + let slice = merge_slices_mutate(x, 5); + assert(slice[2] == 5); + assert(slice.len() == 3); + + let slice = merge_slices_mutate_in_loop(x, 5); + assert(slice[2] == 5); + assert(slice.len() == 3); +} + +// Test returning a merged slice without a mutation +fn merge_slices_return(x: Field, y: Field) -> [Field] { + let slice = [0; 2]; + if x != y { + if x != 20 { + slice.push_back(y) + } else { + slice + } + } else { + slice + } +} + +// Test mutating a slice inside of an if statement +fn merge_slices_mutate(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); + } + slice +} + +// Test mutating a slice inside of a loop in an if statement +fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { + let mut slice = [0; 2]; + if x != y { + for i in 0..5 { + slice = slice.push_back(i); + } + } else { + slice = slice.push_back(x); + } + slice +} diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 99af0b4eed0..38df0374a96 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -261,20 +261,34 @@ impl<'block> BrilligBlock<'block> { let output_registers = vecmap(result_ids, |value_id| { self.allocate_external_call_result(*value_id, dfg) }); - self.brillig_context.foreign_call_instruction( func_name.to_owned(), &input_registers, &output_registers, ); - for output_register in output_registers { + for (i, output_register) in output_registers.iter().enumerate() { if let RegisterOrMemory::HeapVector(HeapVector { size, .. }) = output_register { // Update the stack pointer so that we do not overwrite // dynamic memory returned from other external calls - self.brillig_context.update_stack_pointer(size); + self.brillig_context.update_stack_pointer(*size); + + // Update the dynamic slice length maintained in SSA + if let RegisterOrMemory::RegisterIndex(len_index) = + output_registers[i - 1] + { + let element_size = dfg[result_ids[i]].get_type().element_size(); + self.brillig_context.mov_instruction(len_index, *size); + self.brillig_context.usize_op_in_place( + len_index, + BinaryIntOp::UnsignedDiv, + element_size, + ); + } else { + unreachable!("ICE: a vector must be preceded by a register containing its length"); + } } // Single values and allocation of fixed sized arrays has already been handled // inside of `allocate_external_call_result` @@ -304,7 +318,17 @@ impl<'block> BrilligBlock<'block> { dfg, ); let param_id = arguments[0]; - self.convert_ssa_array_len(param_id, result_register, dfg); + // Slices are represented as a tuple in the form: (length, slice contents). + // Thus, we can expect the first argument to a field in the case of a slice + // or an array in the case of an array. + if let Type::Numeric(_) = dfg.type_of_value(param_id) { + let len_variable = self.convert_ssa_value(arguments[0], dfg); + let len_register_index = + self.function_context.extract_register(len_variable); + self.brillig_context.mov_instruction(result_register, len_register_index); + } else { + self.convert_ssa_array_len(arguments[0], result_register, dfg); + } } Value::Intrinsic( Intrinsic::SlicePushBack @@ -325,12 +349,27 @@ impl<'block> BrilligBlock<'block> { let source = self.convert_ssa_register_value(arguments[0], dfg); let radix = self.convert_ssa_register_value(arguments[1], dfg); let limb_count = self.convert_ssa_register_value(arguments[2], dfg); + + let results = dfg.instruction_results(instruction_id); + + let target_len_variable = self.function_context.get_or_create_variable( + self.brillig_context, + results[0], + dfg, + ); + let target_len = self.function_context.extract_register(target_len_variable); + let target_slice = self.function_context.create_variable( self.brillig_context, - dfg.instruction_results(instruction_id)[0], + results[1], dfg, ); + let heap_vec = self.brillig_context.extract_heap_vector(target_slice); + + // Update the user-facing slice length + self.brillig_context.mov_instruction(target_len, limb_count); + self.brillig_context.radix_instruction( source, heap_vec, @@ -342,14 +381,28 @@ impl<'block> BrilligBlock<'block> { Value::Intrinsic(Intrinsic::ToBits(endianness)) => { let source = self.convert_ssa_register_value(arguments[0], dfg); let limb_count = self.convert_ssa_register_value(arguments[1], dfg); + + let results = dfg.instruction_results(instruction_id); + + let target_len_variable = self.function_context.get_or_create_variable( + self.brillig_context, + results[0], + dfg, + ); + let target_len = self.function_context.extract_register(target_len_variable); + let target_slice = self.function_context.create_variable( self.brillig_context, - dfg.instruction_results(instruction_id)[0], + results[1], dfg, ); let radix = self.brillig_context.make_constant(2_usize.into()); let heap_vec = self.brillig_context.extract_heap_vector(target_slice); + + // Update the user-facing slice length + self.brillig_context.mov_instruction(target_len, limb_count); + self.brillig_context.radix_instruction( source, heap_vec, @@ -587,52 +640,88 @@ impl<'block> BrilligBlock<'block> { instruction_id: InstructionId, arguments: &[ValueId], ) { - let slice_id = arguments[0]; + let slice_id = arguments[1]; let element_size = dfg.type_of_value(slice_id).element_size(); let source_variable = self.convert_ssa_value(slice_id, dfg); let source_vector = self.convert_array_or_vector_to_vector(source_variable); + let results = dfg.instruction_results(instruction_id); match intrinsic { Value::Intrinsic(Intrinsic::SlicePushBack) => { - let target_variable = self.function_context.create_variable( + let target_len = match self.function_context.get_or_create_variable( self.brillig_context, - dfg.instruction_results(instruction_id)[0], + results[0], dfg, - ); + ) { + RegisterOrMemory::RegisterIndex(register_index) => register_index, + _ => unreachable!("ICE: first value of a slice must be a register index"), + }; + + let target_variable = + self.function_context.create_variable(self.brillig_context, results[1], dfg); + let target_vector = self.brillig_context.extract_heap_vector(target_variable); - let item_values = vecmap(&arguments[1..element_size + 1], |arg| { + let item_values = vecmap(&arguments[2..element_size + 2], |arg| { self.convert_ssa_value(*arg, dfg) }); + + self.update_slice_length(target_len, arguments[0], dfg, BinaryIntOp::Add); + self.slice_push_back_operation(target_vector, source_vector, &item_values); } Value::Intrinsic(Intrinsic::SlicePushFront) => { - let target_variable = self.function_context.create_variable( + let target_len = match self.function_context.get_or_create_variable( self.brillig_context, - dfg.instruction_results(instruction_id)[0], + results[0], dfg, - ); + ) { + RegisterOrMemory::RegisterIndex(register_index) => register_index, + _ => unreachable!("ICE: first value of a slice must be a register index"), + }; + + let target_variable = + self.function_context.create_variable(self.brillig_context, results[1], dfg); let target_vector = self.brillig_context.extract_heap_vector(target_variable); - let item_values = vecmap(&arguments[1..element_size + 1], |arg| { + let item_values = vecmap(&arguments[2..element_size + 2], |arg| { self.convert_ssa_value(*arg, dfg) }); + + self.update_slice_length(target_len, arguments[0], dfg, BinaryIntOp::Add); + self.slice_push_front_operation(target_vector, source_vector, &item_values); } Value::Intrinsic(Intrinsic::SlicePopBack) => { - let results = dfg.instruction_results(instruction_id); + let target_len = match self.function_context.get_or_create_variable( + self.brillig_context, + results[0], + dfg, + ) { + RegisterOrMemory::RegisterIndex(register_index) => register_index, + _ => unreachable!("ICE: first value of a slice must be a register index"), + }; let target_variable = - self.function_context.create_variable(self.brillig_context, results[0], dfg); + self.function_context.create_variable(self.brillig_context, results[1], dfg); let target_vector = self.brillig_context.extract_heap_vector(target_variable); - let pop_variables = vecmap(&results[1..element_size + 1], |result| { + let pop_variables = vecmap(&results[2..element_size + 2], |result| { self.function_context.create_variable(self.brillig_context, *result, dfg) }); + self.update_slice_length(target_len, arguments[0], dfg, BinaryIntOp::Sub); + self.slice_pop_back_operation(target_vector, source_vector, &pop_variables); } Value::Intrinsic(Intrinsic::SlicePopFront) => { - let results = dfg.instruction_results(instruction_id); + let target_len = match self.function_context.get_or_create_variable( + self.brillig_context, + results[element_size], + dfg, + ) { + RegisterOrMemory::RegisterIndex(register_index) => register_index, + _ => unreachable!("ICE: first value of a slice must be a register index"), + }; let pop_variables = vecmap(&results[0..element_size], |result| { self.function_context.create_variable(self.brillig_context, *result, dfg) @@ -640,16 +729,26 @@ impl<'block> BrilligBlock<'block> { let target_variable = self.function_context.create_variable( self.brillig_context, - results[element_size], + results[element_size + 1], dfg, ); let target_vector = self.brillig_context.extract_heap_vector(target_variable); + self.update_slice_length(target_len, arguments[0], dfg, BinaryIntOp::Sub); + self.slice_pop_front_operation(target_vector, source_vector, &pop_variables); } Value::Intrinsic(Intrinsic::SliceInsert) => { - let results = dfg.instruction_results(instruction_id); - let target_id = results[0]; + let target_len = match self.function_context.get_or_create_variable( + self.brillig_context, + results[0], + dfg, + ) { + RegisterOrMemory::RegisterIndex(register_index) => register_index, + _ => unreachable!("ICE: first value of a slice must be a register index"), + }; + + let target_id = results[1]; let target_variable = self.function_context.create_variable(self.brillig_context, target_id, dfg); @@ -657,7 +756,7 @@ impl<'block> BrilligBlock<'block> { // Remove if indexing in insert is changed to flattened indexing // https://github.com/noir-lang/noir/issues/1889#issuecomment-1668048587 - let user_index = self.convert_ssa_register_value(arguments[1], dfg); + let user_index = self.convert_ssa_register_value(arguments[2], dfg); let converted_index = self.brillig_context.make_constant(element_size.into()); @@ -668,16 +767,26 @@ impl<'block> BrilligBlock<'block> { BinaryIntOp::Mul, ); - let items = vecmap(&arguments[2..element_size + 2], |arg| { + let items = vecmap(&arguments[3..element_size + 3], |arg| { self.convert_ssa_value(*arg, dfg) }); + self.update_slice_length(target_len, arguments[0], dfg, BinaryIntOp::Add); + self.slice_insert_operation(target_vector, source_vector, converted_index, &items); self.brillig_context.deallocate_register(converted_index); } Value::Intrinsic(Intrinsic::SliceRemove) => { - let results = dfg.instruction_results(instruction_id); - let target_id = results[0]; + let target_len = match self.function_context.get_or_create_variable( + self.brillig_context, + results[0], + dfg, + ) { + RegisterOrMemory::RegisterIndex(register_index) => register_index, + _ => unreachable!("ICE: first value of a slice must be a register index"), + }; + + let target_id = results[1]; let target_variable = self.function_context.create_variable(self.brillig_context, target_id, dfg); @@ -685,7 +794,7 @@ impl<'block> BrilligBlock<'block> { // Remove if indexing in remove is changed to flattened indexing // https://github.com/noir-lang/noir/issues/1889#issuecomment-1668048587 - let user_index = self.convert_ssa_register_value(arguments[1], dfg); + let user_index = self.convert_ssa_register_value(arguments[2], dfg); let converted_index = self.brillig_context.make_constant(element_size.into()); self.brillig_context.memory_op( @@ -695,10 +804,12 @@ impl<'block> BrilligBlock<'block> { BinaryIntOp::Mul, ); - let removed_items = vecmap(&results[1..element_size + 1], |result| { + let removed_items = vecmap(&results[2..element_size + 2], |result| { self.function_context.create_variable(self.brillig_context, *result, dfg) }); + self.update_slice_length(target_len, arguments[0], dfg, BinaryIntOp::Sub); + self.slice_remove_operation( target_vector, source_vector, @@ -712,6 +823,29 @@ impl<'block> BrilligBlock<'block> { } } + /// Slices have a tuple structure (slice length, slice contents) to enable logic + /// that uses dynamic slice lengths (such as with merging slices in the flattening pass). + /// This method codegens an update to the slice length. + /// + /// The binary operation performed on the slice length is always an addition or subtraction of `1`. + /// This is because the slice length holds the user length (length as displayed by a `.len()` call), + /// and not a flattened length used internally to represent arrays of tuples. + /// The length inside of `RegisterOrMemory::HeapVector` represents the entire flattened number + /// of fields in the vector. + fn update_slice_length( + &mut self, + target_len: RegisterIndex, + source_value: ValueId, + dfg: &DataFlowGraph, + binary_op: BinaryIntOp, + ) { + let source_len_variable = + self.function_context.get_or_create_variable(self.brillig_context, source_value, dfg); + let source_len = self.function_context.extract_register(source_len_variable); + + self.brillig_context.usize_op(source_len, target_len, binary_op, 1); + } + /// Converts an SSA cast to a sequence of Brillig opcodes. /// Casting is only necessary when shrinking the bit size of a numeric value. fn convert_cast( @@ -897,7 +1031,7 @@ impl<'block> BrilligBlock<'block> { let vector = self.brillig_context.extract_heap_vector(variable); // Set the pointer to the current stack frame - // The stack pointer will then be update by the caller of this method + // The stack pointer will then be updated by the caller of this method // once the external call is resolved and the array size is known self.brillig_context.set_array_pointer(vector.pointer); variable diff --git a/crates/noirc_evaluator/src/ssa.rs b/crates/noirc_evaluator/src/ssa.rs index 557df2a727a..46ee26fc839 100644 --- a/crates/noirc_evaluator/src/ssa.rs +++ b/crates/noirc_evaluator/src/ssa.rs @@ -58,7 +58,9 @@ pub(crate) fn optimize_into_acir( .simplify_cfg() .print(print_ssa_passes, "After Simplifying:") // Run mem2reg before flattening to handle any promotion - // of values that can be accessed after loop unrolling + // of values that can be accessed after loop unrolling. + // If there are slice mergers uncovered by loop unrolling + // and this pass is missed, slice merging will fail inside of flattening. .mem2reg() .print(print_ssa_passes, "After Mem2Reg:") .flatten_cfg() diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 5d900a4598a..d682c30d5ab 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -780,7 +780,15 @@ impl AcirContext { limb_vars.reverse(); } - Ok(vec![AcirValue::Array(limb_vars.into())]) + // `Intrinsic::ToRadix` returns slices which are represented + // by tuples with the structure (length, slice contents) + Ok(vec![ + AcirValue::Var( + self.add_constant(FieldElement::from(limb_vars.len() as u128)), + AcirType::field(), + ), + AcirValue::Array(limb_vars.into()), + ]) } /// Returns `AcirVar`s constrained to be the bit decomposition of the provided input diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs index 96ff3acc426..3124cfc885e 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -940,18 +940,27 @@ impl Context { dfg: &DataFlowGraph, ) -> Result { let mut var = self.convert_numeric_value(value_id, dfg)?; - let truncation_target = match &dfg[value_id] { - Value::Instruction { instruction, .. } => &dfg[*instruction], - _ => unreachable!("ICE: Truncates are only ever applied to the result of a binary op"), + match &dfg[value_id] { + Value::Instruction { instruction, .. } => { + if matches!( + &dfg[*instruction], + Instruction::Binary(Binary { operator: BinaryOp::Sub, .. }) + ) { + // Subtractions must first have the integer modulus added before truncation can be + // applied. This is done in order to prevent underflow. + let integer_modulus = + self.acir_context.add_constant(FieldElement::from(2_u128.pow(bit_size))); + var = self.acir_context.add_var(var, integer_modulus)?; + } + } + Value::Param { .. } => { + // Binary operations on params may have been entirely simplified if the operation + // results in the identity of the parameter + } + _ => unreachable!( + "ICE: Truncates are only ever applied to the result of a binary op or a param" + ), }; - if matches!(truncation_target, Instruction::Binary(Binary { operator: BinaryOp::Sub, .. })) - { - // Subtractions must first have the integer modulus added before truncation can be - // applied. This is done in order to prevent underflow. - let integer_modulus = - self.acir_context.add_constant(FieldElement::from(2_u128.pow(bit_size))); - var = self.acir_context.add_var(var, integer_modulus)?; - } self.acir_context.truncate_var(var, bit_size, max_bit_size) } @@ -982,14 +991,16 @@ impl Context { let field = self.convert_value(arguments[0], dfg).into_var()?; let radix = self.convert_value(arguments[1], dfg).into_var()?; let limb_size = self.convert_value(arguments[2], dfg).into_var()?; - let result_type = Self::array_element_type(dfg, result_ids[0]); + + let result_type = Self::array_element_type(dfg, result_ids[1]); self.acir_context.radix_decompose(endian, field, radix, limb_size, result_type) } Intrinsic::ToBits(endian) => { let field = self.convert_value(arguments[0], dfg).into_var()?; let bit_size = self.convert_value(arguments[1], dfg).into_var()?; - let result_type = Self::array_element_type(dfg, result_ids[0]); + + let result_type = Self::array_element_type(dfg, result_ids[1]); self.acir_context.bit_decompose(endian, field, bit_size, result_type) } diff --git a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs index e73ff02ba08..e6b6f19026a 100644 --- a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -12,7 +12,7 @@ use crate::ssa::ir::{ value::{Value, ValueId}, }; -use super::{Endian, SimplifyResult}; +use super::{Binary, BinaryOp, Endian, Instruction, SimplifyResult}; /// Try to simplify this call instruction. If the instruction can be simplified to a known value, /// that value is returned. Otherwise None is returned. @@ -34,7 +34,18 @@ pub(super) fn simplify_call( if let Some(constant_args) = constant_args { let field = constant_args[0]; let limb_count = constant_args[1].to_u128() as u32; - SimplifyResult::SimplifiedTo(constant_to_radix(endian, field, 2, limb_count, dfg)) + + let result_slice = constant_to_radix(endian, field, 2, limb_count, dfg); + + let length = dfg + .try_get_array_length(result_slice) + .expect("ICE: a constant array should have an associated length"); + let len_value = + dfg.make_constant(FieldElement::from(length as u128), Type::field()); + + // `Intrinsic::ToBits` returns slices which are represented + // by tuples with the structure (length, slice contents) + SimplifyResult::SimplifiedToMultiple(vec![len_value, result_slice]) } else { SimplifyResult::None } @@ -44,51 +55,64 @@ pub(super) fn simplify_call( let field = constant_args[0]; let radix = constant_args[1].to_u128() as u32; let limb_count = constant_args[2].to_u128() as u32; - SimplifyResult::SimplifiedTo(constant_to_radix( - endian, field, radix, limb_count, dfg, - )) + + let result_slice = constant_to_radix(endian, field, radix, limb_count, dfg); + + let length = dfg + .try_get_array_length(result_slice) + .expect("ICE: a constant array should have an associated length"); + let len_value = + dfg.make_constant(FieldElement::from(length as u128), Type::field()); + + // `Intrinsic::ToRadix` returns slices which are represented + // by tuples with the structure (length, slice contents) + SimplifyResult::SimplifiedToMultiple(vec![len_value, result_slice]) } else { SimplifyResult::None } } Intrinsic::ArrayLen => { - let slice = dfg.get_array_constant(arguments[0]); - 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]) { + if let Some(length) = dfg.try_get_array_length(arguments[0]) { let length = FieldElement::from(length as u128); SimplifyResult::SimplifiedTo(dfg.make_constant(length, Type::field())) + } else if matches!(dfg.type_of_value(arguments[1]), Type::Slice(_)) { + SimplifyResult::SimplifiedTo(arguments[0]) } else { SimplifyResult::None } } Intrinsic::SlicePushBack => { - let slice = dfg.get_array_constant(arguments[0]); + let slice = dfg.get_array_constant(arguments[1]); if let Some((mut slice, element_type)) = slice { - for elem in &arguments[1..] { + for elem in &arguments[2..] { slice.push_back(*elem); } + + let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Add); + let new_slice = dfg.make_array(slice, element_type); - SimplifyResult::SimplifiedTo(new_slice) + SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]) } else { SimplifyResult::None } } Intrinsic::SlicePushFront => { - let slice = dfg.get_array_constant(arguments[0]); + let slice = dfg.get_array_constant(arguments[1]); if let Some((mut slice, element_type)) = slice { - for elem in arguments[1..].iter().rev() { + for elem in arguments[2..].iter().rev() { slice.push_front(*elem); } + + let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Add); + let new_slice = dfg.make_array(slice, element_type); - SimplifyResult::SimplifiedTo(new_slice) + SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]) } else { SimplifyResult::None } } Intrinsic::SlicePopBack => { - let slice = dfg.get_array_constant(arguments[0]); + let slice = dfg.get_array_constant(arguments[1]); if let Some((mut slice, typ)) = slice { let element_count = typ.element_size(); let mut results = VecDeque::with_capacity(element_count + 1); @@ -104,13 +128,16 @@ pub(super) fn simplify_call( let new_slice = dfg.make_array(slice, typ); results.push_front(new_slice); + let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Sub); + + results.push_front(new_slice_length); SimplifyResult::SimplifiedToMultiple(results.into()) } else { SimplifyResult::None } } Intrinsic::SlicePopFront => { - let slice = dfg.get_array_constant(arguments[0]); + let slice = dfg.get_array_constant(arguments[1]); if let Some((mut slice, typ)) = slice { let element_count = typ.element_size(); @@ -119,6 +146,10 @@ pub(super) fn simplify_call( slice.pop_front().expect("There are no elements in this slice to be removed") }); + let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Sub); + + results.push(new_slice_length); + let new_slice = dfg.make_array(slice, typ); // The slice is the last item returned for pop_front @@ -129,26 +160,28 @@ pub(super) fn simplify_call( } } Intrinsic::SliceInsert => { - let slice = dfg.get_array_constant(arguments[0]); - let index = dfg.get_numeric_constant(arguments[1]); + let slice = dfg.get_array_constant(arguments[1]); + let index = dfg.get_numeric_constant(arguments[2]); if let (Some((mut slice, typ)), Some(index)) = (slice, index) { - let elements = &arguments[2..]; + let elements = &arguments[3..]; let mut index = index.to_u128() as usize * elements.len(); - for elem in &arguments[2..] { + for elem in &arguments[3..] { slice.insert(index, *elem); index += 1; } + let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Add); + let new_slice = dfg.make_array(slice, typ); - SimplifyResult::SimplifiedTo(new_slice) + SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]) } else { SimplifyResult::None } } Intrinsic::SliceRemove => { - let slice = dfg.get_array_constant(arguments[0]); - let index = dfg.get_numeric_constant(arguments[1]); + let slice = dfg.get_array_constant(arguments[1]); + let index = dfg.get_numeric_constant(arguments[2]); 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); @@ -160,6 +193,11 @@ pub(super) fn simplify_call( let new_slice = dfg.make_array(slice, typ); results.insert(0, new_slice); + + let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Sub); + + results.insert(0, new_slice_length); + SimplifyResult::SimplifiedToMultiple(results) } else { SimplifyResult::None @@ -182,6 +220,21 @@ pub(super) fn simplify_call( } } +/// Slices have a tuple structure (slice length, slice contents) to enable logic +/// that uses dynamic slice lengths (such as with merging slices in the flattening pass). +/// This method codegens an update to the slice length. +/// +/// The binary operation performed on the slice length is always an addition or subtraction of `1`. +/// This is because the slice length holds the user length (length as displayed by a `.len()` call), +/// and not a flattened length used internally to represent arrays of tuples. +fn update_slice_length(slice_len: ValueId, dfg: &mut DataFlowGraph, operator: BinaryOp) -> ValueId { + let one = dfg.make_constant(FieldElement::one(), Type::field()); + let block = dfg.make_block(); + let instruction = Instruction::Binary(Binary { lhs: slice_len, operator, rhs: one }); + let call_stack = dfg.get_value_call_stack(slice_len); + dfg.insert_instruction_and_results(instruction, block, None, call_stack).first() +} + /// Try to simplify this black box call. If the call can be simplified to a known value, /// that value is returned. Otherwise [`SimplifyResult::None`] is returned. fn simplify_black_box_func( diff --git a/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 3df27868737..1640afabe26 100644 --- a/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -145,7 +145,7 @@ use crate::ssa::{ function_inserter::FunctionInserter, instruction::{BinaryOp, Instruction, InstructionId, TerminatorInstruction}, types::Type, - value::ValueId, + value::{Value, ValueId}, }, ssa_gen::Ssa, }; @@ -391,13 +391,78 @@ impl<'f> Context<'f> { typ @ Type::Array(_, _) => { self.merge_array_values(typ, then_condition, else_condition, then_value, else_value) } - // TODO(#1889) - Type::Slice(_) => panic!("Cannot return slices from an if expression"), + typ @ Type::Slice(_) => { + self.merge_slice_values(typ, then_condition, else_condition, then_value, else_value) + } Type::Reference => panic!("Cannot return references from an if expression"), Type::Function => panic!("Cannot return functions from an if expression"), } } + fn merge_slice_values( + &mut self, + typ: Type, + then_condition: ValueId, + else_condition: ValueId, + then_value_id: ValueId, + else_value_id: ValueId, + ) -> ValueId { + let mut merged = im::Vector::new(); + + let element_types = match &typ { + Type::Slice(elements) => elements, + _ => panic!("Expected slice type"), + }; + + let then_value = self.inserter.function.dfg[then_value_id].clone(); + let else_value = self.inserter.function.dfg[else_value_id].clone(); + + let len = match then_value { + Value::Array { array, .. } => array.len(), + _ => panic!("Expected array value"), + }; + + let else_len = match else_value { + Value::Array { array, .. } => array.len(), + _ => panic!("Expected array value"), + }; + + let len = len.max(else_len); + + for i in 0..len { + for (element_index, element_type) in element_types.iter().enumerate() { + let index_value = ((i * element_types.len() + element_index) as u128).into(); + let index = self.inserter.function.dfg.make_constant(index_value, Type::field()); + + let typevars = Some(vec![element_type.clone()]); + + let mut get_element = |array, typevars, len| { + // The smaller slice is filled with placeholder data. Codegen for slice accesses must + // include checks against the dynamic slice length so that this placeholder data is not incorrectly accessed. + if (len - 1) < index_value.to_u128() as usize { + let zero = FieldElement::zero(); + self.inserter.function.dfg.make_constant(zero, Type::field()) + } else { + let get = Instruction::ArrayGet { array, index }; + self.insert_instruction_with_typevars(get, typevars).first() + } + }; + + let then_element = get_element(then_value_id, typevars.clone(), len); + let else_element = get_element(else_value_id, typevars, else_len); + + merged.push_back(self.merge_values( + then_condition, + else_condition, + then_element, + else_element, + )); + } + } + + self.inserter.function.dfg.make_array(merged, typ) + } + /// Given an if expression that returns an array: `if c { array1 } else { array2 }`, /// this function will recursively merge array1 and array2 into a single resulting array /// by creating a new array containing the result of self.merge_values for each element. diff --git a/crates/noirc_evaluator/src/ssa/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa/ssa_builder/mod.rs index 0958b0d7a67..6a5b24e3e9f 100644 --- a/crates/noirc_evaluator/src/ssa/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa/ssa_builder/mod.rs @@ -397,15 +397,22 @@ mod tests { let input = builder.numeric_constant(FieldElement::from(7_u128), Type::field()); let length = builder.numeric_constant(FieldElement::from(8_u128), Type::field()); let result_types = vec![Type::Array(Rc::new(vec![Type::bool()]), 8)]; - let call_result = builder.insert_call(to_bits_id, vec![input, length], result_types)[0]; + let call_results = + builder.insert_call(to_bits_id, vec![input, length], result_types).into_owned(); - let array = match &builder.current_function.dfg[call_result] { + let slice_len = match &builder.current_function.dfg[call_results[0]] { + Value::NumericConstant { constant, .. } => *constant, + _ => panic!(), + }; + assert_eq!(slice_len, FieldElement::from(256u128)); + + let slice = match &builder.current_function.dfg[call_results[1]] { Value::Array { array, .. } => array, _ => panic!(), }; - assert_eq!(array[0], one); - assert_eq!(array[1], one); - assert_eq!(array[2], one); - assert_eq!(array[3], zero); + assert_eq!(slice[0], one); + assert_eq!(slice[1], one); + assert_eq!(slice[2], one); + assert_eq!(slice[3], zero); } } diff --git a/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs b/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs index bcba7bf5992..b04e4263f07 100644 --- a/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -186,6 +186,13 @@ impl<'a> FunctionContext<'a> { let fmt_str_tuple = ast::Type::Tuple(final_fmt_str_fields); Self::map_type_helper(&fmt_str_tuple, f) } + ast::Type::Slice(elements) => { + let element_types = Self::convert_type(elements).flatten(); + Tree::Branch(vec![ + Tree::Leaf(f(Type::field())), + Tree::Leaf(f(Type::Slice(Rc::new(element_types)))), + ]) + } other => Tree::Leaf(f(Self::convert_non_tuple_type(other))), } } @@ -219,10 +226,7 @@ impl<'a> FunctionContext<'a> { ast::Type::Unit => panic!("convert_non_tuple_type called on a unit type"), ast::Type::Tuple(_) => panic!("convert_non_tuple_type called on a tuple: {typ}"), ast::Type::Function(_, _, _) => Type::Function, - ast::Type::Slice(element) => { - let element_types = Self::convert_type(element).flatten(); - Type::Slice(Rc::new(element_types)) - } + ast::Type::Slice(_) => panic!("convert_non_tuple_type called on a slice: {typ}"), ast::Type::MutableReference(element) => { // Recursive call to panic if element is a tuple Self::convert_non_tuple_type(element); @@ -256,8 +260,10 @@ impl<'a> FunctionContext<'a> { if let Type::Numeric(NumericType::Unsigned { bit_size }) = typ { let to_bits = self.builder.import_intrinsic_id(Intrinsic::ToBits(Endian::Little)); let length = self.builder.field_constant(FieldElement::from(bit_size as i128)); - let result_types = vec![Type::Array(Rc::new(vec![Type::bool()]), bit_size as usize)]; - let rhs_bits = self.builder.insert_call(to_bits, vec![rhs, length], result_types)[0]; + let result_types = + vec![Type::field(), Type::Array(Rc::new(vec![Type::bool()]), bit_size as usize)]; + let rhs_bits = self.builder.insert_call(to_bits, vec![rhs, length], result_types); + let rhs_bits = rhs_bits[1]; let one = self.builder.field_constant(FieldElement::one()); let mut r = one; for i in 1..bit_size + 1 { @@ -576,18 +582,30 @@ impl<'a> FunctionContext<'a> { } /// Compile the given `array[index]` expression as a reference. - /// This will return a triple of (array, index, lvalue_ref) where the lvalue_ref records the + /// This will return a triple of (array, index, lvalue_ref, Option) where the lvalue_ref records the /// structure of the lvalue expression for use by `assign_new_value`. + /// The optional length is for indexing slices rather than arrays since slices + /// are represented as a tuple in the form: (length, slice contents). fn index_lvalue( &mut self, array: &ast::LValue, index: &ast::Expression, - ) -> (ValueId, ValueId, LValue) { + ) -> (ValueId, ValueId, LValue, Option) { let (old_array, array_lvalue) = self.extract_current_value_recursive(array); - let old_array = old_array.into_leaf().eval(self); - let array_lvalue = Box::new(array_lvalue); let index = self.codegen_non_tuple_expression(index); - (old_array, index, LValue::Index { old_array, index, array_lvalue }) + let array_lvalue = Box::new(array_lvalue); + let array_values = old_array.clone().into_value_list(self); + + // A slice is represented as a tuple (length, slice contents). + // We need to fetch the second value. + if array_values.len() > 1 { + let slice_lvalue = + LValue::SliceIndex { old_slice: old_array, index, slice_lvalue: array_lvalue }; + (array_values[1], index, slice_lvalue, Some(array_values[0])) + } else { + let array_lvalue = LValue::Index { old_array: array_values[0], index, array_lvalue }; + (array_values[0], index, array_lvalue, None) + } } fn extract_current_value_recursive(&mut self, lvalue: &ast::LValue) -> (Values, LValue) { @@ -602,8 +620,9 @@ impl<'a> FunctionContext<'a> { } } ast::LValue::Index { array, index, element_type, location } => { - let (old_array, index, index_lvalue) = self.index_lvalue(array, index); - let element = self.codegen_array_index(old_array, index, element_type, *location); + let (old_array, index, index_lvalue, max_length) = self.index_lvalue(array, index); + let element = + self.codegen_array_index(old_array, index, element_type, *location, max_length); (element, index_lvalue) } ast::LValue::MemberAccess { object, field_index: index } => { @@ -629,19 +648,17 @@ impl<'a> FunctionContext<'a> { match lvalue { LValue::Ident => unreachable!("Cannot assign to a variable without a reference"), LValue::Index { old_array: mut array, index, array_lvalue } => { - let element_size = self.builder.field_constant(self.element_size(array)); - - // The actual base index is the user's index * the array element type's size - let mut index = self.builder.insert_binary(index, BinaryOp::Mul, element_size); - let one = self.builder.field_constant(FieldElement::one()); + array = self.assign_lvalue_index(new_value, array, index); + self.assign_new_value(*array_lvalue, array.into()); + } + LValue::SliceIndex { old_slice: slice, index, slice_lvalue } => { + let mut slice_values = slice.into_value_list(self); - new_value.for_each(|value| { - let value = value.eval(self); - array = self.builder.insert_array_set(array, index, value); - index = self.builder.insert_binary(index, BinaryOp::Add, one); - }); + slice_values[1] = self.assign_lvalue_index(new_value, slice_values[1], index); - self.assign_new_value(*array_lvalue, array.into()); + // The size of the slice does not change in a slice index assignment so we can reuse the same length value + let new_slice = Tree::Branch(vec![slice_values[0].into(), slice_values[1].into()]); + self.assign_new_value(*slice_lvalue, new_slice); } LValue::MemberAccess { old_object, index, object_lvalue } => { let new_object = Self::replace_field(old_object, index, new_value); @@ -653,6 +670,26 @@ impl<'a> FunctionContext<'a> { } } + fn assign_lvalue_index( + &mut self, + new_value: Values, + mut array: ValueId, + index: ValueId, + ) -> ValueId { + let element_size = self.builder.field_constant(self.element_size(array)); + + // The actual base index is the user's index * the array element type's size + let mut index = self.builder.insert_binary(index, BinaryOp::Mul, element_size); + let one = self.builder.field_constant(FieldElement::one()); + + new_value.for_each(|value| { + let value = value.eval(self); + array = self.builder.insert_array_set(array, index, value); + index = self.builder.insert_binary(index, BinaryOp::Add, one); + }); + array + } + fn element_size(&self, array: ValueId) -> FieldElement { let size = self.builder.type_of_value(array).element_size(); FieldElement::from(size as u128) @@ -826,6 +863,7 @@ impl SharedContext { pub(super) enum LValue { Ident, Index { old_array: ValueId, index: ValueId, array_lvalue: Box }, + SliceIndex { old_slice: Values, index: ValueId, slice_lvalue: Box }, MemberAccess { old_object: Values, index: usize, object_lvalue: Box }, Dereference { reference: Values }, } diff --git a/crates/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/crates/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 790c99fee05..cc3a7c02a75 100644 --- a/crates/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -9,6 +9,8 @@ use iter_extended::vecmap; use noirc_errors::Location; use noirc_frontend::monomorphization::ast::{self, Expression, Program}; +use crate::ssa::ir::types::NumericType; + use self::{ context::FunctionContext, value::{Tree, Values}, @@ -118,8 +120,18 @@ impl<'a> FunctionContext<'a> { match literal { ast::Literal::Array(array) => { let elements = vecmap(&array.contents, |element| self.codegen_expression(element)); - let typ = Self::convert_non_tuple_type(&array.typ); - self.codegen_array(elements, typ) + + let typ = Self::convert_type(&array.typ).flatten(); + match array.typ { + ast::Type::Array(_, _) => self.codegen_array(elements, typ[0].clone()), + ast::Type::Slice(_) => { + let slice_length = + self.builder.field_constant(array.contents.len() as u128); + let slice_contents = self.codegen_array(elements, typ[1].clone()); + Tree::Branch(vec![slice_length.into(), slice_contents]) + } + _ => unreachable!("ICE: array literal type must be an array or a slice, but got {}", array.typ), + } } ast::Literal::Integer(value, typ) => { let typ = Self::convert_non_tuple_type(typ); @@ -241,9 +253,22 @@ impl<'a> FunctionContext<'a> { } fn codegen_index(&mut self, index: &ast::Index) -> Values { - let array = self.codegen_non_tuple_expression(&index.collection); + let array_or_slice = self.codegen_expression(&index.collection).into_value_list(self); let index_value = self.codegen_non_tuple_expression(&index.index); - self.codegen_array_index(array, index_value, &index.element_type, index.location) + // Slices are represented as a tuple in the form: (length, slice contents). + // Thus, slices require two value ids for their representation. + let (array, slice_length) = if array_or_slice.len() > 1 { + (array_or_slice[1], Some(array_or_slice[0])) + } else { + (array_or_slice[0], None) + }; + self.codegen_array_index( + array, + index_value, + &index.element_type, + index.location, + slice_length, + ) } /// This is broken off from codegen_index so that it can also be @@ -258,6 +283,7 @@ impl<'a> FunctionContext<'a> { index: super::ir::value::ValueId, element_type: &ast::Type, location: Location, + length: Option, ) -> Values { // base_index = index * type_size let type_size = Self::convert_type(element_type).size_of_type(); @@ -269,10 +295,48 @@ impl<'a> FunctionContext<'a> { Self::map_type(element_type, |typ| { let offset = self.make_offset(base_index, field_index); field_index += 1; + + let array_type = &self.builder.type_of_value(array); + match array_type { + Type::Slice(_) => { + self.codegen_slice_access_check(index, length); + } + Type::Array(..) => { + // Nothing needs to done to prepare an array access on an array + } + _ => unreachable!("must have array or slice but got {array_type}"), + } self.builder.insert_array_get(array, offset, typ).into() }) } + /// Prepare a slice access. + /// Check that the index being used to access a slice element + /// is less than the dynamic slice length. + fn codegen_slice_access_check( + &mut self, + index: super::ir::value::ValueId, + length: Option, + ) { + let array_len = length.expect("ICE: a length must be supplied for indexing slices"); + // Check the type of the index value for valid comparisons + let array_len = match self.builder.type_of_value(index) { + Type::Numeric(numeric_type) => match numeric_type { + // If the index itself is an integer, keep the array length as a Field + NumericType::Unsigned { .. } | NumericType::Signed { .. } => array_len, + // If the index and the array length are both Fields we will not be able to perform a less than comparison on them. + // Thus, we cast the array length to a u64 before performing the less than comparison + NumericType::NativeField => self + .builder + .insert_cast(array_len, Type::Numeric(NumericType::Unsigned { bit_size: 64 })), + }, + _ => unreachable!("ICE: array index must be a numeric type"), + }; + + let is_offset_out_of_bounds = self.builder.insert_binary(index, BinaryOp::Lt, array_len); + self.builder.insert_constrain(is_offset_out_of_bounds); + } + fn codegen_cast(&mut self, cast: &ast::Cast) -> Values { let lhs = self.codegen_non_tuple_expression(&cast.lhs); let typ = Self::convert_non_tuple_type(&cast.r#type);