fix(ast_fuzzer): bound literal vector indices, and diagnose leftover ACIR memory operations

compiler/noirc_evaluator/src/ssa/opt/checks.rs

@@ -32,7 +32,7 @@
 /// ```ignore
 /// checks::for_each_instruction(function, |instruction, dfg| {
 ///     checks::assert_not_if_else(instruction);
-///     checks::assert_not_load_or_store(instruction);
+///     checks::assert_not_load_or_store(function, instruction, dfg);
 /// });
 /// ```
 pub(super) fn for_each_instruction(
@@ -117,10 +117,35 @@
 }
 
 /// Panics if the instruction is a Load or Store.
-pub(super) fn assert_not_load_or_store(instruction: &Instruction) {
-    assert!(
-        !matches!(instruction, Instruction::Load { .. } | Instruction::Store { .. }),
-        "Load or Store instruction found"
+///
+/// ACIR has no memory operations, so by this point `mem2reg` must have promoted every allocation.
+/// One surviving here means it could not: something kept the address first-class past the last
+/// `mem2reg` run, which is usually an aggregate that still holds the reference — an array of
+/// references which is live, or one which is dead but was not recognised as such, leaving the
+/// address ineligible for promotion.
+///
+/// Dynamically selecting a reference out of an array is the known way to keep an address
+/// first-class that far, and is reported to the user as
+/// [`RuntimeError::DynamicIndexingWithReference`][crate::errors::RuntimeError::DynamicIndexingWithReference]
+/// by [`verify_no_dynamic_indices_to_references`][crate::ssa::validation::dynamic_array_indices::verify_no_dynamic_indices_to_references],
+/// which runs earlier in the pipeline. Anything reaching this assertion got past that check, so
+/// the printed address and its type are the place to start.
+pub(super) fn assert_not_load_or_store(
+    function: &Function,
+    instruction: &Instruction,
+    dfg: &DataFlowGraph,
+) {
+    let (kind, address) = match instruction {
+        Instruction::Load { address } => ("Load", *address),
+        Instruction::Store { address, .. } => ("Store", *address),
+        _ => return,
+    };
+
+    panic!(
+        "{kind} instruction found in ACIR function '{}': the address {address} of type {} was not \
+         promoted by mem2reg, so a memory operation reached ACIR generation",
+        function.name(),
+        dfg.type_of_value(address),
     );
 }
 
@@ -203,3 +228,33 @@
         false
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::ssa::ssa_gen::Ssa;
+
+    use super::assert_not_load_or_store;
+
+    /// A memory operation surviving into ACIR is a compiler bug with no user-facing cause to
+    /// report, so the panic has to carry enough to start debugging from: which function, which
+    /// address, and what that address is a reference to.
+    #[test]
+    #[should_panic(
+        expected = "Store instruction found in ACIR function 'main': the address v0 of type &mut Field was not promoted by mem2reg"
+    )]
+    fn load_or_store_assertion_names_the_function_and_address() {
+        let src = "
+        acir(inline) fn main f0 {
+          b0():
+            v0 = allocate -> &mut Field
+            store Field 1 at v0
+            return
+        }
+        ";
+        let ssa = Ssa::from_str(src).unwrap();
+        let function = ssa.main();
+        super::for_each_instruction(function, |instruction, dfg| {
+            assert_not_load_or_store(function, instruction, dfg);
+        });
+    }
+}

compiler/noirc_evaluator/src/ssa/opt/mutable_array_set.rs

@@ -64,11 +64,11 @@ fn mutable_array_set_optimization_pre_check(func: &Function) {
 
     // flatten_cfg must have run
     super::checks::assert_cfg_is_flattened(func);
-    super::checks::for_each_instruction(func, |instruction, _dfg| {
+    super::checks::for_each_instruction(func, |instruction, dfg| {
         // remove_if_else must have run
         super::checks::assert_not_if_else(instruction);
         // mem2reg must have run (no Load/Store remaining)
-        super::checks::assert_not_load_or_store(instruction);
+        super::checks::assert_not_load_or_store(func, instruction, dfg);
         // No mutable array sets should exist yet (they are created by this pass)
         super::checks::assert_not_mutable_array_set(instruction);
     });

compiler/noirc_evaluator/src/ssa/validation/dynamic_array_indices.rs

@@ -117,6 +117,35 @@ mod tests {
         assert!(matches!(result, Err(RuntimeError::DynamicIndexingWithReference { .. })));
     }
 
+    #[test]
+    fn dynamic_array_set_of_references() {
+        // Writing a reference into an array at a dynamic index selects which reference the array
+        // ends up aliasing, so it is as unresolvable before ACIR generation as reading one out.
+        // fn main(c: u32) -> pub bool {
+        //     let mut x = false;
+        //     let mut b: [&mut bool; 2] = [&mut false, &mut true];
+        //     b[c] = &mut x;
+        //     *b[0]
+        // }
+        let src = r#"
+            acir(inline) predicate_pure fn main f0 {
+            b0(v0: u32):
+                v1 = allocate -> &mut u1
+                v2 = allocate -> &mut u1
+                store u1 0 at v1
+                store u1 1 at v2
+                v3 = make_array [v1, v2] : [&mut u1; 2]
+                v4 = array_set v3, index v0, value v1
+                v5 = array_get v4, index u32 0 -> &mut u1
+                v6 = load v5 -> u1
+                return v6
+            }"#;
+
+        let ssa = Ssa::from_str(src).unwrap();
+        let result = verify_no_dynamic_indices_to_references(&ssa);
+        assert!(matches!(result, Err(RuntimeError::DynamicIndexingWithReference { .. })));
+    }
+
     #[test]
     fn no_error_in_brillig() {
         // unconstrained fn main(c: u32) -> pub bool {

test_programs/compile_failure/dynamic_array_index_references/Nargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "dynamic_array_index_references"
+type = "bin"
+authors = [""]
+
+[dependencies]

test_programs/compile_failure/dynamic_array_index_references/Prover.toml

@@ -0,0 +1 @@
+i = "1"

test_programs/compile_failure/dynamic_array_index_references/src/main.nr

@@ -0,0 +1,14 @@
+// Reading an array of references at a dynamic index is a dynamic selection of a reference, which
+// cannot be resolved before ACIR generation. Rejecting it here is what keeps the address out of
+// ACIR: were it to survive, mem2reg would have no way to promote the allocation behind it and a
+// later pass would hit its "no memory operations in ACIR" assertion instead of reporting anything
+// a user can act on.
+fn main(i: u32) -> pub Field {
+    let mut x = 10;
+    let mut y = 20;
+
+    let r: [&mut Field; 2] = [&mut x, &mut y];
+
+    y = 77;
+    *r[i]
+}