diff --git a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs index f41006a7c85..0c46d5ab394 100644 --- a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs +++ b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs @@ -1575,9 +1575,6 @@ impl AcirContext { } Some(optional_value) => { let mut values = Vec::new(); - if let AcirValue::DynamicArray(_) = optional_value { - unreachable!("Dynamic array should already be initialized"); - } self.initialize_array_inner(&mut values, optional_value)?; values } diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index f47fbaa3465..d4dde7d1ccb 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -169,6 +169,11 @@ impl Context<'_> { Ok(()) } + /// Attempts a compile-time read/write from an array. + /// + /// This relies on all previous operations on this array being done at known indices so that the `AcirValue` at each + /// position is known (even if the value of this `AcirValue` is unknown). This can then be done only for + /// `AcirValue::Array` as an `AcirValue::DynamicArray` has been mutated at an unknown index. fn handle_constant_index_wrapper( &mut self, instruction: InstructionId, @@ -194,12 +199,16 @@ impl Context<'_> { Ok(false) } } - AcirValue::DynamicArray(_) => Ok(false), + AcirValue::DynamicArray(_) => { + // We do not perform any compile-time reads/writes to dynamic arrays as we'd need to promote this into + // a regular array by reading all of its elements. It's then better to defer to the dynamic index + // codepath so we just issue a single read/write. + Ok(false) + } } } - /// Handle constant index: if there is no predicate and we have the array values, - /// we can perform the operation directly on the array + /// See [Self::handle_constant_index_wrapper] fn handle_constant_index( &mut self, instruction: InstructionId, @@ -243,13 +252,6 @@ impl Context<'_> { // as if the predicate were true. This is as if the predicate were to resolve to false then // the result should not affect the rest of circuit execution. let value = array[index].clone(); - // An `Array` might contain a `DynamicArray`, however if we define the result this way, - // we would bypass the array initialization and the handling of dynamic values that - // happens in `array_get_value`. Rather than repeat it here, let the non-special-case - // handling take over by returning `false`. - if matches!(value, AcirValue::DynamicArray(_)) { - return Ok(false); - } self.define_result(dfg, instruction, value); Ok(true) } diff --git a/compiler/noirc_evaluator/src/acir/types.rs b/compiler/noirc_evaluator/src/acir/types.rs index 81ae44575d6..7ccbf09fdef 100644 --- a/compiler/noirc_evaluator/src/acir/types.rs +++ b/compiler/noirc_evaluator/src/acir/types.rs @@ -110,9 +110,10 @@ impl Debug for AcirDynamicArray { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, - "id: {}, len: {}, element_type_sizes: {:?}", + "id: {}, len: {}, value_types: {:?}, element_type_sizes: {:?}", self.block_id.0, self.len, + self.value_types, self.element_type_sizes.map(|block_id| block_id.0) ) } @@ -146,6 +147,13 @@ impl AcirValue { } } + /// Fetch a flat list of ([AcirVar], [AcirType]). + /// + /// # Panics + /// If [AcirValue::DynamicArray] is supplied or an inner element of an [AcirValue::Array]. + /// This is because an [AcirValue::DynamicArray] is simply a pointer to an array + /// and fetching its internal [AcirValue::Var] would require laying down opcodes to read its content. + /// This method should only be used where dynamic arrays are not a possible type. pub(super) fn flatten(self) -> Vec<(AcirVar, AcirType)> { match self { AcirValue::Var(var, typ) => vec![(var, typ)], @@ -154,13 +162,16 @@ impl AcirValue { } } + /// Fetch a flat list of the [NumericType] contained within an array + /// An [AcirValue::DynamicArray] should already have a field representing + /// its types and should be supported here unlike [AcirValue::flatten] pub(super) fn flat_numeric_types(self) -> Vec { match self { - AcirValue::Array(_) => { - self.flatten().into_iter().map(|(_, typ)| typ.to_numeric_type()).collect() + AcirValue::Array(array) => { + array.into_iter().flat_map(|elem| elem.flat_numeric_types()).collect() } AcirValue::DynamicArray(AcirDynamicArray { value_types, .. }) => value_types, - _ => unreachable!("An AcirValue::Var cannot be used as an array value"), + AcirValue::Var(_, typ) => vec![typ.to_numeric_type()], } } } diff --git a/test_programs/execution_success/regression_9860/Nargo.toml b/test_programs/execution_success/regression_9860/Nargo.toml new file mode 100644 index 00000000000..dc17e9bcc35 --- /dev/null +++ b/test_programs/execution_success/regression_9860/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_9860" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/regression_9860/Prover.toml b/test_programs/execution_success/regression_9860/Prover.toml new file mode 100644 index 00000000000..076e5aad42c --- /dev/null +++ b/test_programs/execution_success/regression_9860/Prover.toml @@ -0,0 +1,2 @@ +a = [[0, 1, 2, 3], [1, 2, 3, 0], [2, 3, 0, 1], [3, 0, 1, 2]] +return = [[0, 0, 2, 3], [1, 2, 3, 0], [2, 0, 0, 1], [3, 0, 1, 2]] diff --git a/test_programs/execution_success/regression_9860/src/main.nr b/test_programs/execution_success/regression_9860/src/main.nr new file mode 100644 index 00000000000..132992df166 --- /dev/null +++ b/test_programs/execution_success/regression_9860/src/main.nr @@ -0,0 +1,7 @@ +fn main(mut a: pub [[u32; 4]; 4]) -> pub [[u32; 4]; 4] { + let idx_b = a[0][0]; + let idx_f = a[2][3]; + a[2][idx_f] = a[2][2]; + a[idx_b][1] = 0; + a +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__expanded.snap new file mode 100644 index 00000000000..3e2060934f7 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__expanded.snap @@ -0,0 +1,11 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main(mut a: pub [[u32; 4]; 4]) -> pub [[u32; 4]; 4] { + let idx_b: u32 = a[0_u32][0_u32]; + let idx_f: u32 = a[2_u32][3_u32]; + a[2_u32][idx_f] = a[2_u32][2_u32]; + a[idx_b][1_u32] = 0_u32; + a +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__stdout.snap new file mode 100644 index 00000000000..63e8508338e --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- +[regression_9860] Circuit output: [[0, 0, 2, 3], [1, 2, 3, 0], [2, 0, 0, 1], [3, 0, 1, 2]]