From 4e773fca068f45f6bd59da46ff9d1e8e858d229e Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 17 Sep 2025 13:04:46 +0100 Subject: [PATCH 01/12] Add integration test --- test_programs/execution_success/regression_9860/Nargo.toml | 6 ++++++ .../execution_success/regression_9860/Prover.toml | 2 ++ .../execution_success/regression_9860/src/main.nr | 7 +++++++ .../regression_9860/execute__tests__stdout.snap | 5 +++++ 4 files changed, 20 insertions(+) create mode 100644 test_programs/execution_success/regression_9860/Nargo.toml create mode 100644 test_programs/execution_success/regression_9860/Prover.toml create mode 100644 test_programs/execution_success/regression_9860/src/main.nr create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__stdout.snap 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__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]] From b1ab718166226b8ca64a6d151a6025f67003a404 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 17 Sep 2025 13:50:38 +0100 Subject: [PATCH 02/12] Do not take the constant index shortcut when setting arrays if the value is dynamic --- compiler/noirc_evaluator/src/acir/arrays.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index f47fbaa3465..7b37d444036 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -226,6 +226,13 @@ impl Context<'_> { } if let Some(store_value) = store_value { + // We should avoid storing a DynamicArray directly inside an Array. Instead we must allow + // the regular `array_set_value` to take place which reads the item from the block the + // dynamic array refers to, and writes items one by one. Otherwise subsequent operations + // with non-constant indexes will fail. + if matches!(store_value, AcirValue::DynamicArray(_)) { + return Ok(false); + } let side_effects_always_enabled = self.acir_context.is_constant_one(&self.current_side_effects_enabled_var); From 152fedcf9fa9bbfc7b3af8a00135a42903ee46b3 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 17 Sep 2025 13:50:46 +0100 Subject: [PATCH 03/12] Add insta --- .../regression_9860/execute__tests__expanded.snap | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/regression_9860/execute__tests__expanded.snap 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 +} From 6549bb77f361b530e32258e4c009bd999f35c073 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 17 Sep 2025 20:01:14 +0000 Subject: [PATCH 04/12] fix ArrayValue::flat_numeric_types --- compiler/noirc_evaluator/src/acir/arrays.rs | 8 +------- compiler/noirc_evaluator/src/acir/types.rs | 9 +++++---- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 7b37d444036..90319355bc9 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -226,13 +226,6 @@ impl Context<'_> { } if let Some(store_value) = store_value { - // We should avoid storing a DynamicArray directly inside an Array. Instead we must allow - // the regular `array_set_value` to take place which reads the item from the block the - // dynamic array refers to, and writes items one by one. Otherwise subsequent operations - // with non-constant indexes will fail. - if matches!(store_value, AcirValue::DynamicArray(_)) { - return Ok(false); - } let side_effects_always_enabled = self.acir_context.is_constant_one(&self.current_side_effects_enabled_var); @@ -600,6 +593,7 @@ impl Context<'_> { }; let value_types = self.convert_value(array, dfg).flat_numeric_types(); + dbg!(value_types.clone()); // Compiler sanity check assert_eq!( value_types.len(), diff --git a/compiler/noirc_evaluator/src/acir/types.rs b/compiler/noirc_evaluator/src/acir/types.rs index 81ae44575d6..0c291f5069a 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) ) } @@ -156,11 +157,11 @@ impl AcirValue { 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()], } } } From 0468590c5f4c393bde7d7bd45486b76d8e7a5bec Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 17 Sep 2025 20:04:25 +0000 Subject: [PATCH 05/12] cleanup --- compiler/noirc_evaluator/src/acir/arrays.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 90319355bc9..f47fbaa3465 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -593,7 +593,6 @@ impl Context<'_> { }; let value_types = self.convert_value(array, dfg).flat_numeric_types(); - dbg!(value_types.clone()); // Compiler sanity check assert_eq!( value_types.len(), From a32165a5dff94febf7ce1806b193486f7152e70c Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 Sep 2025 10:17:21 +0000 Subject: [PATCH 06/12] feat: allow initializing dynamic arrays --- compiler/noirc_evaluator/src/acir/acir_context/mod.rs | 3 --- compiler/noirc_evaluator/src/acir/arrays.rs | 7 ------- 2 files changed, 10 deletions(-) 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..b6bb0131cda 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -243,13 +243,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) } From d288b5fd7d2714ecc79b69f5f130faadc9875448 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 Sep 2025 11:46:49 +0000 Subject: [PATCH 07/12] chore: more docs --- compiler/noirc_evaluator/src/acir/arrays.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index b6bb0131cda..a7d54357760 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -194,7 +194,12 @@ 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) + } } } From 42eb2e254e56743ec49a0dc531b0794d80e7df61 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 Sep 2025 11:51:52 +0000 Subject: [PATCH 08/12] docs --- compiler/noirc_evaluator/src/acir/arrays.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index a7d54357760..61e19276a57 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 a `AcirValue::DynamicArray`'s has been mutated at an unknown index. fn handle_constant_index_wrapper( &mut self, instruction: InstructionId, From f4576b8fa88d5363f5b906be7f7fd18245e42aff Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 Sep 2025 11:52:34 +0000 Subject: [PATCH 09/12] . --- compiler/noirc_evaluator/src/acir/arrays.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 61e19276a57..241453e9e9c 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -208,8 +208,7 @@ impl Context<'_> { } } - /// 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, From f4ff32ca081a81e5997f15455b134bdc1187b9dd Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 Sep 2025 12:11:37 +0000 Subject: [PATCH 10/12] . --- compiler/noirc_evaluator/src/acir/arrays.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 241453e9e9c..4cea86306eb 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -173,7 +173,7 @@ impl Context<'_> { /// /// 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 a `AcirValue::DynamicArray`'s has been mutated at an unknown index. + /// `AcirValue::Array` as an `AcirValue::DynamicArray` has been mutated at an unknown index. fn handle_constant_index_wrapper( &mut self, instruction: InstructionId, From cce4c1062a4c4da12668c08e9b800fa0dc4e7354 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 18 Sep 2025 08:27:35 -0400 Subject: [PATCH 11/12] fix(acir_gen): Do not call into AcirValue::flatten during AcirValue::flat_numeric_types (#9894) --- compiler/noirc_evaluator/src/acir/arrays.rs | 7 ------- compiler/noirc_evaluator/src/acir/types.rs | 19 +++++++++++++++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 7b37d444036..f47fbaa3465 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -226,13 +226,6 @@ impl Context<'_> { } if let Some(store_value) = store_value { - // We should avoid storing a DynamicArray directly inside an Array. Instead we must allow - // the regular `array_set_value` to take place which reads the item from the block the - // dynamic array refers to, and writes items one by one. Otherwise subsequent operations - // with non-constant indexes will fail. - if matches!(store_value, AcirValue::DynamicArray(_)) { - return Ok(false); - } let side_effects_always_enabled = self.acir_context.is_constant_one(&self.current_side_effects_enabled_var); 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()], } } } From 486c785c198211ad5d8e794f5f527ee189a6ed25 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 Sep 2025 12:30:57 +0000 Subject: [PATCH 12/12] . --- compiler/noirc_evaluator/src/acir/arrays.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 4cea86306eb..d4dde7d1ccb 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -171,7 +171,7 @@ impl Context<'_> { /// 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 + /// 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(