fix: remove compile-time error for invalid indices (#5466)

# Description

## Problem\*

Resolves #5464 

## Summary\*

Similarly to how we removed compile-time assertion failures in order to
have correct test error messages, we must do the same for invalid
indices.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

compiler/noirc_evaluator/src/errors.rs

@@ -19,8 +19,6 @@ use serde::{Deserialize, Serialize};
 pub enum RuntimeError {
     #[error(transparent)]
     InternalError(#[from] InternalError),
-    #[error("Index out of bounds, array has size {array_size}, but index was {index}")]
-    IndexOutOfBounds { index: usize, array_size: usize, call_stack: CallStack },
     #[error("Range constraint of {num_bits} bits is too large for the Field size")]
     InvalidRangeConstraint { num_bits: u32, call_stack: CallStack },
     #[error("The value `{value:?}` cannot fit into `{typ}` which has range `{range}`")]
@@ -145,7 +143,6 @@ impl RuntimeError {
                 | InternalError::UndeclaredAcirVar { call_stack }
                 | InternalError::Unexpected { call_stack, .. },
             )
-            | RuntimeError::IndexOutOfBounds { call_stack, .. }
             | RuntimeError::InvalidRangeConstraint { call_stack, .. }
             | RuntimeError::TypeConversion { call_stack, .. }
             | RuntimeError::UnInitialized { call_stack, .. }

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -1085,45 +1085,30 @@ impl<'a> Context<'a> {
             }
         };
 
-        let side_effects_always_enabled =
-            self.acir_context.is_constant_one(&self.current_side_effects_enabled_var);
-        let index_out_of_bounds = index >= array_size;
-
-        // Note that the value of `side_effects_always_enabled` doesn't affect the value which we return here for valid
-        // indices, just whether we return an error for invalid indices at compile time or defer until execution.
-        match (side_effects_always_enabled, index_out_of_bounds) {
-            (true, false) => {
-                let value = match store_value {
-                    Some(store_value) => AcirValue::Array(array.update(index, store_value)),
-                    None => array[index].clone(),
-                };
+        if index >= array_size {
+            return Ok(false);
+        }
+
+        if let Some(store_value) = store_value {
+            let side_effects_always_enabled =
+                self.acir_context.is_constant_one(&self.current_side_effects_enabled_var);
 
+            if side_effects_always_enabled {
+                // If we know that this write will always occur then we can perform it at compile time.
+                let value = AcirValue::Array(array.update(index, store_value));
                 self.define_result(dfg, instruction, value);
                 Ok(true)
+            } else {
+                // If a predicate is applied however we must wait until runtime.
+                Ok(false)
             }
-            (false, false) => {
-                if store_value.is_none() {
-                    // If there is a predicate and the index is not out of range, we can optimistically perform the
-                    // read at compile time as if the predicate is true.
-                    //
-                    // This is as if the predicate is false, any side-effects will be disabled so the value returned
-                    // will not affect the rest of execution.
-                    self.define_result(dfg, instruction, array[index].clone());
-                    Ok(true)
-                } else {
-                    // We do not do this for a array writes however.
-                    Ok(false)
-                }
-            }
-
-            // Report the error if side effects are enabled.
-            (true, true) => {
-                let call_stack = self.acir_context.get_call_stack();
-                Err(RuntimeError::IndexOutOfBounds { index, array_size, call_stack })
-            }
-            // Index is out of bounds but predicate may result in this array operation being skipped
-            // so we don't return an error now.
-            (false, true) => Ok(false),
+        } else {
+            // If the index is not out of range, we can optimistically perform the read at compile time
+            // 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();
+            self.define_result(dfg, instruction, value);
+            Ok(true)
         }
     }
 

noir_stdlib/src/collections/bounded_vec.nr

@@ -14,7 +14,7 @@ impl<T, let MaxLen: u32> BoundedVec<T, MaxLen> {
     /// Get an element from the vector at the given index.
     /// Panics if the given index points beyond the end of the vector (`self.len()`).
     pub fn get(self, index: u32) -> T {
-        assert(index < self.len);
+        assert(index < self.len, "Attempted to read past end of BoundedVec");
         self.get_unchecked(index)
     }
 
@@ -152,25 +152,16 @@ impl<T, let MaxLen: u32, let Len: u32> From<[T; Len]> for BoundedVec<T, MaxLen>
 }
 
 mod bounded_vec_tests {
-    // TODO: Allow imports from "super"
-    use crate::collections::bounded_vec::BoundedVec;
 
-    #[test]
-    fn empty_equality() {
-        let mut bounded_vec1: BoundedVec<Field, 3> = BoundedVec::new();
-        let mut bounded_vec2: BoundedVec<Field, 3> = BoundedVec::new();
-
-        assert_eq(bounded_vec1, bounded_vec2);
-    }
+    mod get {
+        use crate::collections::bounded_vec::BoundedVec;
 
-    #[test]
-    fn inequality() {
-        let mut bounded_vec1: BoundedVec<Field, 3> = BoundedVec::new();
-        let mut bounded_vec2: BoundedVec<Field, 3> = BoundedVec::new();
-        bounded_vec1.push(1);
-        bounded_vec2.push(2);
+        #[test(should_fail_with = "Attempted to read past end of BoundedVec")]
+        fn panics_when_reading_elements_past_end_of_vec() {
+            let vec: BoundedVec<Field, 5> = BoundedVec::new();
 
-        assert(bounded_vec1 != bounded_vec2);
+            crate::println(vec.get(0));
+        }
     }
 
     mod set {
@@ -295,4 +286,26 @@ mod bounded_vec_tests {
             assert_eq(bounded_vec.storage()[1], 2);
         }
     }
+
+    mod trait_eq {
+        use crate::collections::bounded_vec::BoundedVec;
+
+        #[test]
+        fn empty_equality() {
+            let mut bounded_vec1: BoundedVec<Field, 3> = BoundedVec::new();
+            let mut bounded_vec2: BoundedVec<Field, 3> = BoundedVec::new();
+
+            assert_eq(bounded_vec1, bounded_vec2);
+        }
+
+        #[test]
+        fn inequality() {
+            let mut bounded_vec1: BoundedVec<Field, 3> = BoundedVec::new();
+            let mut bounded_vec2: BoundedVec<Field, 3> = BoundedVec::new();
+            bounded_vec1.push(1);
+            bounded_vec2.push(2);
+
+            assert(bounded_vec1 != bounded_vec2);
+        }
+    }
 }