[1 changes] feat: configurable external check failures (noir-lang/noi…

…r#6810)

chore: move constant creation out of loop (noir-lang/noir#6836)
fix: implement `as_field` and `from_field` in the interpreter (noir-lang/noir#6829)
chore: Use Vec for callstacks (noir-lang/noir#6821)
feat: replace `eval_global_as_array_length` with type/interpreter evaluation (noir-lang/noir#6469)
chore: refactor `DataFlowGraph.insert_instruction_and_results` (noir-lang/noir#6823)
chore(docs): updating noirjs tutorial for 1.0.0 (noir-lang/noir#6792)
feat: Sync from aztec-packages (noir-lang/noir#6824)
chore: Have rust-analyzer use the stable toolchain (noir-lang/noir#6825)

.noir-sync-commit

@@ -1 +1 @@
-f337992de96ef656681ebfc96a30c2c9c9b82a70
+73ccd45590222fc82642a6a9aa657c2915fc2c58

noir/noir-repo/.github/critical_libraries_status/README.md

@@ -0,0 +1,20 @@
+# Critical Libraries Status
+
+This directory contains one `.failures.jsonl` file per external directory that is checked by CI.
+CI will run the external repository tests and compare the test failures against those recorded
+in these files. If there's a difference, CI will fail.
+
+This allows us to mark some tests as expected to fail if we introduce breaking changes.
+When tests are fixed on the external repository, CI will let us know that we need to remove
+the `.failures.jsonl` failures on our side.
+
+The format of the `.failures.jsonl` files is one JSON per line with a failure:
+
+```json
+{"suite":"one","name":"foo"}
+```
+
+If it's expected that an external repository doesn't compile (because a PR introduces breaking changes
+to, say, the type system) you can remove the `.failures.jsonl` file for that repository and CI
+will pass again. Once the repository compiles again, CI will let us know and require us to put
+back the `.failures.jsonl` file.

noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl

@@ -0,0 +1,4 @@
+{"event":"started","name":"one","test_count":1,"type":"suite"}
+{"event":"started","name":"foo","suite":"one","type":"test"}
+{"event":"failed","exec_time":0.05356625,"name":"foo","suite":"one","type":"test"}
+{"event":"ok","failed":0,"ignored":0,"passed":1,"type":"suite"}

noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq

@@ -0,0 +1 @@
+{"suite":"one","name":"foo"}

noir/noir-repo/.github/scripts/check_test_results.sh

@@ -0,0 +1,38 @@
+#!/bin/bash
+set -eu
+
+# Usage: ./check_test_results.sh <expected.json> <actual.jsonl>
+# Compares the output of two test results of the same repository.
+# If any of the files doesn't exist or is empty, the script will consider that the test suite
+# couldn't be compiled.
+
+function process_json_lines() {
+  cat $1 | jq -c 'select(.type == "test" and .event == "failed") | {suite: .suite, name: .name}' | jq -s -c 'sort_by(.suite, .name) | .[]' > $1.jq
+}
+
+if [ -f $1 ] && [ -f $2 ]; then
+  # Both files exist, let's compare them
+  $(process_json_lines $2)
+  if ! diff $1 $2.jq; then
+    echo "Error: test failures don't match expected failures"
+    echo "Lines prefixed with '>' are new test failures (you could add them to '$1')"
+    echo "Lines prefixed with '<' are tests that were expected to fail but passed (you could remove them from '$1')"
+  fi
+elif [ -f $1 ]; then
+  # Only the expected file exists, which means the actual test couldn't be compiled.
+  echo "Error: external library tests couldn't be compiled."
+  echo "You could rename '$1' to '$1.does_not_compile' if it's expected that the external library can't be compiled."
+  exit -1
+elif [ -f $2 ]; then
+  # Only the actual file exists, which means we are expecting the external library
+  # not to compile but it did.
+  echo "Error: expected external library not to compile, but it did."
+  echo "You could create '$1' with these contents:"
+  $(process_json_lines $2)
+  cat $2.jq
+  exit -1
+else
+  # Both files don't exists, which means we are expecting the external library not
+  # to compile, and it didn't, so all is good.
+  exit 0
+fi

noir/noir-repo/.github/workflows/reports.yml

@@ -490,49 +490,3 @@ jobs:
         with:
           header: memory
           message: ${{ steps.memory_report.outputs.markdown }}
-
-  generate_compilation_report:
-    name: Compilation time
-    needs: [build-nargo]
-    runs-on: ubuntu-22.04
-    permissions:
-      pull-requests: write
-
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Download nargo binary
-        uses: actions/download-artifact@v4
-        with:
-          name: nargo
-          path: ./nargo
-
-      - name: Set nargo on PATH
-        run: |
-          nargo_binary="${{ github.workspace }}/nargo/nargo"
-          chmod +x $nargo_binary
-          echo "$(dirname $nargo_binary)" >> $GITHUB_PATH
-          export PATH="$PATH:$(dirname $nargo_binary)"
-          nargo -V
-
-      - name: Generate Compilation report
-        working-directory: ./test_programs
-        run: |
-          ./compilation_report.sh
-          mv compilation_report.json ../compilation_report.json
-
-      - name: Parse compilation report
-        id: compilation_report
-        uses: noir-lang/noir-bench-report@0d7464a8c39170523932d7846b6e6b458a294aea
-        with:
-          report: compilation_report.json
-          header: |
-            # Compilation Report
-          memory_report: false
-
-      - name: Add memory report to sticky comment
-        if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
-        uses: marocchino/sticky-pull-request-comment@v2
-        with:
-          header: compilation
-          message: ${{ steps.compilation_report.outputs.markdown }}

noir/noir-repo/.github/workflows/test-js-packages.yml

@@ -543,8 +543,6 @@ jobs:
   external-repo-checks:
     needs: [build-nargo, critical-library-list]
     runs-on: ubuntu-22.04
-    # Only run when 'run-external-checks' label is present
-    if: contains(github.event.pull_request.labels.*.name, 'run-external-checks')
     timeout-minutes: 30
     strategy:
       fail-fast: false
@@ -557,11 +555,17 @@ jobs:
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/parity-lib }
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-lib }
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/reset-kernel-lib }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-lib }
           - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/types }
+          # Use 1 test threads for rollup-lib because each test requires a lot of memory, and multiple ones in parallel exceed the maximum memory limit.
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-lib, nargo_args: "--test-threads 1" }
 
     name: Check external repo - ${{ matrix.project.repo }}/${{ matrix.project.path }}
     steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          path: noir-repo
+
       - name: Checkout
         uses: actions/checkout@v4
         with:
@@ -592,9 +596,14 @@ jobs:
       
       - name: Run nargo test
         working-directory: ./test-repo/${{ matrix.project.path }}
-        run: nargo test -q --silence-warnings
+        run: |
+          out=$(nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }}) && echo "$out" > ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
         env:
           NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true
+
+      - name: Compare test results
+        working-directory: ./noir-repo
+        run: .github/scripts/check_test_results.sh .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.failures.jsonl .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
 
   # This is a job which depends on all test jobs and reports the overall status.
   # This allows us to add/remove test jobs without having to update the required workflows.

noir/noir-repo/.vscode/settings.json

@@ -7,5 +7,6 @@
     },
     "[javascript]": {
         "editor.defaultFormatter": "esbenp.prettier-vscode"
-    }
+    },
+    "rust-analyzer.server.extraEnv": { "RUSTUP_TOOLCHAIN": "stable" }
 }

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/integration-tests/package.json

@@ -13,7 +13,7 @@
     "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore  --max-warnings 0"
   },
   "dependencies": {
-    "@aztec/bb.js": "portal:../../../../barretenberg/ts",
+    "@aztec/bb.js": "0.66.0",
     "@noir-lang/noir_js": "workspace:*",
     "@noir-lang/noir_wasm": "workspace:*",
     "@nomicfoundation/hardhat-chai-matchers": "^2.0.0",

noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs

@@ -2905,7 +2905,6 @@ mod test {
         ssa::{
             function_builder::FunctionBuilder,
             ir::{
-                call_stack::CallStack,
                 function::FunctionId,
                 instruction::BinaryOp,
                 map::Id,
@@ -2932,8 +2931,7 @@ mod test {
             builder.new_function("foo".into(), foo_id, inline_type);
         }
         // Set a call stack for testing whether `brillig_locations` in the `GeneratedAcir` was accurately set.
-        let mut stack = CallStack::unit(Location::dummy());
-        stack.push_back(Location::dummy());
+        let stack = vec![Location::dummy(), Location::dummy()];
         let call_stack =
             builder.current_function.dfg.call_stack_data.get_or_insert_locations(stack);
         builder.set_call_stack(call_stack);

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

@@ -201,7 +201,7 @@ impl RuntimeError {
             RuntimeError::UnknownLoopBound { .. } => {
                 let primary_message = self.to_string();
                 let location =
-                    self.call_stack().back().expect("Expected RuntimeError to have a location");
+                    self.call_stack().last().expect("Expected RuntimeError to have a location");
 
                 Diagnostic::simple_error(
                     primary_message,
@@ -212,7 +212,7 @@ impl RuntimeError {
             _ => {
                 let message = self.to_string();
                 let location =
-                    self.call_stack().back().unwrap_or_else(|| panic!("Expected RuntimeError to have a location. Error message: {message}"));
+                    self.call_stack().last().unwrap_or_else(|| panic!("Expected RuntimeError to have a location. Error message: {message}"));
 
                 Diagnostic::simple_error(message, String::new(), location.span)
             }

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

@@ -319,7 +319,6 @@ impl DependencyContext {
                         Value::Function(callee) => match all_functions[&callee].runtime() {
                             RuntimeType::Brillig(_) => {
                                 // Record arguments/results for each Brillig call for the check
-
                                 self.tainted.insert(
                                     *instruction,
                                     BrilligTaintedIds::new(&arguments, &results),

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

@@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize};
 
 use noirc_errors::Location;
 
-pub(crate) type CallStack = im::Vector<Location>;
+pub(crate) type CallStack = Vec<Location>;
 
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub(crate) struct CallStackId(u32);
@@ -57,9 +57,9 @@ impl Default for CallStackHelper {
 impl CallStackHelper {
     /// Construct a CallStack from a CallStackId
     pub(crate) fn get_call_stack(&self, mut call_stack: CallStackId) -> CallStack {
-        let mut result = im::Vector::new();
+        let mut result = Vec::new();
         while let Some(parent) = self.locations[call_stack.index()].parent {
-            result.push_back(self.locations[call_stack.index()].value);
+            result.push(self.locations[call_stack.index()].value);
             call_stack = parent;
         }
         result

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

@@ -165,6 +165,36 @@ impl DataFlowGraph {
         id
     }
 
+    fn insert_instruction_without_simplification(
+        &mut self,
+        instruction_data: Instruction,
+        block: BasicBlockId,
+        ctrl_typevars: Option<Vec<Type>>,
+        call_stack: CallStackId,
+    ) -> InstructionId {
+        let id = self.make_instruction(instruction_data, ctrl_typevars);
+        self.blocks[block].insert_instruction(id);
+        self.locations.insert(id, call_stack);
+        id
+    }
+
+    pub(crate) fn insert_instruction_and_results_without_simplification(
+        &mut self,
+        instruction_data: Instruction,
+        block: BasicBlockId,
+        ctrl_typevars: Option<Vec<Type>>,
+        call_stack: CallStackId,
+    ) -> InsertInstructionResult {
+        let id = self.insert_instruction_without_simplification(
+            instruction_data,
+            block,
+            ctrl_typevars,
+            call_stack,
+        );
+
+        InsertInstructionResult::Results(id, self.instruction_results(id))
+    }
+
     /// Inserts a new instruction at the end of the given block and returns its results
     pub(crate) fn insert_instruction_and_results(
         &mut self,
@@ -184,7 +214,8 @@ impl DataFlowGraph {
             result @ (SimplifyResult::SimplifiedToInstruction(_)
             | SimplifyResult::SimplifiedToInstructionMultiple(_)
             | SimplifyResult::None) => {
-                let instructions = result.instructions().unwrap_or(vec![instruction]);
+                let mut instructions = result.instructions().unwrap_or(vec![instruction]);
+                assert!(!instructions.is_empty(), "`SimplifyResult::SimplifiedToInstructionMultiple` must not return empty vector");
 
                 if instructions.len() > 1 {
                     // There's currently no way to pass results from one instruction in `instructions` on to the next.
@@ -196,17 +227,22 @@ impl DataFlowGraph {
                     );
                 }
 
-                let mut last_id = None;
-
+                // Pull off the last instruction as we want to return its results.
+                let last_instruction = instructions.pop().expect("`instructions` can't be empty");
                 for instruction in instructions {
-                    let id = self.make_instruction(instruction, ctrl_typevars.clone());
-                    self.blocks[block].insert_instruction(id);
-                    self.locations.insert(id, call_stack);
-                    last_id = Some(id);
+                    self.insert_instruction_without_simplification(
+                        instruction,
+                        block,
+                        ctrl_typevars.clone(),
+                        call_stack,
+                    );
                 }
-
-                let id = last_id.expect("There should be at least 1 simplified instruction");
-                InsertInstructionResult::Results(id, self.instruction_results(id))
+                self.insert_instruction_and_results_without_simplification(
+                    last_instruction,
+                    block,
+                    ctrl_typevars,
+                    call_stack,
+                )
             }
         }
     }