fix(brillig): SliceRefCount reads from the appropriate pointer

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -715,7 +715,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                                 BrilligBinaryOp::LessThan,
                             );
                         }
-                        Intrinsic::ArrayRefCount | Intrinsic::SliceRefCount => {
+                        Intrinsic::ArrayRefCount => {
                             let array = self.convert_ssa_value(arguments[0], dfg);
                             let result = dfg.instruction_results(instruction_id)[0];
 
@@ -729,6 +729,20 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                             let array = array.extract_register();
                             self.brillig_context.load_instruction(destination, array);
                         }
+                        Intrinsic::SliceRefCount => {
+                            let array = self.convert_ssa_value(arguments[1], dfg);
+                            let result = dfg.instruction_results(instruction_id)[0];
+
+                            let destination = self.variables.define_variable(
+                                self.function_context,
+                                self.brillig_context,
+                                result,
+                                dfg,
+                            );
+                            let destination = destination.extract_register();
+                            let array = array.extract_register();
+                            self.brillig_context.load_instruction(destination, array);
+                        }
                         Intrinsic::IsUnconstrained
                         | Intrinsic::DerivePedersenGenerators
                         | Intrinsic::ApplyRangeConstraint

compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_pop_front.rs

@@ -68,26 +68,60 @@ pub(super) fn compile_vector_pop_front_procedure<F: AcirField + DebugToString>(
         BrilligBinaryOp::Sub,
     );
 
-    // FIXME: https://github.com/noir-lang/noir/issues/7976
-    // There used to be a branch here for `if is_rc_one` but it was removed due to issues with
-    // mutating the reference count while popping from the front of the vector.
-    brillig_context.codegen_initialize_vector(target_vector, target_size, None);
-
-    let target_vector_items_pointer =
-        brillig_context.codegen_make_vector_items_pointer(target_vector);
-
-    let source_copy_pointer = brillig_context.allocate_register();
-    brillig_context.memory_op_instruction(
-        source_items_pointer.address,
-        item_pop_count_arg,
-        source_copy_pointer,
-        BrilligBinaryOp::Add,
+    let is_rc_one = brillig_context.allocate_register();
+    brillig_context.codegen_usize_op(
+        source_rc.address,
+        is_rc_one,
+        BrilligBinaryOp::Equals,
+        1_usize,
     );
-    // Now we copy the source vector starting at index removed_items.len() into the target vector
-    brillig_context.codegen_mem_copy(source_copy_pointer, target_vector_items_pointer, target_size);
 
-    brillig_context.deallocate_register(source_copy_pointer);
-    brillig_context.deallocate_register(target_vector_items_pointer);
+    brillig_context.codegen_branch(is_rc_one, |brillig_context, is_rc_one| {
+        if is_rc_one {
+            // We reuse the source vector, moving the metadata to the right (decreasing capacity)
+            brillig_context.memory_op_instruction(
+                source_vector.pointer,
+                item_pop_count_arg,
+                target_vector.pointer,
+                BrilligBinaryOp::Add,
+            );
+            brillig_context.memory_op_instruction(
+                source_capacity.address,
+                item_pop_count_arg,
+                source_capacity.address,
+                BrilligBinaryOp::Sub,
+            );
+            brillig_context.codegen_initialize_vector_metadata(
+                target_vector,
+                target_size,
+                Some(source_capacity),
+            );
+        } else {
+            brillig_context.codegen_initialize_vector(target_vector, target_size, None);
+
+            let target_vector_items_pointer =
+                brillig_context.codegen_make_vector_items_pointer(target_vector);
+
+            let source_copy_pointer = brillig_context.allocate_register();
+            brillig_context.memory_op_instruction(
+                source_items_pointer.address,
+                item_pop_count_arg,
+                source_copy_pointer,
+                BrilligBinaryOp::Add,
+            );
+            // Now we copy the source vector starting at index removed_items.len() into the target vector
+            brillig_context.codegen_mem_copy(
+                source_copy_pointer,
+                target_vector_items_pointer,
+                target_size,
+            );
+
+            brillig_context.deallocate_register(source_copy_pointer);
+            brillig_context.deallocate_register(target_vector_items_pointer);
+        }
+    });
+
+    brillig_context.deallocate_register(is_rc_one);
 
     brillig_context.deallocate_single_addr(target_size);
     brillig_context.deallocate_single_addr(source_rc);

test_programs/execution_success/reference_counts_slices_inliner_0/Nargo.toml

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

test_programs/execution_success/reference_counts_slices_inliner_0/src/main.nr

@@ -0,0 +1,122 @@
+// This test is exactly the same as `reference_counts_inliner_0` which uses
+// arrays rather than slices.
+// This test exists to make sure that our reference counting debug methods match
+// between arrays and slices.
+// We could most likely combine the code for these tests (e.g. using generics),
+// but it is simpler to debug isolated tests.
+// It should only be necessary to have a test at one inliner setting, as we
+// are just checking for discrepancies between the array and slice debugging builtin functions.
+// The actual functionality of reference counting is tested with the `reference_counts_*` tests.
+// We went with testing at an inliner aggressiveness of zero, as this is generally
+// the most useful inliner setting for unconstrained functions.
+use std::mem::slice_refcount;
+
+fn main() {
+    let mut slice = &[0, 1, 2];
+    assert_refcount(slice, 1, true);
+
+    borrow(slice, slice_refcount(slice));
+    borrow_mut(&mut slice, slice_refcount(slice));
+    let _ = copy_mut(slice, slice_refcount(slice));
+
+    borrow_mut_two(&mut slice, &mut slice, slice_refcount(slice));
+
+    let mut u32_slice = &[0, 1, 2];
+    let rc1 = slice_refcount(slice);
+    let rc2 = slice_refcount(u32_slice);
+    borrow_mut_two_separate(&mut slice, &mut u32_slice, rc1, rc2);
+
+    // Safety: test
+    regression_7297();
+}
+
+fn borrow(slice: [Field], rc_before_call: u32) {
+    assert_refcount(slice, rc_before_call, true);
+    println(slice[0]);
+}
+
+fn borrow_mut(slice: &mut [Field], rc_before_call: u32) {
+    assert_refcount(*slice, rc_before_call, true);
+    slice[0] = 3;
+    println(slice[0]);
+}
+
+// Returns a new slice (a copy) to prevent SSA from optimizing away mutations.
+fn copy_mut(mut slice: [Field], rc_before_call: u32) -> [Field] {
+    assert_refcount(slice, rc_before_call, true);
+    slice = &[4, slice[1], slice[2]];
+    println(slice[0]);
+    slice
+}
+
+fn borrow_mut_two(slice1: &mut [Field], slice2: &mut [Field], rc_before_call: u32) {
+    assert_refcount(*slice1, rc_before_call, true);
+    assert_refcount(*slice2, rc_before_call + 1, true); // should be a copy
+    slice1[0] = 5;
+    slice2[0] = 6;
+    println(slice1[0]); // slice1 & 2 alias, so this should also print 6
+    println(slice2[0]);
+}
+
+fn borrow_mut_two_separate(
+    slice1: &mut [Field],
+    slice2: &mut [u32],
+    rc_before_call1: u32,
+    rc_before_call2: u32,
+) {
+    assert_refcount(*slice1, rc_before_call1, true);
+    assert_refcount(*slice2, rc_before_call2, true);
+    slice1[0] = 7;
+    slice2[0] = 8;
+    println(slice1[0]);
+    println(slice2[0]);
+}
+
+fn assert_refcount<T>(slice: [T], mut expected: u32, expect_copy: bool) {
+    let count = slice_refcount(slice);
+
+    if expect_copy {
+        expected += 1;
+    }
+
+    if std::runtime::is_unconstrained() {
+        if count != expected {
+            println(f"actual = {count}, expected = {expected}");
+        }
+        assert_eq(count, expected);
+    } else {
+        assert_eq(count, 0);
+    }
+}
+
+fn regression_7297() {
+    let mut slice: [Field] = &[0, 1, 2];
+
+    let refcount_0 = slice_refcount(slice);
+    borrow_mut_two(&mut slice, &mut slice, refcount_0);
+
+    let refcount_1 = slice_refcount(slice);
+    let slice_2 = copy_mut(slice, refcount_1 + 1);
+    let refcount_2 = slice_refcount(slice);
+
+    assert_eq(slice[0], 6, "the original should not be mutated by copy_mut, only borrow_mut_two");
+    assert_eq(slice_2[0], 4, "the copy should have the expected content");
+
+    if std::runtime::is_unconstrained() {
+        assert(
+            refcount_1 != 0,
+            "borrow_mut_two should create a fresh slice and not decrease its RC",
+        );
+
+        assert_eq(
+            refcount_1,
+            2,
+            "There is 1 clone after `borrow_mut_two` and before `refcount_1` is defined (cloned before slice_refcount call)",
+        );
+        assert_eq(
+            refcount_2,
+            refcount_1 + 3,
+            "after refcount_1 we clone once in passing slice to copy_mut, once to slice_refcount after, and once within copy_mut",
+        );
+    }
+}

test_programs/execution_success/reference_counts_slices_inliner_0/stdout.txt

@@ -0,0 +1,10 @@
+0x00
+0x03
+0x04
+0x06
+0x06
+0x07
+8
+0x06
+0x06
+0x04

test_programs/gates_report_brillig_execution.sh

@@ -19,6 +19,7 @@ excluded_dirs=(
   "reference_counts_inliner_min"
   "reference_counts_inliner_0"
   "reference_counts_inliner_max"
+  "reference_counts_slices_inliner_0"
 )
 
 current_dir=$(pwd)

tooling/debugger/ignored-tests.txt

@@ -5,6 +5,7 @@ macros
 reference_counts_inliner_0
 reference_counts_inliner_min
 reference_counts_inliner_max
+reference_counts_slices_inliner_0
 references
 regression_4709
 regression_7323

tooling/nargo_cli/build.rs

@@ -71,10 +71,11 @@ const INLINER_MIN_OVERRIDES: [(&str, i64); 1] = [
 const INLINER_MAX_OVERRIDES: [(&str, i64); 0] = [];
 
 /// These tests should only be run on exactly 1 inliner setting (the one given here)
-const INLINER_OVERRIDES: [(&str, i64); 3] = [
+const INLINER_OVERRIDES: [(&str, i64); 4] = [
     ("reference_counts_inliner_0", 0),
     ("reference_counts_inliner_min", i64::MIN),
     ("reference_counts_inliner_max", i64::MAX),
+    ("reference_counts_slices_inliner_0", 0),
 ];
 
 /// Some tests are expected to have warnings