diff --git a/.noir-sync-commit b/.noir-sync-commit index 55d843a2d9a..0421ede08e4 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -f81244c6bb29e8869f489d536141eebf6f68f00a +e9d3ccf5bae50241d8183ba5972dce62fbd700a4 diff --git a/noir/noir-repo/.github/workflows/formatting.yml b/noir/noir-repo/.github/workflows/formatting.yml index 08c02af519f..ab92d452c79 100644 --- a/noir/noir-repo/.github/workflows/formatting.yml +++ b/noir/noir-repo/.github/workflows/formatting.yml @@ -15,18 +15,11 @@ concurrency: jobs: clippy: name: cargo clippy - runs-on: ${{ matrix.runner }} + runs-on: ubuntu-latest timeout-minutes: 30 env: RUSTFLAGS: -Dwarnings - strategy: - fail-fast: false - matrix: - include: - - runner: ubuntu-latest - target: x86_64-unknown-linux-gnu - steps: - name: Checkout uses: actions/checkout@v4 @@ -34,18 +27,41 @@ jobs: - name: Setup toolchain uses: dtolnay/rust-toolchain@1.74.1 with: - targets: ${{ matrix.target }} + targets: x86_64-unknown-linux-gnu components: clippy, rustfmt - uses: Swatinem/rust-cache@v2 with: - key: ${{ matrix.target }} + key: x86_64-unknown-linux-gnu cache-on-failure: true save-if: ${{ github.event_name != 'merge_group' }} - name: Run `cargo clippy` run: cargo clippy --all-targets --workspace --locked --release + rustfmt: + name: cargo fmt + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + RUSTFLAGS: -Dwarnings + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup toolchain + uses: dtolnay/rust-toolchain@1.74.1 + with: + targets: x86_64-unknown-linux-gnu + components: clippy, rustfmt + + - uses: Swatinem/rust-cache@v2 + with: + key: x86_64-unknown-linux-gnu + cache-on-failure: true + save-if: ${{ github.event_name != 'merge_group' }} + - name: Run `cargo fmt` run: cargo fmt --all --check @@ -88,7 +104,6 @@ jobs: run: | mkdir dist cp ./target/release/nargo ./dist/nargo - 7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz - name: Upload artifact uses: actions/upload-artifact@v4 diff --git a/noir/noir-repo/.github/workflows/gates_report.yml b/noir/noir-repo/.github/workflows/gates_report.yml deleted file mode 100644 index 0b0a527b69e..00000000000 --- a/noir/noir-repo/.github/workflows/gates_report.yml +++ /dev/null @@ -1,94 +0,0 @@ -name: Report gates diff - -on: - push: - branches: - - master - pull_request: - -jobs: - build-nargo: - runs-on: ubuntu-latest - strategy: - matrix: - target: [x86_64-unknown-linux-gnu] - - steps: - - name: Checkout Noir repo - uses: actions/checkout@v4 - - - name: Setup toolchain - uses: dtolnay/rust-toolchain@1.74.1 - - - uses: Swatinem/rust-cache@v2 - with: - key: ${{ matrix.target }} - cache-on-failure: true - save-if: ${{ github.event_name != 'merge_group' }} - - - name: Build Nargo - run: cargo build --package nargo_cli --release - - - name: Package artifacts - run: | - mkdir dist - cp ./target/release/nargo ./dist/nargo - 7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz - - - name: Upload artifact - uses: actions/upload-artifact@v4 - with: - name: nargo - path: ./dist/* - retention-days: 3 - - - compare_gates_reports: - needs: [build-nargo] - runs-on: ubuntu-latest - permissions: - pull-requests: write - - steps: - - uses: actions/checkout@v4 - - - name: Install `bb` - run: | - ./scripts/install_bb.sh - echo "$HOME/.bb/" >> $GITHUB_PATH - - - name: Download nargo binary - uses: actions/download-artifact@v4 - with: - name: nargo - path: ./nargo - - - name: Set nargo on PATH - run: | - nargo_binary="${{ github.workspace }}/nargo/nargo" - chmod +x $nargo_binary - echo "$(dirname $nargo_binary)" >> $GITHUB_PATH - export PATH="$PATH:$(dirname $nargo_binary)" - nargo -V - - - name: Generate gates report - working-directory: ./test_programs - run: | - ./rebuild.sh - ./gates_report.sh - mv gates_report.json ../gates_report.json - - - name: Compare gates reports - id: gates_diff - uses: noir-lang/noir-gates-diff@1931aaaa848a1a009363d6115293f7b7fc72bb87 - with: - report: gates_report.json - summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) - - - name: Add gates diff to sticky comment - if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' - uses: marocchino/sticky-pull-request-comment@v2 - with: - # delete the comment in case changes no longer impact circuit sizes - delete: ${{ !steps.gates_diff.outputs.markdown }} - message: ${{ steps.gates_diff.outputs.markdown }} diff --git a/noir/noir-repo/.github/workflows/gates_report_brillig.yml b/noir/noir-repo/.github/workflows/gates_report_brillig.yml deleted file mode 100644 index e7ec30923f0..00000000000 --- a/noir/noir-repo/.github/workflows/gates_report_brillig.yml +++ /dev/null @@ -1,92 +0,0 @@ -name: Report Brillig bytecode size diff - -on: - push: - branches: - - master - pull_request: - -jobs: - build-nargo: - runs-on: ubuntu-latest - strategy: - matrix: - target: [x86_64-unknown-linux-gnu] - - steps: - - name: Checkout Noir repo - uses: actions/checkout@v4 - - - name: Setup toolchain - uses: dtolnay/rust-toolchain@1.74.1 - - - uses: Swatinem/rust-cache@v2 - with: - key: ${{ matrix.target }} - cache-on-failure: true - save-if: ${{ github.event_name != 'merge_group' }} - - - name: Build Nargo - run: cargo build --package nargo_cli --release - - - name: Package artifacts - run: | - mkdir dist - cp ./target/release/nargo ./dist/nargo - 7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz - - - name: Upload artifact - uses: actions/upload-artifact@v4 - with: - name: nargo - path: ./dist/* - retention-days: 3 - - compare_brillig_bytecode_size_reports: - needs: [build-nargo] - runs-on: ubuntu-latest - permissions: - pull-requests: write - - steps: - - uses: actions/checkout@v4 - - - name: Download nargo binary - uses: actions/download-artifact@v4 - with: - name: nargo - path: ./nargo - - - name: Set nargo on PATH - run: | - nargo_binary="${{ github.workspace }}/nargo/nargo" - chmod +x $nargo_binary - echo "$(dirname $nargo_binary)" >> $GITHUB_PATH - export PATH="$PATH:$(dirname $nargo_binary)" - nargo -V - - - name: Generate Brillig bytecode size report - working-directory: ./test_programs - run: | - chmod +x gates_report_brillig.sh - ./gates_report_brillig.sh - mv gates_report_brillig.json ../gates_report_brillig.json - - - name: Compare Brillig bytecode size reports - id: brillig_bytecode_diff - uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48 - with: - report: gates_report_brillig.json - header: | - # Changes to Brillig bytecode sizes - brillig_report: true - summaryQuantile: 0.9 # only display the 10% most significant bytecode size diffs in the summary (defaults to 20%) - - - name: Add bytecode size diff to sticky comment - if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' - uses: marocchino/sticky-pull-request-comment@v2 - with: - header: brillig - # delete the comment in case changes no longer impact brillig bytecode sizes - delete: ${{ !steps.brillig_bytecode_diff.outputs.markdown }} - message: ${{ steps.brillig_bytecode_diff.outputs.markdown }} \ No newline at end of file diff --git a/noir/noir-repo/.github/workflows/gates_report_brillig_execution.yml b/noir/noir-repo/.github/workflows/gates_report_brillig_execution.yml deleted file mode 100644 index 0ef98f5045b..00000000000 --- a/noir/noir-repo/.github/workflows/gates_report_brillig_execution.yml +++ /dev/null @@ -1,92 +0,0 @@ -name: Report Brillig opcodes executed diff - -on: - push: - branches: - - master - pull_request: - -jobs: - build-nargo: - runs-on: ubuntu-latest - strategy: - matrix: - target: [x86_64-unknown-linux-gnu] - - steps: - - name: Checkout Noir repo - uses: actions/checkout@v4 - - - name: Setup toolchain - uses: dtolnay/rust-toolchain@1.74.1 - - - uses: Swatinem/rust-cache@v2 - with: - key: ${{ matrix.target }} - cache-on-failure: true - save-if: ${{ github.event_name != 'merge_group' }} - - - name: Build Nargo - run: cargo build --package nargo_cli --release - - - name: Package artifacts - run: | - mkdir dist - cp ./target/release/nargo ./dist/nargo - 7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz - - - name: Upload artifact - uses: actions/upload-artifact@v4 - with: - name: nargo - path: ./dist/* - retention-days: 3 - - compare_brillig_execution_reports: - needs: [build-nargo] - runs-on: ubuntu-latest - permissions: - pull-requests: write - - steps: - - uses: actions/checkout@v4 - - - name: Download nargo binary - uses: actions/download-artifact@v4 - with: - name: nargo - path: ./nargo - - - name: Set nargo on PATH - run: | - nargo_binary="${{ github.workspace }}/nargo/nargo" - chmod +x $nargo_binary - echo "$(dirname $nargo_binary)" >> $GITHUB_PATH - export PATH="$PATH:$(dirname $nargo_binary)" - nargo -V - - - name: Generate Brillig execution report - working-directory: ./test_programs - run: | - chmod +x gates_report_brillig_execution.sh - ./gates_report_brillig_execution.sh - mv gates_report_brillig_execution.json ../gates_report_brillig_execution.json - - - name: Compare Brillig execution reports - id: brillig_execution_diff - uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48 - with: - report: gates_report_brillig_execution.json - header: | - # Changes to number of Brillig opcodes executed - brillig_report: true - summaryQuantile: 0.9 # only display the 10% most significant bytecode size diffs in the summary (defaults to 20%) - - - name: Add bytecode size diff to sticky comment - if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' - uses: marocchino/sticky-pull-request-comment@v2 - with: - header: brillig_execution - # delete the comment in case changes no longer impact brillig bytecode sizes - delete: ${{ !steps.brillig_execution_diff.outputs.markdown }} - message: ${{ steps.brillig_execution_diff.outputs.markdown }} \ No newline at end of file diff --git a/noir/noir-repo/.github/workflows/lockfile.yml b/noir/noir-repo/.github/workflows/lockfile.yml deleted file mode 100644 index 190e01745af..00000000000 --- a/noir/noir-repo/.github/workflows/lockfile.yml +++ /dev/null @@ -1,23 +0,0 @@ -name: Lockfile check - -on: - pull_request: - -# This will cancel previous runs when a branch or PR is updated -concurrency: - group: ${{ github.workflow }}-${{ github.head_ref || github.ref || github.run_id }} - cancel-in-progress: true - -jobs: - yarn-lock: - runs-on: ubuntu-latest - timeout-minutes: 30 - - steps: - - name: Checkout - uses: actions/checkout@v4 - - # Errors if installation would result in modifications to yarn.lock - - name: Install - run: yarn --immutable - shell: bash diff --git a/noir/noir-repo/.github/workflows/release.yml b/noir/noir-repo/.github/workflows/release.yml index 7e0909224e5..59c3d9a1415 100644 --- a/noir/noir-repo/.github/workflows/release.yml +++ b/noir/noir-repo/.github/workflows/release.yml @@ -15,7 +15,7 @@ jobs: steps: - name: Run release-please id: release - uses: google-github-actions/release-please-action@v4 + uses: googleapis/release-please-action@v4 with: token: ${{ secrets.NOIR_RELEASES_TOKEN }} diff --git a/noir/noir-repo/.github/workflows/reports.yml b/noir/noir-repo/.github/workflows/reports.yml new file mode 100644 index 00000000000..8f8aeabb65e --- /dev/null +++ b/noir/noir-repo/.github/workflows/reports.yml @@ -0,0 +1,235 @@ +name: Reports + +on: + push: + branches: + - master + pull_request: + +jobs: + build-nargo: + runs-on: ubuntu-latest + + steps: + - name: Checkout Noir repo + uses: actions/checkout@v4 + + - name: Setup toolchain + uses: dtolnay/rust-toolchain@1.74.1 + + - uses: Swatinem/rust-cache@v2 + with: + key: x86_64-unknown-linux-gnu + cache-on-failure: true + save-if: ${{ github.event_name != 'merge_group' }} + + - name: Build Nargo + run: cargo build --package nargo_cli --release + + - name: Package artifacts + run: | + mkdir dist + cp ./target/release/nargo ./dist/nargo + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: nargo + path: ./dist/* + retention-days: 3 + + + compare_gates_reports: + name: Circuit sizes + needs: [build-nargo] + runs-on: ubuntu-latest + permissions: + pull-requests: write + + steps: + - uses: actions/checkout@v4 + + - name: Install `bb` + run: | + ./scripts/install_bb.sh + echo "$HOME/.bb/" >> $GITHUB_PATH + + - name: Download nargo binary + uses: actions/download-artifact@v4 + with: + name: nargo + path: ./nargo + + - name: Set nargo on PATH + run: | + nargo_binary="${{ github.workspace }}/nargo/nargo" + chmod +x $nargo_binary + echo "$(dirname $nargo_binary)" >> $GITHUB_PATH + export PATH="$PATH:$(dirname $nargo_binary)" + nargo -V + + - name: Generate gates report + working-directory: ./test_programs + run: | + ./rebuild.sh + ./gates_report.sh + mv gates_report.json ../gates_report.json + + - name: Compare gates reports + id: gates_diff + uses: noir-lang/noir-gates-diff@1931aaaa848a1a009363d6115293f7b7fc72bb87 + with: + report: gates_report.json + summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) + + - name: Add gates diff to sticky comment + if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' + uses: marocchino/sticky-pull-request-comment@v2 + with: + # delete the comment in case changes no longer impact circuit sizes + delete: ${{ !steps.gates_diff.outputs.markdown }} + message: ${{ steps.gates_diff.outputs.markdown }} + + compare_brillig_bytecode_size_reports: + name: Brillig bytecode sizes + needs: [build-nargo] + runs-on: ubuntu-latest + permissions: + pull-requests: write + + steps: + - uses: actions/checkout@v4 + + - name: Download nargo binary + uses: actions/download-artifact@v4 + with: + name: nargo + path: ./nargo + + - name: Set nargo on PATH + run: | + nargo_binary="${{ github.workspace }}/nargo/nargo" + chmod +x $nargo_binary + echo "$(dirname $nargo_binary)" >> $GITHUB_PATH + export PATH="$PATH:$(dirname $nargo_binary)" + nargo -V + + - name: Generate Brillig bytecode size report + working-directory: ./test_programs + run: | + ./gates_report_brillig.sh + mv gates_report_brillig.json ../gates_report_brillig.json + + - name: Compare Brillig bytecode size reports + id: brillig_bytecode_diff + uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48 + with: + report: gates_report_brillig.json + header: | + # Changes to Brillig bytecode sizes + brillig_report: true + summaryQuantile: 0.9 # only display the 10% most significant bytecode size diffs in the summary (defaults to 20%) + + - name: Add bytecode size diff to sticky comment + if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: brillig + # delete the comment in case changes no longer impact brillig bytecode sizes + delete: ${{ !steps.brillig_bytecode_diff.outputs.markdown }} + message: ${{ steps.brillig_bytecode_diff.outputs.markdown }} + + compare_brillig_execution_reports: + name: Brillig execution trace sizes + needs: [build-nargo] + runs-on: ubuntu-latest + permissions: + pull-requests: write + + steps: + - uses: actions/checkout@v4 + + - name: Download nargo binary + uses: actions/download-artifact@v4 + with: + name: nargo + path: ./nargo + + - name: Set nargo on PATH + run: | + nargo_binary="${{ github.workspace }}/nargo/nargo" + chmod +x $nargo_binary + echo "$(dirname $nargo_binary)" >> $GITHUB_PATH + export PATH="$PATH:$(dirname $nargo_binary)" + nargo -V + + - name: Generate Brillig execution report + working-directory: ./test_programs + run: | + ./gates_report_brillig_execution.sh + mv gates_report_brillig_execution.json ../gates_report_brillig_execution.json + + - name: Compare Brillig execution reports + id: brillig_execution_diff + uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48 + with: + report: gates_report_brillig_execution.json + header: | + # Changes to number of Brillig opcodes executed + brillig_report: true + summaryQuantile: 0.9 # only display the 10% most significant bytecode size diffs in the summary (defaults to 20%) + + - name: Add bytecode size diff to sticky comment + if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: brillig_execution + # delete the comment in case changes no longer impact brillig bytecode sizes + delete: ${{ !steps.brillig_execution_diff.outputs.markdown }} + message: ${{ steps.brillig_execution_diff.outputs.markdown }} + + generate_memory_report: + name: Peak memory usage + needs: [build-nargo] + runs-on: ubuntu-latest + permissions: + pull-requests: write + + steps: + - uses: actions/checkout@v4 + + - name: Download nargo binary + uses: actions/download-artifact@v4 + with: + name: nargo + path: ./nargo + + - name: Set nargo on PATH + run: | + nargo_binary="${{ github.workspace }}/nargo/nargo" + chmod +x $nargo_binary + echo "$(dirname $nargo_binary)" >> $GITHUB_PATH + export PATH="$PATH:$(dirname $nargo_binary)" + nargo -V + + - name: Generate Memory report + working-directory: ./test_programs + run: | + ./memory_report.sh + mv memory_report.json ../memory_report.json + + - name: Parse memory report + id: memory_report + uses: noir-lang/noir-bench-report@ccb0d806a91d3bd86dba0ba3d580a814eed5673c + with: + report: memory_report.json + header: | + # Memory Report + memory_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: memory + message: ${{ steps.memory_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 422a30ed08f..6a9a918b955 100644 --- a/noir/noir-repo/.github/workflows/test-js-packages.yml +++ b/noir/noir-repo/.github/workflows/test-js-packages.yml @@ -13,6 +13,19 @@ concurrency: cancel-in-progress: true jobs: + yarn-lock: + runs-on: ubuntu-latest + timeout-minutes: 30 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + # Errors if installation would result in modifications to yarn.lock + - name: Install + run: yarn --immutable + shell: bash + build-nargo: runs-on: ubuntu-22.04 timeout-minutes: 30 @@ -78,7 +91,6 @@ jobs: ./tooling/noirc_abi_wasm/web retention-days: 10 - build-noir-wasm: runs-on: ubuntu-latest timeout-minutes: 30 @@ -582,6 +594,7 @@ jobs: # We want this job to always run (even if the dependant jobs fail) as we want this job to fail rather than skipping. if: ${{ always() }} needs: + - yarn-lock - test-acvm_js-node - test-acvm_js-browser - test-noirc-abi diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs index 9817d0a6738..e331efda894 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -1,6 +1,6 @@ //! Dead Instruction Elimination (DIE) pass: Removes any instruction without side-effects for //! which the results are unused. -use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; +use fxhash::FxHashSet as HashSet; use im::Vector; use noirc_errors::Location; use rayon::iter::{IntoParallelRefMutIterator, ParallelIterator}; @@ -18,8 +18,6 @@ use crate::ssa::{ ssa_gen::{Ssa, SSA_WORD_SIZE}, }; -use super::rc::{pop_rc_for, RcInstruction}; - impl Ssa { /// Performs Dead Instruction Elimination (DIE) to remove any instructions with /// unused results. @@ -108,8 +106,6 @@ impl Context { let instructions_len = block.instructions().len(); - let mut rc_tracker = RcTracker::default(); - // Indexes of instructions that might be out of bounds. // We'll remove those, but before that we'll insert bounds checks for them. let mut possible_index_out_of_bounds_indexes = Vec::new(); @@ -137,12 +133,8 @@ impl Context { }); } } - - rc_tracker.track_inc_rcs_to_remove(*instruction_id, function); } - self.instructions_to_remove.extend(rc_tracker.rc_pairs_to_remove); - // If there are some instructions that might trigger an out of bounds error, // first add constrain checks. Then run the DIE pass again, which will remove those // but leave the constrains (any any value needed by those constrains) @@ -521,79 +513,6 @@ fn apply_side_effects( (lhs, rhs) } -#[derive(Default)] -struct RcTracker { - // We can track IncrementRc instructions per block to determine whether they are useless. - // IncrementRc and DecrementRc instructions are normally side effectual instructions, but we remove - // them if their value is not used anywhere in the function. However, even when their value is used, their existence - // is pointless logic if there is no array set between the increment and the decrement of the reference counter. - // We track per block whether an IncrementRc instruction has a paired DecrementRc instruction - // with the same value but no array set in between. - // If we see an inc/dec RC pair within a block we can safely remove both instructions. - rcs_with_possible_pairs: HashMap>, - rc_pairs_to_remove: HashSet, - - // We also separately track all IncrementRc instructions and all arrays which have been mutably borrowed. - // If an array has not been mutably borrowed we can then safely remove all IncrementRc instructions on that array. - inc_rcs: HashMap>, - - // The SSA often creates patterns where after simplifications we end up with repeat - // IncrementRc instructions on the same value. We track whether the previous instruction was an IncrementRc, - // and if the current instruction is also an IncrementRc on the same value we remove the current instruction. - // `None` if the previous instruction was anything other than an IncrementRc - previous_inc_rc: Option, -} - -impl RcTracker { - fn track_inc_rcs_to_remove(&mut self, instruction_id: InstructionId, function: &Function) { - let instruction = &function.dfg[instruction_id]; - - if let Instruction::IncrementRc { value } = instruction { - if let Some(previous_value) = self.previous_inc_rc { - if previous_value == *value { - self.rc_pairs_to_remove.insert(instruction_id); - } - } - self.previous_inc_rc = Some(*value); - } else { - self.previous_inc_rc = None; - } - - // DIE loops over a block in reverse order, so we insert an RC instruction for possible removal - // when we see a DecrementRc and check whether it was possibly mutated when we see an IncrementRc. - match instruction { - Instruction::IncrementRc { value } => { - if let Some(inc_rc) = - pop_rc_for(*value, function, &mut self.rcs_with_possible_pairs) - { - if !inc_rc.possibly_mutated { - self.rc_pairs_to_remove.insert(inc_rc.id); - self.rc_pairs_to_remove.insert(instruction_id); - } - } - - self.inc_rcs.entry(*value).or_default().insert(instruction_id); - } - Instruction::DecrementRc { value } => { - let typ = function.dfg.type_of_value(*value); - - // We assume arrays aren't mutated until we find an array_set - let dec_rc = - RcInstruction { id: instruction_id, array: *value, possibly_mutated: false }; - self.rcs_with_possible_pairs.entry(typ).or_default().push(dec_rc); - } - Instruction::ArraySet { array, .. } => { - let typ = function.dfg.type_of_value(*array); - if let Some(dec_rcs) = self.rcs_with_possible_pairs.get_mut(&typ) { - for dec_rc in dec_rcs { - dec_rc.possibly_mutated = true; - } - } - } - _ => {} - } - } -} #[cfg(test)] mod test { use std::sync::Arc; @@ -602,7 +521,7 @@ mod test { use crate::ssa::{ function_builder::FunctionBuilder, - ir::{instruction::Instruction, map::Id, types::Type}, + ir::{map::Id, types::Type}, opt::assert_normalized_ssa_equals, Ssa, }; @@ -674,30 +593,6 @@ mod test { assert_normalized_ssa_equals(ssa, expected); } - #[test] - fn remove_useless_paired_rcs_even_when_used() { - let src = " - acir(inline) fn main f0 { - b0(v0: [Field; 2]): - inc_rc v0 - v2 = array_get v0, index u32 0 -> Field - dec_rc v0 - return v2 - } - "; - let ssa = Ssa::from_str(src).unwrap(); - - let expected = " - acir(inline) fn main f0 { - b0(v0: [Field; 2]): - v2 = array_get v0, index u32 0 -> Field - return v2 - } - "; - let ssa = ssa.dead_instruction_elimination(); - assert_normalized_ssa_equals(ssa, expected); - } - #[test] fn keep_paired_rcs_with_array_set() { let src = " @@ -767,66 +662,6 @@ mod test { assert_eq!(main.dfg[b1].instructions().len(), 2); } - #[test] - fn keep_inc_rc_on_borrowed_array_set() { - // acir(inline) fn main f0 { - // b0(v0: [u32; 2]): - // inc_rc v0 - // v3 = array_set v0, index u32 0, value u32 1 - // inc_rc v0 - // inc_rc v0 - // inc_rc v0 - // v4 = array_get v3, index u32 1 - // return v4 - // } - let main_id = Id::test_new(0); - - // Compiling main - let mut builder = FunctionBuilder::new("main".into(), main_id); - let array_type = Type::Array(Arc::new(vec![Type::unsigned(32)]), 2); - let v0 = builder.add_parameter(array_type.clone()); - builder.increment_array_reference_count(v0); - let zero = builder.numeric_constant(0u128, Type::unsigned(32)); - let one = builder.numeric_constant(1u128, Type::unsigned(32)); - let v3 = builder.insert_array_set(v0, zero, one); - builder.increment_array_reference_count(v0); - builder.increment_array_reference_count(v0); - builder.increment_array_reference_count(v0); - - let v4 = builder.insert_array_get(v3, one, Type::unsigned(32)); - - builder.terminate_with_return(vec![v4]); - - let ssa = builder.finish(); - let main = ssa.main(); - - // The instruction count never includes the terminator instruction - assert_eq!(main.dfg[main.entry_block()].instructions().len(), 6); - - // We expect the output to be unchanged - // Expected output: - // - // acir(inline) fn main f0 { - // b0(v0: [u32; 2]): - // inc_rc v0 - // v3 = array_set v0, index u32 0, value u32 1 - // inc_rc v0 - // v4 = array_get v3, index u32 1 - // return v4 - // } - let ssa = ssa.dead_instruction_elimination(); - let main = ssa.main(); - - let instructions = main.dfg[main.entry_block()].instructions(); - // We expect only the repeated inc_rc instructions to be collapsed into a single inc_rc. - assert_eq!(instructions.len(), 4); - - assert!(matches!(&main.dfg[instructions[0]], Instruction::IncrementRc { .. })); - assert!(matches!(&main.dfg[instructions[1]], Instruction::ArraySet { .. })); - assert!(matches!(&main.dfg[instructions[2]], Instruction::IncrementRc { .. })); - assert!(matches!(&main.dfg[instructions[3]], Instruction::ArrayGet { .. })); - } - #[test] fn does_not_remove_inc_or_dec_rc_of_if_they_are_loaded_from_a_reference() { let src = " 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 06f51b16842..e1ecc972eeb 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 @@ -428,8 +428,8 @@ impl<'a> Parser<'a> { Some(if self.eat_colon() { let expression = self.parse_expression_or_error(); (ident, expression) - } else if self.at(Token::Assign) { - // If we find '=' instead of ':', assume the user meant ':`, error and continue + } else if self.at(Token::DoubleColon) || self.at(Token::Assign) { + // If we find '=' or '::' instead of ':', assume the user meant ':`, error and continue self.expected_token(Token::Colon); self.bump(); let expression = self.parse_expression_or_error(); @@ -1369,6 +1369,34 @@ mod tests { assert_eq!(expr.to_string(), "y"); } + #[test] + fn parses_constructor_recovers_if_double_colon_instead_of_colon() { + let src = " + Foo { x: 1, y:: z } + ^^ + "; + let (src, span) = get_source_with_error_span(src); + let mut parser = Parser::for_str(&src); + let expr = parser.parse_expression_or_error(); + + let error = get_single_error(&parser.errors, span); + assert_eq!(error.to_string(), "Expected a ':' but found '::'"); + + let ExpressionKind::Constructor(mut constructor) = expr.kind else { + panic!("Expected constructor"); + }; + assert_eq!(constructor.typ.to_string(), "Foo"); + assert_eq!(constructor.fields.len(), 2); + + let (name, expr) = constructor.fields.remove(0); + assert_eq!(name.to_string(), "x"); + assert_eq!(expr.to_string(), "1"); + + let (name, expr) = constructor.fields.remove(0); + assert_eq!(name.to_string(), "y"); + assert_eq!(expr.to_string(), "z"); + } + #[test] fn parses_parses_if_true() { let src = "if true { 1 }"; diff --git a/noir/noir-repo/cspell.json b/noir/noir-repo/cspell.json index 15bba2cb5f8..5c707e92e21 100644 --- a/noir/noir-repo/cspell.json +++ b/noir/noir-repo/cspell.json @@ -155,6 +155,7 @@ "nargo", "neovim", "newtype", + "nextest", "nightlies", "nixpkgs", "noirc", diff --git a/noir/noir-repo/test_programs/gates_report_brillig.sh b/noir/noir-repo/test_programs/gates_report_brillig.sh old mode 100644 new mode 100755 diff --git a/noir/noir-repo/test_programs/gates_report_brillig_execution.sh b/noir/noir-repo/test_programs/gates_report_brillig_execution.sh old mode 100644 new mode 100755 diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs index 9306e38a48a..97c7ad86d5a 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs @@ -1586,6 +1586,54 @@ fn main() { assert_eq!(changed, expected); } + #[test] + async fn test_auto_import_inserts_after_last_use_in_nested_module() { + let src = r#"mod foo { + pub mod bar { + pub fn hello_world() {} + } +} + +mod baz { + fn qux() {} +} + +mod other { + use baz::qux; + + fn main() { + hel>|< + } +}"#; + + let expected = r#"mod foo { + pub mod bar { + pub fn hello_world() {} + } +} + +mod baz { + fn qux() {} +} + +mod other { + use baz::qux; + use super::foo::bar::hello_world; + + fn main() { + hel + } +}"#; + let mut items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = items.remove(0); + + let changed = + apply_text_edits(&src.replace(">|<", ""), &item.additional_text_edits.unwrap()); + assert_eq!(changed, expected); + } + #[test] async fn test_does_not_auto_import_test_functions() { let src = r#" 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 f9a3f429029..246ff653245 100644 --- a/noir/noir-repo/tooling/lsp/src/use_segment_positions.rs +++ b/noir/noir-repo/tooling/lsp/src/use_segment_positions.rs @@ -318,7 +318,7 @@ fn new_use_completion_item_additional_text_edits( request: UseCompletionItemAdditionTextEditsRequest, ) -> Vec { let line = request.auto_import_line as u32; - let character = (request.nesting * 4) as u32; + let character = 0; let indent = " ".repeat(request.nesting * 4); let mut newlines = "\n"; @@ -331,6 +331,6 @@ fn new_use_completion_item_additional_text_edits( vec![TextEdit { range: Range { start: Position { line, character }, end: Position { line, character } }, - new_text: format!("use {};{}{}", request.full_path, newlines, indent), + new_text: format!("{}use {};{}", indent, request.full_path, newlines), }] }