[1 changes] feat: lsp rename/find-all-references for local variables (n…

…oir-lang/noir#5439)

feat: remove duplicated array reads at constant indices (noir-lang/noir#5445)
fix: Account for the expected kind when resolving turbofish generics (noir-lang/noir#5448)
fix: Fix issue with unresolved results (noir-lang/noir#5453)
feat: apply `no_predicates` in stdlib (noir-lang/noir#5454)
fix: prevent `no_predicates` from removing predicates in calling function (noir-lang/noir#5452)
feat: lsp rename/find-all-references for globals (noir-lang/noir#5415)
feat: remove redundant `EnableSideEffects` instructions (noir-lang/noir#5440)

.noir-sync-commit

@@ -1 +1 @@
-30c50f52a6d58163e39006b73f4eb5003afc239b
+bb6913ac53620fabd73e24ca1a2b1369225903ec

noir/noir-repo/.github/workflows/mirror-external_libs.yml

@@ -0,0 +1,8 @@
+name: Mirror Repositories
+on:
+  workflow_dispatch: {} 
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo Dummy workflow TODO

noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs

@@ -162,63 +162,27 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
             VMStatus::Finished { .. } => Ok(BrilligSolverStatus::Finished),
             VMStatus::InProgress => Ok(BrilligSolverStatus::InProgress),
             VMStatus::Failure { reason, call_stack } => {
+                let call_stack = call_stack
+                    .iter()
+                    .map(|brillig_index| OpcodeLocation::Brillig {
+                        acir_index: self.acir_index,
+                        brillig_index: *brillig_index,
+                    })
+                    .collect();
                 let payload = match reason {
                     FailureReason::RuntimeError { message } => {
                         Some(ResolvedAssertionPayload::String(message))
                     }
                     FailureReason::Trap { revert_data_offset, revert_data_size } => {
-                        // Since noir can only revert with strings currently, we can parse return data as a string
-                        if revert_data_size == 0 {
-                            None
-                        } else {
-                            let memory = self.vm.get_memory();
-                            let mut revert_values_iter = memory
-                                [revert_data_offset..(revert_data_offset + revert_data_size)]
-                                .iter();
-                            let error_selector = ErrorSelector::new(
-                                revert_values_iter
-                                    .next()
-                                    .expect("Incorrect revert data size")
-                                    .try_into()
-                                    .expect("Error selector is not u64"),
-                            );
-
-                            match error_selector {
-                                STRING_ERROR_SELECTOR => {
-                                    // If the error selector is 0, it means the error is a string
-                                    let string = revert_values_iter
-                                        .map(|memory_value| {
-                                            let as_u8: u8 = memory_value
-                                                .try_into()
-                                                .expect("String item is not u8");
-                                            as_u8 as char
-                                        })
-                                        .collect();
-                                    Some(ResolvedAssertionPayload::String(string))
-                                }
-                                _ => {
-                                    // If the error selector is not 0, it means the error is a custom error
-                                    Some(ResolvedAssertionPayload::Raw(RawAssertionPayload {
-                                        selector: error_selector,
-                                        data: revert_values_iter
-                                            .map(|value| value.to_field())
-                                            .collect(),
-                                    }))
-                                }
-                            }
-                        }
+                        extract_failure_payload_from_memory(
+                            self.vm.get_memory(),
+                            revert_data_offset,
+                            revert_data_size,
+                        )
                     }
                 };
-                Err(OpcodeResolutionError::BrilligFunctionFailed {
-                    payload,
-                    call_stack: call_stack
-                        .iter()
-                        .map(|brillig_index| OpcodeLocation::Brillig {
-                            acir_index: self.acir_index,
-                            brillig_index: *brillig_index,
-                        })
-                        .collect(),
-                })
+
+                Err(OpcodeResolutionError::BrilligFunctionFailed { payload, call_stack })
             }
             VMStatus::ForeignCallWait { function, inputs } => {
                 Ok(BrilligSolverStatus::ForeignCallWait(ForeignCallWaitInfo { function, inputs }))
@@ -283,6 +247,50 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
     }
 }
 
+/// Extracts a `ResolvedAssertionPayload` from a block of memory of a Brillig VM instance.
+///
+/// Returns `None` if the amount of memory requested is zero.
+fn extract_failure_payload_from_memory<F: AcirField>(
+    memory: &[MemoryValue<F>],
+    revert_data_offset: usize,
+    revert_data_size: usize,
+) -> Option<ResolvedAssertionPayload<F>> {
+    // Since noir can only revert with strings currently, we can parse return data as a string
+    if revert_data_size == 0 {
+        None
+    } else {
+        let mut revert_values_iter =
+            memory[revert_data_offset..(revert_data_offset + revert_data_size)].iter();
+        let error_selector = ErrorSelector::new(
+            revert_values_iter
+                .next()
+                .expect("Incorrect revert data size")
+                .try_into()
+                .expect("Error selector is not u64"),
+        );
+
+        match error_selector {
+            STRING_ERROR_SELECTOR => {
+                // If the error selector is 0, it means the error is a string
+                let string = revert_values_iter
+                    .map(|memory_value| {
+                        let as_u8: u8 = memory_value.try_into().expect("String item is not u8");
+                        as_u8 as char
+                    })
+                    .collect();
+                Some(ResolvedAssertionPayload::String(string))
+            }
+            _ => {
+                // If the error selector is not 0, it means the error is a custom error
+                Some(ResolvedAssertionPayload::Raw(RawAssertionPayload {
+                    selector: error_selector,
+                    data: revert_values_iter.map(|value| value.to_field()).collect(),
+                }))
+            }
+        }
+    }
+}
+
 /// Encapsulates a request from a Brillig VM process that encounters a [foreign call opcode][acir::brillig_vm::Opcode::ForeignCall]
 /// where the result of the foreign call has not yet been provided.
 ///

noir/noir-repo/acvm-repo/acvm_js/build.sh

@@ -25,7 +25,7 @@ function run_if_available {
 require_command jq
 require_command cargo
 require_command wasm-bindgen
-#require_command wasm-opt
+require_command wasm-opt
 
 self_path=$(dirname "$(readlink -f "$0")")
 pname=$(cargo read-manifest | jq -r '.name')

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

@@ -36,6 +36,7 @@ use self::{
 };
 
 mod acir_gen;
+mod checks;
 pub(super) mod function_builder;
 pub mod ir;
 mod opt;

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -976,16 +976,23 @@ impl<'a> Context<'a> {
             }
         };
 
-        if self.handle_constant_index(instruction, dfg, index, array, store_value)? {
+        let array_id = dfg.resolve(array);
+        let array_typ = dfg.type_of_value(array_id);
+        // Compiler sanity checks
+        assert!(!array_typ.is_nested_slice(), "ICE: Nested slice type has reached ACIR generation");
+        let (Type::Array(_, _) | Type::Slice(_)) = &array_typ else {
+            unreachable!("ICE: expected array or slice type");
+        };
+
+        if self.handle_constant_index_wrapper(instruction, dfg, array, index, store_value)? {
             return Ok(());
         }
 
         // Get an offset such that the type of the array at the offset is the same as the type at the 'index'
         // If we find one, we will use it when computing the index under the enable_side_effect predicate
         // If not, array_get(..) will use a fallback costing one multiplication in the worst case.
         // cf. https://github.com/noir-lang/noir/pull/4971
-        let array_id = dfg.resolve(array);
-        let array_typ = dfg.type_of_value(array_id);
+
         // For simplicity we compute the offset only for simple arrays
         let is_simple_array = dfg.instruction_results(instruction).len() == 1
             && can_omit_element_sizes_array(&array_typ);
@@ -1018,83 +1025,106 @@ impl<'a> Context<'a> {
         Ok(())
     }
 
-    /// Handle constant index: if there is no predicate and we have the array values,
-    /// we can perform the operation directly on the array
-    fn handle_constant_index(
+    fn handle_constant_index_wrapper(
         &mut self,
         instruction: InstructionId,
         dfg: &DataFlowGraph,
+        array: ValueId,
         index: ValueId,
-        array_id: ValueId,
         store_value: Option<ValueId>,
     ) -> Result<bool, RuntimeError> {
-        let index_const = dfg.get_numeric_constant(index);
-        let value_type = dfg.type_of_value(array_id);
+        let array_id = dfg.resolve(array);
+        let array_typ = dfg.type_of_value(array_id);
         // Compiler sanity checks
-        assert!(
-            !value_type.is_nested_slice(),
-            "ICE: Nested slice type has reached ACIR generation"
-        );
-        let (Type::Array(_, _) | Type::Slice(_)) = &value_type else {
+        assert!(!array_typ.is_nested_slice(), "ICE: Nested slice type has reached ACIR generation");
+        let (Type::Array(_, _) | Type::Slice(_)) = &array_typ else {
             unreachable!("ICE: expected array or slice type");
         };
 
         match self.convert_value(array_id, dfg) {
             AcirValue::Var(acir_var, _) => {
-                return Err(RuntimeError::InternalError(InternalError::Unexpected {
+                Err(RuntimeError::InternalError(InternalError::Unexpected {
                     expected: "an array value".to_string(),
                     found: format!("{acir_var:?}"),
                     call_stack: self.acir_context.get_call_stack(),
                 }))
             }
             AcirValue::Array(array) => {
-                if let Some(index_const) = index_const {
-                    let array_size = array.len();
-                    let index = match index_const.try_to_u64() {
-                        Some(index_const) => index_const as usize,
-                        None => {
-                            let call_stack = self.acir_context.get_call_stack();
-                            return Err(RuntimeError::TypeConversion {
-                                from: "array index".to_string(),
-                                into: "u64".to_string(),
-                                call_stack,
-                            });
-                        }
-                    };
+                // `AcirValue::Array` supports reading/writing to constant indices at compile-time in some cases.
+                if let Some(constant_index) = dfg.get_numeric_constant(index) {
+                    let store_value = store_value.map(|value| self.convert_value(value, dfg));
+                    self.handle_constant_index(instruction, dfg, array, constant_index, store_value)
+                } else {
+                    Ok(false)
+                }
+            }
+            AcirValue::DynamicArray(_) => Ok(false),
+        }
+    }
 
-                    if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) {
-                        // Report the error if side effects are enabled.
-                        if index >= array_size {
-                            let call_stack = self.acir_context.get_call_stack();
-                            return Err(RuntimeError::IndexOutOfBounds {
-                                index,
-                                array_size,
-                                call_stack,
-                            });
-                        } else {
-                            let value = match store_value {
-                                Some(store_value) => {
-                                    let store_value = self.convert_value(store_value, dfg);
-                                    AcirValue::Array(array.update(index, store_value))
-                                }
-                                None => array[index].clone(),
-                            };
+    /// Handle constant index: if there is no predicate and we have the array values,
+    /// we can perform the operation directly on the array
+    fn handle_constant_index(
+        &mut self,
+        instruction: InstructionId,
+        dfg: &DataFlowGraph,
+        array: Vector<AcirValue>,
+        index: FieldElement,
+        store_value: Option<AcirValue>,
+    ) -> Result<bool, RuntimeError> {
+        let array_size: usize = array.len();
+        let index = match index.try_to_u64() {
+            Some(index_const) => index_const as usize,
+            None => {
+                let call_stack = self.acir_context.get_call_stack();
+                return Err(RuntimeError::TypeConversion {
+                    from: "array index".to_string(),
+                    into: "u64".to_string(),
+                    call_stack,
+                });
+            }
+        };
 
-                            self.define_result(dfg, instruction, value);
-                            return Ok(true);
-                        }
-                    }
-                    // If there is a predicate and the index is not out of range, we can directly perform the read
-                    else if index < array_size && store_value.is_none() {
-                        self.define_result(dfg, instruction, array[index].clone());
-                        return Ok(true);
-                    }
+        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(),
+                };
+
+                self.define_result(dfg, instruction, value);
+                Ok(true)
+            }
+            (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)
                 }
             }
-            AcirValue::DynamicArray(_) => (),
-        };
 
-        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),
+        }
     }
 
     /// We need to properly setup the inputs for array operations in ACIR.

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/check_for_underconstrained_values.rs → noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs

@@ -105,7 +105,7 @@ impl Context {
             .iter()
             .chain(function.returns())
             .filter(|id| function.dfg.get_numeric_constant(**id).is_none())
-            .copied();
+            .map(|value_id| function.dfg.resolve(*value_id));
 
         let mut connected_sets_indices: HashSet<usize> = HashSet::new();
 
@@ -169,13 +169,13 @@ impl Context {
             // Insert non-constant instruction arguments
             function.dfg[*instruction].for_each_value(|value_id| {
                 if function.dfg.get_numeric_constant(value_id).is_none() {
-                    instruction_arguments_and_results.insert(value_id);
+                    instruction_arguments_and_results.insert(function.dfg.resolve(value_id));
                 }
             });
             // And non-constant results
             for value_id in function.dfg.instruction_results(*instruction).iter() {
                 if function.dfg.get_numeric_constant(*value_id).is_none() {
-                    instruction_arguments_and_results.insert(*value_id);
+                    instruction_arguments_and_results.insert(function.dfg.resolve(*value_id));
                 }
             }
 

noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/mod.rs

@@ -0,0 +1 @@
+mod check_for_underconstrained_values;

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -379,9 +379,17 @@ impl Instruction {
             {
                 true
             }
-            Instruction::EnableSideEffects { .. }
-            | Instruction::ArrayGet { .. }
-            | Instruction::ArraySet { .. } => true,
+
+            // `ArrayGet`s which read from "known good" indices from an array don't need a predicate.
+            Instruction::ArrayGet { array, index } => {
+                #[allow(clippy::match_like_matches_macro)]
+                match (dfg.type_of_value(*array), dfg.get_numeric_constant(*index)) {
+                    (Type::Array(_, len), Some(index)) if index.to_u128() < (len as u128) => false,
+                    _ => true,
+                }
+            }
+
+            Instruction::EnableSideEffects { .. } | Instruction::ArraySet { .. } => true,
 
             Instruction::Call { func, .. } => match dfg[*func] {
                 Value::Function(_) => true,