fix: Don't remove necessary RC instructions in DIE pass (noir-lang/no…

…ir#6585)

feat: Add `BoundedVec::from_parts` and `BoundedVec::from_parts_unchecked` (noir-lang/noir#6691)
chore: fix warning when compiling `noir_wasm` (noir-lang/noir#6686)
chore: fix tests in `noirc_abi_wasm` (noir-lang/noir#6688)
feat(tooling): Skip program transformation when loaded from cache (noir-lang/noir#6689)
feat(ssa): Simplify array get from set that writes to the same dynamic index (noir-lang/noir#6684)
feat: Reduce memory consumption by storing array length as `u32` during SSA (noir-lang/noir#6606)
chore: add `ram_blowup_regression` to memory report (noir-lang/noir#6683)
chore: update noir-bench-report version (noir-lang/noir#6675)
fix: Prevent hoisting binary instructions which can overflow (noir-lang/noir#6672)
feat(ssa): Hoisting of array get using known induction variable maximum (noir-lang/noir#6639)
feat: better error message when trying to invoke struct function field (noir-lang/noir#6661)
feat: add memory report into the CI (noir-lang/noir#6630)
feat: allow ignoring test failures from foreign calls (noir-lang/noir#6660)
chore: refactor foreign call executors (noir-lang/noir#6659)
fix: correct signed integer handling in `noirc_abi` (noir-lang/noir#6638)
fix: allow multiple `_` parameters, and disallow `_` as an expression you can read from (noir-lang/noir#6657)
feat: allow filtering which SSA passes are printed (noir-lang/noir#6636)
fix: use correct type for attribute arguments (noir-lang/noir#6640)
fix: always return an array of `u8`s when simplifying `Intrinsic::ToRadix` calls (noir-lang/noir#6663)
feat(ssa): Option to set the maximum acceptable Brillig bytecode increase in unrolling (noir-lang/noir#6641)
feat: Sync from aztec-packages (noir-lang/noir#6656)
chore: refactor poseidon2 (noir-lang/noir#6655)
fix: correct types returned by constant EC operations simplified within SSA (noir-lang/noir#6652)
feat: Sync from aztec-packages (noir-lang/noir#6634)
fix: used signed division for signed modulo (noir-lang/noir#6635)
fix(ssa): don't deduplicate constraints in blocks that are not dominated (noir-lang/noir#6627)
chore: pin foundry version in CI (noir-lang/noir#6642)
feat(ssa): Deduplicate intrinsics with predicates (noir-lang/noir#6615)
chore: improve error message of `&T` (noir-lang/noir#6633)
fix: LSP code action wasn't triggering on beginning or end of identifier (noir-lang/noir#6616)
chore!: remove `ec` module from stdlib (noir-lang/noir#6612)
fix(LSP): use generic self type to narrow down methods to complete (noir-lang/noir#6617)
fix!: Disallow `#[export]` on associated methods (noir-lang/noir#6626)
chore: redo typo PR by donatik27 (noir-lang/noir#6575)
chore: redo typo PR by Dimitrolito (noir-lang/noir#6614)
feat: simplify `jmpif`s by reversing branches if condition is negated (noir-lang/noir#5891)
fix: Do not warn on unused functions marked with #[export] (noir-lang/noir#6625)
chore: Add panic for compiler error described in #6620 (noir-lang/noir#6621)
feat(perf): Track last loads per block in mem2reg and remove them if possible (noir-lang/noir#6088)
fix(ssa): Track all local allocations during flattening (noir-lang/noir#6619)
feat(comptime): Implement blackbox functions in comptime interpreter (noir-lang/noir#6551)
chore: derive PartialEq and Hash for FieldElement (noir-lang/noir#6610)
chore: ignore almost-empty directories in nargo_cli tests (noir-lang/noir#6611)
chore: remove temporary allocations from `num_bits` (noir-lang/noir#6600)
chore: Release Noir(1.0.0-beta.0) (noir-lang/noir#6562)
feat: Add `array_refcount` and `slice_refcount` builtins for debugging (noir-lang/noir#6584)
chore!: Require types of globals to be specified (noir-lang/noir#6592)
fix: don't report visibility errors when elaborating comptime value (noir-lang/noir#6498)
fix: preserve newlines between comments when formatting statements (noir-lang/noir#6601)
fix: parse a bit more SSA stuff (noir-lang/noir#6599)
chore!: remove eddsa from stdlib (noir-lang/noir#6591)
chore: Typo in oracles how to (noir-lang/noir#6598)
feat(ssa): Loop invariant code motion (noir-lang/noir#6563)
fix: remove `compiler_version` from new `Nargo.toml` (noir-lang/noir#6590)
feat: Avoid incrementing reference counts in some cases (noir-lang/noir#6568)
chore: fix typo in test name (noir-lang/noir#6589)
fix: consider prereleases to be compatible with pre-1.0.0 releases (noir-lang/noir#6580)
feat: try to inline brillig calls with all constant arguments  (noir-lang/noir#6548)
fix: correct type when simplifying `derive_pedersen_generators` (noir-lang/noir#6579)
feat: Sync from aztec-packages (noir-lang/noir#6576)

.noir-sync-commit

@@ -1 +1 @@
-3992d79bf37e71c9f4e62b5bb2a8bb91db6228c5
+440d94d8149ede5f211437e9405f65b460cfcbf8

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs

@@ -127,8 +127,9 @@ impl Context {
                         .push(instructions_len - instruction_index - 1);
                 }
             } else {
-                use Instruction::*;
-                if matches!(instruction, IncrementRc { .. } | DecrementRc { .. }) {
+                // We can't remove rc instructions if they're loaded from a reference
+                // since we'd have no way of knowing whether the reference is still used.
+                if Self::is_inc_dec_instruction_on_known_array(instruction, &function.dfg) {
                     self.rc_instructions.push((*instruction_id, block_id));
                 } else {
                     instruction.for_each_value(|value| {
@@ -140,7 +141,6 @@ impl Context {
             rc_tracker.track_inc_rcs_to_remove(*instruction_id, function);
         }
 
-        self.instructions_to_remove.extend(rc_tracker.get_non_mutated_arrays());
         self.instructions_to_remove.extend(rc_tracker.rc_pairs_to_remove);
 
         // If there are some instructions that might trigger an out of bounds error,
@@ -337,6 +337,28 @@ impl Context {
 
         inserted_check
     }
+
+    /// True if this is a `Instruction::IncrementRc` or `Instruction::DecrementRc`
+    /// operating on an array directly from a `Instruction::MakeArray` or an
+    /// intrinsic known to return a fresh array.
+    fn is_inc_dec_instruction_on_known_array(
+        instruction: &Instruction,
+        dfg: &DataFlowGraph,
+    ) -> bool {
+        use Instruction::*;
+        if let IncrementRc { value } | DecrementRc { value } = instruction {
+            if let Value::Instruction { instruction, .. } = &dfg[*value] {
+                return match &dfg[*instruction] {
+                    MakeArray { .. } => true,
+                    Call { func, .. } => {
+                        matches!(&dfg[*func], Value::Intrinsic(_) | Value::ForeignFunction(_))
+                    }
+                    _ => false,
+                };
+            }
+        }
+        false
+    }
 }
 
 fn instruction_might_result_in_out_of_bounds(
@@ -510,10 +532,11 @@ struct RcTracker {
     // If we see an inc/dec RC pair within a block we can safely remove both instructions.
     rcs_with_possible_pairs: HashMap<Type, Vec<RcInstruction>>,
     rc_pairs_to_remove: HashSet<InstructionId>,
+
     // We also separately track all IncrementRc instructions and all arrays which have been mutably borrowed.
     // If an array has not been mutably borrowed we can then safely remove all IncrementRc instructions on that array.
     inc_rcs: HashMap<ValueId, HashSet<InstructionId>>,
-    mut_borrowed_arrays: HashSet<ValueId>,
+
     // The SSA often creates patterns where after simplifications we end up with repeat
     // IncrementRc instructions on the same value. We track whether the previous instruction was an IncrementRc,
     // and if the current instruction is also an IncrementRc on the same value we remove the current instruction.
@@ -566,35 +589,10 @@ impl RcTracker {
                         dec_rc.possibly_mutated = true;
                     }
                 }
-
-                self.mut_borrowed_arrays.insert(*array);
-            }
-            Instruction::Store { value, .. } => {
-                // We are very conservative and say that any store of an array value means it has the potential
-                // to be mutated. This is done due to the tracking of mutable borrows still being per block.
-                let typ = function.dfg.type_of_value(*value);
-                if matches!(&typ, Type::Array(..) | Type::Slice(..)) {
-                    self.mut_borrowed_arrays.insert(*value);
-                }
             }
             _ => {}
         }
     }
-
-    fn get_non_mutated_arrays(&self) -> HashSet<InstructionId> {
-        self.inc_rcs
-            .keys()
-            .filter_map(|value| {
-                if !self.mut_borrowed_arrays.contains(value) {
-                    Some(&self.inc_rcs[value])
-                } else {
-                    None
-                }
-            })
-            .flatten()
-            .copied()
-            .collect()
-    }
 }
 #[cfg(test)]
 mod test {
@@ -830,32 +828,23 @@ mod test {
     }
 
     #[test]
-    fn remove_inc_rcs_that_are_never_mutably_borrowed() {
+    fn does_not_remove_inc_or_dec_rc_of_if_they_are_loaded_from_a_reference() {
         let src = "
-            acir(inline) fn main f0 {
-              b0(v0: [Field; 2]):
-                inc_rc v0
-                inc_rc v0
-                inc_rc v0
-                v2 = array_get v0, index u32 0 -> Field
-                inc_rc v0
-                return v2
+            brillig(inline) fn borrow_mut f0 {
+              b0(v0: &mut [Field; 3]):
+                v1 = load v0 -> [Field; 3]
+                inc_rc v1 // this one shouldn't be removed
+                v2 = load v0 -> [Field; 3]
+                inc_rc v2 // this one shouldn't be removed
+                v3 = load v0 -> [Field; 3]
+                v6 = array_set v3, index u32 0, value Field 5
+                store v6 at v0
+                dec_rc v6
+                return
             }
             ";
         let ssa = Ssa::from_str(src).unwrap();
-        let main = ssa.main();
-
-        // The instruction count never includes the terminator instruction
-        assert_eq!(main.dfg[main.entry_block()].instructions().len(), 5);
-
-        let expected = "
-            acir(inline) fn main f0 {
-              b0(v0: [Field; 2]):
-                v2 = array_get v0, index u32 0 -> Field
-                return v2
-            }
-            ";
         let ssa = ssa.dead_instruction_elimination();
-        assert_normalized_ssa_equals(ssa, expected);
+        assert_normalized_ssa_equals(ssa, src);
     }
 }

noir/noir-repo/docs/docs/noir/standard_library/containers/boundedvec.md

@@ -246,6 +246,42 @@ Example:
 let bounded_vec: BoundedVec<Field, 10> = BoundedVec::from_array([1, 2, 3])
 ```
 
+### from_parts
+
+```rust
+pub fn from_parts(mut array: [T; MaxLen], len: u32) -> Self
+```
+
+Creates a new BoundedVec from the given array and length.
+The given length must be less than or equal to the length of the array.
+
+This function will zero out any elements at or past index `len` of `array`.
+This incurs an extra runtime cost of O(MaxLen). If you are sure your array is
+zeroed after that index, you can use `from_parts_unchecked` to remove the extra loop.
+
+Example:
+
+#include_code from-parts noir_stdlib/src/collections/bounded_vec.nr rust
+
+### from_parts_unchecked
+
+```rust
+pub fn from_parts_unchecked(array: [T; MaxLen], len: u32) -> Self
+```
+
+Creates a new BoundedVec from the given array and length.
+The given length must be less than or equal to the length of the array.
+
+This function is unsafe because it expects all elements past the `len` index
+of `array` to be zeroed, but does not check for this internally. Use `from_parts`
+for a safe version of this function which does zero out any indices past the
+given length. Invalidating this assumption can notably cause `BoundedVec::eq`
+to give incorrect results since it will check even elements past `len`.
+
+Example:
+
+#include_code from-parts-unchecked noir_stdlib/src/collections/bounded_vec.nr rust
+
 ### map
 
 ```rust

noir/noir-repo/noir_stdlib/src/collections/bounded_vec.nr

@@ -420,6 +420,58 @@ impl<T, let MaxLen: u32> BoundedVec<T, MaxLen> {
         }
         ret
     }
+
+    /// Creates a new BoundedVec from the given array and length.
+    /// The given length must be less than or equal to the length of the array.
+    ///
+    /// This function will zero out any elements at or past index `len` of `array`.
+    /// This incurs an extra runtime cost of O(MaxLen). If you are sure your array is
+    /// zeroed after that index, you can use `from_parts_unchecked` to remove the extra loop.
+    ///
+    /// Example:
+    ///
+    /// ```noir
+    /// let vec: BoundedVec<u32, 4> = BoundedVec::from_parts([1, 2, 3, 0], 3);
+    /// assert_eq(vec.len(), 3);
+    /// ```
+    pub fn from_parts(mut array: [T; MaxLen], len: u32) -> Self {
+        assert(len <= MaxLen);
+        let zeroed = crate::mem::zeroed();
+        for i in 0..MaxLen {
+            if i >= len {
+                array[i] = zeroed;
+            }
+        }
+        BoundedVec { storage: array, len }
+    }
+
+    /// Creates a new BoundedVec from the given array and length.
+    /// The given length must be less than or equal to the length of the array.
+    ///
+    /// This function is unsafe because it expects all elements past the `len` index
+    /// of `array` to be zeroed, but does not check for this internally. Use `from_parts`
+    /// for a safe version of this function which does zero out any indices past the
+    /// given length. Invalidating this assumption can notably cause `BoundedVec::eq`
+    /// to give incorrect results since it will check even elements past `len`.
+    ///
+    /// Example:
+    ///
+    /// ```noir
+    /// let vec: BoundedVec<u32, 4> = BoundedVec::from_parts_unchecked([1, 2, 3, 0], 3);
+    /// assert_eq(vec.len(), 3);
+    ///
+    /// // invalid use!
+    /// let vec1: BoundedVec<u32, 4> = BoundedVec::from_parts_unchecked([1, 2, 3, 1], 3);
+    /// let vec2: BoundedVec<u32, 4> = BoundedVec::from_parts_unchecked([1, 2, 3, 2], 3);
+    ///
+    /// // both vecs have length 3 so we'd expect them to be equal, but this
+    /// // fails because elements past the length are still checked in eq
+    /// assert_eq(vec1, vec2); // fails
+    /// ```
+    pub fn from_parts_unchecked(array: [T; MaxLen], len: u32) -> Self {
+        assert(len <= MaxLen);
+        BoundedVec { storage: array, len }
+    }
 }
 
 impl<T, let MaxLen: u32> Eq for BoundedVec<T, MaxLen>
@@ -431,7 +483,11 @@ where
         //
         // We make the assumption that the user has used the proper interface for working with `BoundedVec`s
         // rather than directly manipulating the internal fields as this can result in an inconsistent internal state.
-        (self.len == other.len) & (self.storage == other.storage)
+        if self.len == other.len {
+            self.storage == other.storage
+        } else {
+            false
+        }
     }
 }
 
@@ -598,4 +654,38 @@ mod bounded_vec_tests {
             assert(bounded_vec1 != bounded_vec2);
         }
     }
+
+    mod from_parts {
+        use crate::collections::bounded_vec::BoundedVec;
+
+        #[test]
+        fn from_parts() {
+            // docs:start:from-parts
+            let vec: BoundedVec<u32, 4> = BoundedVec::from_parts([1, 2, 3, 0], 3);
+            assert_eq(vec.len(), 3);
+
+            // Any elements past the given length are zeroed out, so these
+            // two BoundedVecs will be completely equal
+            let vec1: BoundedVec<u32, 4> = BoundedVec::from_parts([1, 2, 3, 1], 3);
+            let vec2: BoundedVec<u32, 4> = BoundedVec::from_parts([1, 2, 3, 2], 3);
+            assert_eq(vec1, vec2);
+            // docs:end:from-parts
+        }
+
+        #[test]
+        fn from_parts_unchecked() {
+            // docs:start:from-parts-unchecked
+            let vec: BoundedVec<u32, 4> = BoundedVec::from_parts_unchecked([1, 2, 3, 0], 3);
+            assert_eq(vec.len(), 3);
+
+            // invalid use!
+            let vec1: BoundedVec<u32, 4> = BoundedVec::from_parts_unchecked([1, 2, 3, 1], 3);
+            let vec2: BoundedVec<u32, 4> = BoundedVec::from_parts_unchecked([1, 2, 3, 2], 3);
+
+            // both vecs have length 3 so we'd expect them to be equal, but this
+            // fails because elements past the length are still checked in eq
+            assert(vec1 != vec2);
+            // docs:end:from-parts-unchecked
+        }
+    }
 }

noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr

@@ -1,6 +1,6 @@
 fn main() {
     let mut array = [0, 1, 2];
-    assert_refcount(array, 1);
+    assert_refcount(array, 2);
 
     borrow(array, std::mem::array_refcount(array));
     borrow_mut(&mut array, std::mem::array_refcount(array));
@@ -13,13 +13,13 @@ fn borrow(array: [Field; 3], rc_before_call: u32) {
 }
 
 fn borrow_mut(array: &mut [Field; 3], rc_before_call: u32) {
-    assert_refcount(*array, rc_before_call + 0); // Issue! This should be rc_before_call + 1
+    assert_refcount(*array, rc_before_call + 1);
     array[0] = 5;
     println(array[0]);
 }
 
 fn copy_mut(mut array: [Field; 3], rc_before_call: u32) {
-    assert_refcount(array, rc_before_call + 0); // Issue! This should be rc_before_call + 1
+    assert_refcount(array, rc_before_call + 1);
     array[0] = 6;
     println(array[0]);
 }