diff --git a/.noir-sync-commit b/.noir-sync-commit index a6780a1e9ef5..ab7c61a3bd70 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -f337992de96ef656681ebfc96a30c2c9c9b82a70 \ No newline at end of file +913be5b013323449963d31ab00f521f572cfcd67 diff --git a/noir/noir-repo/test_programs/execution_success/cast_and_shift_global/Prover.toml b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/aztec-nr.failures.jsonl similarity index 100% rename from noir/noir-repo/test_programs/execution_success/cast_and_shift_global/Prover.toml rename to noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/aztec-nr.failures.jsonl diff --git a/noir/noir-repo/test_programs/execution_success/trait_inheritance/Prover.toml b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-contracts.failures.jsonl similarity index 100% rename from noir/noir-repo/test_programs/execution_success/trait_inheritance/Prover.toml rename to noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-contracts.failures.jsonl diff --git a/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/blob.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/blob.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/parity-lib.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/parity-lib.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/private-kernel-lib.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/private-kernel-lib.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/rollup-lib.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/rollup-lib.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/types.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/types.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/README.md b/noir/noir-repo/.github/critical_libraries_status/README.md new file mode 100644 index 000000000000..47e9d7ad6ed3 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl new file mode 100644 index 000000000000..cb56f792778c --- /dev/null +++ b/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"} \ No newline at end of file diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq b/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq new file mode 100644 index 000000000000..1123e7e68e4e --- /dev/null +++ b/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq @@ -0,0 +1 @@ +{"suite":"one","name":"foo"} diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl.jq b/noir/noir-repo/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl.jq new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/eddsa/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/eddsa/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/mimc/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/mimc/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir-bignum/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir-bignum/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir-edwards/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir-edwards/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_base64/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_base64/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_bigcurve/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_bigcurve/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_rsa/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_rsa/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_sort/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_sort/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_string_search/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/noir_string_search/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/schnorr/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/schnorr/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/critical_libraries_status/noir-lang/sparse_array/.failures.jsonl b/noir/noir-repo/.github/critical_libraries_status/noir-lang/sparse_array/.failures.jsonl new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/noir/noir-repo/.github/scripts/check_test_results.sh b/noir/noir-repo/.github/scripts/check_test_results.sh new file mode 100755 index 000000000000..c844c025876d --- /dev/null +++ b/noir/noir-repo/.github/scripts/check_test_results.sh @@ -0,0 +1,38 @@ +#!/bin/bash +set -eu + +# Usage: ./check_test_results.sh +# 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 \ No newline at end of file diff --git a/noir/noir-repo/.github/workflows/reports.yml b/noir/noir-repo/.github/workflows/reports.yml index 1e355dc9e6bb..e8a229843187 100644 --- a/noir/noir-repo/.github/workflows/reports.yml +++ b/noir/noir-repo/.github/workflows/reports.yml @@ -76,7 +76,7 @@ jobs: - name: Compare gates reports id: gates_diff - uses: noir-lang/noir-gates-diff@1931aaaa848a1a009363d6115293f7b7fc72bb87 + uses: noir-lang/noir-gates-diff@84ada11295b9a1e1da7325af4e45e2db9f775175 with: report: gates_report.json summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) @@ -121,7 +121,7 @@ jobs: - name: Compare Brillig bytecode size reports id: brillig_bytecode_diff - uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48 + uses: noir-lang/noir-gates-diff@84ada11295b9a1e1da7325af4e45e2db9f775175 with: report: gates_report_brillig.json header: | @@ -170,7 +170,7 @@ jobs: - name: Compare Brillig execution reports id: brillig_execution_diff - uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48 + uses: noir-lang/noir-gates-diff@84ada11295b9a1e1da7325af4e45e2db9f775175 with: report: gates_report_brillig_execution.json header: | @@ -225,8 +225,8 @@ jobs: retention-days: 3 overwrite: true - generate_compilation_report: - name: Compilation time + generate_compilation_and_execution_report: + name: Compilation and execution time needs: [build-nargo] runs-on: ubuntu-22.04 permissions: @@ -252,10 +252,15 @@ jobs: - name: Generate Compilation report working-directory: ./test_programs run: | - ./compilation_report.sh - cat compilation_report.json + ./compilation_report.sh 0 1 mv compilation_report.json ../compilation_report.json + - name: Generate Execution report + working-directory: ./test_programs + run: | + ./execution_report.sh 0 1 + mv execution_report.json ../execution_report.json + - name: Upload compilation report uses: actions/upload-artifact@v4 with: @@ -263,8 +268,16 @@ jobs: path: compilation_report.json retention-days: 3 overwrite: true + + - name: Upload execution report + uses: actions/upload-artifact@v4 + with: + name: in_progress_execution_report + path: execution_report.json + retention-days: 3 + overwrite: true - external_repo_compilation_report: + external_repo_compilation_and_execution_report: needs: [build-nargo] runs-on: ubuntu-latest timeout-minutes: 15 @@ -272,15 +285,15 @@ jobs: fail-fast: false matrix: include: - - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts } - - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/parity-root } - - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner } - - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail } - - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts, is_library: true } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/parity-root, take_average: true } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner, take_average: true } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail, take_average: true } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset, take_average: true } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public } - name: External repo compilation report - ${{ matrix.project.repo }}/${{ matrix.project.path }} + name: External repo compilation and execution reports - ${{ matrix.project.repo }}/${{ matrix.project.path }} steps: - name: Download nargo binary uses: actions/download-artifact@v4 @@ -302,6 +315,13 @@ jobs: sparse-checkout: | test_programs/compilation_report.sh sparse-checkout-cone-mode: false + + - uses: actions/checkout@v4 + with: + path: scripts + sparse-checkout: | + test_programs/execution_report.sh + sparse-checkout-cone-mode: false - name: Checkout uses: actions/checkout@v4 @@ -310,15 +330,67 @@ jobs: path: test-repo ref: ${{ matrix.project.ref }} - - name: Generate compilation report + - name: Generate compilation report without averages working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ !matrix.project.take_average }} run: | mv /home/runner/work/noir/noir/scripts/test_programs/compilation_report.sh ./compilation_report.sh chmod +x ./compilation_report.sh ./compilation_report.sh 1 + + - name: Generate execution report + working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ !matrix.project.is_library }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh + ./execution_report.sh 1 + + - name: Generate compilation report with averages + working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ matrix.project.take_average }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/compilation_report.sh ./compilation_report.sh + chmod +x ./compilation_report.sh + ./compilation_report.sh 1 1 + + - name: Generate execution report without averages + working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ !matrix.project.is_library && !matrix.project.take_average }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh + ./execution_report.sh 1 + + - name: Generate execution report with averages + working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ !matrix.project.is_library && matrix.project.take_average }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh + ./execution_report.sh 1 1 + + - name: Generate compilation report with averages + working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ matrix.project.take_average }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/compilation_report.sh ./compilation_report.sh + chmod +x ./compilation_report.sh + ./compilation_report.sh 1 1 + + - name: Generate execution report without averages + working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ !matrix.project.is_library && !matrix.project.take_average }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh + ./execution_report.sh 1 + + - name: Generate execution report with averages + working-directory: ./test-repo/${{ matrix.project.path }} + if: ${{ !matrix.project.is_library && matrix.project.take_average }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh + ./execution_report.sh 1 1 - name: Move compilation report - id: report + id: compilation_report shell: bash run: | PACKAGE_NAME=${{ matrix.project.path }} @@ -326,18 +398,36 @@ jobs: mv ./test-repo/${{ matrix.project.path }}/compilation_report.json ./compilation_report_$PACKAGE_NAME.json echo "compilation_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT + - name: Move execution report + id: execution_report + shell: bash + if: ${{ !matrix.project.is_library }} + run: | + PACKAGE_NAME=${{ matrix.project.path }} + PACKAGE_NAME=$(basename $PACKAGE_NAME) + mv ./test-repo/${{ matrix.project.path }}/execution_report.json ./execution_report_$PACKAGE_NAME.json + echo "execution_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT + - name: Upload compilation report uses: actions/upload-artifact@v4 with: - name: compilation_report_${{ steps.report.outputs.compilation_report_name }} - path: compilation_report_${{ steps.report.outputs.compilation_report_name }}.json + name: compilation_report_${{ steps.compilation_report.outputs.compilation_report_name }} + path: compilation_report_${{ steps.compilation_report.outputs.compilation_report_name }}.json + retention-days: 3 + overwrite: true + + - name: Upload execution report + uses: actions/upload-artifact@v4 + with: + name: execution_report_${{ steps.execution_report.outputs.execution_report_name }} + path: execution_report_${{ steps.execution_report.outputs.execution_report_name }}.json retention-days: 3 overwrite: true upload_compilation_report: name: Upload compilation report - needs: [generate_compilation_report, external_repo_compilation_report] - # We want this job to run even if one variation of the matrix in `external_repo_compilation_report` fails + needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report] + # We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails if: always() runs-on: ubuntu-latest permissions: @@ -364,7 +454,7 @@ jobs: - name: Parse compilation report id: compilation_report - uses: noir-lang/noir-bench-report@0d7464a8c39170523932d7846b6e6b458a294aea + uses: noir-lang/noir-bench-report@e408e131e96c3615b4f820d7d642360fb4d6e2f4 with: report: compilation_report.json header: | @@ -477,7 +567,7 @@ jobs: - name: Parse memory report id: memory_report - uses: noir-lang/noir-bench-report@0d7464a8c39170523932d7846b6e6b458a294aea + uses: noir-lang/noir-bench-report@e408e131e96c3615b4f820d7d642360fb4d6e2f4 with: report: memory_report.json header: | @@ -491,48 +581,47 @@ jobs: header: memory message: ${{ steps.memory_report.outputs.markdown }} - generate_compilation_report: - name: Compilation time - needs: [build-nargo] - runs-on: ubuntu-22.04 + upload_execution_report: + name: Upload execution report + needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report] + # We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails + if: always() + runs-on: ubuntu-latest permissions: pull-requests: write steps: - uses: actions/checkout@v4 - - name: Download nargo binary + - name: Download initial execution report uses: actions/download-artifact@v4 with: - name: nargo - path: ./nargo + name: in_progress_execution_report - - 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: Download matrix execution reports + uses: actions/download-artifact@v4 + with: + pattern: execution_report_* + path: ./reports - - name: Generate Compilation report - working-directory: ./test_programs + - name: Merge execution reports using jq run: | - ./compilation_report.sh - mv compilation_report.json ../compilation_report.json + mv ./.github/scripts/merge-bench-reports.sh merge-bench-reports.sh + ./merge-bench-reports.sh execution_report - - name: Parse compilation report - id: compilation_report - uses: noir-lang/noir-bench-report@0d7464a8c39170523932d7846b6e6b458a294aea + - name: Parse execution report + id: execution_report + uses: noir-lang/noir-bench-report@e408e131e96c3615b4f820d7d642360fb4d6e2f4 with: - report: compilation_report.json + report: execution_report.json header: | - # Compilation Report - memory_report: false + # Execution Report + execution_report: true - 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 }} + header: execution_time + message: ${{ steps.execution_report.outputs.markdown }} + diff --git a/noir/noir-repo/.github/workflows/test-js-packages.yml b/noir/noir-repo/.github/workflows/test-js-packages.yml index d227b4eb00b0..e593389a9712 100644 --- a/noir/noir-repo/.github/workflows/test-js-packages.yml +++ b/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. diff --git a/noir/noir-repo/.gitignore b/noir/noir-repo/.gitignore index 8442d688fbf8..9bfb8a89331b 100644 --- a/noir/noir-repo/.gitignore +++ b/noir/noir-repo/.gitignore @@ -36,6 +36,7 @@ gates_report.json gates_report_brillig.json gates_report_brillig_execution.json compilation_report.json +execution_report.json # Github Actions scratch space # This gives a location to download artifacts into the repository in CI without making git dirty. diff --git a/noir/noir-repo/.vscode/settings.json b/noir/noir-repo/.vscode/settings.json index fb8ea527881d..cd1c5f886dfa 100644 --- a/noir/noir-repo/.vscode/settings.json +++ b/noir/noir-repo/.vscode/settings.json @@ -7,5 +7,6 @@ }, "[javascript]": { "editor.defaultFormatter": "esbenp.prettier-vscode" - } + }, + "rust-analyzer.server.extraEnv": { "RUSTUP_TOOLCHAIN": "stable" } } diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index f14fb841372a..bb45ea80517f 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -3184,6 +3184,7 @@ dependencies = [ "serde_json", "serde_with", "similar-asserts", + "smallvec", "test-case", "thiserror", "tracing", @@ -4487,6 +4488,9 @@ name = "smallvec" version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" +dependencies = [ + "serde", +] [[package]] name = "smawk" diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs index 700589d20409..d0ec7d02201c 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs @@ -42,13 +42,13 @@ pub enum BlackBoxFunc { /// https://tools.ietf.org/html/rfc7693 /// - inputs are a byte array, i.e a vector of (witness, 8) /// - output is a byte array of length 32, i.e. an array of 32 - /// (witness, 8), constrained to be the blake2s of the inputs. + /// (witness, 8), constrained to be the blake2s of the inputs. Blake2s, /// Computes the Blake3 hash of the inputs /// - inputs are a byte array, i.e a vector of (witness, 8) /// - output is a byte array of length 32, i.e an array of 32 - /// (witness, 8), constrained to be the blake3 of the inputs. + /// (witness, 8), constrained to be the blake3 of the inputs. Blake3, /// Verifies a ECDSA signature over the secp256k1 curve. diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs index 4ff581bf17ab..e651e6998a4a 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs @@ -281,7 +281,7 @@ impl Deserialize<'a>> Program { where D: Deserializer<'de>, { - let bytecode_b64: String = serde::Deserialize::deserialize(deserializer)?; + let bytecode_b64: String = Deserialize::deserialize(deserializer)?; let program_bytes = base64::engine::general_purpose::STANDARD .decode(bytecode_b64) .map_err(D::Error::custom)?; diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs index f47c40b0dd79..dec58b5f90b9 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs @@ -47,6 +47,7 @@ pub enum Opcode { /// - **express a constraint** on witnesses; for instance to express that a /// witness `w` is a boolean, you can add the opcode: `w*w-w=0` /// - or, to **compute the value** of an arithmetic operation of some inputs. + /// /// For instance, to multiply two witnesses `x` and `y`, you would use the /// opcode `z-x*y=0`, which would constrain `z` to be `x*y`. /// diff --git a/noir/noir-repo/acvm-repo/acir/src/native_types/expression/mod.rs b/noir/noir-repo/acvm-repo/acir/src/native_types/expression/mod.rs index 2bbbc39d0ca9..cdb8974526fa 100644 --- a/noir/noir-repo/acvm-repo/acir/src/native_types/expression/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/native_types/expression/mod.rs @@ -85,6 +85,7 @@ impl Expression { /// /// - `mul_term` in an expression contains degree-2 terms /// - `linear_combinations` contains degree-1 terms + /// /// Hence, it is sufficient to check that there are no `mul_terms` /// /// Examples: @@ -98,11 +99,11 @@ impl Expression { /// Returns `true` if the expression can be seen as a degree-1 univariate polynomial /// /// - `mul_terms` in an expression can be univariate, however unless the coefficient - /// is zero, it is always degree-2. + /// is zero, it is always degree-2. /// - `linear_combinations` contains the sum of degree-1 terms, these terms do not - /// need to contain the same variable and so it can be multivariate. However, we - /// have thus far only checked if `linear_combinations` contains one term, so this - /// method will return false, if the `Expression` has not been simplified. + /// need to contain the same variable and so it can be multivariate. However, we + /// have thus far only checked if `linear_combinations` contains one term, so this + /// method will return false, if the `Expression` has not been simplified. /// /// Hence, we check in the simplest case if an expression is a degree-1 univariate, /// by checking if it contains no `mul_terms` and it contains one `linear_combination` term. diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs index 43e32101cc58..e95c6207c3c3 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs @@ -404,7 +404,7 @@ mod tests { ], q_c: FieldElement::zero(), }), - Opcode::BlackBoxFuncCall(acir::circuit::opcodes::BlackBoxFuncCall::RANGE { + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input: FunctionInput::witness(Witness(3), 32), }), ]; diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index ccad3510682f..7ce34dbc6fea 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -10,7 +10,7 @@ use crate::pwg::OpcodeResolutionError; /// Resolve BigInt opcodes by storing BigInt values (and their moduli) by their ID in the BigIntSolver /// - When it encounters a bigint operation opcode, it performs the operation on the stored values -/// and store the result using the provided ID. +/// and store the result using the provided ID. /// - When it gets a to_bytes opcode, it simply looks up the value and resolves the output witness accordingly. #[derive(Default)] pub(crate) struct AcvmBigIntSolver { diff --git a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs index 8b164b7c0f23..b43f7512b6e2 100644 --- a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs +++ b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs @@ -1457,7 +1457,7 @@ fn poseidon2_permutation_zeroes() { #[test] fn sha256_compression_zeros() { let results = solve_array_input_blackbox_call( - [(FieldElement::zero(), false); 24].try_into().unwrap(), + [(FieldElement::zero(), false); 24].into(), 8, None, sha256_compression_op, diff --git a/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs b/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs index 540862843ab0..dab611a81e83 100644 --- a/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs +++ b/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs @@ -8,7 +8,7 @@ use crate::BlackBoxResolutionError; /// Resolve BigInt opcodes by storing BigInt values (and their moduli) by their ID in a HashMap: /// - When it encounters a bigint operation opcode, it performs the operation on the stored values -/// and store the result using the provided ID. +/// and store the result using the provided ID. /// - When it gets a to_bytes opcode, it simply looks up the value and resolves the output witness accordingly. #[derive(Default, Debug, Clone, PartialEq, Eq)] diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs index 9318e4d2b5c0..1b311504b5ca 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/lib.rs @@ -150,6 +150,10 @@ pub struct CompileOptions { /// A lower value keeps the original program if it was smaller, even if it has more jumps. #[arg(long, hide = true, allow_hyphen_values = true)] pub max_bytecode_increase_percent: Option, + + /// Used internally to test for non-determinism in the compiler. + #[clap(long, hide = true)] + pub check_non_determinism: bool, } pub fn parse_expression_width(input: &str) -> Result { diff --git a/noir/noir-repo/compiler/noirc_errors/src/reporter.rs b/noir/noir-repo/compiler/noirc_errors/src/reporter.rs index e57775d9a7f8..20a765fdaa5f 100644 --- a/noir/noir-repo/compiler/noirc_errors/src/reporter.rs +++ b/noir/noir-repo/compiler/noirc_errors/src/reporter.rs @@ -230,7 +230,7 @@ pub fn report<'files>( let color_choice = if std::io::stderr().is_terminal() { ColorChoice::Auto } else { ColorChoice::Never }; let writer = StandardStream::stderr(color_choice); - let config = codespan_reporting::term::Config::default(); + let config = term::Config::default(); let stack_trace = stack_trace(files, &custom_diagnostic.call_stack); let diagnostic = convert_diagnostic(custom_diagnostic, file, stack_trace, deny_warnings); diff --git a/noir/noir-repo/compiler/noirc_evaluator/Cargo.toml b/noir/noir-repo/compiler/noirc_evaluator/Cargo.toml index 72fba8aadc2d..15531fafff7f 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/Cargo.toml +++ b/noir/noir-repo/compiler/noirc_evaluator/Cargo.toml @@ -28,6 +28,7 @@ tracing.workspace = true chrono = "0.4.37" rayon.workspace = true cfg-if.workspace = true +smallvec = { version = "1.13.2", features = ["serde"] } [dev-dependencies] proptest.workspace = true diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs index 78188ea2b657..bb277751b9e0 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs @@ -954,6 +954,7 @@ impl> AcirContext { offset: AcirVar, bits: u32, ) -> Result<(), RuntimeError> { + #[allow(unused_qualifications)] const fn num_bits() -> usize { std::mem::size_of::() * 8 } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/acir/generated_acir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/acir/generated_acir.rs index b6a5a817ea70..a2b161688c00 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/acir/generated_acir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/acir/generated_acir.rs @@ -497,7 +497,7 @@ impl GeneratedAcir { /// This implies that either `y` or `t` or both is `0`. /// - If `t == 0`, then by definition `t == 0`. /// - If `y == 0`, this does not mean anything at this point in time, due to it having no - /// constraints. + /// constraints. /// /// Naively, we could apply the following constraint: `y == 1 - t`. /// This along with the previous `y * t == 0` constraint means that @@ -604,7 +604,7 @@ impl GeneratedAcir { ) { // Check whether we have a call to this Brillig function already exists. // This helps us optimize the Brillig metadata to only be stored once per Brillig entry point. - let inserted_func_before = self.brillig_locations.get(&brillig_function_index).is_some(); + let inserted_func_before = self.brillig_locations.contains_key(&brillig_function_index); let opcode = AcirOpcode::BrilligCall { id: brillig_function_index, inputs, outputs, predicate }; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs index e7b011b6d7b1..95e0dd121321 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs @@ -1,7 +1,6 @@ //! This file holds the pass to convert from Noir's SSA IR to ACIR. use fxhash::FxHashMap as HashMap; -use im::Vector; use std::collections::{BTreeMap, HashSet}; use std::fmt::Debug; @@ -248,7 +247,7 @@ impl Debug for AcirDynamicArray { #[derive(Debug, Clone)] pub(crate) enum AcirValue { Var(AcirVar, AcirType), - Array(Vector), + Array(im::Vector), DynamicArray(AcirDynamicArray), } @@ -1118,7 +1117,7 @@ impl<'a> Context<'a> { &mut self, instruction: InstructionId, dfg: &DataFlowGraph, - array: Vector, + array: im::Vector, index: FieldElement, store_value: Option, ) -> Result { @@ -1303,7 +1302,7 @@ impl<'a> Context<'a> { match typ { Type::Numeric(_) => self.array_get_value(&Type::field(), call_data_block, offset), Type::Array(arc, len) => { - let mut result = Vector::new(); + let mut result = im::Vector::new(); for _i in 0..*len { for sub_type in arc.iter() { let element = self.get_from_call_data(offset, call_data_block, sub_type)?; @@ -1394,7 +1393,7 @@ impl<'a> Context<'a> { Ok(AcirValue::Var(read, typ)) } Type::Array(element_types, len) => { - let mut values = Vector::new(); + let mut values = im::Vector::new(); for _ in 0..len { for typ in element_types.as_ref() { values.push_back(self.array_get_value(typ, block_id, var_index)?); @@ -1682,7 +1681,7 @@ impl<'a> Context<'a> { let read = self.acir_context.read_from_memory(source, &index_var)?; Ok::(AcirValue::Var(read, AcirType::field())) })?; - let array: Vector = init_values.into(); + let array: im::Vector = init_values.into(); self.initialize_array(destination, array_len, Some(AcirValue::Array(array)))?; Ok(()) } @@ -2053,8 +2052,9 @@ impl<'a> Context<'a> { /// /// There are some edge cases to consider: /// - Constants are not explicitly type casted, so we need to check for this and - /// return the type of the other operand, if we have a constant. + /// return the type of the other operand, if we have a constant. /// - 0 is not seen as `Field 0` but instead as `Unit 0` + /// /// TODO: The latter seems like a bug, if we cannot differentiate between a function returning /// TODO nothing and a 0. /// @@ -2273,7 +2273,7 @@ impl<'a> Context<'a> { let slice = self.convert_value(slice_contents, dfg); let mut new_elem_size = Self::flattened_value_size(&slice); - let mut new_slice = Vector::new(); + let mut new_slice = im::Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; let elements_to_push = &arguments[2..]; @@ -2344,7 +2344,7 @@ impl<'a> Context<'a> { let one = self.acir_context.add_constant(FieldElement::one()); let new_slice_length = self.acir_context.add_var(slice_length, one)?; - let mut new_slice = Vector::new(); + let mut new_slice = im::Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; let elements_to_push = &arguments[2..]; @@ -2418,7 +2418,7 @@ impl<'a> Context<'a> { } let slice = self.convert_value(slice_contents, dfg); - let mut new_slice = Vector::new(); + let mut new_slice = im::Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; let mut results = vec![ @@ -2444,7 +2444,7 @@ impl<'a> Context<'a> { let slice = self.convert_value(slice_contents, dfg); - let mut new_slice = Vector::new(); + let mut new_slice = im::Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; let element_size = slice_typ.element_size(); @@ -2631,7 +2631,7 @@ impl<'a> Context<'a> { let slice_size = Self::flattened_value_size(&slice); - let mut new_slice = Vector::new(); + let mut new_slice = im::Vector::new(); self.slice_intrinsic_input(&mut new_slice, slice)?; // Compiler sanity check @@ -2760,8 +2760,6 @@ impl<'a> Context<'a> { unreachable!("Expected static_assert to be removed by this point") } Intrinsic::StrAsBytes => unreachable!("Expected as_bytes to be removed by this point"), - Intrinsic::FromField => unreachable!("Expected from_field to be removed by this point"), - Intrinsic::AsField => unreachable!("Expected as_field to be removed by this point"), Intrinsic::IsUnconstrained => { unreachable!("Expected is_unconstrained to be removed by this point") } @@ -2783,7 +2781,7 @@ impl<'a> Context<'a> { fn slice_intrinsic_input( &mut self, - old_slice: &mut Vector, + old_slice: &mut im::Vector, input: AcirValue, ) -> Result<(), RuntimeError> { match input { @@ -2905,7 +2903,6 @@ mod test { ssa::{ function_builder::FunctionBuilder, ir::{ - call_stack::CallStack, function::FunctionId, instruction::BinaryOp, map::Id, @@ -2932,8 +2929,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); @@ -3358,8 +3354,8 @@ mod test { // We have two normal Brillig functions that was called multiple times. // We should have a single locations map for each function's debug metadata. assert_eq!(main_acir.brillig_locations.len(), 2); - assert!(main_acir.brillig_locations.get(&BrilligFunctionId(0)).is_some()); - assert!(main_acir.brillig_locations.get(&BrilligFunctionId(1)).is_some()); + assert!(main_acir.brillig_locations.contains_key(&BrilligFunctionId(0))); + assert!(main_acir.brillig_locations.contains_key(&BrilligFunctionId(1))); } // Test that given multiple primitive operations that are represented by Brillig directives (e.g. invert/quotient), @@ -3494,7 +3490,7 @@ mod test { // We have one normal Brillig functions that was called twice. // We should have a single locations map for each function's debug metadata. assert_eq!(main_acir.brillig_locations.len(), 1); - assert!(main_acir.brillig_locations.get(&BrilligFunctionId(0)).is_some()); + assert!(main_acir.brillig_locations.contains_key(&BrilligFunctionId(0))); } // Test that given both normal Brillig calls, Brillig stdlib calls, and non-inlined ACIR calls, that we accurately generate ACIR. @@ -3587,7 +3583,7 @@ mod test { ); assert_eq!(main_acir.brillig_locations.len(), 1); - assert!(main_acir.brillig_locations.get(&BrilligFunctionId(0)).is_some()); + assert!(main_acir.brillig_locations.contains_key(&BrilligFunctionId(0))); let foo_acir = &acir_functions[1]; let foo_opcodes = foo_acir.opcodes(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 0b02cd29f0f4..5bcddc212758 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -635,9 +635,7 @@ impl<'block> BrilligBlock<'block> { let array = array.extract_register(); self.brillig_context.load_instruction(destination, array); } - Intrinsic::FromField - | Intrinsic::AsField - | Intrinsic::IsUnconstrained + Intrinsic::IsUnconstrained | Intrinsic::DerivePedersenGenerators | Intrinsic::ApplyRangeConstraint | Intrinsic::StrAsBytes diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index be4a6b84bc16..3654a95a03f3 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -27,7 +27,7 @@ pub(crate) struct GeneratedBrillig { pub(crate) locations: BTreeMap, pub(crate) error_types: BTreeMap, pub(crate) name: String, - pub(crate) procedure_locations: HashMap, + pub(crate) procedure_locations: BTreeMap, } #[derive(Default, Debug, Clone)] @@ -61,7 +61,7 @@ pub(crate) struct BrilligArtifact { /// This is created as artifacts are linked together and allows us to determine /// which opcodes originate from reusable procedures.s /// The range is inclusive for both start and end opcode locations. - pub(crate) procedure_locations: HashMap, + pub(crate) procedure_locations: BTreeMap, } /// A pointer to a location in the opcode. diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs b/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs index ee662c50b75c..1e484f8af591 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs +++ b/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) } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 48be8eb7ad85..2f167ec8ab3a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -294,11 +294,9 @@ impl DependencyContext { Intrinsic::ArrayLen | Intrinsic::ArrayRefCount | Intrinsic::ArrayAsStrUnchecked - | Intrinsic::AsField | Intrinsic::AsSlice | Intrinsic::BlackBox(..) | Intrinsic::DerivePedersenGenerators - | Intrinsic::FromField | Intrinsic::Hint(..) | Intrinsic::SlicePushBack | Intrinsic::SlicePushFront @@ -316,10 +314,9 @@ impl DependencyContext { self.update_children(&arguments, &results); } }, - Value::Function(callee) => match all_functions[&callee].runtime() { + 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), @@ -575,12 +572,10 @@ impl Context { Intrinsic::ArrayLen | Intrinsic::ArrayAsStrUnchecked | Intrinsic::ArrayRefCount - | Intrinsic::AsField | Intrinsic::AsSlice | Intrinsic::BlackBox(..) | Intrinsic::Hint(Hint::BlackBox) | Intrinsic::DerivePedersenGenerators - | Intrinsic::FromField | Intrinsic::SliceInsert | Intrinsic::SlicePushBack | Intrinsic::SlicePushFront @@ -596,7 +591,7 @@ impl Context { self.value_sets.push(instruction_arguments_and_results); } }, - Value::Function(callee) => match all_functions[&callee].runtime() { + Value::Function(callee) => match all_functions[callee].runtime() { RuntimeType::Brillig(_) => { // For calls to Brillig functions we memorize the mapping of results to argument ValueId's and InstructionId's // The latter are needed to produce the callstack later diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs index 48af34d466cf..068fff7d2840 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs @@ -252,7 +252,7 @@ impl FunctionBuilder { for size in ssa_param_sizes { let visibilities: Vec = flattened_params_databus_visibility.drain(0..size).collect(); - let visibility = visibilities.get(0).copied().unwrap_or(DatabusVisibility::None); + let visibility = visibilities.first().copied().unwrap_or(DatabusVisibility::None); assert!( visibilities.iter().all(|v| *v == visibility), "inconsistent databus visibility for ssa param" diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs index 113609f4a116..e1df616bc66b 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs +++ b/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; +pub(crate) type CallStack = Vec; #[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 diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 72ba369f98cf..9ccf7f11512a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -42,7 +42,7 @@ pub(crate) struct DataFlowGraph { /// Call instructions require the func signature, but /// other instructions may need some more reading on my part #[serde_as(as = "HashMap")] - results: HashMap>, + results: HashMap>, /// Storage for all of the values defined in this /// function. @@ -165,6 +165,36 @@ impl DataFlowGraph { id } + fn insert_instruction_without_simplification( + &mut self, + instruction_data: Instruction, + block: BasicBlockId, + ctrl_typevars: Option>, + 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>, + 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, + ) } } } @@ -306,16 +342,18 @@ impl DataFlowGraph { /// Returns the results of the instruction pub(crate) fn make_instruction_results( &mut self, - instruction_id: InstructionId, + instruction: InstructionId, ctrl_typevars: Option>, ) { - let result_types = self.instruction_result_types(instruction_id, ctrl_typevars); - let results = vecmap(result_types.into_iter().enumerate(), |(position, typ)| { - let instruction = instruction_id; - self.values.insert(Value::Instruction { typ, position, instruction }) + let mut results = smallvec::SmallVec::new(); + let mut position = 0; + self.for_each_instruction_result_type(instruction, ctrl_typevars, |this, typ| { + let result = this.values.insert(Value::Instruction { typ, position, instruction }); + position += 1; + results.push(result); }); - self.results.insert(instruction_id, results); + self.results.insert(instruction, results); } /// Return the result types of this instruction. @@ -326,18 +364,21 @@ impl DataFlowGraph { /// the type of an instruction that does not require them. Compared to passing an empty Vec, /// Option has the benefit of panicking if it is accidentally used for a Call instruction, /// rather than silently returning the empty Vec and continuing. - fn instruction_result_types( - &self, + fn for_each_instruction_result_type( + &mut self, instruction_id: InstructionId, ctrl_typevars: Option>, - ) -> Vec { + mut f: impl FnMut(&mut Self, Type), + ) { let instruction = &self.instructions[instruction_id]; match instruction.result_type() { - InstructionResultType::Known(typ) => vec![typ], - InstructionResultType::Operand(value) => vec![self.type_of_value(value)], - InstructionResultType::None => vec![], + InstructionResultType::Known(typ) => f(self, typ), + InstructionResultType::Operand(value) => f(self, self.type_of_value(value)), + InstructionResultType::None => (), InstructionResultType::Unknown => { - ctrl_typevars.expect("Control typevars required but not given") + for typ in ctrl_typevars.expect("Control typevars required but not given") { + f(self, typ); + } } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 3072bb1d72d6..5d9bfc89f614 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -43,9 +43,9 @@ pub(crate) type InstructionId = Id; /// These are similar to built-ins in other languages. /// These can be classified under two categories: /// - Opcodes which the IR knows the target machine has -/// special support for. (LowLevel) +/// special support for. (LowLevel) /// - Opcodes which have no function definition in the -/// source code and must be processed by the IR. +/// source code and must be processed by the IR. #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] pub(crate) enum Intrinsic { ArrayLen, @@ -65,8 +65,6 @@ pub(crate) enum Intrinsic { ToRadix(Endian), BlackBox(BlackBoxFunc), Hint(Hint), - FromField, - AsField, AsWitness, IsUnconstrained, DerivePedersenGenerators, @@ -97,8 +95,6 @@ impl std::fmt::Display for Intrinsic { Intrinsic::ToRadix(Endian::Little) => write!(f, "to_le_radix"), Intrinsic::BlackBox(function) => write!(f, "{function}"), Intrinsic::Hint(Hint::BlackBox) => write!(f, "black_box"), - Intrinsic::FromField => write!(f, "from_field"), - Intrinsic::AsField => write!(f, "as_field"), Intrinsic::AsWitness => write!(f, "as_witness"), Intrinsic::IsUnconstrained => write!(f, "is_unconstrained"), Intrinsic::DerivePedersenGenerators => write!(f, "derive_pedersen_generators"), @@ -140,8 +136,6 @@ impl Intrinsic { | Intrinsic::SlicePushFront | Intrinsic::SliceInsert | Intrinsic::StrAsBytes - | Intrinsic::FromField - | Intrinsic::AsField | Intrinsic::IsUnconstrained | Intrinsic::DerivePedersenGenerators | Intrinsic::FieldLessThan => false, @@ -213,8 +207,6 @@ impl Intrinsic { "to_be_radix" => Some(Intrinsic::ToRadix(Endian::Big)), "to_le_bits" => Some(Intrinsic::ToBits(Endian::Little)), "to_be_bits" => Some(Intrinsic::ToBits(Endian::Big)), - "from_field" => Some(Intrinsic::FromField), - "as_field" => Some(Intrinsic::AsField), "as_witness" => Some(Intrinsic::AsWitness), "is_unconstrained" => Some(Intrinsic::IsUnconstrained), "derive_pedersen_generators" => Some(Intrinsic::DerivePedersenGenerators), @@ -556,10 +548,25 @@ impl Instruction { /// If true the instruction will depend on `enable_side_effects` context during acir-gen. pub(crate) fn requires_acir_gen_predicate(&self, dfg: &DataFlowGraph) -> bool { match self { - Instruction::Binary(binary) - if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) => - { - true + Instruction::Binary(binary) => { + match binary.operator { + BinaryOp::Add + | BinaryOp::Sub + | BinaryOp::Mul + | BinaryOp::Div + | BinaryOp::Mod => { + // Some binary math can overflow or underflow, but this is only the case + // for unsigned types (here we assume the type of binary.lhs is the same) + dfg.type_of_value(binary.rhs).is_unsigned() + } + BinaryOp::Eq + | BinaryOp::Lt + | BinaryOp::And + | BinaryOp::Or + | BinaryOp::Xor + | BinaryOp::Shl + | BinaryOp::Shr => false, + } } Instruction::ArrayGet { array, index } => { @@ -577,7 +584,6 @@ impl Instruction { _ => false, }, Instruction::Cast(_, _) - | Instruction::Binary(_) | Instruction::Not(_) | Instruction::Truncate { .. } | Instruction::Constrain(_, _, _) @@ -952,9 +958,11 @@ impl Instruction { } } Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + let then_condition = dfg.resolve(*then_condition); + let else_condition = dfg.resolve(*else_condition); let typ = dfg.type_of_value(*then_value); - if let Some(constant) = dfg.get_numeric_constant(*then_condition) { + if let Some(constant) = dfg.get_numeric_constant(then_condition) { if constant.is_one() { return SimplifiedTo(*then_value); } else if constant.is_zero() { @@ -968,10 +976,51 @@ impl Instruction { return SimplifiedTo(then_value); } - if matches!(&typ, Type::Numeric(_)) { - let then_condition = *then_condition; - let else_condition = *else_condition; + if let Value::Instruction { instruction, .. } = &dfg[then_value] { + if let Instruction::IfElse { + then_condition: inner_then_condition, + then_value: inner_then_value, + else_condition: inner_else_condition, + .. + } = dfg[*instruction] + { + if then_condition == inner_then_condition { + let instruction = Instruction::IfElse { + then_condition, + then_value: inner_then_value, + else_condition: inner_else_condition, + else_value, + }; + return SimplifiedToInstruction(instruction); + } + // TODO: We could check to see if `then_condition == inner_else_condition` + // but we run into issues with duplicate NOT instructions having distinct ValueIds. + } + }; + + if let Value::Instruction { instruction, .. } = &dfg[else_value] { + if let Instruction::IfElse { + then_condition: inner_then_condition, + else_condition: inner_else_condition, + else_value: inner_else_value, + .. + } = dfg[*instruction] + { + if then_condition == inner_then_condition { + let instruction = Instruction::IfElse { + then_condition, + then_value, + else_condition: inner_else_condition, + else_value: inner_else_value, + }; + return SimplifiedToInstruction(instruction); + } + // TODO: We could check to see if `then_condition == inner_else_condition` + // but we run into issues with duplicate NOT instructions having distinct ValueIds. + } + }; + if matches!(&typ, Type::Numeric(_)) { let result = ValueMerger::merge_numeric_values( dfg, block, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index fa7d314ff332..6da4c7702c8c 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -330,31 +330,6 @@ pub(super) fn simplify_call( Intrinsic::BlackBox(bb_func) => { simplify_black_box_func(bb_func, arguments, dfg, block, call_stack) } - Intrinsic::AsField => { - let instruction = Instruction::Cast(arguments[0], NumericType::NativeField); - SimplifyResult::SimplifiedToInstruction(instruction) - } - Intrinsic::FromField => { - let incoming_type = Type::field(); - let target_type = return_type.clone().unwrap(); - - let truncate = Instruction::Truncate { - value: arguments[0], - bit_size: target_type.bit_size(), - max_bit_size: incoming_type.bit_size(), - }; - let truncated_value = dfg - .insert_instruction_and_results( - truncate, - block, - Some(vec![incoming_type]), - call_stack, - ) - .first(); - - let instruction = Instruction::Cast(truncated_value, target_type.unwrap_numeric()); - SimplifyResult::SimplifiedToInstruction(instruction) - } Intrinsic::AsWitness => SimplifyResult::None, Intrinsic::IsUnconstrained => SimplifyResult::None, Intrinsic::DerivePedersenGenerators => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/list.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/list.rs deleted file mode 100644 index 9a84d304444d..000000000000 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/list.rs +++ /dev/null @@ -1,187 +0,0 @@ -use serde::{Deserialize, Serialize}; -use std::sync::Arc; - -/// A shared linked list type intended to be cloned -#[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct List { - head: Arc>, - len: usize, -} - -#[derive(Default, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -enum Node { - #[default] - Nil, - Cons(T, Arc>), -} - -impl Default for List { - fn default() -> Self { - List { head: Arc::new(Node::Nil), len: 0 } - } -} - -impl List { - pub fn new() -> Self { - Self::default() - } - - /// This is actually a push_front since we just create a new head for the - /// list. This is done so that the tail of the list can still be shared. - /// In the case of call stacks, the last node will be main, while the top - /// of the call stack will be the head of this list. - pub fn push_back(&mut self, value: T) { - self.len += 1; - self.head = Arc::new(Node::Cons(value, self.head.clone())); - } - - /// It is more efficient to iterate from the head of the list towards the tail. - /// For callstacks this means from the top of the call stack towards main. - fn iter_rev(&self) -> IterRev { - IterRev { head: &self.head, len: self.len } - } - - pub fn clear(&mut self) { - *self = Self::default(); - } - - pub fn append(&mut self, other: Self) - where - T: Copy + std::fmt::Debug, - { - let other = other.into_iter().collect::>(); - - for item in other { - self.push_back(item); - } - } - - pub fn len(&self) -> usize { - self.len - } - - pub fn is_empty(&self) -> bool { - self.len == 0 - } - - fn pop_front(&mut self) -> Option - where - T: Copy, - { - match self.head.as_ref() { - Node::Nil => None, - Node::Cons(value, rest) => { - let value = *value; - self.head = rest.clone(); - self.len -= 1; - Some(value) - } - } - } - - pub fn truncate(&mut self, len: usize) - where - T: Copy, - { - if self.len > len { - for _ in 0..self.len - len { - self.pop_front(); - } - } - } - - pub fn unit(item: T) -> Self { - let mut this = Self::default(); - this.push_back(item); - this - } - - pub fn back(&self) -> Option<&T> { - match self.head.as_ref() { - Node::Nil => None, - Node::Cons(item, _) => Some(item), - } - } -} - -pub struct IterRev<'a, T> { - head: &'a Node, - len: usize, -} - -impl IntoIterator for List -where - T: Copy + std::fmt::Debug, -{ - type Item = T; - - type IntoIter = std::iter::Rev>; - - fn into_iter(self) -> Self::IntoIter { - let items: Vec<_> = self.iter_rev().copied().collect(); - items.into_iter().rev() - } -} - -impl<'a, T> IntoIterator for &'a List { - type Item = &'a T; - - type IntoIter = std::iter::Rev< as IntoIterator>::IntoIter>; - - fn into_iter(self) -> Self::IntoIter { - let items: Vec<_> = self.iter_rev().collect(); - items.into_iter().rev() - } -} - -impl<'a, T> Iterator for IterRev<'a, T> { - type Item = &'a T; - - fn next(&mut self) -> Option { - match self.head { - Node::Nil => None, - Node::Cons(value, rest) => { - self.head = rest; - Some(value) - } - } - } - - fn size_hint(&self) -> (usize, Option) { - (0, Some(self.len)) - } -} - -impl<'a, T> ExactSizeIterator for IterRev<'a, T> {} - -impl std::fmt::Debug for List -where - T: std::fmt::Debug, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "[")?; - for (i, item) in self.iter_rev().enumerate() { - if i != 0 { - write!(f, ", ")?; - } - write!(f, "{item:?}")?; - } - write!(f, "]") - } -} - -impl std::fmt::Display for List -where - T: std::fmt::Display, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "[")?; - for (i, item) in self.iter_rev().enumerate() { - if i != 0 { - write!(f, ", ")?; - } - write!(f, "{item}")?; - } - write!(f, "]") - } -} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/mod.rs index 88e0d8900db3..686606452fba 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/mod.rs @@ -6,7 +6,6 @@ pub(crate) mod dom; pub(crate) mod function; pub(crate) mod function_inserter; pub(crate) mod instruction; -pub mod list; pub(crate) mod map; pub(crate) mod post_order; pub(crate) mod printer; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 434202292575..aec172dddcd2 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -34,7 +34,7 @@ //! the following transformations of certain instructions within the block are expected: //! //! 1. A constraint is multiplied by the condition and changes the constraint to -//! an equality with c: +//! an equality with c: //! //! constrain v0 //! ============ @@ -211,6 +211,13 @@ struct Context<'f> { /// /// The `ValueId` here is that which is returned by the allocate instruction. local_allocations: HashSet, + + /// A map from `cond` to `Not(cond)` + /// + /// `Not` instructions are inserted constantly by this pass and this map helps keep + /// us from unnecessarily inserting extra instructions, and keeps ids unique which + /// helps simplifications. + not_instructions: HashMap, } #[derive(Clone)] @@ -256,6 +263,7 @@ fn flatten_function_cfg(function: &mut Function, no_predicates: &HashMap Context<'f> { let condition_call_stack = self.inserter.function.dfg.get_value_call_stack_id(cond_context.condition); - let else_condition = - self.insert_instruction(Instruction::Not(cond_context.condition), condition_call_stack); + + let else_condition = self.not_instruction(cond_context.condition, condition_call_stack); let else_condition = self.link_condition(else_condition); let old_allocations = std::mem::take(&mut self.local_allocations); @@ -464,6 +472,16 @@ impl<'f> Context<'f> { vec![self.cfg.successors(*block).next().unwrap()] } + fn not_instruction(&mut self, condition: ValueId, call_stack: CallStackId) -> ValueId { + if let Some(existing) = self.not_instructions.get(&condition) { + return *existing; + } + + let not = self.insert_instruction(Instruction::Not(condition), call_stack); + self.not_instructions.insert(condition, not); + not + } + /// Process the 'exit' block of a conditional statement fn else_stop(&mut self, block: &BasicBlockId) -> Vec { let mut cond_context = self.condition_stack.pop().unwrap(); @@ -671,8 +689,7 @@ impl<'f> Context<'f> { .insert_instruction_with_typevars(load, Some(vec![typ]), call_stack) .first(); - let else_condition = - self.insert_instruction(Instruction::Not(condition), call_stack); + let else_condition = self.not_instruction(condition, call_stack); let instruction = Instruction::IfElse { then_condition: condition, @@ -706,7 +723,6 @@ impl<'f> Context<'f> { let argument_type = self.inserter.function.dfg.type_of_value(field); let cast = Instruction::Cast(condition, argument_type.unwrap_numeric()); - let casted_condition = self.insert_instruction(cast, call_stack); let field = self.insert_instruction( Instruction::binary(BinaryOp::Mul, field, casted_condition), @@ -789,7 +805,7 @@ impl<'f> Context<'f> { fn var_or_one(&mut self, var: ValueId, condition: ValueId, call_stack: CallStackId) -> ValueId { let field = self.insert_instruction(Instruction::binary(BinaryOp::Mul, var, condition), call_stack); - let not_condition = self.insert_instruction(Instruction::Not(condition), call_stack); + let not_condition = self.not_instruction(condition, call_stack); self.insert_instruction( Instruction::binary(BinaryOp::Add, field, not_condition), call_stack, @@ -910,7 +926,6 @@ mod test { v8 = mul v5, v2 v9 = add v7, v8 store v9 at v1 - v10 = not v0 enable_side_effects u1 1 return } @@ -949,15 +964,14 @@ mod test { v8 = mul v5, v2 v9 = add v7, v8 store v9 at v1 - v10 = not v0 - enable_side_effects v10 - v11 = load v1 -> Field - v12 = cast v10 as Field - v13 = cast v0 as Field - v15 = mul v12, Field 6 - v16 = mul v13, v11 - v17 = add v15, v16 - store v17 at v1 + enable_side_effects v3 + v10 = load v1 -> Field + v11 = cast v3 as Field + v12 = cast v0 as Field + v14 = mul v11, Field 6 + v15 = mul v12, v10 + v16 = add v14, v15 + store v16 at v1 enable_side_effects u1 1 return } @@ -1085,16 +1099,14 @@ mod test { v23 = mul v20, Field 6 v24 = mul v21, v16 v25 = add v23, v24 - enable_side_effects v0 - v26 = not v0 - enable_side_effects v26 - v27 = cast v26 as Field - v28 = cast v0 as Field - v30 = mul v27, Field 3 - v31 = mul v28, v25 - v32 = add v30, v31 + enable_side_effects v3 + v26 = cast v3 as Field + v27 = cast v0 as Field + v29 = mul v26, Field 3 + v30 = mul v27, v25 + v31 = add v29, v30 enable_side_effects u1 1 - return v32 + return v31 }"; let main = ssa.main(); @@ -1288,13 +1300,12 @@ mod test { v16 = mul v14, v11 v17 = add v15, v16 store v17 at v6 - v18 = not v5 - enable_side_effects v18 - v19 = load v6 -> u8 - v20 = cast v18 as u8 - v21 = cast v4 as u8 - v22 = mul v21, v19 - store v22 at v6 + enable_side_effects v12 + v18 = load v6 -> u8 + v19 = cast v12 as u8 + v20 = cast v4 as u8 + v21 = mul v20, v18 + store v21 at v6 enable_side_effects u1 1 constrain v5 == u1 1 return @@ -1460,4 +1471,88 @@ mod test { let merged_ssa = Ssa::from_str(src).unwrap(); let _ = merged_ssa.flatten_cfg(); } + + #[test] + fn eliminates_unnecessary_if_else_instructions_on_numeric_types() { + let src = " + acir(inline) fn main f0 { + b0(v0: bool): + v1 = allocate -> &mut [Field; 1] + store Field 0 at v1 + jmpif v0 then: b1, else: b2 + b1(): + store Field 1 at v1 + store Field 2 at v1 + jmp b2() + b2(): + v3 = load v1 -> Field + return v3 + }"; + + let ssa = Ssa::from_str(src).unwrap(); + + let ssa = ssa.flatten_cfg().mem2reg().fold_constants(); + + let expected = " + acir(inline) fn main f0 { + b0(v0: u1): + v1 = allocate -> &mut [Field; 1] + enable_side_effects v0 + v2 = not v0 + v3 = cast v0 as Field + v4 = cast v2 as Field + v6 = mul v3, Field 2 + v7 = mul v4, v3 + v8 = add v6, v7 + enable_side_effects u1 1 + return v8 + } + "; + + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn eliminates_unnecessary_if_else_instructions_on_array_types() { + let src = " + acir(inline) fn main f0 { + b0(v0: bool, v1: bool): + v2 = make_array [Field 0] : [Field; 1] + v3 = allocate -> &mut [Field; 1] + store v2 at v3 + jmpif v0 then: b1, else: b2 + b1(): + v4 = make_array [Field 1] : [Field; 1] + store v4 at v3 + v5 = make_array [Field 2] : [Field; 1] + store v5 at v3 + jmp b2() + b2(): + v24 = load v3 -> Field + return v24 + }"; + + let ssa = Ssa::from_str(src).unwrap(); + + let ssa = ssa + .flatten_cfg() + .mem2reg() + .remove_if_else() + .fold_constants() + .dead_instruction_elimination(); + + let expected = " + acir(inline) fn main f0 { + b0(v0: u1, v1: u1): + enable_side_effects v0 + v2 = cast v0 as Field + v4 = mul v2, Field 2 + v5 = make_array [v4] : [Field; 1] + enable_side_effects u1 1 + return v5 + } + "; + + assert_normalized_ssa_equals(ssa, expected); + } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/branch_analysis.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/branch_analysis.rs index ce54bb533f7d..85240106f9f6 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/branch_analysis.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/branch_analysis.rs @@ -2,14 +2,14 @@ //! //! The algorithm is split into two parts: //! 1. The outer part: -//! A. An (unrolled) CFG can be though of as a linear sequence of blocks where some nodes split -//! off, but eventually rejoin to a new node and continue the linear sequence. -//! B. Follow this sequence in order, and whenever a split is found call -//! `find_join_point_of_branches` and then recur from the join point it returns until the -//! return instruction is found. +//! A. An (unrolled) CFG can be though of as a linear sequence of blocks where some nodes split +//! off, but eventually rejoin to a new node and continue the linear sequence. +//! B. Follow this sequence in order, and whenever a split is found call +//! `find_join_point_of_branches` and then recur from the join point it returns until the +//! return instruction is found. //! //! 2. The inner part defined by `find_join_point_of_branches`: -//! A. For each of the two branches in a jmpif block: +//! A. For each of the two branches in a jmpif block: //! - Check if either has multiple predecessors. If so, it is a join point. //! - If not, continue to search the linear sequence of successor blocks from that block. //! - If another split point is found, recur in `find_join_point_of_branches` diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index 44c0eb380c2e..c188ed1f80fd 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -263,17 +263,17 @@ mod test { fn simple_loop_invariant_code_motion() { let src = " brillig(inline) fn main f0 { - b0(v0: u32, v1: u32): - jmp b1(u32 0) - b1(v2: u32): - v5 = lt v2, u32 4 + b0(v0: i32, v1: i32): + jmp b1(i32 0) + b1(v2: i32): + v5 = lt v2, i32 4 jmpif v5 then: b3, else: b2 b2(): return b3(): v6 = mul v0, v1 - constrain v6 == u32 6 - v8 = add v2, u32 1 + constrain v6 == i32 6 + v8 = add v2, i32 1 jmp b1(v8) } "; @@ -287,17 +287,17 @@ mod test { // `v6 = mul v0, v1` in b3 should now be `v3 = mul v0, v1` in b0 let expected = " brillig(inline) fn main f0 { - b0(v0: u32, v1: u32): + b0(v0: i32, v1: i32): v3 = mul v0, v1 - jmp b1(u32 0) - b1(v2: u32): - v6 = lt v2, u32 4 + jmp b1(i32 0) + b1(v2: i32): + v6 = lt v2, i32 4 jmpif v6 then: b3, else: b2 b2(): return b3(): - constrain v3 == u32 6 - v9 = add v2, u32 1 + constrain v3 == i32 6 + v9 = add v2, i32 1 jmp b1(v9) } "; @@ -312,25 +312,25 @@ mod test { // is hoisted to the parent loop's pre-header block. let src = " brillig(inline) fn main f0 { - b0(v0: u32, v1: u32): - jmp b1(u32 0) - b1(v2: u32): - v6 = lt v2, u32 4 + b0(v0: i32, v1: i32): + jmp b1(i32 0) + b1(v2: i32): + v6 = lt v2, i32 4 jmpif v6 then: b3, else: b2 b2(): return b3(): - jmp b4(u32 0) - b4(v3: u32): - v7 = lt v3, u32 4 + jmp b4(i32 0) + b4(v3: i32): + v7 = lt v3, i32 4 jmpif v7 then: b6, else: b5 b5(): - v9 = add v2, u32 1 + v9 = add v2, i32 1 jmp b1(v9) b6(): v10 = mul v0, v1 - constrain v10 == u32 6 - v12 = add v3, u32 1 + constrain v10 == i32 6 + v12 = add v3, i32 1 jmp b4(v12) } "; @@ -344,25 +344,25 @@ mod test { // `v10 = mul v0, v1` in b6 should now be `v4 = mul v0, v1` in b0 let expected = " brillig(inline) fn main f0 { - b0(v0: u32, v1: u32): + b0(v0: i32, v1: i32): v4 = mul v0, v1 - jmp b1(u32 0) - b1(v2: u32): - v7 = lt v2, u32 4 + jmp b1(i32 0) + b1(v2: i32): + v7 = lt v2, i32 4 jmpif v7 then: b3, else: b2 b2(): return b3(): - jmp b4(u32 0) - b4(v3: u32): - v8 = lt v3, u32 4 + jmp b4(i32 0) + b4(v3: i32): + v8 = lt v3, i32 4 jmpif v8 then: b6, else: b5 b5(): - v10 = add v2, u32 1 + v10 = add v2, i32 1 jmp b1(v10) b6(): - constrain v4 == u32 6 - v12 = add v3, u32 1 + constrain v4 == i32 6 + v12 = add v3, i32 1 jmp b4(v12) } "; @@ -386,19 +386,19 @@ mod test { // hoist `v7 = mul v6, v0`. let src = " brillig(inline) fn main f0 { - b0(v0: u32, v1: u32): - jmp b1(u32 0) - b1(v2: u32): - v5 = lt v2, u32 4 + b0(v0: i32, v1: i32): + jmp b1(i32 0) + b1(v2: i32): + v5 = lt v2, i32 4 jmpif v5 then: b3, else: b2 b2(): return b3(): v6 = mul v0, v1 v7 = mul v6, v0 - v8 = eq v7, u32 12 - constrain v7 == u32 12 - v9 = add v2, u32 1 + v8 = eq v7, i32 12 + constrain v7 == i32 12 + v9 = add v2, i32 1 jmp b1(v9) } "; @@ -411,19 +411,19 @@ mod test { let expected = " brillig(inline) fn main f0 { - b0(v0: u32, v1: u32): + b0(v0: i32, v1: i32): v3 = mul v0, v1 v4 = mul v3, v0 - v6 = eq v4, u32 12 - jmp b1(u32 0) - b1(v2: u32): - v9 = lt v2, u32 4 + v6 = eq v4, i32 12 + jmp b1(i32 0) + b1(v2: i32): + v9 = lt v2, i32 4 jmpif v9 then: b3, else: b2 b2(): return b3(): - constrain v4 == u32 12 - v11 = add v2, u32 1 + constrain v4 == i32 12 + v11 = add v2, i32 1 jmp b1(v11) } "; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 1e5cd8bdfbd0..4356a23335c3 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -32,9 +32,11 @@ //! - We also track the last instance of a load instruction to each address in a block. //! If we see that the last load instruction was from the same address as the current load instruction, //! we move to replace the result of the current load with the result of the previous load. +//! //! This removal requires a couple conditions: -//! - No store occurs to that address before the next load, -//! - The address is not used as an argument to a call +//! - No store occurs to that address before the next load, +//! - The address is not used as an argument to a call +//! //! This optimization helps us remove repeated loads for which there are not known values. //! - On `Instruction::Store { address, value }`: //! - If the address of the store is known: @@ -200,7 +202,7 @@ impl<'f> PerFunctionContext<'f> { .get(store_address) .map_or(false, |expression| matches!(expression, Expression::Dereference(_))); - if self.last_loads.get(store_address).is_none() + if !self.last_loads.contains_key(store_address) && !store_alias_used && !is_dereference { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index e85e2c4a4417..e99f239e82ed 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -176,8 +176,6 @@ impl Context { | Intrinsic::ToRadix(_) | Intrinsic::BlackBox(_) | Intrinsic::Hint(Hint::BlackBox) - | Intrinsic::FromField - | Intrinsic::AsField | Intrinsic::AsSlice | Intrinsic::AsWitness | Intrinsic::IsUnconstrained diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index 79f2354aff67..9a931e36ea2a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -235,8 +235,6 @@ fn slice_capacity_change( | Intrinsic::StrAsBytes | Intrinsic::BlackBox(_) | Intrinsic::Hint(Hint::BlackBox) - | Intrinsic::FromField - | Intrinsic::AsField | Intrinsic::AsWitness | Intrinsic::IsUnconstrained | Intrinsic::DerivePedersenGenerators diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs index 87e680932c6b..eff6576b87fe 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs @@ -39,6 +39,8 @@ impl Function { } } + let is_unconstrained = matches!(self.runtime(), RuntimeType::Brillig(_)).into(); + let is_within_unconstrained = self.dfg.make_constant(is_unconstrained, NumericType::bool()); for instruction_id in is_unconstrained_calls { let call_returns = self.dfg.instruction_results(instruction_id); let original_return_id = call_returns[0]; @@ -46,9 +48,6 @@ impl Function { // We replace the result with a fresh id. This will be unused, so the DIE pass will remove the leftover intrinsic call. self.dfg.replace_result(instruction_id, original_return_id); - let is_unconstrained = matches!(self.runtime(), RuntimeType::Brillig(_)).into(); - let is_within_unconstrained = - self.dfg.make_constant(is_unconstrained, NumericType::bool()); // Replace all uses of the original return value with the constant self.dfg.set_value_from_id(original_return_id, is_within_unconstrained); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs index 5628e12b9aef..b671d5011a14 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs @@ -78,7 +78,7 @@ impl RuntimeSeparatorContext { if within_brillig { for called_func_id in called_functions.iter() { - let called_func = &ssa.functions[&called_func_id]; + let called_func = &ssa.functions[called_func_id]; if matches!(called_func.runtime(), RuntimeType::Acir(_)) { self.acir_functions_called_from_brillig.insert(*called_func_id); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index 2a2722361953..ab4256197b91 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -18,6 +18,8 @@ //! //! When unrolling ACIR code, we remove reference count instructions because they are //! only used by Brillig bytecode. +use std::collections::BTreeSet; + use acvm::{acir::AcirField, FieldElement}; use im::HashSet; @@ -117,7 +119,7 @@ pub(super) struct Loop { back_edge_start: BasicBlockId, /// All the blocks contained within the loop, including `header` and `back_edge_start`. - pub(super) blocks: HashSet, + pub(super) blocks: BTreeSet, } pub(super) struct Loops { @@ -238,7 +240,7 @@ impl Loop { back_edge_start: BasicBlockId, cfg: &ControlFlowGraph, ) -> Self { - let mut blocks = HashSet::default(); + let mut blocks = BTreeSet::default(); blocks.insert(header); let mut insert = |block, stack: &mut Vec| { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs index c77fe7513a16..bd65fc333771 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs @@ -1,5 +1,4 @@ use std::fmt::Display; -use std::sync::atomic::{AtomicU32, Ordering}; use acvm::acir::AcirField; use acvm::FieldElement; @@ -154,6 +153,7 @@ impl StatementKind { r#type, expression, comptime: false, + is_global_let: false, attributes, }) } @@ -562,6 +562,7 @@ pub struct LetStatement { // True if this should only be run during compile-time pub comptime: bool, + pub is_global_let: bool, } #[derive(Debug, PartialEq, Eq, Clone)] @@ -839,10 +840,9 @@ impl ForRange { block: Expression, for_loop_span: Span, ) -> Statement { - /// Counter used to generate unique names when desugaring - /// code in the parser requires the creation of fresh variables. - /// The parser is stateless so this is a static global instead. - static UNIQUE_NAME_COUNTER: AtomicU32 = AtomicU32::new(0); + // Counter used to generate unique names when desugaring + // code in the parser requires the creation of fresh variables. + let mut unique_name_counter: u32 = 0; match self { ForRange::Range(..) => { @@ -853,7 +853,8 @@ impl ForRange { let start_range = ExpressionKind::integer(FieldElement::zero()); let start_range = Expression::new(start_range, array_span); - let next_unique_id = UNIQUE_NAME_COUNTER.fetch_add(1, Ordering::Relaxed); + let next_unique_id = unique_name_counter; + unique_name_counter += 1; let array_name = format!("$i{next_unique_id}"); let array_span = array.span; let array_ident = Ident::new(array_name, array_span); @@ -886,7 +887,7 @@ impl ForRange { })); let end_range = Expression::new(end_range, array_span); - let next_unique_id = UNIQUE_NAME_COUNTER.fetch_add(1, Ordering::Relaxed); + let next_unique_id = unique_name_counter; let index_name = format!("$i{next_unique_id}"); let fresh_identifier = Ident::new(index_name.clone(), array_span); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs index f05fc721581d..c9c2cdcfea15 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs @@ -523,7 +523,9 @@ pub fn build_debug_crate_file() -> String { __debug_var_assign_oracle(var_id, value); } pub fn __debug_var_assign(var_id: u32, value: T) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_var_assign_inner(var_id, value); }} } @@ -534,7 +536,9 @@ pub fn build_debug_crate_file() -> String { __debug_var_drop_oracle(var_id); } pub fn __debug_var_drop(var_id: u32) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_var_drop_inner(var_id); }} } @@ -545,7 +549,9 @@ pub fn build_debug_crate_file() -> String { __debug_fn_enter_oracle(fn_id); } pub fn __debug_fn_enter(fn_id: u32) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_fn_enter_inner(fn_id); }} } @@ -556,7 +562,9 @@ pub fn build_debug_crate_file() -> String { __debug_fn_exit_oracle(fn_id); } pub fn __debug_fn_exit(fn_id: u32) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_fn_exit_inner(fn_id); }} } @@ -567,7 +575,9 @@ pub fn build_debug_crate_file() -> String { __debug_dereference_assign_oracle(var_id, value); } pub fn __debug_dereference_assign(var_id: u32, value: T) { - unsafe {{ + unsafe { + //@safety: debug context + { __debug_dereference_assign_inner(var_id, value); }} } @@ -594,6 +604,7 @@ pub fn build_debug_crate_file() -> String { }} pub fn __debug_member_assign_{n}(var_id: u32, value: T, {var_sig}) {{ unsafe {{ + //@safety: debug context __debug_inner_member_assign_{n}(var_id, value, {vars}); }} }} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs index b4ea06f10306..c24cc1a9c865 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -32,7 +32,7 @@ use crate::{ Kind, QuotedType, Shared, StructType, Type, }; -use super::{Elaborator, LambdaContext}; +use super::{Elaborator, LambdaContext, UnsafeBlockStatus}; impl<'context> Elaborator<'context> { pub(crate) fn elaborate_expression(&mut self, expr: Expression) -> (ExprId, Type) { @@ -59,7 +59,7 @@ impl<'context> Elaborator<'context> { return self.elaborate_comptime_block(comptime, expr.span) } ExpressionKind::Unsafe(block_expression, _) => { - self.elaborate_unsafe_block(block_expression) + self.elaborate_unsafe_block(block_expression, expr.span) } ExpressionKind::Resolved(id) => return (id, self.interner.id_type(id)), ExpressionKind::Interned(id) => { @@ -140,15 +140,36 @@ impl<'context> Elaborator<'context> { (HirBlockExpression { statements }, block_type) } - fn elaborate_unsafe_block(&mut self, block: BlockExpression) -> (HirExpression, Type) { + fn elaborate_unsafe_block( + &mut self, + block: BlockExpression, + span: Span, + ) -> (HirExpression, Type) { // Before entering the block we cache the old value of `in_unsafe_block` so it can be restored. - let old_in_unsafe_block = self.in_unsafe_block; - self.in_unsafe_block = true; + let old_in_unsafe_block = self.unsafe_block_status; + let is_nested_unsafe_block = + !matches!(old_in_unsafe_block, UnsafeBlockStatus::NotInUnsafeBlock); + if is_nested_unsafe_block { + let span = Span::from(span.start()..span.start() + 6); // Only highlight the `unsafe` keyword + self.push_err(TypeCheckError::NestedUnsafeBlock { span }); + } + + self.unsafe_block_status = UnsafeBlockStatus::InUnsafeBlockWithoutUnconstrainedCalls; let (hir_block_expression, typ) = self.elaborate_block_expression(block); - // Finally, we restore the original value of `self.in_unsafe_block`. - self.in_unsafe_block = old_in_unsafe_block; + if let UnsafeBlockStatus::InUnsafeBlockWithoutUnconstrainedCalls = self.unsafe_block_status + { + let span = Span::from(span.start()..span.start() + 6); // Only highlight the `unsafe` keyword + self.push_err(TypeCheckError::UnnecessaryUnsafeBlock { span }); + } + + // Finally, we restore the original value of `self.in_unsafe_block`, + // but only if this isn't a nested unsafe block (that way if we found an unconstrained call + // for this unsafe block we'll also consider the outer one as finding one, and we don't double error) + if !is_nested_unsafe_block { + self.unsafe_block_status = old_in_unsafe_block; + } (HirExpression::Unsafe(hir_block_expression), typ) } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index fe1d1e38e1a6..593ea6b20e8c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -4,8 +4,8 @@ use std::{ }; use crate::{ - ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, usage_tracker::UsageTracker, - StructField, StructType, TypeBindings, + ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, node_interner::GlobalValue, + usage_tracker::UsageTracker, StructField, StructType, TypeBindings, }; use crate::{ ast::{ @@ -79,6 +79,16 @@ pub struct LambdaContext { pub scope_index: usize, } +/// Determines whether we are in an unsafe block and, if so, whether +/// any unconstrained calls were found in it (because if not we'll warn +/// that the unsafe block is not needed). +#[derive(Copy, Clone)] +enum UnsafeBlockStatus { + NotInUnsafeBlock, + InUnsafeBlockWithoutUnconstrainedCalls, + InUnsafeBlockWithConstrainedCalls, +} + pub struct Elaborator<'context> { scopes: ScopeForest, @@ -90,7 +100,7 @@ pub struct Elaborator<'context> { pub(crate) file: FileId, - in_unsafe_block: bool, + unsafe_block_status: UnsafeBlockStatus, nested_loops: usize, /// Contains a mapping of the current struct or functions's generics to @@ -202,7 +212,7 @@ impl<'context> Elaborator<'context> { def_maps, usage_tracker, file: FileId::dummy(), - in_unsafe_block: false, + unsafe_block_status: UnsafeBlockStatus::NotInUnsafeBlock, nested_loops: 0, generics: Vec::new(), lambda_stack: Vec::new(), @@ -1652,20 +1662,13 @@ impl<'context> Elaborator<'context> { self.push_err(ResolverError::MutableGlobal { span }); } - let comptime = let_stmt.comptime; - - let (let_statement, _typ) = if comptime { - self.elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id))) - } else { - self.elaborate_let(let_stmt, Some(global_id)) - }; + let (let_statement, _typ) = self + .elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id))); let statement_id = self.interner.get_global(global_id).let_statement; self.interner.replace_statement(statement_id, let_statement); - if comptime { - self.elaborate_comptime_global(global_id); - } + self.elaborate_comptime_global(global_id); if let Some(name) = name { self.interner.register_global(global_id, name, global.visibility, self.module_id()); @@ -1696,7 +1699,17 @@ impl<'context> Elaborator<'context> { self.debug_comptime(location, |interner| value.display(interner).to_string()); - self.interner.get_global_mut(global_id).value = Some(value); + self.interner.get_global_mut(global_id).value = GlobalValue::Resolved(value); + } + } + + /// If the given global is unresolved, elaborate it and return true + fn elaborate_global_if_unresolved(&mut self, global_id: &GlobalId) -> bool { + if let Some(global) = self.unresolved_globals.remove(global_id) { + self.elaborate_global(global); + true + } else { + false } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs index a68991becb7f..51673342fefe 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -9,7 +9,7 @@ use crate::hir::resolution::visibility::item_in_module_is_visible; use crate::locations::ReferencesTracker; use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; -use crate::Type; +use crate::{Shared, Type, TypeAlias}; use super::types::SELF_TYPE_NAME; use super::Elaborator; @@ -218,18 +218,8 @@ impl<'context> Elaborator<'context> { ), ModuleDefId::TypeAliasId(id) => { let type_alias = self.interner.get_type_alias(id); - let type_alias = type_alias.borrow(); - - let module_id = match &type_alias.typ { - Type::Struct(struct_id, _generics) => struct_id.borrow().id.module_id(), - Type::Error => { - return Err(PathResolutionError::Unresolved(last_ident.clone())); - } - _ => { - // For now we only allow type aliases that point to structs. - // The more general case is captured here: https://github.com/noir-lang/noir/issues/6398 - panic!("Type alias in path not pointing to struct not yet supported") - } + let Some(module_id) = get_type_alias_module_def_id(&type_alias) else { + return Err(PathResolutionError::Unresolved(last_ident.clone())); }; ( @@ -345,3 +335,18 @@ fn merge_intermediate_path_resolution_item_with_module_def_id( }, } } + +fn get_type_alias_module_def_id(type_alias: &Shared) -> Option { + let type_alias = type_alias.borrow(); + + match &type_alias.typ { + Type::Struct(struct_id, _generics) => Some(struct_id.borrow().id.module_id()), + Type::Alias(type_alias, _generics) => get_type_alias_module_def_id(type_alias), + Type::Error => None, + _ => { + // For now we only allow type aliases that point to structs. + // The more general case is captured here: https://github.com/noir-lang/noir/issues/6398 + panic!("Type alias in path not pointing to struct not yet supported") + } + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs index 3fbdadbbee8b..133473219f66 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -604,17 +604,11 @@ impl<'context> Elaborator<'context> { alias_generics }; - // Now instantiate the underlying struct with those generics, the struct might + // Now instantiate the underlying struct or alias with those generics, the struct might // have more generics than those in the alias, like in this example: // // type Alias = Struct; - let typ = type_alias.get_type(&generics); - let Type::Struct(_, generics) = typ else { - // See https://github.com/noir-lang/noir/issues/6398 - panic!("Expected type alias to point to struct") - }; - - generics + get_type_alias_generics(&type_alias, &generics) } PathResolutionItem::TraitFunction(trait_id, Some(generics), _func_id) => { let trait_ = self.interner.get_trait(trait_id); @@ -666,9 +660,7 @@ impl<'context> Elaborator<'context> { self.interner.add_function_reference(func_id, hir_ident.location); } DefinitionKind::Global(global_id) => { - if let Some(global) = self.unresolved_globals.remove(&global_id) { - self.elaborate_global(global); - } + self.elaborate_global_if_unresolved(&global_id); if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, global_id); } @@ -882,3 +874,14 @@ impl<'context> Elaborator<'context> { (id, typ) } } + +fn get_type_alias_generics(type_alias: &TypeAlias, generics: &[Type]) -> Vec { + let typ = type_alias.get_type(generics); + match typ { + Type::Struct(_, generics) => generics, + Type::Alias(type_alias, generics) => { + get_type_alias_generics(&type_alias.borrow(), &generics) + } + _ => panic!("Expected type alias to point to struct or alias"), + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs index 93009f490710..8a46d85d5637 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs @@ -76,6 +76,7 @@ impl<'context> Elaborator<'context> { ) -> (HirStatement, Type) { let expr_span = let_stmt.expression.span; let (expression, expr_type) = self.elaborate_expression(let_stmt.expression); + let type_contains_unspecified = let_stmt.r#type.contains_unspecified(); let annotated_type = self.resolve_inferred_type(let_stmt.r#type); @@ -123,7 +124,9 @@ impl<'context> Elaborator<'context> { let attributes = let_stmt.attributes; let comptime = let_stmt.comptime; - let let_ = HirLetStatement { pattern, r#type, expression, attributes, comptime }; + let is_global_let = let_stmt.is_global_let; + let let_ = + HirLetStatement::new(pattern, r#type, expression, attributes, comptime, is_global_let); (HirStatement::Let(let_), Type::Unit) } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index 2e4809f35117..550ee41fbd4c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -1,6 +1,5 @@ use std::{borrow::Cow, rc::Rc}; -use acvm::acir::AcirField; use iter_extended::vecmap; use noirc_errors::{Location, Span}; use rustc_hash::FxHashMap as HashMap; @@ -12,7 +11,6 @@ use crate::{ UnresolvedTypeData, UnresolvedTypeExpression, WILDCARD_TYPE, }, hir::{ - comptime::{Interpreter, Value}, def_collector::dc_crate::CompilationError, resolution::{errors::ResolverError, import::PathResolutionError}, type_check::{ @@ -30,14 +28,14 @@ use crate::{ traits::{NamedType, ResolvedTraitBound, Trait, TraitConstraint}, }, node_interner::{ - DefinitionKind, DependencyId, ExprId, GlobalId, ImplSearchErrorKind, NodeInterner, TraitId, + DependencyId, ExprId, GlobalValue, ImplSearchErrorKind, NodeInterner, TraitId, TraitImplKind, TraitMethodId, }, token::SecondaryAttribute, Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, UnificationError, }; -use super::{lints, path_resolution::PathResolutionItem, Elaborator}; +use super::{lints, path_resolution::PathResolutionItem, Elaborator, UnsafeBlockStatus}; pub const SELF_TYPE_NAME: &str = "Self"; @@ -415,17 +413,41 @@ impl<'context> Elaborator<'context> { .map(|let_statement| Kind::numeric(let_statement.r#type)) .unwrap_or(Kind::u32()); - // TODO(https://github.com/noir-lang/noir/issues/6238): - // support non-u32 generics here - if !kind.unifies(&Kind::u32()) { - let error = TypeCheckError::EvaluatedGlobalIsntU32 { - expected_kind: Kind::u32().to_string(), - expr_kind: kind.to_string(), - expr_span: path.span(), - }; - self.push_err(error); - } - Some(Type::Constant(self.eval_global_as_array_length(id, path).into(), kind)) + let Some(stmt) = self.interner.get_global_let_statement(id) else { + if self.elaborate_global_if_unresolved(&id) { + return self.lookup_generic_or_global_type(path); + } else { + let path = path.clone(); + self.push_err(ResolverError::NoSuchNumericTypeVariable { path }); + return None; + } + }; + + let rhs = stmt.expression; + let span = self.interner.expr_span(&rhs); + + let GlobalValue::Resolved(global_value) = &self.interner.get_global(id).value + else { + self.push_err(ResolverError::UnevaluatedGlobalType { span }); + return None; + }; + + let Some(global_value) = global_value.to_field_element() else { + let global_value = global_value.clone(); + if global_value.is_integral() { + self.push_err(ResolverError::NegativeGlobalType { span, global_value }); + } else { + self.push_err(ResolverError::NonIntegralGlobalType { span, global_value }); + } + return None; + }; + + let Ok(global_value) = kind.ensure_value_fits(global_value, span) else { + self.push_err(ResolverError::GlobalLargerThanKind { span, global_value, kind }); + return None; + }; + + Some(Type::Constant(global_value, kind)) } _ => None, } @@ -633,31 +655,6 @@ impl<'context> Elaborator<'context> { .or_else(|| self.resolve_trait_method_by_named_generic(path)) } - fn eval_global_as_array_length(&mut self, global_id: GlobalId, path: &Path) -> u32 { - let Some(stmt) = self.interner.get_global_let_statement(global_id) else { - if let Some(global) = self.unresolved_globals.remove(&global_id) { - self.elaborate_global(global); - return self.eval_global_as_array_length(global_id, path); - } else { - let path = path.clone(); - self.push_err(ResolverError::NoSuchNumericTypeVariable { path }); - return 0; - } - }; - - let length = stmt.expression; - let span = self.interner.expr_span(&length); - let result = try_eval_array_length_id(self.interner, length, span); - - match result.map(|length| length.try_into()) { - Ok(Ok(length_value)) => return length_value, - Ok(Err(_cast_err)) => self.push_err(ResolverError::IntegerTooLarge { span }), - Err(Some(error)) => self.push_err(error), - Err(None) => (), - } - 0 - } - pub(super) fn unify( &mut self, actual: &Type, @@ -1486,8 +1483,14 @@ impl<'context> Elaborator<'context> { func_type_is_unconstrained || self.is_unconstrained_call(call.func); let crossing_runtime_boundary = is_current_func_constrained && is_unconstrained_call; if crossing_runtime_boundary { - if !self.in_unsafe_block { - self.push_err(TypeCheckError::Unsafe { span }); + match self.unsafe_block_status { + UnsafeBlockStatus::NotInUnsafeBlock => { + self.push_err(TypeCheckError::Unsafe { span }); + } + UnsafeBlockStatus::InUnsafeBlockWithoutUnconstrainedCalls => { + self.unsafe_block_status = UnsafeBlockStatus::InUnsafeBlockWithConstrainedCalls; + } + UnsafeBlockStatus::InUnsafeBlockWithConstrainedCalls => (), } if let Some(called_func_id) = self.interner.lookup_function_from_expr(&call.func) { @@ -1834,88 +1837,6 @@ fn bind_generic(param: &ResolvedGeneric, arg: &Type, bindings: &mut TypeBindings } } -pub fn try_eval_array_length_id( - interner: &NodeInterner, - rhs: ExprId, - span: Span, -) -> Result> { - // Arbitrary amount of recursive calls to try before giving up - let fuel = 100; - try_eval_array_length_id_with_fuel(interner, rhs, span, fuel) -} - -fn try_eval_array_length_id_with_fuel( - interner: &NodeInterner, - rhs: ExprId, - span: Span, - fuel: u32, -) -> Result> { - if fuel == 0 { - // If we reach here, it is likely from evaluating cyclic globals. We expect an error to - // be issued for them after name resolution so issue no error now. - return Err(None); - } - - match interner.expression(&rhs) { - HirExpression::Literal(HirLiteral::Integer(int, false)) => { - int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) - } - HirExpression::Ident(ident, _) => { - if let Some(definition) = interner.try_definition(ident.id) { - match definition.kind { - DefinitionKind::Global(global_id) => { - let let_statement = interner.get_global_let_statement(global_id); - if let Some(let_statement) = let_statement { - let expression = let_statement.expression; - try_eval_array_length_id_with_fuel(interner, expression, span, fuel - 1) - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - _ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - HirExpression::Infix(infix) => { - let lhs = try_eval_array_length_id_with_fuel(interner, infix.lhs, span, fuel - 1)?; - let rhs = try_eval_array_length_id_with_fuel(interner, infix.rhs, span, fuel - 1)?; - - match infix.operator.kind { - BinaryOpKind::Add => Ok(lhs + rhs), - BinaryOpKind::Subtract => Ok(lhs - rhs), - BinaryOpKind::Multiply => Ok(lhs * rhs), - BinaryOpKind::Divide => Ok(lhs / rhs), - BinaryOpKind::Equal => Ok((lhs == rhs) as u128), - BinaryOpKind::NotEqual => Ok((lhs != rhs) as u128), - BinaryOpKind::Less => Ok((lhs < rhs) as u128), - BinaryOpKind::LessEqual => Ok((lhs <= rhs) as u128), - BinaryOpKind::Greater => Ok((lhs > rhs) as u128), - BinaryOpKind::GreaterEqual => Ok((lhs >= rhs) as u128), - BinaryOpKind::And => Ok(lhs & rhs), - BinaryOpKind::Or => Ok(lhs | rhs), - BinaryOpKind::Xor => Ok(lhs ^ rhs), - BinaryOpKind::ShiftRight => Ok(lhs >> rhs), - BinaryOpKind::ShiftLeft => Ok(lhs << rhs), - BinaryOpKind::Modulo => Ok(lhs % rhs), - } - } - HirExpression::Cast(cast) => { - let lhs = try_eval_array_length_id_with_fuel(interner, cast.lhs, span, fuel - 1)?; - let lhs_value = Value::Field(lhs.into()); - let evaluated_value = - Interpreter::evaluate_cast_one_step(&cast, rhs, lhs_value, interner) - .map_err(|error| Some(ResolverError::ArrayLengthInterpreter { error }))?; - - evaluated_value - .to_u128() - .ok_or_else(|| Some(ResolverError::InvalidArrayLengthExpr { span })) - } - _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } -} - /// Gives an error if a user tries to create a mutable reference /// to an immutable variable. fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result<(), ResolverError> { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs index 3df20b39209e..6ff918328a1f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -243,6 +243,9 @@ pub enum InterpreterError { CannotInterpretFormatStringWithErrors { location: Location, }, + GlobalsDependencyCycle { + location: Location, + }, // These cases are not errors, they are just used to prevent us from running more code // until the loop can be resumed properly. These cases will never be displayed to users. @@ -319,7 +322,8 @@ impl InterpreterError { | InterpreterError::CannotResolveExpression { location, .. } | InterpreterError::CannotSetFunctionBody { location, .. } | InterpreterError::UnknownArrayLength { location, .. } - | InterpreterError::CannotInterpretFormatStringWithErrors { location } => *location, + | InterpreterError::CannotInterpretFormatStringWithErrors { location } + | InterpreterError::GlobalsDependencyCycle { location } => *location, InterpreterError::FailedToParseMacro { error, file, .. } => { Location::new(error.span(), *file) @@ -674,6 +678,11 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { "Some of the variables to interpolate could not be evaluated".to_string(); CustomDiagnostic::simple_error(msg, secondary, location.span) } + InterpreterError::GlobalsDependencyCycle { location } => { + let msg = "This global recursively depends on itself".to_string(); + let secondary = String::new(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index dfa55a9d79b6..e9e37243e07d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -20,6 +20,7 @@ use crate::monomorphization::{ perform_impl_bindings, perform_instantiation_bindings, resolve_trait_method, undo_instantiation_bindings, }; +use crate::node_interner::GlobalValue; use crate::token::{FmtStrFragment, Tokens}; use crate::TypeVariable; use crate::{ @@ -568,26 +569,39 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { DefinitionKind::Local(_) => self.lookup(&ident), DefinitionKind::Global(global_id) => { // Avoid resetting the value if it is already known - if let Some(value) = &self.elaborator.interner.get_global(*global_id).value { - Ok(value.clone()) - } else { - let global_id = *global_id; - let crate_of_global = self.elaborator.interner.get_global(global_id).crate_id; - let let_ = - self.elaborator.interner.get_global_let_statement(global_id).ok_or_else( - || { + let global_id = *global_id; + let global_info = self.elaborator.interner.get_global(global_id); + let global_crate_id = global_info.crate_id; + match &global_info.value { + GlobalValue::Resolved(value) => Ok(value.clone()), + GlobalValue::Resolving => { + // Note that the error we issue here isn't very informative (it doesn't include the actual cycle) + // but the general dependency cycle detector will give a better error later on during compilation. + let location = self.elaborator.interner.expr_location(&id); + Err(InterpreterError::GlobalsDependencyCycle { location }) + } + GlobalValue::Unresolved => { + let let_ = self + .elaborator + .interner + .get_global_let_statement(global_id) + .ok_or_else(|| { let location = self.elaborator.interner.expr_location(&id); InterpreterError::VariableNotInScope { location } - }, - )?; + })?; - if let_.comptime || crate_of_global != self.crate_id { - self.evaluate_let(let_.clone())?; - } + self.elaborator.interner.get_global_mut(global_id).value = + GlobalValue::Resolving; + + if let_.runs_comptime() || global_crate_id != self.crate_id { + self.evaluate_let(let_.clone())?; + } - let value = self.lookup(&ident)?; - self.elaborator.interner.get_global_mut(global_id).value = Some(value.clone()); - Ok(value) + let value = self.lookup(&ident)?; + self.elaborator.interner.get_global_mut(global_id).value = + GlobalValue::Resolved(value.clone()); + Ok(value) + } } } DefinitionKind::NumericGeneric(type_variable, numeric_typ) => { @@ -1380,13 +1394,19 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { fn evaluate_cast(&mut self, cast: &HirCastExpression, id: ExprId) -> IResult { let evaluated_lhs = self.evaluate(cast.lhs)?; - Self::evaluate_cast_one_step(cast, id, evaluated_lhs, self.elaborator.interner) + let location = self.elaborator.interner.expr_location(&id); + Self::evaluate_cast_one_step( + &cast.r#type, + location, + evaluated_lhs, + self.elaborator.interner, + ) } /// evaluate_cast without recursion pub fn evaluate_cast_one_step( - cast: &HirCastExpression, - id: ExprId, + typ: &Type, + location: Location, evaluated_lhs: Value, interner: &NodeInterner, ) -> IResult { @@ -1417,7 +1437,6 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { (if value { FieldElement::one() } else { FieldElement::zero() }, false) } value => { - let location = interner.expr_location(&id); let typ = value.get_type().into_owned(); return Err(InterpreterError::NonNumericCasted { typ, location }); } @@ -1434,7 +1453,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } // Now actually cast the lhs, bit casting and wrapping as necessary - match cast.r#type.follow_bindings() { + match typ.follow_bindings() { Type::FieldElement => { if lhs_is_negative { lhs = FieldElement::zero() - lhs; @@ -1443,8 +1462,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } Type::Integer(sign, bit_size) => match (sign, bit_size) { (Signedness::Unsigned, IntegerBitSize::One) => { - let location = interner.expr_location(&id); - Err(InterpreterError::TypeUnsupported { typ: cast.r#type.clone(), location }) + Err(InterpreterError::TypeUnsupported { typ: typ.clone(), location }) } (Signedness::Unsigned, IntegerBitSize::Eight) => cast_to_int!(lhs, to_u128, u8, U8), (Signedness::Unsigned, IntegerBitSize::Sixteen) => { @@ -1457,8 +1475,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { cast_to_int!(lhs, to_u128, u64, U64) } (Signedness::Signed, IntegerBitSize::One) => { - let location = interner.expr_location(&id); - Err(InterpreterError::TypeUnsupported { typ: cast.r#type.clone(), location }) + Err(InterpreterError::TypeUnsupported { typ: typ.clone(), location }) } (Signedness::Signed, IntegerBitSize::Eight) => cast_to_int!(lhs, to_i128, i8, I8), (Signedness::Signed, IntegerBitSize::Sixteen) => { @@ -1472,10 +1489,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } }, Type::Bool => Ok(Value::Bool(!lhs.is_zero() || lhs_is_negative)), - typ => { - let location = interner.expr_location(&id); - Err(InterpreterError::CastToNonNumericType { typ, location }) - } + typ => Err(InterpreterError::CastToNonNumericType { typ, location }), } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 3d8ccf789261..2e672e3d0d59 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -64,6 +64,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "array_len" => array_len(interner, arguments, location), "array_refcount" => Ok(Value::U32(0)), "assert_constant" => Ok(Value::Bool(true)), + "as_field" => as_field(interner, arguments, location), "as_slice" => as_slice(interner, arguments, location), "ctstring_eq" => ctstring_eq(arguments, location), "ctstring_hash" => ctstring_hash(arguments, location), @@ -116,6 +117,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "field_less_than" => field_less_than(arguments, location), "fmtstr_as_ctstring" => fmtstr_as_ctstring(interner, arguments, location), "fmtstr_quoted_contents" => fmtstr_quoted_contents(interner, arguments, location), + "from_field" => from_field(interner, arguments, return_type, location), "fresh_type_variable" => fresh_type_variable(interner), "function_def_add_attribute" => function_def_add_attribute(self, arguments, location), "function_def_body" => function_def_body(interner, arguments, location), @@ -290,6 +292,16 @@ fn array_as_str_unchecked( Ok(Value::String(Rc::new(string))) } +// fn as_field(x: T) -> Field {} +fn as_field( + interner: &NodeInterner, + arguments: Vec<(Value, Location)>, + location: Location, +) -> IResult { + let (value, value_location) = check_one_argument(arguments, location)?; + Interpreter::evaluate_cast_one_step(&Type::FieldElement, value_location, value, interner) +} + fn as_slice( interner: &NodeInterner, arguments: Vec<(Value, Location)>, @@ -2224,6 +2236,17 @@ fn fmtstr_quoted_contents( Ok(Value::Quoted(Rc::new(tokens))) } +// fn from_field(x: Field) -> T {} +fn from_field( + interner: &NodeInterner, + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + let (value, value_location) = check_one_argument(arguments, location)?; + Interpreter::evaluate_cast_one_step(&return_type, value_location, value, interner) +} + // fn fresh_type_variable() -> Type fn fresh_type_variable(interner: &NodeInterner) -> IResult { Ok(Value::Type(interner.next_type_variable_with_kind(Kind::Any))) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs index 945fb45026d0..e5c55e2b0bec 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -1,6 +1,6 @@ use std::{borrow::Cow, rc::Rc, vec}; -use acvm::{AcirField, FieldElement}; +use acvm::FieldElement; use im::Vector; use iter_extended::{try_vecmap, vecmap}; use noirc_errors::{Location, Span}; @@ -409,6 +409,9 @@ impl Value { Value::Pointer(element, true) => { return element.unwrap_or_clone().into_hir_expression(interner, location); } + Value::Closure(hir_lambda, _args, _typ, _opt_func_id, _module_id) => { + HirExpression::Lambda(hir_lambda) + } Value::TypedExpr(TypedExpr::StmtId(..)) | Value::Expr(..) | Value::Pointer(..) @@ -420,7 +423,6 @@ impl Value { | Value::Zeroed(_) | Value::Type(_) | Value::UnresolvedType(_) - | Value::Closure(..) | Value::ModuleDefinition(_) => { let typ = self.get_type().into_owned(); let value = self.display(interner).to_string(); @@ -502,19 +504,28 @@ impl Value { Ok(vec![token]) } - /// Converts any unsigned `Value` into a `u128`. - /// Returns `None` for negative integers. - pub(crate) fn to_u128(&self) -> Option { + /// Returns false for non-integral `Value`s. + pub(crate) fn is_integral(&self) -> bool { + use Value::*; + matches!( + self, + Field(_) | I8(_) | I16(_) | I32(_) | I64(_) | U8(_) | U16(_) | U32(_) | U64(_) + ) + } + + /// Converts any non-negative `Value` into a `FieldElement`. + /// Returns `None` for negative integers and non-integral `Value`s. + pub(crate) fn to_field_element(&self) -> Option { match self { - Self::Field(value) => Some(value.to_u128()), - Self::I8(value) => (*value >= 0).then_some(*value as u128), - Self::I16(value) => (*value >= 0).then_some(*value as u128), - Self::I32(value) => (*value >= 0).then_some(*value as u128), - Self::I64(value) => (*value >= 0).then_some(*value as u128), - Self::U8(value) => Some(*value as u128), - Self::U16(value) => Some(*value as u128), - Self::U32(value) => Some(*value as u128), - Self::U64(value) => Some(*value as u128), + Self::Field(value) => Some(*value), + Self::I8(value) => (*value >= 0).then_some((*value as u128).into()), + Self::I16(value) => (*value >= 0).then_some((*value as u128).into()), + Self::I32(value) => (*value >= 0).then_some((*value as u128).into()), + Self::I64(value) => (*value >= 0).then_some((*value as u128).into()), + Self::U8(value) => Some((*value as u128).into()), + Self::U16(value) => Some((*value as u128).into()), + Self::U32(value) => Some((*value as u128).into()), + Self::U64(value) => Some((*value as u128).into()), _ => None, } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index 774836f89921..8817904c6f56 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -5,10 +5,13 @@ use thiserror::Error; use crate::{ ast::{Ident, UnsupportedNumericGenericType}, - hir::{comptime::InterpreterError, type_check::TypeCheckError}, + hir::{ + comptime::{InterpreterError, Value}, + type_check::TypeCheckError, + }, parser::ParserError, usage_tracker::UnusedItem, - Type, + Kind, Type, }; use super::import::PathResolutionError; @@ -101,6 +104,14 @@ pub enum ResolverError { MutableGlobal { span: Span }, #[error("Globals must have a specified type")] UnspecifiedGlobalType { span: Span, expected_type: Type }, + #[error("Global failed to evaluate")] + UnevaluatedGlobalType { span: Span }, + #[error("Globals used in a type position must be non-negative")] + NegativeGlobalType { span: Span, global_value: Value }, + #[error("Globals used in a type position must be integers")] + NonIntegralGlobalType { span: Span, global_value: Value }, + #[error("Global value `{global_value}` is larger than its kind's maximum value")] + GlobalLargerThanKind { span: Span, global_value: FieldElement, kind: Kind }, #[error("Self-referential structs are not supported")] SelfReferentialStruct { span: Span }, #[error("#[no_predicates] attribute is only allowed on constrained functions")] @@ -443,6 +454,34 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, + ResolverError::UnevaluatedGlobalType { span } => { + Diagnostic::simple_error( + "Global failed to evaluate".to_string(), + String::new(), + *span, + ) + } + ResolverError::NegativeGlobalType { span, global_value } => { + Diagnostic::simple_error( + "Globals used in a type position must be non-negative".to_string(), + format!("But found value `{global_value:?}`"), + *span, + ) + } + ResolverError::NonIntegralGlobalType { span, global_value } => { + Diagnostic::simple_error( + "Globals used in a type position must be integers".to_string(), + format!("But found value `{global_value:?}`"), + *span, + ) + } + ResolverError::GlobalLargerThanKind { span, global_value, kind } => { + Diagnostic::simple_error( + format!("Global value `{global_value}` is larger than its kind's maximum value"), + format!("Global's kind inferred to be `{kind}`"), + *span, + ) + } ResolverError::SelfReferentialStruct { span } => { Diagnostic::simple_error( "Self-referential structs are not supported".into(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/scope/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/scope/mod.rs index 1a9087a7408e..ebb271b86b73 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/scope/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/scope/mod.rs @@ -25,10 +25,10 @@ It's not implemented yet, because nothing has been benched pub struct Scope(pub HashMap); impl Scope { - pub fn find(&mut self, key: &Q) -> Option<&mut V> + pub fn find(&mut self, key: &Q) -> Option<&mut V> where K: std::borrow::Borrow, - Q: std::hash::Hash + Eq, + Q: ?Sized + std::hash::Hash + Eq, { self.0.get_mut(key) } @@ -75,10 +75,10 @@ impl ScopeTree { // Recursively search for a key in the scope tree. // Returns the value if found, along with the index it was found at. - pub fn find(&mut self, key: &Q) -> Option<(&mut V, usize)> + pub fn find(&mut self, key: &Q) -> Option<(&mut V, usize)> where K: std::borrow::Borrow, - Q: std::hash::Hash + Eq, + Q: ?Sized + std::hash::Hash + Eq, { for (i, scope) in self.0.iter_mut().enumerate().rev() { if let Some(value_found) = scope.find(key) { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs index 15b8d50c78bf..16422e0ef8bc 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -66,9 +66,6 @@ pub enum TypeCheckError { from_value: FieldElement, span: Span, }, - // TODO(https://github.com/noir-lang/noir/issues/6238): implement handling for larger types - #[error("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)")] - EvaluatedGlobalIsntU32 { expected_kind: String, expr_kind: String, expr_span: Span }, #[error("Expected {expected:?} found {found:?}")] ArityMisMatch { expected: usize, found: usize, span: Span }, #[error("Return type in a function cannot be public")] @@ -208,6 +205,10 @@ pub enum TypeCheckError { CyclicType { typ: Type, span: Span }, #[error("Type annotations required before indexing this array or slice")] TypeAnnotationsNeededForIndex { span: Span }, + #[error("Unnecessary `unsafe` block")] + UnnecessaryUnsafeBlock { span: Span }, + #[error("Unnecessary `unsafe` block")] + NestedUnsafeBlock { span: Span }, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -275,15 +276,6 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { *span, ) } - // TODO(https://github.com/noir-lang/noir/issues/6238): implement - // handling for larger types - TypeCheckError::EvaluatedGlobalIsntU32 { expected_kind, expr_kind, expr_span } => { - Diagnostic::simple_warning( - format!("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)"), - String::new(), - *expr_span, - ) - } TypeCheckError::TraitMethodParameterTypeMismatch { method_name, expected_typ, actual_typ, parameter_index, parameter_span } => { Diagnostic::simple_error( format!("Parameter #{parameter_index} of method `{method_name}` must be of type {expected_typ}, not {actual_typ}"), @@ -529,6 +521,20 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { *span, ) }, + TypeCheckError::UnnecessaryUnsafeBlock { span } => { + Diagnostic::simple_warning( + "Unnecessary `unsafe` block".into(), + "".into(), + *span, + ) + }, + TypeCheckError::NestedUnsafeBlock { span } => { + Diagnostic::simple_warning( + "Unnecessary `unsafe` block".into(), + "Because it's nested inside another `unsafe` block".into(), + *span, + ) + }, } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs index 0258cfd8ddb8..c42b8230290f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs @@ -31,15 +31,31 @@ pub struct HirLetStatement { pub expression: ExprId, pub attributes: Vec, pub comptime: bool, + pub is_global_let: bool, } impl HirLetStatement { + pub fn new( + pattern: HirPattern, + r#type: Type, + expression: ExprId, + attributes: Vec, + comptime: bool, + is_global_let: bool, + ) -> HirLetStatement { + Self { pattern, r#type, expression, attributes, comptime, is_global_let } + } + pub fn ident(&self) -> HirIdent { match &self.pattern { HirPattern::Identifier(ident) => ident.clone(), _ => panic!("can only fetch hir ident from HirPattern::Identifier"), } } + + pub fn runs_comptime(&self) -> bool { + self.comptime || self.is_global_let + } } #[derive(Debug, Clone)] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs index 2c9a44c079d0..0ed6399296b5 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs @@ -226,6 +226,10 @@ impl Kind { // Kind::Integer unifies with Kind::IntegerOrField (Kind::Integer | Kind::IntegerOrField, Kind::Integer | Kind::IntegerOrField) => true, + // Kind::IntegerOrField unifies with Kind::Numeric(_) + (Kind::IntegerOrField, Kind::Numeric(_typ)) + | (Kind::Numeric(_typ), Kind::IntegerOrField) => true, + // Kind::Numeric unifies along its Type argument (Kind::Numeric(lhs), Kind::Numeric(rhs)) => { let mut bindings = TypeBindings::new(); @@ -255,7 +259,8 @@ impl Kind { match self { Kind::IntegerOrField => Some(Type::default_int_or_field_type()), Kind::Integer => Some(Type::default_int_type()), - Kind::Any | Kind::Normal | Kind::Numeric(..) => None, + Kind::Numeric(typ) => Some(*typ.clone()), + Kind::Any | Kind::Normal => None, } } @@ -267,7 +272,7 @@ impl Kind { } /// Ensure the given value fits in self.integral_maximum_size() - fn ensure_value_fits( + pub(crate) fn ensure_value_fits( &self, value: FieldElement, span: Span, @@ -2210,6 +2215,7 @@ impl Type { Type::NamedGeneric(binding, _) | Type::TypeVariable(binding) => { substitute_binding(binding) } + // Do not substitute_helper fields, it can lead to infinite recursion // and we should not match fields when type checking anyway. Type::Struct(fields, args) => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs index a5c4b2cd7721..99799b9a1c0f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs @@ -726,7 +726,7 @@ impl<'a> Lexer<'a> { } fn parse_comment(&mut self, start: u32) -> SpannedTokenResult { - let doc_style = match self.peek_char() { + let mut doc_style = match self.peek_char() { Some('!') => { self.next_char(); Some(DocStyle::Inner) @@ -735,9 +735,17 @@ impl<'a> Lexer<'a> { self.next_char(); Some(DocStyle::Outer) } + Some('@') => Some(DocStyle::Safety), _ => None, }; - let comment = self.eat_while(None, |ch| ch != '\n'); + let mut comment = self.eat_while(None, |ch| ch != '\n'); + if doc_style == Some(DocStyle::Safety) { + if comment.starts_with("@safety") { + comment = comment["@safety".len()..].to_string(); + } else { + doc_style = None; + } + } if !comment.is_ascii() { let span = Span::from(start..self.position); @@ -752,7 +760,7 @@ impl<'a> Lexer<'a> { } fn parse_block_comment(&mut self, start: u32) -> SpannedTokenResult { - let doc_style = match self.peek_char() { + let mut doc_style = match self.peek_char() { Some('!') => { self.next_char(); Some(DocStyle::Inner) @@ -761,6 +769,7 @@ impl<'a> Lexer<'a> { self.next_char(); Some(DocStyle::Outer) } + Some('@') => Some(DocStyle::Safety), _ => None, }; @@ -787,7 +796,13 @@ impl<'a> Lexer<'a> { ch => content.push(ch), } } - + if doc_style == Some(DocStyle::Safety) { + if content.starts_with("@safety") { + content = content["@safety".len()..].to_string(); + } else { + doc_style = None; + } + } if depth == 0 { if !content.is_ascii() { let span = Span::from(start..self.position); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs index f35515045dbe..ffd755e606f1 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs @@ -345,6 +345,7 @@ impl Display for FmtStrFragment { pub enum DocStyle { Outer, Inner, + Safety, } #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -424,11 +425,13 @@ impl fmt::Display for Token { Token::LineComment(ref s, style) => match style { Some(DocStyle::Inner) => write!(f, "//!{s}"), Some(DocStyle::Outer) => write!(f, "///{s}"), + Some(DocStyle::Safety) => write!(f, "//@safety{s}"), None => write!(f, "//{s}"), }, Token::BlockComment(ref s, style) => match style { Some(DocStyle::Inner) => write!(f, "/*!{s}*/"), Some(DocStyle::Outer) => write!(f, "/**{s}*/"), + Some(DocStyle::Safety) => write!(f, "/*@safety{s}*/"), None => write!(f, "/*{s}*/"), }, Token::Quote(ref stream) => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs index b31a5744d09c..8c07d71de212 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -11,7 +11,7 @@ use crate::ast::{FunctionKind, IntegerBitSize, Signedness, UnaryOp, Visibility}; use crate::hir::comptime::InterpreterError; use crate::hir::type_check::{NoMatchingImplFoundError, TypeCheckError}; -use crate::node_interner::{ExprId, ImplSearchErrorKind}; +use crate::node_interner::{ExprId, GlobalValue, ImplSearchErrorKind}; use crate::token::FmtStrFragment; use crate::{ debug::DebugInstrumenter, @@ -895,7 +895,7 @@ impl<'interner> Monomorphizer<'interner> { DefinitionKind::Global(global_id) => { let global = self.interner.get_global(*global_id); - let expr = if let Some(value) = global.value.clone() { + let expr = if let GlobalValue::Resolved(value) = global.value.clone() { value .into_hir_expression(self.interner, global.location) .map_err(MonomorphizationError::InterpreterError)? @@ -1133,7 +1133,7 @@ impl<'interner> Monomorphizer<'interner> { | HirType::Error | HirType::Quoted(_) => Ok(()), HirType::Constant(_value, kind) => { - if kind.is_error() { + if kind.is_error() || kind.default_type().is_none() { Err(MonomorphizationError::UnknownConstant { location }) } else { Ok(()) @@ -1154,7 +1154,6 @@ impl<'interner> Monomorphizer<'interner> { Ok(()) } - HirType::TypeVariable(ref binding) => { let type_var_kind = match &*binding.borrow() { TypeBinding::Bound(binding) => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index 6d70ea2fd6d6..1df2cd767215 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs @@ -414,7 +414,7 @@ pub struct DefinitionId(usize); impl DefinitionId { //dummy id for error reporting pub fn dummy_id() -> DefinitionId { - DefinitionId(std::usize::MAX) + DefinitionId(usize::MAX) } } @@ -425,7 +425,7 @@ pub struct GlobalId(usize); impl GlobalId { // Dummy id for error reporting pub fn dummy_id() -> Self { - GlobalId(std::usize::MAX) + GlobalId(usize::MAX) } } @@ -496,7 +496,7 @@ pub struct TypeAliasId(pub usize); impl TypeAliasId { pub fn dummy_id() -> TypeAliasId { - TypeAliasId(std::usize::MAX) + TypeAliasId(usize::MAX) } } @@ -609,7 +609,14 @@ pub struct GlobalInfo { pub crate_id: CrateId, pub location: Location, pub let_statement: StmtId, - pub value: Option, + pub value: GlobalValue, +} + +#[derive(Debug, Clone)] +pub enum GlobalValue { + Unresolved, + Resolving, + Resolved(comptime::Value), } #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -849,6 +856,7 @@ impl NodeInterner { let id = GlobalId(self.globals.len()); let location = Location::new(ident.span(), file); let name = ident.to_string(); + let definition_id = self.push_definition(name, mutable, comptime, DefinitionKind::Global(id), location); @@ -860,7 +868,7 @@ impl NodeInterner { crate_id, let_statement, location, - value: None, + value: GlobalValue::Unresolved, }); self.global_attributes.insert(id, attributes); id @@ -884,6 +892,7 @@ impl NodeInterner { ) -> GlobalId { let statement = self.push_stmt(HirStatement::Error); let span = name.span(); + let id = self .push_global(name, local_id, crate_id, statement, file, attributes, mutable, comptime); self.push_stmt_location(statement, span, file); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs index 899928528e61..7cb8731593b8 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs @@ -108,6 +108,8 @@ pub enum ParserErrorReason { WrongNumberOfAttributeArguments { name: String, min: usize, max: usize, found: usize }, #[error("The `deprecated` attribute expects a string argument")] DeprecatedAttributeExpectsAStringArgument, + #[error("Unsafe block must start with a safety comment")] + MissingSafetyComment, } /// Represents a parsing error, or a parsing error in the making. @@ -181,7 +183,11 @@ impl ParserError { } pub fn is_warning(&self) -> bool { - matches!(self.reason(), Some(ParserErrorReason::ExperimentalFeature(_))) + matches!( + self.reason(), + Some(ParserErrorReason::ExperimentalFeature(_)) + | Some(ParserErrorReason::MissingSafetyComment) + ) } } @@ -272,6 +278,11 @@ impl<'a> From<&'a ParserError> for Diagnostic { "Noir doesn't have immutable references, only mutable references".to_string(), error.span, ), + ParserErrorReason::MissingSafetyComment => Diagnostic::simple_warning( + "Missing Safety Comment".into(), + "Unsafe block must start with a safety comment: //@safety".into(), + error.span, + ), other => Diagnostic::simple_error(format!("{other}"), String::new(), error.span), }, None => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs index fcc58c5d8331..9ce288c97fa8 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs @@ -154,10 +154,7 @@ impl<'a> Parser<'a> { where F: FnOnce(&mut Parser<'a>) -> Option, { - match self.parse_result(parsing_function) { - Ok(item) => item, - Err(_) => None, - } + self.parse_result(parsing_function).unwrap_or_default() } /// Bumps this parser by one token. Returns the token that was previously the "current" token. diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/expression.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/expression.rs index 526a0c3dd6e9..6b059bb0c8c1 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -8,7 +8,7 @@ use crate::{ MemberAccessExpression, MethodCallExpression, Statement, TypePath, UnaryOp, UnresolvedType, }, parser::{labels::ParsingRuleLabel, parser::parse_many::separated_by_comma, ParserErrorReason}, - token::{Keyword, Token, TokenKind}, + token::{DocStyle, Keyword, SpannedToken, Token, TokenKind}, }; use super::{ @@ -375,6 +375,20 @@ impl<'a> Parser<'a> { return None; } + let next_token = self.next_token.token(); + if matches!( + next_token, + Token::LineComment(_, Some(DocStyle::Safety)) + | Token::BlockComment(_, Some(DocStyle::Safety)) + ) { + //Checks the safety comment is there, and skip it + let span = self.current_token_span; + self.eat_left_brace(); + self.token = SpannedToken::new(Token::LeftBrace, span); + } else { + self.push_error(ParserErrorReason::MissingSafetyComment, self.current_token_span); + } + let start_span = self.current_token_span; if let Some(block) = self.parse_block() { Some(ExpressionKind::Unsafe(block, self.span_since(start_span))) @@ -389,9 +403,7 @@ impl<'a> Parser<'a> { /// /// VariableExpression = Path fn parse_path_expr(&mut self, allow_constructors: bool) -> Option { - let Some(path) = self.parse_path() else { - return None; - }; + let path = self.parse_path()?; if allow_constructors && self.eat_left_brace() { let typ = UnresolvedType::from_path(path); @@ -421,9 +433,7 @@ impl<'a> Parser<'a> { } fn parse_constructor_field(&mut self) -> Option<(Ident, Expression)> { - let Some(ident) = self.eat_ident() else { - return None; - }; + let ident = self.eat_ident()?; Some(if self.eat_colon() { let expression = self.parse_expression_or_error(); @@ -534,9 +544,7 @@ impl<'a> Parser<'a> { /// TypePathExpression = PrimitiveType '::' identifier ( '::' GenericTypeArgs )? fn parse_type_path_expr(&mut self) -> Option { let start_span = self.current_token_span; - let Some(typ) = self.parse_primitive_type() else { - return None; - }; + let typ = self.parse_primitive_type()?; let typ = UnresolvedType { typ, span: self.span_since(start_span) }; self.eat_or_error(Token::DoubleColon); @@ -963,7 +971,8 @@ mod tests { #[test] fn parses_unsafe_expression() { - let src = "unsafe { 1 }"; + let src = "unsafe { //@safety: test + 1 }"; let expr = parse_expression_no_errors(src); let ExpressionKind::Unsafe(block, _) = expr.kind else { panic!("Expected unsafe expression"); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs index 5ece510022ad..3a4f6d922a4d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs @@ -23,6 +23,7 @@ impl<'a> Parser<'a> { // and throw the error in name resolution. let attributes = self.validate_secondary_attributes(attributes); + let is_global_let = true; let Some(ident) = self.eat_ident() else { self.eat_semicolons(); @@ -35,6 +36,7 @@ impl<'a> Parser<'a> { expression: Expression { kind: ExpressionKind::Error, span: Span::default() }, attributes, comptime, + is_global_let, }; }; @@ -53,7 +55,7 @@ impl<'a> Parser<'a> { self.expected_token(Token::Semicolon); } - LetStatement { pattern, r#type: typ, expression, attributes, comptime } + LetStatement { pattern, r#type: typ, expression, attributes, comptime, is_global_let } } } @@ -105,6 +107,7 @@ mod tests { assert_eq!("foo", name.to_string()); assert!(matches!(let_statement.r#type.typ, UnresolvedTypeData::Unspecified)); assert!(!let_statement.comptime); + assert!(let_statement.is_global_let); assert_eq!(visibility, ItemVisibility::Private); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/pattern.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/pattern.rs index c4dcab55d736..50779e9ccffa 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/pattern.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/pattern.rs @@ -154,9 +154,7 @@ impl<'a> Parser<'a> { /// InternedPattern = interned_pattern fn parse_interned_pattern(&mut self) -> Option { - let Some(token) = self.eat_kind(TokenKind::InternedPattern) else { - return None; - }; + let token = self.eat_kind(TokenKind::InternedPattern)?; match token.into_token() { Token::InternedPattern(pattern) => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/statement.rs index ff6117c9348b..14c8a415a38f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/statement.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/statement.rs @@ -364,7 +364,14 @@ impl<'a> Parser<'a> { Expression { kind: ExpressionKind::Error, span: self.current_token_span } }; - Some(LetStatement { pattern, r#type, expression, attributes, comptime: false }) + Some(LetStatement { + pattern, + r#type, + expression, + attributes, + comptime: false, + is_global_let: false, + }) } /// ConstrainStatement @@ -373,9 +380,7 @@ impl<'a> Parser<'a> { /// | 'assert_eq' Arguments fn parse_constrain_statement(&mut self) -> Option { let start_span = self.current_token_span; - let Some(kind) = self.parse_constrain_kind() else { - return None; - }; + let kind = self.parse_constrain_kind()?; Some(match kind { ConstrainKind::Assert | ConstrainKind::AssertEq => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/type_expression.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/type_expression.rs index 7dd59aedb459..83b04bb157a2 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/type_expression.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/type_expression.rs @@ -158,9 +158,7 @@ impl<'a> Parser<'a> { /// ConstantTypeExpression = int fn parse_constant_type_expression(&mut self) -> Option { - let Some(int) = self.eat_int() else { - return None; - }; + let int = self.eat_int()?; Some(UnresolvedTypeExpression::Constant(int, self.previous_token_span)) } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/where_clause.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/where_clause.rs index a753ffb6fd24..8945e6f29f5f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/where_clause.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/where_clause.rs @@ -36,9 +36,7 @@ impl<'a> Parser<'a> { } fn parse_single_where_clause(&mut self) -> Option<(UnresolvedType, Vec)> { - let Some(typ) = self.parse_type() else { - return None; - }; + let typ = self.parse_type()?; self.eat_or_error(Token::Colon); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index 8ddf1b571e62..3d908d1aa0c8 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -1287,11 +1287,15 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { check_rewrite(src, expected_rewrite); } +// TODO(https://github.com/noir-lang/noir/issues/6780): currently failing +// with a stack overflow #[test] +#[ignore] fn deny_cyclic_globals() { let src = r#" global A: u32 = B; global B: u32 = A; + fn main() {} "#; @@ -1991,9 +1995,6 @@ fn numeric_generic_u16_array_size() { )); } -// TODO(https://github.com/noir-lang/noir/issues/6238): -// The EvaluatedGlobalIsntU32 warning is a stopgap -// (originally from https://github.com/noir-lang/noir/issues/6125) #[test] fn numeric_generic_field_larger_than_u32() { let src = r#" @@ -2006,29 +2007,16 @@ fn numeric_generic_field_larger_than_u32() { } "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - assert!(matches!( - errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); - assert!(matches!( - errors[1].0, - CompilationError::ResolverError(ResolverError::IntegerTooLarge { .. }) - )); + assert_eq!(errors.len(), 0); } -// TODO(https://github.com/noir-lang/noir/issues/6238): -// The EvaluatedGlobalIsntU32 warning is a stopgap -// (originally from https://github.com/noir-lang/noir/issues/6126) #[test] fn numeric_generic_field_arithmetic_larger_than_u32() { let src = r#" struct Foo {} - impl Foo { - fn size(self) -> Field { - F - } + fn size(_x: Foo) -> Field { + F } // 2^32 - 1 @@ -2039,21 +2027,11 @@ fn numeric_generic_field_arithmetic_larger_than_u32() { } fn main() { - let _ = foo::().size(); + let _ = size(foo::()); } "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - - assert!(matches!( - errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); - - assert!(matches!( - errors[1].0, - CompilationError::ResolverError(ResolverError::UnusedVariable { .. }) - )); + assert_eq!(errors.len(), 0); } #[test] @@ -2180,25 +2158,11 @@ fn numeric_generics_type_kind_mismatch() { } "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 3); - - // TODO(https://github.com/noir-lang/noir/issues/6238): - // The EvaluatedGlobalIsntU32 warning is a stopgap + assert_eq!(errors.len(), 1); assert!(matches!( errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); - - assert!(matches!( - errors[1].0, CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), )); - - // TODO(https://github.com/noir-lang/noir/issues/6238): see above - assert!(matches!( - errors[2].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); } #[test] @@ -3219,20 +3183,20 @@ fn dont_infer_globals_to_u32_from_type_use() { } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 3); - assert!(matches!( - errors[0].0, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); - assert!(matches!( - errors[1].0, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); - assert!(matches!( - errors[2].0, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); + let mut errors = get_program_errors(src); + assert_eq!(errors.len(), 6); + for (error, _file_id) in errors.drain(0..3) { + assert!(matches!( + error, + CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) + )); + } + for (error, _file_id) in errors { + assert!(matches!( + error, + CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }) + )); + } } #[test] @@ -3242,7 +3206,8 @@ fn dont_infer_partial_global_types() { pub global NESTED_ARRAY: [[Field; _]; 3] = [[]; 3]; pub global STR: str<_> = "hi"; pub global NESTED_STR: [str<_>] = &["hi"]; - pub global FMT_STR: fmtstr<_, _> = f"hi {ARRAY}"; + pub global FORMATTED_VALUE: str<5> = "there"; + pub global FMT_STR: fmtstr<_, _> = f"hi {FORMATTED_VALUE}"; pub global TUPLE_WITH_MULTIPLE: ([str<_>], [[Field; _]; 3]) = (&["hi"], [[]; 3]); fn main() { } @@ -3318,13 +3283,9 @@ fn non_u32_as_array_length() { "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); + assert_eq!(errors.len(), 1); assert!(matches!( errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }) - )); - assert!(matches!( - errors[1].0, CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }) )); } @@ -3894,3 +3855,66 @@ fn disallows_underscore_on_right_hand_side() { assert_eq!(name, "_"); } + +#[test] +fn errors_on_cyclic_globals() { + let src = r#" + pub comptime global A: u32 = B; + pub comptime global B: u32 = A; + + fn main() { } + "#; + let errors = get_program_errors(src); + + assert!(errors.iter().any(|(error, _)| matches!( + error, + CompilationError::InterpreterError(InterpreterError::GlobalsDependencyCycle { .. }) + ))); + assert!(errors.iter().any(|(error, _)| matches!( + error, + CompilationError::ResolverError(ResolverError::DependencyCycle { .. }) + ))); +} + +#[test] +fn warns_on_unneeded_unsafe() { + let src = r#" + fn main() { + unsafe { + //@safety: test + foo() + } + } + + fn foo() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + &errors[0].0, + CompilationError::TypeError(TypeCheckError::UnnecessaryUnsafeBlock { .. }) + )); +} + +#[test] +fn warns_on_nested_unsafe() { + let src = r#" + fn main() { + unsafe { + //@safety: test + unsafe { + //@safety: test + foo() + } + } + } + + unconstrained fn foo() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + &errors[0].0, + CompilationError::TypeError(TypeCheckError::NestedUnsafeBlock { .. }) + )); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/aliases.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/aliases.rs index 8d3433299f65..1a5621c8adbb 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/aliases.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/aliases.rs @@ -47,3 +47,31 @@ fn alias_in_let_pattern() { "#; assert_no_errors(src); } + +#[test] +fn double_alias_in_path() { + let src = r#" + struct Foo {} + + impl Foo { + fn new() -> Self { + Self {} + } + } + + type FooAlias1 = Foo; + type FooAlias2 = FooAlias1; + + fn main() { + let _ = FooAlias2::new(); + } + "#; + assert_no_errors(src); +} + +#[test] +fn double_generic_alias_in_path() { + let src = r#" + "#; + assert_no_errors(src); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs index 3328fe439ae1..83de9c077ab3 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs @@ -153,3 +153,49 @@ fn arithmetic_generics_checked_cast_indirect_zeros() { panic!("unexpected error: {:?}", monomorphization_error); } } + +#[test] +fn global_numeric_generic_larger_than_u32() { + // Regression test for https://github.com/noir-lang/noir/issues/6125 + let source = r#" + global A: Field = 4294967297; + + fn foo() { } + + fn main() { + let _ = foo::(); + } + "#; + let errors = get_program_errors(source); + assert_eq!(errors.len(), 0); +} + +#[test] +fn global_arithmetic_generic_larger_than_u32() { + // Regression test for https://github.com/noir-lang/noir/issues/6126 + let source = r#" + struct Foo {} + + impl Foo { + fn size(self) -> Field { + let _ = self; + F + } + } + + // 2^32 - 1 + global A: Field = 4294967295; + + // Avoiding overflow succeeds: + // fn foo() -> Foo { + fn foo() -> Foo { + Foo {} + } + + fn main() { + let _ = foo::().size(); + } + "#; + let errors = get_program_errors(source); + assert_eq!(errors.len(), 0); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs index ce72240d146d..7ae3974a125f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs @@ -88,6 +88,7 @@ fn constrained_reference_to_unconstrained() { let x_ref = &mut x; if x == 5 { unsafe { + //@safety: test context mut_ref_input(x_ref, y); } } diff --git a/noir/noir-repo/cspell.json b/noir/noir-repo/cspell.json index 5c707e92e215..9a4bca1e339c 100644 --- a/noir/noir-repo/cspell.json +++ b/noir/noir-repo/cspell.json @@ -32,6 +32,7 @@ "boilerplates", "bridgekeeper", "brillig", + "bunx", "bytecount", "cachix", "callsite", @@ -242,9 +243,11 @@ "walkdir", "wasi", "wasmer", + "webapps", "Weierstraß", "whitespace", "whitespaces", + "YOLO", "zkhash", "zshell" ], diff --git a/noir/noir-repo/docs/docs/how_to/how-to-solidity-verifier.md b/noir/noir-repo/docs/docs/how_to/how-to-solidity-verifier.mdx similarity index 64% rename from noir/noir-repo/docs/docs/how_to/how-to-solidity-verifier.md rename to noir/noir-repo/docs/docs/how_to/how-to-solidity-verifier.mdx index 2cc0f8e57ce6..da36b60920d7 100644 --- a/noir/noir-repo/docs/docs/how_to/how-to-solidity-verifier.md +++ b/noir/noir-repo/docs/docs/how_to/how-to-solidity-verifier.mdx @@ -20,24 +20,37 @@ sidebar_position: 0 pagination_next: tutorials/noirjs_app --- -Noir has the ability to generate a verifier contract in Solidity, which can be deployed in many EVM-compatible blockchains such as Ethereum. +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + +Noir is universal. The witness and the compiled program can be fed into a proving backend such as Aztec's [Barretenberg](https://github.com/AztecProtocol/aztec-packages/tree/master/barretenberg), which can then generate a verifier contract for deployment on blockchains. This allows for a powerful feature set, as one can make use of the conciseness and the privacy provided by Noir in an immutable ledger. Applications can range from simple P2P guessing games, to complex private DeFi interactions. -This guide shows you how to generate a Solidity Verifier and deploy it on the [Remix IDE](https://remix.ethereum.org/). It is assumed that: +Although not strictly in the domain of Noir itself, this guide shows how to generate a Solidity Verifier with Barretenberg and deploy it on the [Remix IDE](https://remix.ethereum.org/). It is assumed that: +- You will be using Barretenberg as your proving backend +- You will be using an EVM blockchain to verify your proof - You are comfortable with the Solidity programming language and understand how contracts are deployed on the Ethereum network - You have Noir installed and you have a Noir program. If you don't, [get started](../getting_started/quick_start.md) with Nargo and the example Hello Noir circuit - You are comfortable navigating RemixIDE. If you aren't or you need a refresher, you can find some video tutorials [here](https://www.youtube.com/channel/UCjTUPyFEr2xDGN6Cg8nKDaA) that could help you. ## Rundown -Generating a Solidity Verifier contract is actually a one-command process. However, compiling it and deploying it can have some caveats. Here's the rundown of this guide: +Generating a Solidity Verifier with Barretenberg contract is actually a one-command process. However, compiling it and deploying it can have some caveats. Here's the rundown of this guide: 1. How to generate a solidity smart contract 2. How to compile the smart contract in the RemixIDE 3. How to deploy it to a testnet +:::info[Which proving system to use?] + +Barretenberg currently provides two provers: `UltraPlonk` and `UltraHonk`. In a nutshell, `UltraHonk` is faster and uses less RAM, but its verifier contract is much more expensive. `UltraPlonk` is optimized for on-chain verification, but proving is more expensive. + +In any case, we provide instructions for both. Choose your poison ☠️ + +::: + ## Step 1 - Generate a contract This is by far the most straightforward step. Just run: @@ -46,25 +59,31 @@ This is by far the most straightforward step. Just run: nargo compile ``` -This will compile your source code into a Noir build artifact to be stored in the `./target` directory, you can then generate the smart contract using the commands: +This will compile your source code into a Noir build artifact to be stored in the `./target` directory. From here on, it's Barretenberg's work. You can generate the smart contract using the commands: + + + + +```sh +bb write_vk_ultra_keccak_honk -b ./target/.json +bb contract_ultra_honk +``` + + + ```sh -# Here we pass the path to the newly generated Noir artifact. bb write_vk -b ./target/.json bb contract ``` -replacing `` with the name of your Noir project. A new `contract` folder would then be generated in your project directory, containing the Solidity -file `contract.sol`. It can be deployed to any EVM blockchain acting as a verifier smart contract. + + -You can find more information about `bb` and the default Noir proving backend on [this page](../getting_started/quick_start.md#proving-backend). +replacing `` with the name of your Noir project. A `Verifier.sol` contract is now in the target folder and can be deployed to any EVM blockchain acting as a verifier smart contract. -:::info - -It is possible to generate verifier contracts of Noir programs for other smart contract platforms as long as the proving backend supplies an implementation. +You can find more information about `bb` and the default Noir proving backend on [this page](../getting_started/quick_start.md#proving-backend). -Barretenberg, the default proving backend for Nargo, supports generation of verifier contracts, for the time being these are only in Solidity. -::: ## Step 2 - Compiling @@ -85,17 +104,12 @@ To compile our the verifier, we can navigate to the compilation tab: ![Compilation Tab](@site/static/img/how-tos/solidity_verifier_2.png) -Remix should automatically match a suitable compiler version. However, hitting the "Compile" button will most likely generate a "Stack too deep" error: +Remix should automatically match a suitable compiler version. However, hitting the "Compile" button will most likely tell you the contract is too big to deploy on mainnet, or complain about a stack too deep: -![Stack too deep](@site/static/img/how-tos/solidity_verifier_3.png) +![Contract code too big](@site/static/img/how-tos/solidity_verifier_6.png) +![Stack too deep](@site/static/img/how-tos/solidity_verifier_8.png) -This is due to the verify function needing to put many variables on the stack, but enabling the optimizer resolves the issue. To do this, let's open the "Advanced Configurations" tab and enable optimization. The default 200 runs will suffice. - -:::info - -This time we will see a warning about an unused function parameter. This is expected, as the `verify` function doesn't use the `_proof` parameter inside a solidity block, it is loaded from calldata and used in assembly. - -::: +To avoid this, you can just use some optimization. Open the "Advanced Configurations" tab and enable optimization. The default 200 runs will suffice. ![Compilation success](@site/static/img/how-tos/solidity_verifier_4.png) @@ -103,54 +117,81 @@ This time we will see a warning about an unused function parameter. This is expe At this point we should have a compiled contract ready to deploy. If we navigate to the deploy section in Remix, we will see many different environments we can deploy to. The steps to deploy on each environment would be out-of-scope for this guide, so we will just use the default Remix VM. -Looking closely, we will notice that our "Solidity Verifier" is actually three contracts working together: - -- An `UltraVerificationKey` library which simply stores the verification key for our circuit. -- An abstract contract `BaseUltraVerifier` containing most of the verifying logic. -- A main `UltraVerifier` contract that inherits from the Base and uses the Key contract. - -Remix will take care of the dependencies for us so we can simply deploy the UltraVerifier contract by selecting it and hitting "deploy": +Looking closely, we will notice that our "Solidity Verifier" is composed on multiple contracts working together. Remix will take care of the dependencies for us so we can simply deploy the Verifier contract by selecting it and hitting "deploy": -![Deploying UltraVerifier](@site/static/img/how-tos/solidity_verifier_5.png) + + -A contract will show up in the "Deployed Contracts" section, where we can retrieve the Verification Key Hash. This is particularly useful for double-checking that the deployer contract is the correct one. +![Deploying HonkVerifier](@site/static/img/how-tos/solidity_verifier_7.png) -:::note + + -Why "UltraVerifier"? +![Deploying PlonkVerifier](@site/static/img/how-tos/solidity_verifier_9.png) -To be precise, the Noir compiler (`nargo`) doesn't generate the verifier contract directly. It compiles the Noir code into an intermediate language (ACIR), which is then executed by the backend. So it is the backend that returns the verifier smart contract, not Noir. + + -In this case, the Barretenberg Backend uses the UltraPlonk proving system, hence the "UltraVerifier" name. - -::: +A contract will show up in the "Deployed Contracts" section. ## Step 4 - Verifying -To verify a proof using the Solidity verifier contract, we call the `verify` function in this extended contract: +To verify a proof using the Solidity verifier contract, we call the `verify` function: ```solidity function verify(bytes calldata _proof, bytes32[] calldata _publicInputs) external view returns (bool) ``` -When using the default example in the [Hello Noir](../getting_started/quick_start.md) guide, the easiest way to confirm that the verifier contract is doing its job is by calling the `verify` function via remix with the required parameters. Note that the public inputs must be passed in separately to the rest of the proof so we must split the proof as returned from `bb`. +First generate a proof with `bb`. We need a `Prover.toml` file for our inputs. Run: -First generate a proof with `bb` at the location `./proof` using the steps in [get started](../getting_started/quick_start.md), this proof is in a binary format but we want to convert it into a hex string to pass into Remix, this can be done with the +```bash +nargo check +``` + +This will generate a `Prover.toml` you can fill with the values you want to prove. We can now execute the circuit with `nargo` and then use the proving backend to prove: + + + + +```bash +nargo execute +bb prove_ultra_keccak_honk -b ./target/.json -w ./target/ -o ./target/proof +``` + +:::tip[Public inputs] +Barretenberg attaches the public inputs to the proof, which in this case it's not very useful. If you're up for some JS, `bb.js` has [a method for it](https://github.com/AztecProtocol/aztec-packages/blob/master/barretenberg/ts/src/proof/index.ts), but in the CLI you can use this ugly snippet: + +```bash +cat ./target/proof | od -An -v -t x1 | tr -d $' \n' | sed 's/^.\{8\}//' | (read hex; echo "${hex:0:192}${hex:256}") +``` + +Beautiful. This assumes a circuit with one public input (32 bytes, for Barretenberg). For more inputs, you can just increment `hex:256` with 32 more bytes for each public input. + +::: + + + ```bash -# This value must be changed to match the number of public inputs (including return values!) in your program. -NUM_PUBLIC_INPUTS=1 -PUBLIC_INPUT_BYTES=32*NUM_PUBLIC_INPUTS -HEX_PUBLIC_INPUTS=$(head -c $PUBLIC_INPUT_BYTES ./proof | od -An -v -t x1 | tr -d $' \n') -HEX_PROOF=$(tail -c +$(($PUBLIC_INPUT_BYTES + 1)) ./proof | od -An -v -t x1 | tr -d $' \n') +nargo execute +bb prove -b ./target/.json -w ./target/ -o ./target/proof +``` + -echo "Public inputs:" -echo $HEX_PUBLIC_INPUTS +:::tip[Public inputs] +Barretenberg attaches the public inputs to the proof, which in this case it's not very useful. If you're up for some JS, `bb.js` has [a method for it](https://github.com/AztecProtocol/aztec-packages/blob/master/barretenberg/ts/src/proof/index.ts), but in the CLI you can use this ugly snippet: -echo "Proof:" -echo "0x$HEX_PROOF" +```bash +tail -c +33 ./target/proof | od -An -v -t x1 | tr -d $' \n' ``` +Beautiful. This assumes a circuit with one public input (32 bytes, for Barretenberg). For more inputs, you can just add 32 more bytes for each public input to the `tail` command. + +::: + + + + Remix expects that the public inputs will be split into an array of `bytes32` values so `HEX_PUBLIC_INPUTS` needs to be split up into 32 byte chunks which are prefixed with `0x` accordingly. A programmatic example of how the `verify` function is called can be seen in the example zk voting application [here](https://github.com/noir-lang/noir-examples/blob/33e598c257e2402ea3a6b68dd4c5ad492bce1b0a/foundry-voting/src/zkVote.sol#L35): @@ -188,7 +229,7 @@ the `verify` function will expect the public inputs array (second function param Passing only two inputs will result in an error such as `PUBLIC_INPUT_COUNT_INVALID(3, 2)`. -In this case, the inputs parameter to `verify` would be an array ordered as `[pubkey_x, pubkey_y, return`. +In this case, the inputs parameter to `verify` would be an array ordered as `[pubkey_x, pubkey_y, return]`. ::: diff --git a/noir/noir-repo/docs/docs/noir/concepts/data_types/fields.md b/noir/noir-repo/docs/docs/noir/concepts/data_types/fields.md index b5af20f7d7e2..c339e3d05e4c 100644 --- a/noir/noir-repo/docs/docs/noir/concepts/data_types/fields.md +++ b/noir/noir-repo/docs/docs/noir/concepts/data_types/fields.md @@ -132,7 +132,7 @@ example: ```rust fn main() { let field = 2 - field.assert_max_bit_size(32); + field.assert_max_bit_size::<32>(); } ``` diff --git a/noir/noir-repo/docs/docs/noir/concepts/unconstrained.md b/noir/noir-repo/docs/docs/noir/concepts/unconstrained.md index b5221b8d2ddf..8e07d719f4be 100644 --- a/noir/noir-repo/docs/docs/noir/concepts/unconstrained.md +++ b/noir/noir-repo/docs/docs/noir/concepts/unconstrained.md @@ -67,6 +67,7 @@ We can then run `u72_to_u8` as unconstrained brillig code in order to calculate ```rust fn main(num: u72) -> pub [u8; 8] { let out = unsafe { + //@safety: 'out' is properly constrained below in 'assert(num == reconstructed_num);' u72_to_u8(num) }; @@ -96,6 +97,7 @@ This ends up taking off another ~250 gates from our circuit! We've ended up with Note that in order to invoke unconstrained functions we need to wrap them in an `unsafe` block, to make it clear that the call is unconstrained. +Furthermore, a warning is emitted unless the `unsafe` block starts with a `//@safety: ...` comment explaining why it is fine to call the unconstrained function. Generally we want to use brillig whenever there's something that's easy to verify but hard to compute within the circuit. For example, if you wanted to calculate a square root of a number it'll be a much better idea to calculate this in brillig and then assert that if you square the result you get back your number. diff --git a/noir/noir-repo/docs/docs/tutorials/noirjs_app.md b/noir/noir-repo/docs/docs/tutorials/noirjs_app.md index 6e69ea0bbedf..8967ee005cef 100644 --- a/noir/noir-repo/docs/docs/tutorials/noirjs_app.md +++ b/noir/noir-repo/docs/docs/tutorials/noirjs_app.md @@ -1,186 +1,91 @@ --- -title: Building a web app with NoirJS +title: Building a web app with Noir and Barretenberg description: Learn how to setup a new app that uses Noir to generate and verify zero-knowledge SNARK proofs in a typescript or javascript environment. keywords: [how to, guide, javascript, typescript, noir, barretenberg, zero-knowledge, proofs, app] sidebar_position: 0 pagination_next: noir/concepts/data_types/index --- -NoirJS is a set of packages meant to work both in a browser and a server environment. In this tutorial, we will build a simple web app using them. From here, you should get an idea on how to proceed with your own Noir projects! +NoirJS is a Typescript package meant to work both in a browser and a server environment. -You can find the complete app code for this guide [here](https://github.com/noir-lang/tiny-noirjs-app). - -## Setup - -:::note - -Feel free to use whatever versions, just keep in mind that Nargo and the NoirJS packages are meant to be in sync. For example, Nargo 0.31.x matches `noir_js@0.31.x`, etc. - -In this guide, we will be pinned to 0.31.0. - -::: - -Before we start, we want to make sure we have Node, Nargo and the Barretenberg proving system (`bb`) installed. - -We start by opening a terminal and executing `node --version`. If we don't get an output like `v20.10.0`, that means node is not installed. Let's do that by following the handy [nvm guide](https://github.com/nvm-sh/nvm?tab=readme-ov-file#install--update-script). - -As for `Nargo`, we can follow the [Nargo guide](../getting_started/quick_start.md) to install it. If you're lazy, just paste this on a terminal and run `noirup`: - -```sh -curl -L https://raw.githubusercontent.com/noir-lang/noirup/main/install | bash -``` +In this tutorial, we will combine NoirJS with Aztec's Barretenberg backend to build a simple web app. From here, you should get an idea on how to proceed with your own Noir projects! -Follow the instructions on [this page](https://github.com/AztecProtocol/aztec-packages/tree/master/barretenberg/cpp/src/barretenberg/bb#installation) to install `bb`. -Version 0.41.0 is compatible with `nargo` version 0.31.0, which you can install with `bbup -v 0.41.0` once `bbup` is installed. - -Easy enough. Onwards! - -## Our project - -ZK is a powerful technology. An app that doesn't reveal one of the inputs to _anyone_ is almost unbelievable, yet Noir makes it as easy as a single line of code. - -In fact, it's so simple that it comes nicely packaged in `nargo`. Let's do that! +You can find the complete app code for this guide [here](https://github.com/noir-lang/tiny-noirjs-app). -### Nargo +## Dependencies -Run: +Before we start, we want to make sure we have Node installed. For convenience (and speed), we can just install [Bun](https://bun.sh) as our package manager, and Node will work out-of-the-box: ```bash -nargo new circuit +curl -fsSL https://bun.sh/install | bash ``` -And... That's about it. Your program is ready to be compiled and run. +Let's go barebones. Doing the bare minimum is not only simple, but also allows you to easily adapt it to almost any frontend framework. -To compile, let's `cd` into the `circuit` folder to enter our project, and call: +Barebones means we can immediately start with the dependencies even on an empty folder 😈: ```bash -nargo compile +bun i @noir-lang/noir_wasm@1.0.0-beta.0 @noir-lang/noir_js@1.0.0-beta.0 @aztec/bb.js@0.63.1 ``` -This compiles our circuit into `json` format and add it to a new `target` folder. +Wait, what are these dependencies? -:::info +- `noir_wasm` is the `wasm` version of the Noir compiler. Although most developers prefer to use `nargo` for compiling, there's nothing wrong with `noir_wasm`. We like `noir_wasm`. +- `noir_js` is the main Noir package. It will execute our program, and generate the witness that will be sent to the backend. +- `bb.js` is the Typescript interface for Aztec's Barretenberg proving backend. It also uses the `wasm` version in order to run on the browser. -At this point in the tutorial, your folder structure should look like this: +:::info -```tree -. -└── circuit <---- our working directory - ├── Nargo.toml - ├── src - │ └── main.nr - └── target - └── circuit.json -``` +In this guide, we will install versions pinned to 1.0.0-beta.0. These work with Barretenberg version 0.63.1, so we are using that one version too. Feel free to try with older or later versions, though! ::: -### Node and Vite - -If you want to explore Nargo, feel free to go on a side-quest now and follow the steps in the -[getting started](../getting_started/quick_start.md) guide. However, we want our app to run on the browser, so we need Vite. +## Setting up our Noir program -Vite is a powerful tool to generate static websites. While it provides all kinds of features, let's just go barebones with some good old vanilla JS. +ZK is a powerful technology. An app that reveals computational correctness but doesn't reveal some of its inputs is almost unbelievable, yet Noir makes it as easy as a single line of code. -To do this this, go back to the previous folder (`cd ..`) and create a new vite project by running `npm create vite` and choosing "Vanilla" and "Javascript". +:::tip -A wild `vite-project` directory should now appear in your root folder! Let's not waste any time and dive right in: +It's not just you. We also enjoy syntax highlighting. [Check out the Language Server](../tooling/language_server.md) -```bash -cd vite-project -``` +::: -### Setting Up Vite and Configuring the Project - -Before we proceed with any coding, let's get our environment tailored for Noir. We'll start by laying down the foundations with a `vite.config.js` file. This little piece of configuration is our secret sauce for making sure everything meshes well with the NoirJS libraries and other special setups we might need, like handling WebAssembly modules. Here’s how you get that going: - -#### Creating the vite.config.js - -In your freshly minted `vite-project` folder, create a new file named `vite.config.js` and open it in your code editor. Paste the following to set the stage: - -```javascript -import { defineConfig } from 'vite'; -import copy from 'rollup-plugin-copy'; -import fs from 'fs'; -import path from 'path'; - -const wasmContentTypePlugin = { - name: 'wasm-content-type-plugin', - configureServer(server) { - server.middlewares.use(async (req, res, next) => { - if (req.url.endsWith('.wasm')) { - res.setHeader('Content-Type', 'application/wasm'); - const newPath = req.url.replace('deps', 'dist'); - const targetPath = path.join(__dirname, newPath); - const wasmContent = fs.readFileSync(targetPath); - return res.end(wasmContent); - } - next(); - }); - }, -}; +All you need is a `main.nr` and a `Nargo.toml` file. You can follow the [noirup](../getting_started/noir_installation.md) installation and just run `noirup -v 1.0.0-beta.0`, or just create them by hand: -export default defineConfig(({ command }) => { - if (command === 'serve') { - return { - build: { - target: 'esnext', - rollupOptions: { - external: ['@aztec/bb.js'] - } - }, - optimizeDeps: { - esbuildOptions: { - target: 'esnext' - } - }, - plugins: [ - copy({ - targets: [{ src: 'node_modules/**/*.wasm', dest: 'node_modules/.vite/dist' }], - copySync: true, - hook: 'buildStart', - }), - command === 'serve' ? wasmContentTypePlugin : [], - ], - }; - } - - return {}; -}); +```bash +mkdir -p circuit/src +touch circuit/src/main.nr circuit/Nargo.toml ``` -#### Install Dependencies - -Now that our stage is set, install the necessary NoirJS packages along with our other dependencies: +To make our program interesting, let's give it a real use-case scenario: Bob wants to prove he is older than 18, without disclosing his age. Open `main.nr` and write: -```bash -npm install && npm install @noir-lang/backend_barretenberg@0.31.0 @noir-lang/noir_js@0.31.0 -npm install rollup-plugin-copy --save-dev +```rust +fn main(age: u8) { + assert(age >= 18); +} ``` -:::info - -At this point in the tutorial, your folder structure should look like this: +This program accepts a private input called age, and simply proves this number is higher than 18. But to run this code, we need to give the compiler a `Nargo.toml` with at least a name and a type: -```tree -. -└── circuit - └── ...etc... -└── vite-project <---- our working directory - └── ...etc... +```toml +[package] +name = "circuit" +type = "bin" ``` -::: +This is all that we need to get started with Noir. -#### Some cleanup +![my heart is ready for you, noir.js](@site/static/img/memes/titanic.jpeg) -`npx create vite` is amazing but it creates a bunch of files we don't really need for our simple example. Actually, let's just delete everything except for `vite.config.js`, `index.html`, `main.js` and `package.json`. I feel lighter already. +## Setting up our app -![my heart is ready for you, noir.js](@site/static/img/memes/titanic.jpeg) +Remember when apps only had one `html` and one `js` file? Well, that's enough for Noir webapps. Let's create them: -## HTML +```bash +touch index.html index.js +``` -Our app won't run like this, of course. We need some working HTML, at least. Let's open our broken-hearted `index.html` and replace everything with this code snippet: +And add something useful to our HTML file: ```html @@ -200,11 +105,11 @@ Our app won't run like this, of course. We need some working HTML, at least. Let - +

Noir app

- - + +

Logs

@@ -214,32 +119,26 @@ Our app won't run like this, of course. We need some working HTML, at least. Let ``` -It _could_ be a beautiful UI... Depending on which universe you live in. - -## Some good old vanilla Javascript - -Our love for Noir needs undivided attention, so let's just open `main.js` and delete everything (this is where the romantic scenery becomes a bit creepy). +It _could_ be a beautiful UI... Depending on which universe you live in. In any case, we're using some scary CSS to make two boxes that will show cool things on the screen. -Start by pasting in this boilerplate code: +As for the JS, real madmen could just `console.log` everything, but let's say we want to see things happening (the true initial purpose of JS... right?). Here's some boilerplate for that. Just paste it in `index.js`: ```js -function display(container, msg) { - const c = document.getElementById(container); - const p = document.createElement('p'); - p.textContent = msg; - c.appendChild(p); -} +const show = (id, content) => { + const container = document.getElementById(id); + container.appendChild(document.createTextNode(content)); + container.appendChild(document.createElement("br")); +}; -document.getElementById('submitGuess').addEventListener('click', async () => { - try { - // here's where love happens - } catch (err) { - display('logs', 'Oh 💔 Wrong guess'); - } +document.getElementById("submit").addEventListener("click", async () => { + try { + // noir goes here + } catch { + show("logs", "Oh 💔"); + } }); -``` -The display function doesn't do much. We're simply manipulating our website to see stuff happening. For example, if the proof fails, it will simply log a broken heart 😢 +``` :::info @@ -248,30 +147,56 @@ At this point in the tutorial, your folder structure should look like this: ```tree . └── circuit - └── ...same as above -└── vite-project - ├── vite.config.js - ├── main.js - ├── package.json - └── index.html + └── src + └── main.nr + Nargo.toml + index.js + package.json + index.html + ...etc ``` -You'll see other files and folders showing up (like `package-lock.json`, `node_modules`) but you shouldn't have to care about those. - ::: -## Some NoirJS +## Compile compile compile -We're starting with the good stuff now. If you've compiled the circuit as described above, you should have a `json` file we want to import at the very top of our `main.js` file: +Finally we're up for something cool. But before we can execute a Noir program, we need to compile it into ACIR: an abstract representation. Here's where `noir_wasm` comes in. -```ts -import circuit from '../circuit/target/circuit.json'; +`noir_wasm` expects a filesystem so it can resolve dependencies. While we could use the `public` folder, let's just import those using the nice `?url` syntax provided by vite. At the top of the file: + +```js +import { compile, createFileManager } from "@noir-lang/noir_wasm" + +import main from "./circuit/src/main.nr?url"; +import nargoToml from "./circuit/Nargo.toml?url"; ``` -[Noir is backend-agnostic](../index.mdx#whats-new-about-noir). We write Noir, but we also need a proving backend. That's why we need to import and instantiate the two dependencies we installed above: `BarretenbergBackend` and `Noir`. Let's import them right below: +Compiling on the browser is common enough that `createFileManager` already gives us a nice in-memory filesystem we can use. So all we need to compile is fetching these files, writing them to our filesystem, and compile. Add this function: ```js -import { BarretenbergBackend, BarretenbergVerifier as Verifier } from '@noir-lang/backend_barretenberg'; +export async function getCircuit() { + const fm = createFileManager("/"); + const { body } = await fetch(main); + const { body: nargoTomlBody } = await fetch(nargoToml); + + fm.writeFile("./src/main.nr", body); + fm.writeFile("./Nargo.toml", nargoTomlBody); + return await compile(fm); +} +``` + +:::tip + +As you can imagine, with `node` it's all conveniently easier since you get native access to `fs`... + +::: + +## Some more JS + +We're starting with the good stuff now. We want to execute our circuit to get the witness, and then feed that witness to Barretenberg. Luckily, both packages are quite easy to work with. Let's import them at the top of the file: + +```js +import { UltraHonkBackend } from '@aztec/bb.js'; import { Noir } from '@noir-lang/noir_js'; ``` @@ -279,88 +204,103 @@ And instantiate them inside our try-catch block: ```ts // try { -const backend = new BarretenbergBackend(circuit); -const noir = new Noir(circuit); +const { program } = await getCircuit(); +const noir = new Noir(program); +const backend = new UltraHonkBackend(program.bytecode); // } ``` -:::note +:::warning -For the remainder of the tutorial, everything will be happening inside the `try` block +WASMs are not always easy to work with. In our case, `vite` likes serving them with the wrong MIME type. There are different fixes but we found the easiest one is just YOLO instantiating the WASMs manually. Paste this at the top of the file, just below the other imports, and it will work just fine: + +```js +import initNoirC from "@noir-lang/noirc_abi"; +import initACVM from "@noir-lang/acvm_js"; +import acvm from "@noir-lang/acvm_js/web/acvm_js_bg.wasm?url"; +import noirc from "@noir-lang/noirc_abi/web/noirc_abi_wasm_bg.wasm?url"; +await Promise.all([initACVM(fetch(acvm)), initNoirC(fetch(noirc))]); +``` ::: -## Our app +## Executing and proving -Now for the app itself. We're capturing whatever is in the input when people press the submit button. Just add this: +Now for the app itself. We're capturing whatever is in the input when people press the submit button. Inside our `try` block, let's just grab that input and get its value. Noir will gladly execute it, and give us a witness: ```js -const x = parseInt(document.getElementById('guessInput').value); -const input = { x, y: 2 }; +const age = document.getElementById("age").value; +show("logs", "Generating witness... ⏳"); +const { witness } = await noir.execute({ age }); +show("logs", "Generated witness... ✅"); + ``` +:::note + +For the remainder of the tutorial, everything will be happening inside the `try` block + +::: + Now we're ready to prove stuff! Let's feed some inputs to our circuit and calculate the proof: ```js -await setup(); // let's squeeze our wasm inits here - -display('logs', 'Generating proof... ⌛'); -const { witness } = await noir.execute(input); +show("logs", "Generating proof... ⏳"); const proof = await backend.generateProof(witness); -display('logs', 'Generating proof... ✅'); -display('results', proof.proof); +show("logs", "Generated proof... ✅"); +show("results", proof.proof); ``` -You're probably eager to see stuff happening, so go and run your app now! +Our program is technically **done** . You're probably eager to see stuff happening! To serve this in a convenient way, we can use a bundler like `vite` by creating a `vite.config.js` file: + +```bash +touch vite.config.js +``` -From your terminal, run `npm run dev`. If it doesn't open a browser for you, just visit `localhost:5173`. You should now see the worst UI ever, with an ugly input. +`vite` helps us with a little catch: `bb.js` in particular uses top-level awaits which aren't supported everywhere. So we can add this to the `vite.config.js` to make the bundler optimize them: -![Getting Started 0](@site/static/img/noir_getting_started_1.png) +```js +export default { optimizeDeps: { esbuildOptions: { target: "esnext" } } }; +``` + +This should be enough for vite. We don't even need to install it, just run: + +```bash +bunx vite +``` -Now, our circuit says `fn main(x: Field, y: pub Field)`. This means only the `y` value is public, and it's hardcoded above: `input = { x, y: 2 }`. In other words, you won't need to send your secret`x` to the verifier! +If it doesn't open a browser for you, just visit `localhost:5173`. You should now see the worst UI ever, with an ugly input. -By inputting any number other than 2 in the input box and clicking "submit", you should get a valid proof. Otherwise the proof won't even generate correctly. By the way, if you're human, you shouldn't be able to understand anything on the "proof" box. That's OK. We like you, human ❤️. +![Noir Webapp UI](@site/static/img/tutorials/noirjs_webapp/webapp1.png) + +Now, our circuit requires a private input `fn main(age: u8)`, and fails if it is less than 18. Let's see if it works. Submit any number above 18 (as long as it fits in 8 bits) and you should get a valid proof. Otherwise the proof won't even generate correctly. + +By the way, if you're human, you shouldn't be able to understand anything on the "proof" box. That's OK. We like you, human ❤️. ## Verifying Time to celebrate, yes! But we shouldn't trust machines so blindly. Let's add these lines to see our proof being verified: ```js -display('logs', 'Verifying proof... ⌛'); +show('logs', 'Verifying proof... ⌛'); const isValid = await backend.verifyProof(proof); - -// or to cache and use the verification key: -// const verificationKey = await backend.getVerificationKey(); -// const verifier = new Verifier(); -// const isValid = await verifier.verifyProof(proof, verificationKey); - -if (isValid) display('logs', 'Verifying proof... ✅'); +show("logs", `Proof is ${isValid ? "valid" : "invalid"}... ✅`); ``` You have successfully generated a client-side Noir web app! ![coded app without math knowledge](@site/static/img/memes/flextape.jpeg) -## Further Reading - -You can see how noirjs is used in a full stack Next.js hardhat application in the [noir-starter repo here](https://github.com/noir-lang/noir-starter/tree/main/vite-hardhat). The example shows how to calculate a proof in the browser and verify it with a deployed Solidity verifier contract from noirjs. - -You should also check out the more advanced examples in the [noir-examples repo](https://github.com/noir-lang/noir-examples), where you'll find reference usage for some cool apps. +## Next steps -## UltraHonk Backend +At this point, you have a working ZK app that works on the browser. Actually, it works on a mobile phone too! -Barretenberg has recently exposed a new UltraHonk backend. We can use UltraHonk in NoirJS after version 0.33.0. Everything will be the same as the tutorial above, except that the class we need to import will change: +If you want to continue learning by doing, here are some challenges for you: -```js -import { UltraHonkBackend, UltraHonkVerifier as Verifier } from '@noir-lang/backend_barretenberg'; -``` - -The backend will then be instantiated as such: - -```js -const backend = new UltraHonkBackend(circuit); -``` +- Install [nargo](https://noir-lang.org/docs/getting_started/noir_installation) and write [Noir tests](../tooling/testing) +- Change the circuit to accept a [public input](../noir/concepts/data_types/#private--public-types) as the cutoff age. It could be different depending on the purpose, for example! +- Enjoy Noir's Rust-like syntax and write a struct `Country` that implements a trait `MinAge` with a method `get_min_age`. Then, make a struct `Person` have an `u8` as its age and a country of type `Country`. You can pass a `person` in JS just like a JSON object `person: { age, country: { min_age: 18 }}` -Then all the commands to prove and verify your circuit will be same. +The world is your stage, just have fun with ZK! You can see how noirjs is used in a full stack Next.js hardhat application in the [noir-starter repo here](https://github.com/noir-lang/noir-starter/tree/main/vite-hardhat). The example shows how to calculate a proof in the browser and verify it with a deployed Solidity verifier contract from noirjs. -The only feature currently unsupported with UltraHonk are [recursive proofs](../explainers/explainer-recursion.md). +Check out other starters, tools, or just cool projects in the [awesome noir repository](https://github.com/noir-lang/awesome-noir). diff --git a/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_6.png b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_6.png new file mode 100644 index 000000000000..c92e874740a2 Binary files /dev/null and b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_6.png differ diff --git a/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_7.png b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_7.png new file mode 100644 index 000000000000..c8541fa544cb Binary files /dev/null and b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_7.png differ diff --git a/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_8.png b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_8.png new file mode 100644 index 000000000000..125c140acec6 Binary files /dev/null and b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_8.png differ diff --git a/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_9.png b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_9.png new file mode 100644 index 000000000000..150812e33209 Binary files /dev/null and b/noir/noir-repo/docs/static/img/how-tos/solidity_verifier_9.png differ diff --git a/noir/noir-repo/docs/static/img/tutorials/noirjs_webapp/webapp1.png b/noir/noir-repo/docs/static/img/tutorials/noirjs_webapp/webapp1.png new file mode 100644 index 000000000000..7591cd827e3e Binary files /dev/null and b/noir/noir-repo/docs/static/img/tutorials/noirjs_webapp/webapp1.png differ diff --git a/noir/noir-repo/noir_stdlib/src/array/check_shuffle.nr b/noir/noir-repo/noir_stdlib/src/array/check_shuffle.nr index 2e8c227feeee..828ac7332909 100644 --- a/noir/noir-repo/noir_stdlib/src/array/check_shuffle.nr +++ b/noir/noir-repo/noir_stdlib/src/array/check_shuffle.nr @@ -43,6 +43,9 @@ where T: Eq, { unsafe { + /*@safety : shuffle_indices is ensured to be a permutation of 0..N, and then + shuffle_indices is ensured to map lhs to rhs: assert(lhs[i] == rhs[shuffle_indices[i]]), for all i in 0..N + */ let shuffle_indices = __get_shuffle_indices(lhs, rhs); for i in 0..N { diff --git a/noir/noir-repo/noir_stdlib/src/array/mod.nr b/noir/noir-repo/noir_stdlib/src/array/mod.nr index 8bb425854f2d..1309a15d5715 100644 --- a/noir/noir-repo/noir_stdlib/src/array/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/array/mod.nr @@ -185,9 +185,10 @@ where /// ``` pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { unsafe { - // Safety: `sorted` array is checked to be: - // a. a permutation of `input`'s elements - // b. satisfying the predicate `ordering` + /*@safety: `sorted` array is checked to be: + a. a permutation of `input`'s elements + b. satisfying the predicate `ordering` + */ let sorted = quicksort::quicksort(self, ordering); if !is_unconstrained() { diff --git a/noir/noir-repo/noir_stdlib/src/collections/map.nr b/noir/noir-repo/noir_stdlib/src/collections/map.nr index 2b0da1b90ecd..bc0b80124db3 100644 --- a/noir/noir-repo/noir_stdlib/src/collections/map.nr +++ b/noir/noir-repo/noir_stdlib/src/collections/map.nr @@ -271,7 +271,7 @@ impl HashMap { for slot in self._table { if slot.is_valid() { - let (_, value) = unsafe { slot.key_value_unchecked() }; + let (_, value) = slot.key_value_unchecked(); values.push(value); } } diff --git a/noir/noir-repo/noir_stdlib/src/collections/umap.nr b/noir/noir-repo/noir_stdlib/src/collections/umap.nr index 7aebeb437cf8..27af14caa24d 100644 --- a/noir/noir-repo/noir_stdlib/src/collections/umap.nr +++ b/noir/noir-repo/noir_stdlib/src/collections/umap.nr @@ -113,7 +113,11 @@ impl UHashMap { H: Hasher, { // docs:end:contains_key - unsafe { self.get(key) }.is_some() + unsafe { + //@safety : unconstrained context + self.get(key) + } + .is_some() } // Returns true if the map contains no elements. @@ -432,7 +436,10 @@ where // Not marked as deleted and has key-value. if equal & slot.is_valid() { let (key, value) = slot.key_value_unchecked(); - let other_value = unsafe { other.get(key) }; + let other_value = unsafe { + //@safety : unconstrained context + other.get(key) + }; if other_value.is_none() { equal = false; diff --git a/noir/noir-repo/noir_stdlib/src/convert.nr b/noir/noir-repo/noir_stdlib/src/convert.nr index d4bea8b6390d..1e5c1484b5fe 100644 --- a/noir/noir-repo/noir_stdlib/src/convert.nr +++ b/noir/noir-repo/noir_stdlib/src/convert.nr @@ -117,3 +117,51 @@ impl From for Field { } } // docs:end:from-impls + +/// A generic interface for casting between primitive types, +/// equivalent of using the `as` keyword between values. +/// +/// # Example +/// +/// ``` +/// let x: Field = 1234567890; +/// let y: u8 = x as u8; +/// let z: u8 = x.as_(); +/// assert_eq(y, z); +/// ``` +pub trait AsPrimitive { + /// The equivalent of doing `self as T`. + fn as_(self) -> T; +} + +#[generate_as_primitive_impls] +comptime fn generate_as_primitive_impls(_: FunctionDefinition) -> Quoted { + let types = [ + quote { bool }, + quote { u8 }, + quote { u16 }, + quote { u32 }, + quote { u64 }, + quote { i8 }, + quote { i16 }, + quote { i32 }, + quote { i64 }, + quote { Field }, + ]; + + let mut impls = &[]; + for type1 in types { + for type2 in types { + impls = impls.push_back( + quote { + impl AsPrimitive<$type1> for $type2 { + fn as_(self) -> $type1 { + self as $type1 + } + } + }, + ); + } + } + impls.join(quote {}) +} diff --git a/noir/noir-repo/noir_stdlib/src/field/bn254.nr b/noir/noir-repo/noir_stdlib/src/field/bn254.nr index a7ca7d773737..f3cbdca27622 100644 --- a/noir/noir-repo/noir_stdlib/src/field/bn254.nr +++ b/noir/noir-repo/noir_stdlib/src/field/bn254.nr @@ -39,6 +39,10 @@ fn assert_gt_limbs(a: (Field, Field), b: (Field, Field)) { let (alo, ahi) = a; let (blo, bhi) = b; unsafe { + /*@safety: borrow is enforced to be boolean due to its type. + if borrow is 0, it asserts that (alo > blo && ahi >= bhi) + if borrow is 1, it asserts that (alo <= blo && ahi > bhi) + */ let borrow = lte_hint(alo, blo); let rlo = alo - blo - 1 + (borrow as Field) * TWO_POW_128; @@ -55,6 +59,7 @@ pub fn decompose(x: Field) -> (Field, Field) { compute_decomposition(x) } else { unsafe { + /*@safety: decomposition is properly checked below*/ // Take hints of the decomposition let (xlo, xhi) = decompose_hint(x); @@ -74,7 +79,12 @@ pub fn decompose(x: Field) -> (Field, Field) { pub fn assert_gt(a: Field, b: Field) { if is_unconstrained() { - assert(unsafe { field_less_than(b, a) }); + assert( + unsafe { + //@safety: already unconstrained + field_less_than(b, a) + }, + ); } else { // Decompose a and b let a_limbs = decompose(a); @@ -92,13 +102,14 @@ pub fn assert_lt(a: Field, b: Field) { pub fn gt(a: Field, b: Field) -> bool { if is_unconstrained() { unsafe { + //@safety: unsafe in unconstrained field_less_than(b, a) } } else if a == b { false } else { - // Take a hint of the comparison and verify it unsafe { + //@safety: Take a hint of the comparison and verify it if field_less_than(a, b) { assert_gt(b, a); false diff --git a/noir/noir-repo/noir_stdlib/src/field/mod.nr b/noir/noir-repo/noir_stdlib/src/field/mod.nr index a5c1ff7bb620..7e5ad97d64f2 100644 --- a/noir/noir-repo/noir_stdlib/src/field/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/field/mod.nr @@ -238,6 +238,7 @@ pub fn bytes32_to_field(bytes32: [u8; 32]) -> Field { fn lt_fallback(x: Field, y: Field) -> bool { if is_unconstrained() { unsafe { + //@safety : unconstrained context field_less_than(x, y) } } else { diff --git a/noir/noir-repo/noir_stdlib/src/hash/mod.nr b/noir/noir-repo/noir_stdlib/src/hash/mod.nr index 5f7017b60b75..89f6d2a13634 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/mod.nr @@ -96,7 +96,10 @@ fn __derive_generators( // does not assert the limbs are 128 bits // does not assert the decomposition does not overflow the EmbeddedCurveScalar fn from_field_unsafe(scalar: Field) -> EmbeddedCurveScalar { - let (xlo, xhi) = unsafe { crate::field::bn254::decompose_hint(scalar) }; + let (xlo, xhi) = unsafe { + //@safety : xlo and xhi decomposition is checked below + crate::field::bn254::decompose_hint(scalar) + }; // Check that the decomposition is correct assert_eq(scalar, xlo + crate::field::bn254::TWO_POW_128 * xhi); EmbeddedCurveScalar { lo: xlo, hi: xhi } diff --git a/noir/noir-repo/noir_stdlib/src/hash/sha256.nr b/noir/noir-repo/noir_stdlib/src/hash/sha256.nr index b9a2b02c9d9a..a4e21304912b 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/sha256.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/sha256.nr @@ -74,8 +74,10 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { let mut msg_byte_ptr = 0; for i in 0..num_blocks { let msg_start = BLOCK_SIZE * i; - let (new_msg_block, new_msg_byte_ptr) = - unsafe { build_msg_block(msg, message_size, msg_start) }; + let (new_msg_block, new_msg_byte_ptr) = unsafe { + /*@safety : the msg_block is checked below in verify_msg_block*/ + build_msg_block(msg, message_size, msg_start) + }; if msg_start < message_size { msg_block = new_msg_block; @@ -104,8 +106,10 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { // or our message cannot be evenly split into blocks. if modulo != 0 { let msg_start = BLOCK_SIZE * num_blocks; - let (new_msg_block, new_msg_byte_ptr) = - unsafe { build_msg_block(msg, message_size, msg_start) }; + let (new_msg_block, new_msg_byte_ptr) = unsafe { + //@safety : the msg_block is checked below in verify_msg_block + build_msg_block(msg, message_size, msg_start) + }; if msg_start < message_size { msg_block = new_msg_block; @@ -146,7 +150,10 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { msg_byte_ptr = 0; } - msg_block = unsafe { attach_len_to_msg_block(msg_block, msg_byte_ptr, message_size) }; + msg_block = unsafe { + //@safety : the msg_len is checked below in verify_msg_len + attach_len_to_msg_block(msg_block, msg_byte_ptr, message_size) + }; if !is_unconstrained() { verify_msg_len(msg_block, last_block, msg_byte_ptr, message_size); @@ -798,7 +805,10 @@ mod tests { 101, 115, 46, 48, ]; assert_eq(input.len(), 22); - let (msg_block, msg_byte_ptr) = unsafe { build_msg_block(input, input.len(), 0) }; + let (msg_block, msg_byte_ptr) = unsafe { + //@safety : testing context + build_msg_block(input, input.len(), 0) + }; assert_eq(msg_byte_ptr, input.len()); assert_eq(msg_block[0], make_item(input[0], input[1], input[2], input[3])); assert_eq(msg_block[1], make_item(input[4], input[5], input[6], input[7])); @@ -815,7 +825,10 @@ mod tests { 108, 97, 105, 110, 59, 32, 99, 104, 97, 114, 115, 101, 116, ]; assert_eq(input.len(), 68); - let (msg_block, msg_byte_ptr) = unsafe { build_msg_block(input, input.len(), 64) }; + let (msg_block, msg_byte_ptr) = unsafe { + //@safety : testing context + build_msg_block(input, input.len(), 64) + }; assert_eq(msg_byte_ptr, 4); assert_eq(msg_block[0], make_item(input[64], input[65], input[66], input[67])); assert_eq(msg_block[1], 0); @@ -828,7 +841,10 @@ mod tests { 1919905082, 1131376244, 1701737517, 1417244773, 978151789, 1697470053, 1920166255, 1849316213, 1651139939, ]; - let msg_block = unsafe { attach_len_to_msg_block(input, 1, 448) }; + let msg_block = unsafe { + //@safety : testing context + attach_len_to_msg_block(input, 1, 448) + }; assert_eq(msg_block[0], ((1 << 7) as u32) * 256 * 256 * 256); assert_eq(msg_block[1], 0); assert_eq(msg_block[15], 3584); diff --git a/noir/noir-repo/noir_stdlib/src/lib.nr b/noir/noir-repo/noir_stdlib/src/lib.nr index c3a289a8fa2b..cdc82b205099 100644 --- a/noir/noir-repo/noir_stdlib/src/lib.nr +++ b/noir/noir-repo/noir_stdlib/src/lib.nr @@ -28,6 +28,8 @@ pub mod mem; pub mod panic; pub mod hint; +use convert::AsPrimitive; + // Oracle calls are required to be wrapped in an unconstrained function // Thus, the only argument to the `println` oracle is expected to always be an ident #[oracle(print)] @@ -39,12 +41,14 @@ unconstrained fn print_unconstrained(with_newline: bool, input: T) { pub fn println(input: T) { unsafe { + //@safety: a print statement cannot be constrained print_unconstrained(true, input); } } pub fn print(input: T) { unsafe { + //@safety: a print statement cannot be constrained print_unconstrained(false, input); } } @@ -89,28 +93,29 @@ pub fn assert_constant(x: T) {} #[builtin(static_assert)] pub fn static_assert(predicate: bool, message: str) {} -// from_field and as_field are private since they are not valid for every type. -// `as` should be the default for users to cast between primitive types, and in the future -// traits can be used to work with generic types. -#[builtin(from_field)] -fn from_field(x: Field) -> T {} - -#[builtin(as_field)] -fn as_field(x: T) -> Field {} - -pub fn wrapping_add(x: T, y: T) -> T { - crate::from_field(crate::as_field(x) + crate::as_field(y)) +pub fn wrapping_add(x: T, y: T) -> T +where + T: AsPrimitive, + Field: AsPrimitive, +{ + AsPrimitive::as_(x.as_() + y.as_()) } -pub fn wrapping_sub(x: T, y: T) -> T { +pub fn wrapping_sub(x: T, y: T) -> T +where + T: AsPrimitive, + Field: AsPrimitive, +{ //340282366920938463463374607431768211456 is 2^128, it is used to avoid underflow - crate::from_field( - crate::as_field(x) + 340282366920938463463374607431768211456 - crate::as_field(y), - ) + AsPrimitive::as_(x.as_() + 340282366920938463463374607431768211456 - y.as_()) } -pub fn wrapping_mul(x: T, y: T) -> T { - crate::from_field(crate::as_field(x) * crate::as_field(y)) +pub fn wrapping_mul(x: T, y: T) -> T +where + T: AsPrimitive, + Field: AsPrimitive, +{ + AsPrimitive::as_(x.as_() * y.as_()) } #[builtin(as_witness)] diff --git a/noir/noir-repo/noir_stdlib/src/meta/expr.nr b/noir/noir-repo/noir_stdlib/src/meta/expr.nr index bb795a520d38..a6bb4630e813 100644 --- a/noir/noir-repo/noir_stdlib/src/meta/expr.nr +++ b/noir/noir-repo/noir_stdlib/src/meta/expr.nr @@ -669,7 +669,12 @@ comptime fn new_unary_op(op: UnaryOp, rhs: Expr) -> Expr { comptime fn new_unsafe(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { ; }); - quote { unsafe { $exprs }}.as_expr().unwrap() + quote { unsafe { + //@safety: generated by macro + $exprs + }} + .as_expr() + .unwrap() } comptime fn join_expressions(exprs: [Expr], separator: Quoted) -> Quoted { diff --git a/noir/noir-repo/noir_stdlib/src/meta/mod.nr b/noir/noir-repo/noir_stdlib/src/meta/mod.nr index 5d2164a510d3..f2234300ab26 100644 --- a/noir/noir-repo/noir_stdlib/src/meta/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/meta/mod.nr @@ -52,7 +52,7 @@ pub comptime fn derive(s: StructDefinition, traits: [TraitDefinition]) -> Quoted let mut result = quote {}; for trait_to_derive in traits { - let handler = unsafe { HANDLERS.get(trait_to_derive) }; + let handler = HANDLERS.get(trait_to_derive); assert(handler.is_some(), f"No derive function registered for `{trait_to_derive}`"); let trait_impl = handler.unwrap()(s); diff --git a/noir/noir-repo/noir_stdlib/src/uint128.nr b/noir/noir-repo/noir_stdlib/src/uint128.nr index 06febb3ee001..fab8cacc055d 100644 --- a/noir/noir-repo/noir_stdlib/src/uint128.nr +++ b/noir/noir-repo/noir_stdlib/src/uint128.nr @@ -1,5 +1,6 @@ use crate::cmp::{Eq, Ord, Ordering}; use crate::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Not, Rem, Shl, Shr, Sub}; +use super::convert::AsPrimitive; global pow64: Field = 18446744073709551616; //2^64; global pow63: Field = 9223372036854775808; // 2^63; @@ -103,8 +104,16 @@ impl U128 { if ascii < 58 { ascii - 48 } else { - let ascii = - ascii + 32 * (unsafe { U128::uconstrained_check_is_upper_ascii(ascii) as u8 }); + let ascii = ascii + + 32 + * ( + unsafe { + /*@safety : optionally adds 32 and then check (below) the result is in 'a..f' range + This enforces that the input is in either 'A..F' or 'a..f' range + */ + U128::uconstrained_check_is_upper_ascii(ascii) as u8 + } + ); assert(ascii >= 97); // enforce >= 'a' assert(ascii <= 102); // enforce <= 'f' ascii - 87 @@ -139,8 +148,11 @@ impl U128 { } } - pub fn from_integer(i: T) -> U128 { - let f = crate::as_field(i); + pub fn from_integer(i: T) -> U128 + where + T: AsPrimitive, + { + let f = i.as_(); // Reject values which would overflow a u128 f.assert_max_bit_size::<128>(); let lo = f as u64 as Field; @@ -148,8 +160,11 @@ impl U128 { U128 { lo, hi } } - pub fn to_integer(self) -> T { - crate::from_field(self.lo + self.hi * pow64) + pub fn to_integer(self) -> T + where + Field: AsPrimitive, + { + AsPrimitive::as_(self.lo + self.hi * pow64) } fn wrapping_mul(self: Self, b: U128) -> U128 { @@ -206,6 +221,9 @@ impl Mul for U128 { impl Div for U128 { fn div(self: Self, b: U128) -> U128 { unsafe { + /*@safety : euclidian division is asserted to be correct: assert(a == b * q + r); and assert(r < b); + Furthermore, U128 addition and multiplication ensures that b * q + r does not overflow + */ let (q, r) = self.unconstrained_div(b); let a = b * q + r; assert_eq(self, a); @@ -218,6 +236,7 @@ impl Div for U128 { impl Rem for U128 { fn rem(self: Self, b: U128) -> U128 { unsafe { + //@safety : cf div() above let (q, r) = self.unconstrained_div(b); let a = b * q + r; assert_eq(self, a); @@ -437,6 +456,7 @@ mod tests { let c = U128::one(); let d = U128::from_u64s_le(0x0, 0x1); unsafe { + //@safety: testing context let (q, r) = a.unconstrained_div(b); assert_eq(q, c); assert_eq(r, d); @@ -446,12 +466,14 @@ mod tests { let b = U128::one(); // Check the case where a is a multiple of b unsafe { + //@safety: testing context let (c, d) = a.unconstrained_div(b); assert_eq((c, d), (a, U128::zero())); } // Check where b is a multiple of a unsafe { + //@safety: testing context let (c, d) = b.unconstrained_div(a); assert_eq((c, d), (U128::zero(), b)); } @@ -460,6 +482,7 @@ mod tests { let a = U128::from_u64s_le(0x1, 0x0); let b = U128::zero(); unsafe { + //@safety: testing context let (c, d) = a.unconstrained_div(b); assert_eq((c, d), (U128::zero(), U128::zero())); } @@ -467,6 +490,7 @@ mod tests { let a = U128::from_u64s_le(0x0, pow63 as u64); let b = U128::from_u64s_le(0x0, pow63 as u64); unsafe { + //@safety: testing context let (c, d) = a.unconstrained_div(b); assert_eq((c, d), (U128::one(), U128::zero())); } diff --git a/noir/noir-repo/test_programs/compilation_report.sh b/noir/noir-repo/test_programs/compilation_report.sh index d050e7c9c34b..360eecd28491 100755 --- a/noir/noir-repo/test_programs/compilation_report.sh +++ b/noir/noir-repo/test_programs/compilation_report.sh @@ -10,7 +10,7 @@ tests_to_profile=("sha256_regression" "regression_4709" "ram_blowup_regression") echo "{\"compilation_reports\": [ " > $current_dir/compilation_report.json # If there is an argument that means we want to generate a report for only the current directory -if [ "$#" -ne 0 ]; then +if [ "$1" == "1" ]; then base_path="$current_dir" tests_to_profile=(".") fi @@ -32,12 +32,30 @@ for dir in ${tests_to_profile[@]}; do # The default package to run is the supplied list hardcoded at the top of the script PACKAGE_NAME=$dir # Otherwise default to the current directory as the package we want to run - if [ "$#" -ne 0 ]; then + if [ "$1" == "1" ]; then PACKAGE_NAME=$(basename $current_dir) fi - COMPILE_TIME=$((time nargo compile --force --silence-warnings) 2>&1 | grep real | grep -oE '[0-9]+m[0-9]+.[0-9]+s') - echo -e " {\n \"artifact_name\":\"$PACKAGE_NAME\",\n \"time\":\"$COMPILE_TIME\"" >> $current_dir/compilation_report.json + NUM_RUNS=1 + TOTAL_TIME=0 + + # Passing a second argument will take an average of five runs + # rather than + if [ "$2" == "1" ]; then + NUM_RUNS=5 + fi + + for ((i = 1; i <= NUM_RUNS; i++)); do + COMPILE_TIME=$((time nargo compile --force --silence-warnings) 2>&1 | grep real | grep -oE '[0-9]+m[0-9]+.[0-9]+s') + # Convert time to seconds and add to total time + TOTAL_TIME=$(echo "$TOTAL_TIME + $(echo $COMPILE_TIME | sed -E 's/([0-9]+)m([0-9.]+)s/\1 * 60 + \2/' | bc)" | bc) + done + + AVG_TIME=$(echo "$TOTAL_TIME / $NUM_RUNS" | bc -l) + # Keep only last three decimal points + AVG_TIME=$(awk '{printf "%.3f\n", $1}' <<< "$AVG_TIME") + + echo -e " {\n \"artifact_name\":\"$PACKAGE_NAME\",\n \"time\":\""$AVG_TIME"s\"" >> $current_dir/compilation_report.json if (($ITER == $NUM_ARTIFACTS)); then echo "}" >> $current_dir/compilation_report.json diff --git a/noir/noir-repo/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr b/noir/noir-repo/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr index f262635e5089..29431005c29c 100644 --- a/noir/noir-repo/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr +++ b/noir/noir-repo/test_programs/compile_failure/brillig_mut_ref_from_acir/src/main.nr @@ -4,6 +4,7 @@ unconstrained fn mut_ref_identity(value: &mut Field) -> Field { fn main(mut x: Field, y: pub Field) { let returned_x = unsafe { + //@safety: testing context mut_ref_identity(&mut x) }; assert(returned_x == x); diff --git a/noir/noir-repo/test_programs/compile_failure/regression_5008/src/main.nr b/noir/noir-repo/test_programs/compile_failure/regression_5008/src/main.nr index cf79c22683f8..be1adb119aae 100644 --- a/noir/noir-repo/test_programs/compile_failure/regression_5008/src/main.nr +++ b/noir/noir-repo/test_programs/compile_failure/regression_5008/src/main.nr @@ -14,6 +14,7 @@ fn main() { let foo = Foo { bar: &mut Bar { value: 0 } }; unsafe { + //@safety: testing context foo.crash_fn() }; } diff --git a/noir/noir-repo/test_programs/compile_failure/unconstrained_ref/src/main.nr b/noir/noir-repo/test_programs/compile_failure/unconstrained_ref/src/main.nr index 9a4f42469b52..282d598b484c 100644 --- a/noir/noir-repo/test_programs/compile_failure/unconstrained_ref/src/main.nr +++ b/noir/noir-repo/test_programs/compile_failure/unconstrained_ref/src/main.nr @@ -5,6 +5,7 @@ unconstrained fn uncon_ref() -> &mut Field { fn main() { let e = unsafe { + //@safety: testing context uncon_ref() }; } diff --git a/noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/acir_inside_brillig_recursion/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/acir_inside_brillig_recursion/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/acir_inside_brillig_recursion/src/main.nr similarity index 88% rename from noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/acir_inside_brillig_recursion/src/main.nr index 49b7c00b6b97..b4cc69bb00a3 100644 --- a/noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/acir_inside_brillig_recursion/src/main.nr @@ -1,5 +1,6 @@ fn main() { unsafe { + //@safety: testing context assert_eq(fibonacci(3), fibonacci_hint(3)); } } diff --git a/noir/noir-repo/test_programs/compile_success_empty/brillig_cast/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/brillig_cast/src/main.nr index f1a6814f4e7a..581ca8dc16af 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/brillig_cast/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/brillig_cast/src/main.nr @@ -3,6 +3,7 @@ // The features being tested are cast operations on brillig fn main() { unsafe { + //@safety: testing context bool_casts(); field_casts(); uint_casts(); diff --git a/noir/noir-repo/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr index fedfe48cb0dc..15f1b23853d1 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/brillig_field_binary_operations/src/main.nr @@ -1,6 +1,7 @@ // Tests arithmetic operations on fields fn main() { unsafe { + //@safety: testing context let x = 4; let y = 2; assert((x + y) == add(x, y)); diff --git a/noir/noir-repo/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr index 7ecc21dbd2f1..4ee048073558 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr @@ -4,6 +4,7 @@ fn main() { let y: u32 = 2; unsafe { + //@safety: testing context assert((x + y) == add(x, y)); assert((x - y) == sub(x, y)); diff --git a/noir/noir-repo/test_programs/compile_success_empty/brillig_modulo/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/brillig_modulo/src/main.nr index 2ad43bdb7eb5..47f2466f453e 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/brillig_modulo/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/brillig_modulo/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is modulo operations on brillig fn main() { unsafe { + //@safety: testing context assert(modulo(47, 3) == 2); assert(modulo(2, 3) == 2); assert(signed_modulo(5, 3) == 2); diff --git a/noir/noir-repo/test_programs/compile_success_empty/brillig_slice_input/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/brillig_slice_input/src/main.nr index 436a939767ee..41bd2d8c3ef2 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/brillig_slice_input/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/brillig_slice_input/src/main.nr @@ -17,12 +17,14 @@ fn main() { let mut slice = &[]; slice = slice.push_back([Point { x: 13, y: 14 }, Point { x: 20, y: 8 }]); unsafe { + //@safety: testing context let brillig_sum = sum_slice(slice); assert_eq(brillig_sum, 55); } slice = slice.push_back([Point { x: 15, y: 5 }, Point { x: 12, y: 13 }]); unsafe { + //@safety: testing context let brillig_sum = sum_slice(slice); assert_eq(brillig_sum, 100); } diff --git a/noir/noir-repo/test_programs/execution_success/cast_and_shift_global/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/cast_and_shift_global/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/cast_and_shift_global/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/cast_and_shift_global/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/cast_and_shift_global/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/cast_and_shift_global/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/cast_and_shift_global/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/cast_and_shift_global/src/main.nr diff --git a/noir/noir-repo/test_programs/execution_success/check_large_field_bits/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/check_large_field_bits/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/check_large_field_bits/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/check_large_field_bits/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/check_large_field_bits/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/check_large_field_bits/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/check_large_field_bits/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/check_large_field_bits/src/main.nr diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_as_field/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/comptime_as_field/Nargo.toml new file mode 100644 index 000000000000..294832ba3297 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_as_field/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "comptime_as_field" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_as_field/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_as_field/src/main.nr new file mode 100644 index 000000000000..e13db54b80bd --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_as_field/src/main.nr @@ -0,0 +1,6 @@ +fn main() { + comptime { + let _: U128 = U128::from_integer(1); + } +} + diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_from_field/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/comptime_from_field/Nargo.toml new file mode 100644 index 000000000000..38a46ba0dbec --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_from_field/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "comptime_from_field" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_from_field/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_from_field/src/main.nr new file mode 100644 index 000000000000..722c5c7eb329 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_from_field/src/main.nr @@ -0,0 +1,6 @@ +fn main() { + comptime { + let _: Field = U128::from_hex("0x0").to_integer(); + } +} + diff --git a/noir/noir-repo/test_programs/execution_success/comptime_slice_equality/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/comptime_slice_equality/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/comptime_slice_equality/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/comptime_slice_equality/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/comptime_slice_equality/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_slice_equality/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/comptime_slice_equality/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/comptime_slice_equality/src/main.nr diff --git a/noir/noir-repo/test_programs/compile_success_empty/double_generic_alias_in_path/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/double_generic_alias_in_path/Nargo.toml new file mode 100644 index 000000000000..aaebee8d6efc --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/double_generic_alias_in_path/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "double_generic_alias_in_path" +type = "bin" +authors = [""] +compiler_version = ">=0.32.0" + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/double_generic_alias_in_path/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/double_generic_alias_in_path/src/main.nr new file mode 100644 index 000000000000..09f2e5c4b43d --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/double_generic_alias_in_path/src/main.nr @@ -0,0 +1,14 @@ +struct Foo {} + +impl Foo { + fn new() -> Self { + Self {} + } +} + +type FooAlias1 = Foo; +type FooAlias2 = FooAlias1; + +fn main() { + let _ = FooAlias2::new(); +} diff --git a/noir/noir-repo/test_programs/execution_success/empty/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/empty/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/empty/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/empty/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/empty/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/empty/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/empty/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/empty/src/main.nr diff --git a/noir/noir-repo/test_programs/execution_success/is_unconstrained/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/is_unconstrained/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/is_unconstrained/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/is_unconstrained/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/is_unconstrained/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/is_unconstrained/src/main.nr similarity index 89% rename from noir/noir-repo/test_programs/execution_success/is_unconstrained/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/is_unconstrained/src/main.nr index d06366cf6420..dddf2dfec2eb 100644 --- a/noir/noir-repo/test_programs/execution_success/is_unconstrained/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/is_unconstrained/src/main.nr @@ -10,6 +10,7 @@ unconstrained fn unconstrained_intermediate() { fn main() { unsafe { + //@safety: testing context unconstrained_intermediate(); } check(false); diff --git a/noir/noir-repo/test_programs/compile_success_empty/macros_in_comptime/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/macros_in_comptime/src/main.nr index 799091fca09d..9e4439e67cd3 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/macros_in_comptime/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/macros_in_comptime/src/main.nr @@ -6,7 +6,10 @@ global three_field: Field = 3; fn main() { comptime { - unsafe { foo::(5) }; + unsafe { + //@safety: testing context + foo::(5) + }; submodule::bar(); } } diff --git a/noir/noir-repo/test_programs/execution_success/regression_5462/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/regression_5462/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/regression_5462/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/regression_5462/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/regression_5462/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/regression_5462/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/regression_5462/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/regression_5462/src/main.nr diff --git a/noir/noir-repo/test_programs/execution_success/trait_inheritance/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/trait_inheritance/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/trait_inheritance/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/trait_inheritance/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/trait_inheritance/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/trait_inheritance/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/trait_inheritance/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/trait_inheritance/src/main.nr diff --git a/noir/noir-repo/test_programs/execution_success/unit_value/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/unit_value/Nargo.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/unit_value/Nargo.toml rename to noir/noir-repo/test_programs/compile_success_empty/unit_value/Nargo.toml diff --git a/noir/noir-repo/test_programs/execution_success/unit_value/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/unit_value/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/unit_value/src/main.nr rename to noir/noir-repo/test_programs/compile_success_empty/unit_value/src/main.nr diff --git a/noir/noir-repo/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr b/noir/noir-repo/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr index 33b84c2b702f..b8c4137aa74c 100644 --- a/noir/noir-repo/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_no_bug/check_uncostrained_regression/src/main.nr @@ -16,6 +16,7 @@ unconstrained fn convert(trigger: Trigger) -> ResultType { impl Trigger { fn execute(self) -> ResultType { let result = unsafe { + //@safety: testing context convert(self) }; assert(result.a == self.x + 1); diff --git a/noir/noir-repo/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr b/noir/noir-repo/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr index 1d9269eebd5e..7047be3a5772 100644 --- a/noir/noir-repo/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_with_bug/underconstrained_value_detector_5425/src/main.nr @@ -10,6 +10,7 @@ unconstrained fn maximum_price(options: [u32; 3]) -> u32 { fn main(sandwiches: pub [u32; 3], drinks: pub [u32; 3], snacks: pub [u32; 3], best_value: u32) { unsafe { + //@safety: testing context let meal_deal_cost: u32 = 390; let most_expensive_sandwich = maximum_price(sandwiches); let mut sandwich_exists = false; diff --git a/noir/noir-repo/test_programs/execution_failure/brillig_assert_fail/src/main.nr b/noir/noir-repo/test_programs/execution_failure/brillig_assert_fail/src/main.nr index 07256f0c398c..4ca70baae2ed 100644 --- a/noir/noir-repo/test_programs/execution_failure/brillig_assert_fail/src/main.nr +++ b/noir/noir-repo/test_programs/execution_failure/brillig_assert_fail/src/main.nr @@ -4,6 +4,7 @@ fn main(x: Field) { assert( 1 == unsafe { + //@safety: testing context conditional(x as bool) } ); diff --git a/noir/noir-repo/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr b/noir/noir-repo/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr index 4dd06ceb7430..d726e699bab9 100644 --- a/noir/noir-repo/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr +++ b/noir/noir-repo/test_programs/execution_failure/brillig_assert_msg_runtime/src/main.nr @@ -1,6 +1,7 @@ fn main(x: Field) { assert( 1 == unsafe { + //@safety: testing context conditional(x) } ); diff --git a/noir/noir-repo/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr b/noir/noir-repo/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr index 1c1563ffe1d3..b87744399c61 100644 --- a/noir/noir-repo/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr +++ b/noir/noir-repo/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr @@ -15,6 +15,7 @@ fn fold_conditional_wrapper(x: bool) -> Field { #[fold] fn fold_conditional(x: bool) -> Field { unsafe { + //@safety: testing context conditional_wrapper(x) } } diff --git a/noir/noir-repo/test_programs/execution_failure/regression_5202/src/main.nr b/noir/noir-repo/test_programs/execution_failure/regression_5202/src/main.nr index fbcdba433554..a270a9fccad6 100644 --- a/noir/noir-repo/test_programs/execution_failure/regression_5202/src/main.nr +++ b/noir/noir-repo/test_programs/execution_failure/regression_5202/src/main.nr @@ -14,12 +14,14 @@ unconstrained fn should_i_assert() -> bool { fn get_magical_boolean() -> bool { let option = unsafe { + //@safety: testing context get_unconstrained_option() }; let pre_assert = option.is_some().to_field(); if unsafe { + //@safety: testing context should_i_assert() } { // Note that `should_i_assert` is unconstrained, so Noir should not be able to infer diff --git a/noir/noir-repo/test_programs/execution_report.sh b/noir/noir-repo/test_programs/execution_report.sh new file mode 100755 index 000000000000..073871e60f8f --- /dev/null +++ b/noir/noir-repo/test_programs/execution_report.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +set -e + +current_dir=$(pwd) +base_path="$current_dir/execution_success" + +# Tests to be profiled for execution report +tests_to_profile=("sha256_regression" "regression_4709" "ram_blowup_regression") + +echo "{\"execution_reports\": [ " > $current_dir/execution_report.json + +# If there is an argument that means we want to generate a report for only the current directory +if [ "$1" == "1" ]; then + base_path="$current_dir" + tests_to_profile=(".") +fi + +ITER="1" +NUM_ARTIFACTS=${#tests_to_profile[@]} + +for dir in ${tests_to_profile[@]}; do + if [[ " ${excluded_dirs[@]} " =~ " ${dir} " ]]; then + continue + fi + + if [[ ${CI-false} = "true" ]] && [[ " ${ci_excluded_dirs[@]} " =~ " ${dir} " ]]; then + continue + fi + + cd $base_path/$dir + + # The default package to run is the supplied list hardcoded at the top of the script + PACKAGE_NAME=$dir + # Otherwise default to the current directory as the package we want to run + if [ "$1" == "1" ]; then + PACKAGE_NAME=$(basename $current_dir) + fi + + # Check whether a compilation artifact exists. + # Any programs part of this benchmark should already be compiled. + # We want to make sure that compilation time is not included in the execution time. + if [ ! -e ./target/*.json ]; then + echo "Missing compilation artifact for $PACKAGE_NAME" + exit 1 + fi + + + NUM_RUNS=1 + TOTAL_TIME=0 + + if [ "$2" == "1" ]; then + NUM_RUNS=5 + fi + + for ((i = 1; i <= NUM_RUNS; i++)); do + EXECUTION_TIME=$((time nargo execute --silence-warnings) 2>&1 | grep real | grep -oE '[0-9]+m[0-9]+.[0-9]+s') + # Convert to seconds and add to total time + TOTAL_TIME=$(echo "$TOTAL_TIME + $(echo $EXECUTION_TIME | sed -E 's/([0-9]+)m([0-9.]+)s/\1 * 60 + \2/' | bc)" | bc) + done + + AVG_TIME=$(echo "$TOTAL_TIME / $NUM_RUNS" | bc -l) + # Keep only last three decimal points + AVG_TIME=$(awk '{printf "%.3f\n", $1}' <<< "$AVG_TIME") + + echo -e " {\n \"artifact_name\":\"$PACKAGE_NAME\",\n \"time\":\""$AVG_TIME"s\"" >> $current_dir/execution_report.json + + if (($ITER == $NUM_ARTIFACTS)); then + echo "}" >> $current_dir/execution_report.json + else + echo "}, " >> $current_dir/execution_report.json + fi + + ITER=$(( $ITER + 1 )) +done + +echo "]}" >> $current_dir/execution_report.json diff --git a/noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/Prover.toml b/noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/Prover.toml deleted file mode 100644 index 8b137891791f..000000000000 --- a/noir/noir-repo/test_programs/execution_success/acir_inside_brillig_recursion/Prover.toml +++ /dev/null @@ -1 +0,0 @@ - diff --git a/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr b/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr index 3d3992971f73..7738bed52552 100644 --- a/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr @@ -29,9 +29,15 @@ fn main(inputs: str<12>, iv: str<16>, key: str<16>, output: str<32>) { let result: [u8; 16] = std::aes128::aes128_encrypt(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()).as_array(); - let output_bytes: [u8; 16] = unsafe { decode_hex(output) }; + let output_bytes: [u8; 16] = unsafe { + //@safety: testing context + decode_hex(output) + }; assert(result == output_bytes); - let unconstrained_result = unsafe { cipher(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()) }; + let unconstrained_result = unsafe { + //@safety: testing context + cipher(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()) + }; assert(unconstrained_result == output_bytes); } diff --git a/noir/noir-repo/test_programs/execution_success/array_to_slice_constant_length/src/main.nr b/noir/noir-repo/test_programs/execution_success/array_to_slice_constant_length/src/main.nr index 09f996229395..425c48c21f1c 100644 --- a/noir/noir-repo/test_programs/execution_success/array_to_slice_constant_length/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/array_to_slice_constant_length/src/main.nr @@ -5,6 +5,7 @@ unconstrained fn return_array(val: Field) -> [Field; 1] { fn main(val: Field) { unsafe { + //@safety: testing context let array = return_array(val); assert_constant(array.as_slice().len()); } diff --git a/noir/noir-repo/test_programs/execution_success/bigint/src/main.nr b/noir/noir-repo/test_programs/execution_success/bigint/src/main.nr index 5a8b9fb67ef8..65cf990dcaf5 100644 --- a/noir/noir-repo/test_programs/execution_success/bigint/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/bigint/src/main.nr @@ -18,6 +18,7 @@ fn main(mut x: [u8; 5], y: [u8; 5]) { test_unconstrained1(a, b) } else { unsafe { + //@safety: testing context test_unconstrained2(a, b) } }; diff --git a/noir/noir-repo/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr index 1a595ecfb383..bcb995591c78 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_acir_as_brillig/src/main.nr @@ -1,5 +1,6 @@ fn main(x: u32) { unsafe { + //@safety: testing context assert(entry_point(x) == 2); swap_entry_point(x, x + 1); assert(deep_entry_point(x) == 4); diff --git a/noir/noir-repo/test_programs/execution_success/brillig_arrays/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_arrays/src/main.nr index 00d43e2b10de..b4cd13e10910 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_arrays/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_arrays/src/main.nr @@ -3,6 +3,7 @@ // The features being tested are array reads and writes fn main(x: [Field; 3]) { unsafe { + //@safety: testing context read_array(x); read_write_array(x); } diff --git a/noir/noir-repo/test_programs/execution_success/brillig_blake2s/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_blake2s/src/main.nr index c4412bd2bba6..58f45e8d6670 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_blake2s/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_blake2s/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is blake2s in brillig fn main(x: [u8; 5], result: [u8; 32]) { unsafe { + //@safety: testing context assert(blake2s(x) == result); } } diff --git a/noir/noir-repo/test_programs/execution_success/brillig_calls/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_calls/src/main.nr index c76d7651b0a4..5bd3458da517 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_calls/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_calls/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is brillig calls fn main(x: u32) { unsafe { + /*@safety : testing context*/ assert(entry_point(x) == 2); swap_entry_point(x, x + 1); assert(deep_entry_point(x) == 4); diff --git a/noir/noir-repo/test_programs/execution_success/brillig_calls_array/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_calls_array/src/main.nr index 90f588ab9884..8d5e01dd7c40 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_calls_array/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_calls_array/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is brillig calls passing arrays around fn main(x: [u32; 3]) { unsafe { + //@safety: testing context assert(entry_point(x) == 9); another_entry_point(x); } diff --git a/noir/noir-repo/test_programs/execution_success/brillig_calls_conditionals/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_calls_conditionals/src/main.nr index 0fbfd84968f8..e03f04a0cbcc 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_calls_conditionals/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_calls_conditionals/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is brillig calls with conditionals fn main(x: [u32; 3]) { unsafe { + //@safety: testing context assert(entry_point(x[0]) == 7); assert(entry_point(x[1]) == 8); assert(entry_point(x[2]) == 9); diff --git a/noir/noir-repo/test_programs/execution_success/brillig_conditional/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_conditional/src/main.nr index 17484f6e8c5b..5934f982f88c 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_conditional/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_conditional/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is basic conditonal on brillig fn main(x: Field) { unsafe { + //@safety: testing context assert(4 == conditional(x == 1)); } } diff --git a/noir/noir-repo/test_programs/execution_success/brillig_fns_as_values/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_fns_as_values/src/main.nr index 03a92b4b10db..b56542fd0ae4 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_fns_as_values/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_fns_as_values/src/main.nr @@ -4,6 +4,7 @@ struct MyStruct { fn main(x: u32) { unsafe { + //@safety: testing context assert(wrapper(increment, x) == x + 1); assert(wrapper(increment_acir, x) == x + 1); assert(wrapper(decrement, x) == x - 1); diff --git a/noir/noir-repo/test_programs/execution_success/brillig_identity_function/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_identity_function/src/main.nr index f3b45d0de4e1..7bf584579d3e 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_identity_function/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_identity_function/src/main.nr @@ -7,6 +7,7 @@ struct myStruct { // The features being tested is the identity function in Brillig fn main(x: Field) { unsafe { + //@safety: testing context assert(x == identity(x)); // TODO: add support for array comparison let arr = identity_array([x, x]); diff --git a/noir/noir-repo/test_programs/execution_success/brillig_nested_arrays/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_nested_arrays/src/main.nr index 77ab4ea19a6e..e97db35f8006 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_nested_arrays/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_nested_arrays/src/main.nr @@ -29,6 +29,7 @@ unconstrained fn create_and_assert_inside_brillig(x: Field, y: Field) { fn main(x: Field, y: Field) { unsafe { + //@safety: testing context let header = Header { params: [1, 2, 3] }; let note0 = MyNote { array: [1, 2], plain: 3, header }; let note1 = MyNote { array: [4, 5], plain: 6, header }; diff --git a/noir/noir-repo/test_programs/execution_success/brillig_not/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_not/src/main.nr index 4e61b6a54eab..38641f5ee5fb 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_not/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_not/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is not instruction on brillig fn main(x: Field, y: Field) { unsafe { + //@safety: testing context assert(false == not_operator(x as bool)); assert(true == not_operator(y as bool)); } diff --git a/noir/noir-repo/test_programs/execution_success/brillig_oracle/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_oracle/src/main.nr index 8f5b2fa7566d..e59c908fd6fa 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_oracle/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_oracle/src/main.nr @@ -4,6 +4,7 @@ use std::test::OracleMock; // Tests oracle usage in brillig/unconstrained functions fn main(_x: Field) { unsafe { + //@safety: testing context let size = 20; // TODO: Add a method along the lines of `(0..size).to_array()`. let mut mock_oracle_response = [0; 20]; diff --git a/noir/noir-repo/test_programs/execution_success/brillig_recursion/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_recursion/src/main.nr index 5354777a1de7..fa048c30abef 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_recursion/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_recursion/src/main.nr @@ -3,6 +3,7 @@ // The feature being tested is brillig recursion fn main(x: u32) { unsafe { + //@safety: testing context assert(fibonacci(x) == 55); } } diff --git a/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr index d4b74162cfb7..839ac489f2d0 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr @@ -1,5 +1,6 @@ fn main(x: Field, y: Field) -> pub Field { unsafe { + //@safety: testing context let notes = create_notes(x, y); sum_x(notes, x, y) } diff --git a/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr b/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr index 2eaab810d6a0..70423810327b 100644 --- a/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr @@ -25,7 +25,10 @@ unconstrained fn calculate_global_value() -> Field { } // Regression test for https://github.com/noir-lang/noir/issues/4318 -global CALCULATED_GLOBAL: Field = unsafe { calculate_global_value() }; +global CALCULATED_GLOBAL: Field = unsafe { + //@safety: testing context + calculate_global_value() +}; fn main( a: [Field; M + N - N], diff --git a/noir/noir-repo/test_programs/execution_success/hint_black_box/src/main.nr b/noir/noir-repo/test_programs/execution_success/hint_black_box/src/main.nr index 1109c54301f8..d53cf3358788 100644 --- a/noir/noir-repo/test_programs/execution_success/hint_black_box/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/hint_black_box/src/main.nr @@ -23,7 +23,10 @@ fn main(a: u32, b: u32) { //assert_eq(slice_sum(black_box(arr.as_slice())), b); // But we can pass a blackboxed slice to Brillig. - let s = unsafe { brillig_slice_sum(black_box(arr.as_slice())) }; + let s = unsafe { + //@safety: testing context + brillig_slice_sum(black_box(arr.as_slice())) + }; assert_eq(s, b); let mut d = b; diff --git a/noir/noir-repo/test_programs/execution_success/is_unconstrained/Prover.toml b/noir/noir-repo/test_programs/execution_success/is_unconstrained/Prover.toml deleted file mode 100644 index 8b137891791f..000000000000 --- a/noir/noir-repo/test_programs/execution_success/is_unconstrained/Prover.toml +++ /dev/null @@ -1 +0,0 @@ - diff --git a/noir/noir-repo/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr b/noir/noir-repo/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr index f8070aa3ef47..aec0ba3be33a 100644 --- a/noir/noir-repo/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/nested_arrays_from_brillig/src/main.nr @@ -20,7 +20,10 @@ unconstrained fn create_inside_brillig(values: [Field; 6]) -> [MyNote; 2] { } fn main(values: [Field; 6]) { - let notes = unsafe { create_inside_brillig(values) }; + let notes = unsafe { + //@safety: testing context + create_inside_brillig(values) + }; assert(access_nested(notes) == (2 + 4 + 3 + 1)); } diff --git a/noir/noir-repo/test_programs/execution_success/reference_only_used_as_alias/src/main.nr b/noir/noir-repo/test_programs/execution_success/reference_only_used_as_alias/src/main.nr index 8b5b804cf940..c310f052fce1 100644 --- a/noir/noir-repo/test_programs/execution_success/reference_only_used_as_alias/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/reference_only_used_as_alias/src/main.nr @@ -62,6 +62,7 @@ where { |e: Event| { unsafe { + //@safety: testing context func(context.a); } emit_with_keys(context, randomness, e, compute); diff --git a/noir/noir-repo/test_programs/execution_success/regression_5435/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_5435/src/main.nr index 895a6983ad93..4e90002e2a2c 100644 --- a/noir/noir-repo/test_programs/execution_success/regression_5435/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/regression_5435/src/main.nr @@ -3,7 +3,10 @@ fn main(input: Field, enable: bool) { let hash = no_predicate_function(input); // `EnableSideEffects` instruction from above instruction leaks out and removes the predicate from this call, // resulting in execution failure. - unsafe { fail(hash) }; + unsafe { + //@safety: testing context + fail(hash) + }; } } diff --git a/noir/noir-repo/test_programs/execution_success/regression_6451/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_6451/src/main.nr index b13b6c25a7ee..192e09fe3f57 100644 --- a/noir/noir-repo/test_programs/execution_success/regression_6451/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/regression_6451/src/main.nr @@ -10,7 +10,10 @@ fn main(x: Field) { value.assert_max_bit_size::<1>(); // Regression test for #6447 (Aztec Packages issue #9488) - let y = unsafe { empty(x + 1) }; + let y = unsafe { + //@safety: testing context + empty(x + 1) + }; let z = y + x + 1; let z1 = z + y; assert(z + z1 != 3); diff --git a/noir/noir-repo/test_programs/execution_success/regression_6674_3/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_6674_3/src/main.nr index 2c87a4c679ce..a0f2d0668d55 100644 --- a/noir/noir-repo/test_programs/execution_success/regression_6674_3/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/regression_6674_3/src/main.nr @@ -166,7 +166,10 @@ pub unconstrained fn sort_by_counter_desc(array: [T; N]) -> [T; N pub unconstrained fn sort_by(array: [T; N]) -> [T; N] { let mut result = array; - unsafe { get_sorting_index(array) }; + unsafe { + //@safety: testing context + get_sorting_index(array) + }; result } diff --git a/noir/noir-repo/test_programs/execution_success/regression_6834/Nargo.toml b/noir/noir-repo/test_programs/execution_success/regression_6834/Nargo.toml new file mode 100644 index 000000000000..edc3257d52d5 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/regression_6834/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "regression_6834" +version = "0.1.0" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/test_programs/execution_success/regression_6834/Prover.toml b/noir/noir-repo/test_programs/execution_success/regression_6834/Prover.toml new file mode 100644 index 000000000000..9ef840487ad0 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/regression_6834/Prover.toml @@ -0,0 +1,2 @@ +year = 1 +min_age = 1 diff --git a/noir/noir-repo/test_programs/execution_success/regression_6834/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_6834/src/main.nr new file mode 100644 index 000000000000..cea8d163ab56 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/regression_6834/src/main.nr @@ -0,0 +1,9 @@ +fn main(year: u32, min_age: u8) -> pub u32 { + if min_age == 0 { + (year - 2) / 4 + } else if year > 1 { + (year - 2) / 4 + } else { + 0 + } +} diff --git a/noir/noir-repo/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr index fc1e55ee6416..c0d8b336a04b 100644 --- a/noir/noir-repo/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/regression_unsafe_no_predicates/src/main.nr @@ -7,7 +7,10 @@ fn main(x: u8, nest: bool) { #[no_predicates] pub fn unsafe_assert(msg: [u8; N]) -> u8 { - let block = unsafe { get_block(msg) }; + let block = unsafe { + //@safety: testing context + get_block(msg) + }; verify_block(msg, block); block[0] } diff --git a/noir/noir-repo/test_programs/execution_success/u16_support/src/main.nr b/noir/noir-repo/test_programs/execution_success/u16_support/src/main.nr index ca41c7080771..c6a212f908ee 100644 --- a/noir/noir-repo/test_programs/execution_success/u16_support/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/u16_support/src/main.nr @@ -1,6 +1,7 @@ fn main(x: u16) { test_u16(x); unsafe { + //@safety: testing context test_u16_unconstrained(x); } } diff --git a/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr b/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr index 689ba9d4a04a..d1ba1c6175dd 100644 --- a/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr @@ -293,7 +293,10 @@ unconstrained fn doc_tests() { // docs:start:get_example fn get_example(map: UHashMap>) { - let x = unsafe { map.get(12) }; + let x = unsafe { + //@safety: testing context + map.get(12) + }; if x.is_some() { assert(x.unwrap() == 42); @@ -318,7 +321,11 @@ fn entries_examples(map: UHashMap {value}"); } // docs:end:keys_example diff --git a/noir/noir-repo/test_programs/gates_report_brillig.sh b/noir/noir-repo/test_programs/gates_report_brillig.sh index d3f6344dbf47..7343857a3c50 100755 --- a/noir/noir-repo/test_programs/gates_report_brillig.sh +++ b/noir/noir-repo/test_programs/gates_report_brillig.sh @@ -28,6 +28,6 @@ done echo "]" >> Nargo.toml -nargo info --force-brillig --json > gates_report_brillig.json +nargo info --force-brillig --json | jq -r ".programs[].functions = []" > gates_report_brillig.json rm Nargo.toml diff --git a/noir/noir-repo/test_programs/gates_report_brillig_execution.sh b/noir/noir-repo/test_programs/gates_report_brillig_execution.sh index b3f4a8bda987..6a5a699e2d8f 100755 --- a/noir/noir-repo/test_programs/gates_report_brillig_execution.sh +++ b/noir/noir-repo/test_programs/gates_report_brillig_execution.sh @@ -38,6 +38,6 @@ done echo "]" >> Nargo.toml -nargo info --profile-execution --json > gates_report_brillig_execution.json +nargo info --profile-execution --json | jq -r ".programs[].functions = []" > gates_report_brillig_execution.json -rm Nargo.toml \ No newline at end of file +rm Nargo.toml diff --git a/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr index 709180879a05..39167242a7bc 100644 --- a/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr +++ b/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr @@ -597,7 +597,12 @@ mod tests { fn test_expr_as_unsafe() { comptime { - let expr = quote { unsafe { 1; 4; 23 } }.as_expr().unwrap(); + let expr = quote { + unsafe + { + //@safety" test + 1; 4; 23 } + }.as_expr().unwrap(); let exprs = expr.as_unsafe().unwrap(); assert_eq(exprs.len(), 3); } @@ -607,7 +612,9 @@ mod tests { fn test_expr_modify_for_unsafe() { comptime { - let expr = quote { unsafe { 1; 4; 23 } }.as_expr().unwrap(); + let expr = quote { unsafe { + //@safety: test + 1; 4; 23 } }.as_expr().unwrap(); let expr = expr.modify(times_two); let exprs = expr.as_unsafe().unwrap(); assert_eq(exprs.len(), 3); diff --git a/noir/noir-repo/test_programs/noir_test_success/mock_oracle/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/mock_oracle/src/main.nr index 1b427043e917..b026f510418c 100644 --- a/noir/noir-repo/test_programs/noir_test_success/mock_oracle/src/main.nr +++ b/noir/noir-repo/test_programs/noir_test_success/mock_oracle/src/main.nr @@ -35,6 +35,7 @@ unconstrained fn struct_field(point: Point, array: [Field; 4]) -> Field { #[test(should_fail)] fn test_mock_no_returns() { unsafe { + //@safety: testing context OracleMock::mock("void_field"); void_field(); // Some return value must be set } @@ -43,6 +44,7 @@ fn test_mock_no_returns() { #[test] fn test_mock() { unsafe { + //@safety: testing context OracleMock::mock("void_field").returns(10); assert_eq(void_field(), 10); } @@ -51,6 +53,7 @@ fn test_mock() { #[test] fn test_multiple_mock() { unsafe { + //@safety: testing context let first_mock = OracleMock::mock("void_field").returns(10); OracleMock::mock("void_field").returns(42); @@ -65,6 +68,7 @@ fn test_multiple_mock() { #[test] fn test_multiple_mock_times() { unsafe { + //@safety: testing context OracleMock::mock("void_field").returns(10).times(2); OracleMock::mock("void_field").returns(42); @@ -77,6 +81,7 @@ fn test_multiple_mock_times() { #[test] fn test_mock_with_params() { unsafe { + //@safety: testing context OracleMock::mock("field_field").with_params((5,)).returns(10); assert_eq(field_field(5), 10); } @@ -85,6 +90,7 @@ fn test_mock_with_params() { #[test] fn test_multiple_mock_with_params() { unsafe { + //@safety: testing context OracleMock::mock("field_field").with_params((5,)).returns(10); OracleMock::mock("field_field").with_params((7,)).returns(14); @@ -96,6 +102,7 @@ fn test_multiple_mock_with_params() { #[test] fn test_mock_last_params() { unsafe { + //@safety: testing context let mock = OracleMock::mock("field_field").returns(10); assert_eq(field_field(5), 10); @@ -106,6 +113,7 @@ fn test_mock_last_params() { #[test] fn test_mock_last_params_many_calls() { unsafe { + //@safety: testing context let mock = OracleMock::mock("field_field").returns(10); assert_eq(field_field(5), 10); assert_eq(field_field(7), 10); @@ -123,6 +131,7 @@ fn test_mock_struct_field() { let point = Point { x: 14, y: 27 }; unsafe { + //@safety: testing context OracleMock::mock("struct_field").returns(42).times(2); let timeless_mock = OracleMock::mock("struct_field").returns(0); assert_eq(42, struct_field(point, array)); diff --git a/noir/noir-repo/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr index fb90e3d96c5a..99277b629399 100644 --- a/noir/noir-repo/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr +++ b/noir/noir-repo/test_programs/noir_test_success/out_of_bounds_alignment/src/main.nr @@ -14,6 +14,7 @@ fn test_acir() { #[test(should_fail)] fn test_brillig() { unsafe { + //@safety: testing context assert_eq(out_of_bounds_unconstrained_wrapper([0; 50], [0; 50]), 0); } } diff --git a/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs b/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs index a610b08ab77e..f31e123d0cd2 100644 --- a/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs +++ b/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs @@ -22,7 +22,6 @@ enum ACVMCommand { Execute(execute_cmd::ExecuteCommand), } -#[cfg(not(feature = "codegen-docs"))] pub(crate) fn start_cli() -> eyre::Result<()> { let ACVMCli { command } = ACVMCli::parse(); @@ -32,10 +31,3 @@ pub(crate) fn start_cli() -> eyre::Result<()> { Ok(()) } - -#[cfg(feature = "codegen-docs")] -pub(crate) fn start_cli() -> eyre::Result<()> { - let markdown: String = clap_markdown::help_markdown::(); - println!("{markdown}"); - Ok(()) -} diff --git a/noir/noir-repo/tooling/debugger/build.rs b/noir/noir-repo/tooling/debugger/build.rs index ebdf2036894a..63924de8336f 100644 --- a/noir/noir-repo/tooling/debugger/build.rs +++ b/noir/noir-repo/tooling/debugger/build.rs @@ -2,7 +2,6 @@ use std::collections::HashSet; use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf}; -use std::{env, fs}; const GIT_COMMIT: &&str = &"GIT_COMMIT"; @@ -14,7 +13,7 @@ fn main() { build_data::no_debug_rebuilds(); } - let out_dir = env::var("OUT_DIR").unwrap(); + let out_dir = std::env::var("OUT_DIR").unwrap(); let destination = Path::new(&out_dir).join("debug.rs"); let mut test_file = File::create(destination).unwrap(); @@ -39,8 +38,8 @@ fn generate_debugger_tests(test_file: &mut File, test_data_dir: &Path) { let test_data_dir = test_data_dir.join(test_sub_dir); let test_case_dirs = - fs::read_dir(test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir()); - let ignored_tests_contents = fs::read_to_string("ignored-tests.txt").unwrap(); + std::fs::read_dir(test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir()); + let ignored_tests_contents = std::fs::read_to_string("ignored-tests.txt").unwrap(); let ignored_tests = ignored_tests_contents.lines().collect::>(); for test_dir in test_case_dirs { diff --git a/noir/noir-repo/tooling/debugger/src/context.rs b/noir/noir-repo/tooling/debugger/src/context.rs index 77c186fe7077..256bb148ae96 100644 --- a/noir/noir-repo/tooling/debugger/src/context.rs +++ b/noir/noir-repo/tooling/debugger/src/context.rs @@ -1015,10 +1015,10 @@ mod tests { outputs: vec![], predicate: None, }]; - let brillig_funcs = &vec![brillig_bytecode]; + let brillig_funcs = &[brillig_bytecode]; let current_witness_index = 2; let circuit = Circuit { current_witness_index, opcodes, ..Circuit::default() }; - let circuits = &vec![circuit]; + let circuits = &[circuit]; let debug_symbols = vec![]; let file_map = BTreeMap::new(); @@ -1185,7 +1185,7 @@ mod tests { ]; let current_witness_index = 3; let circuit = Circuit { current_witness_index, opcodes, ..Circuit::default() }; - let circuits = &vec![circuit]; + let circuits = &[circuit]; let debug_symbols = vec![]; let file_map = BTreeMap::new(); @@ -1197,7 +1197,7 @@ mod tests { PrintOutput::Stdout, debug_artifact, )); - let brillig_funcs = &vec![brillig_bytecode]; + let brillig_funcs = &[brillig_bytecode]; let mut context = DebugContext::new( &StubbedBlackBoxSolver, circuits, @@ -1283,7 +1283,7 @@ mod tests { }; let circuits = vec![circuit_one, circuit_two]; let debug_artifact = DebugArtifact { debug_symbols: vec![], file_map: BTreeMap::new() }; - let brillig_funcs = &vec![brillig_one, brillig_two]; + let brillig_funcs = &[brillig_one, brillig_two]; let context = DebugContext::new( &StubbedBlackBoxSolver, diff --git a/noir/noir-repo/tooling/lsp/src/modules.rs b/noir/noir-repo/tooling/lsp/src/modules.rs index cadf71b2eec0..b023f3886c3c 100644 --- a/noir/noir-repo/tooling/lsp/src/modules.rs +++ b/noir/noir-repo/tooling/lsp/src/modules.rs @@ -41,9 +41,7 @@ pub(crate) fn relative_module_full_path( interner, ); } else { - let Some(parent_module) = get_parent_module(interner, module_def_id) else { - return None; - }; + let parent_module = get_parent_module(interner, module_def_id)?; full_path = relative_module_id_path( parent_module, diff --git a/noir/noir-repo/tooling/lsp/src/requests/hover.rs b/noir/noir-repo/tooling/lsp/src/requests/hover.rs index bb1ea661719e..78be09653fc2 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/hover.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/hover.rs @@ -5,7 +5,6 @@ use fm::{FileMap, PathString}; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; use noirc_frontend::{ ast::{ItemVisibility, Visibility}, - elaborator::types::try_eval_array_length_id, hir::def_map::ModuleId, hir_def::{ expr::{HirArrayLiteral, HirExpression, HirLiteral}, @@ -93,9 +92,7 @@ fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> Option String { string.push('\n'); } + let mut print_comptime = definition.comptime; + let mut opt_value = None; + + // See if we can figure out what's the global's value + if let Some(stmt) = args.interner.get_global_let_statement(id) { + print_comptime = stmt.comptime; + opt_value = get_global_value(args.interner, stmt.expression); + } + string.push_str(" "); - if definition.comptime { + if print_comptime { string.push_str("comptime "); } if definition.mutable { @@ -217,12 +223,9 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { string.push_str(": "); string.push_str(&format!("{}", typ)); - // See if we can figure out what's the global's value - if let Some(stmt) = args.interner.get_global_let_statement(id) { - if let Some(value) = get_global_value(args.interner, stmt.expression) { - string.push_str(" = "); - string.push_str(&value); - } + if let Some(value) = opt_value { + string.push_str(" = "); + string.push_str(&value); } string.push_str(&go_to_type_links(&typ, args.interner, args.files)); @@ -233,13 +236,6 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { } fn get_global_value(interner: &NodeInterner, expr: ExprId) -> Option { - let span = interner.expr_span(&expr); - - // Globals as array lengths are extremely common, so we try that first. - if let Ok(result) = try_eval_array_length_id(interner, expr, span) { - return Some(result.to_string()); - } - match interner.expression(&expr) { HirExpression::Literal(literal) => match literal { HirLiteral::Array(hir_array_literal) => { diff --git a/noir/noir-repo/tooling/lsp/src/use_segment_positions.rs b/noir/noir-repo/tooling/lsp/src/use_segment_positions.rs index 246ff653245f..2cd406ee7734 100644 --- a/noir/noir-repo/tooling/lsp/src/use_segment_positions.rs +++ b/noir/noir-repo/tooling/lsp/src/use_segment_positions.rs @@ -193,10 +193,13 @@ impl UseSegmentPositions { } fn insert_use_segment_position(&mut self, segment: String, position: UseSegmentPosition) { - if self.use_segment_positions.get(&segment).is_none() { - self.use_segment_positions.insert(segment, position); - } else { - self.use_segment_positions.insert(segment, UseSegmentPosition::NoneOrMultiple); + match self.use_segment_positions.entry(segment) { + std::collections::hash_map::Entry::Vacant(e) => { + e.insert(position); + } + std::collections::hash_map::Entry::Occupied(mut e) => { + e.insert(UseSegmentPosition::NoneOrMultiple); + } } } } diff --git a/noir/noir-repo/tooling/nargo_cli/build.rs b/noir/noir-repo/tooling/nargo_cli/build.rs index 8db2c1786d83..17ce6d4dbfdc 100644 --- a/noir/noir-repo/tooling/nargo_cli/build.rs +++ b/noir/noir-repo/tooling/nargo_cli/build.rs @@ -7,7 +7,7 @@ const GIT_COMMIT: &&str = &"GIT_COMMIT"; fn main() { // Only use build_data if the environment variable isn't set. - if std::env::var(GIT_COMMIT).is_err() { + if env::var(GIT_COMMIT).is_err() { build_data::set_GIT_COMMIT(); build_data::set_GIT_DIRTY(); build_data::no_debug_rebuilds(); @@ -19,9 +19,9 @@ fn main() { // Try to find the directory that Cargo sets when it is running; otherwise fallback to assuming the CWD // is the root of the repository and append the crate path - let root_dir = match std::env::var("CARGO_MANIFEST_DIR") { + let root_dir = match env::var("CARGO_MANIFEST_DIR") { Ok(dir) => PathBuf::from(dir).parent().unwrap().parent().unwrap().to_path_buf(), - Err(_) => std::env::current_dir().unwrap(), + Err(_) => env::current_dir().unwrap(), }; let test_dir = root_dir.join("test_programs"); @@ -215,6 +215,9 @@ fn test_{test_name}(force_brillig: ForceBrillig, inliner_aggressiveness: Inliner // Set the maximum increase so that part of the optimization is exercised (it might fail). nargo.arg("--max-bytecode-increase-percent"); nargo.arg("50"); + + // Check whether the test case is non-deterministic + nargo.arg("--check-non-determinism"); }} {test_content} diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs index 2ecf6959a94f..f718021c351a 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -216,6 +216,20 @@ fn compile_programs( cached_program, )?; + if compile_options.check_non_determinism { + let (program_two, _) = compile_program( + file_manager, + parsed_files, + workspace, + package, + compile_options, + load_cached_program(package), + )?; + if fxhash::hash64(&program) != fxhash::hash64(&program_two) { + panic!("Non deterministic result compiling {}", package.name); + } + } + // Choose the target width for the final, backend specific transformation. let target_width = get_target_width(package.expression_width, compile_options.expression_width); diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs index 49a23a7ea62e..709caf711854 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -162,7 +162,7 @@ pub(crate) fn execute_program( diagnostic.report(&debug_artifact, false); } - Err(crate::errors::CliError::NargoError(err)) + Err(CliError::NargoError(err)) } } } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd/formatters.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd/formatters.rs index 1b9b2d503788..75cf14ba1209 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd/formatters.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd/formatters.rs @@ -417,6 +417,7 @@ impl Formatter for JsonFormatter { let mut json = Map::new(); json.insert("type".to_string(), json!("test")); json.insert("name".to_string(), json!(&test_result.name)); + json.insert("suite".to_string(), json!(&test_result.package_name)); json.insert("exec_time".to_string(), json!(test_result.time_to_run.as_secs_f64())); let mut stdout = String::new(); diff --git a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs index a19408bd5fda..9750eb823a60 100644 --- a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs +++ b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs @@ -202,7 +202,7 @@ fn fuzz_keccak256_equivalence() { }}" ) }, - |data| sha3::Keccak256::digest(data).try_into().unwrap(), + |data| sha3::Keccak256::digest(data).into(), ); } @@ -219,7 +219,7 @@ fn fuzz_keccak256_equivalence_over_135() { }}" ) }, - |data| sha3::Keccak256::digest(data).try_into().unwrap(), + |data| sha3::Keccak256::digest(data).into(), ); } @@ -235,7 +235,7 @@ fn fuzz_sha256_equivalence() { }}" ) }, - |data| sha2::Sha256::digest(data).try_into().unwrap(), + |data| sha2::Sha256::digest(data).into(), ); } @@ -251,7 +251,7 @@ fn fuzz_sha512_equivalence() { }}" ) }, - |data| sha2::Sha512::digest(data).try_into().unwrap(), + |data| sha2::Sha512::digest(data).into(), ); } diff --git a/noir/noir-repo/tooling/nargo_fmt/build.rs b/noir/noir-repo/tooling/nargo_fmt/build.rs index bd2db5f5b186..a95cbe165254 100644 --- a/noir/noir-repo/tooling/nargo_fmt/build.rs +++ b/noir/noir-repo/tooling/nargo_fmt/build.rs @@ -1,10 +1,9 @@ use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf}; -use std::{env, fs}; fn main() { - let out_dir = env::var("OUT_DIR").unwrap(); + let out_dir = std::env::var("OUT_DIR").unwrap(); let destination = Path::new(&out_dir).join("execute.rs"); let mut test_file = File::create(destination).unwrap(); @@ -24,7 +23,7 @@ fn generate_formatter_tests(test_file: &mut File, test_data_dir: &Path) { let outputs_dir = test_data_dir.join("expected"); let test_case_files = - fs::read_dir(inputs_dir).unwrap().flatten().filter(|c| c.path().is_file()); + std::fs::read_dir(inputs_dir).unwrap().flatten().filter(|c| c.path().is_file()); for file in test_case_files { let file_path = file.path(); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs index 9a9386e1911c..4184ff288d7d 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs @@ -228,13 +228,11 @@ impl<'a> Formatter<'a> { /// Writes the current indentation to the buffer, but only if the buffer /// is empty or it ends with a newline (otherwise we'd be indenting when not needed). pub(crate) fn write_indentation(&mut self) { - if !(self.buffer.is_empty() || self.buffer.ends_with_newline()) { - return; - } - - for _ in 0..self.indentation { - for _ in 0..self.config.tab_spaces { - self.write(" "); + if self.buffer.is_empty() || self.buffer.ends_with_newline() { + for _ in 0..self.indentation { + for _ in 0..self.config.tab_spaces { + self.write(" "); + } } } } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs index e20eb4291d18..57512d049df4 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs @@ -1,4 +1,4 @@ -use noirc_frontend::token::Token; +use noirc_frontend::token::{DocStyle, Token}; use super::Formatter; @@ -113,7 +113,8 @@ impl<'a> Formatter<'a> { last_was_block_comment = false; } - Token::LineComment(comment, None) => { + Token::LineComment(comment, None) + | Token::LineComment(comment, Some(DocStyle::Safety)) => { if comment.trim() == "noir-fmt:ignore" { ignore_next = true; } @@ -142,7 +143,8 @@ impl<'a> Formatter<'a> { last_was_block_comment = false; self.written_comments_count += 1; } - Token::BlockComment(comment, None) => { + Token::BlockComment(comment, None) + | Token::BlockComment(comment, Some(DocStyle::Safety)) => { if comment.trim() == "noir-fmt:ignore" { ignore_next = true; } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs index ecc9fab18ced..2f71d3209a75 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs @@ -1910,15 +1910,21 @@ global y = 1; #[test] fn format_unsafe_one_expression() { - let src = "global x = unsafe { 1 } ;"; - let expected = "global x = unsafe { 1 };\n"; + let src = "global x = unsafe { //@safety: testing + 1 } ;"; + let expected = "global x = unsafe { + //@safety: testing + 1 +};\n"; assert_format(src, expected); } #[test] fn format_unsafe_two_expressions() { - let src = "global x = unsafe { 1; 2 } ;"; + let src = "global x = unsafe { //@safety: testing + 1; 2 } ;"; let expected = "global x = unsafe { + //@safety: testing 1; 2 }; @@ -2198,6 +2204,7 @@ global y = 1; let src = "mod moo { fn foo() { let mut sorted_write_tuples = unsafe { + //@safety: testing get_sorted_tuple( final_public_data_writes.storage, |(_, leaf_a): (u32, PublicDataTreeLeaf), (_, leaf_b): (u32, PublicDataTreeLeaf)| full_field_less_than( @@ -2211,6 +2218,7 @@ global y = 1; let expected = "mod moo { fn foo() { let mut sorted_write_tuples = unsafe { + //@safety: testing get_sorted_tuple( final_public_data_writes.storage, |(_, leaf_a): (u32, PublicDataTreeLeaf), (_, leaf_b): (u32, PublicDataTreeLeaf)| { diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs index 50c286ff1617..116d68e308f5 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs @@ -489,9 +489,12 @@ mod tests { #[test] fn format_unsafe_statement() { - let src = " fn foo() { unsafe { 1 } } "; + let src = " fn foo() { unsafe { + //@safety: testing context + 1 } } "; let expected = "fn foo() { unsafe { + //@safety: testing context 1 } } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs index 834280ddba35..76fdcd0d0f83 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs @@ -1,8 +1,4 @@ -use std::{ - cmp::Ordering, - collections::BTreeMap, - fmt::{self, Display}, -}; +use std::{cmp::Ordering, collections::BTreeMap, fmt::Display}; use noirc_frontend::ast::{ItemVisibility, PathKind, UseTree, UseTreeKind}; @@ -157,7 +153,7 @@ impl Segment { } impl Display for Segment { - fn fmt(&self, f: &mut std::fmt::Formatter) -> fmt::Result { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { Segment::Crate => write!(f, "crate"), Segment::Super => write!(f, "super"), diff --git a/noir/noir-repo/tooling/nargo_fmt/tests/expected/unsafe.nr b/noir/noir-repo/tooling/nargo_fmt/tests/expected/unsafe.nr index 7d733c203de0..ec99d90c6620 100644 --- a/noir/noir-repo/tooling/nargo_fmt/tests/expected/unsafe.nr +++ b/noir/noir-repo/tooling/nargo_fmt/tests/expected/unsafe.nr @@ -1,7 +1,9 @@ fn main(x: pub u8, y: u8) { - unsafe {} + unsafe { //@safety: testing + } unsafe { + //@safety: testing assert_eq(x, y); } } diff --git a/noir/noir-repo/tooling/nargo_fmt/tests/input/unsafe.nr b/noir/noir-repo/tooling/nargo_fmt/tests/input/unsafe.nr index 6e12ef975eef..65ed061835fb 100644 --- a/noir/noir-repo/tooling/nargo_fmt/tests/input/unsafe.nr +++ b/noir/noir-repo/tooling/nargo_fmt/tests/input/unsafe.nr @@ -1,7 +1,8 @@ fn main(x: pub u8, y: u8) { - unsafe { } + unsafe {//@safety: testing + } - unsafe { + unsafe {//@safety: testing assert_eq(x, y); } } diff --git a/noir/noir-repo/tooling/nargo_toml/src/lib.rs b/noir/noir-repo/tooling/nargo_toml/src/lib.rs index b5c459776181..fa556d445a46 100644 --- a/noir/noir-repo/tooling/nargo_toml/src/lib.rs +++ b/noir/noir-repo/tooling/nargo_toml/src/lib.rs @@ -93,7 +93,7 @@ pub fn find_package_root(current_path: &Path) -> Result /// * `C:\foo\bar` -> `C:\foo` /// * `//shared/foo/bar` -> `//shared/foo` /// * `/foo` -> `/foo` -/// otherwise empty path. +/// otherwise empty path. fn path_root(path: &Path) -> PathBuf { let mut components = path.components();