Skip to content

fix(expand): output safety comment before statements#8378

Merged
vezenovm merged 2 commits intomasterfrom
ab/nargo-expand-safety-comment
May 6, 2025
Merged

fix(expand): output safety comment before statements#8378
vezenovm merged 2 commits intomasterfrom
ab/nargo-expand-safety-comment

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented May 6, 2025

Description

Problem

Resolves #8371

Summary

Now safety comments will always be added before statements, never in the middle of expressions.

For this program:

global G_A: [u16; 3] = [33700, 47314, 35095];
global G_B: [u16; 3] = [59890, 17417, 14409];
fn main(a: [u16; 3], b: [bool; 1]) -> pub bool {
    // Safety: testing context
    if unsafe { func_1(G_B, true) }[(((a[0] as u32) % (G_B[2] as u32)) % 1)] {
        // Safety: testing context
        let c = unsafe { func_1(a, b[0]) };
        b[0]
    } else {
        ((a[((a[0] as u32) % 3)] as u32) > ((24993 % G_A[1]) as u32))
    }
}
unconstrained fn func_1(a: [u16; 3], b: bool) -> [bool; 1] {
    [false]
}

the output of nargo expand is now:

global G_A: [u16; 3] = [33700, 47314, 35095];

global G_B: [u16; 3] = [59890, 17417, 14409];

fn main(a: [u16; 3], b: [bool; 1]) -> pub bool {
    // Safety: comment added by `nargo expand`
    if unsafe { func_1(G_B, true) }[((a[0] as u32) % (G_B[2] as u32)) % 1] {
        // Safety: comment added by `nargo expand`
        let c: [bool; 1] = unsafe { func_1(a, b[0]) };
        b[0]
    } else {
        (a[(a[0] as u32) % 3] as u32) > ((24993 % G_A[1]) as u32)
    }
}

unconstrained fn func_1(a: [u16; 3], b: bool) -> [bool; 1] {
    [false]
}

Additional Context

In a next PR I'll try to preserve safety comments in the AST so we can output those instead of a generated comment.

Documentation

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@asterite asterite requested a review from a team May 6, 2025 18:17
@vezenovm
Copy link
Contributor

vezenovm commented May 6, 2025

Awesome thanks!

@vezenovm vezenovm added this pull request to the merge queue May 6, 2025
Merged via the queue into master with commit 05df1c4 May 6, 2025
115 checks passed
@vezenovm vezenovm deleted the ab/nargo-expand-safety-comment branch May 6, 2025 19:11
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 12, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: always type-check turbofish, and error when it's not allowed
(noir-lang/noir#8437)
chore: Release Noir(1.0.0-beta.5)
(noir-lang/noir#7955)
feat(greybox_fuzzer): Parallel fuzz tests
(noir-lang/noir#8432)
fix(ssa): Mislabeled instructions with side effects in
EnableSideEffectsIf removal pass
(noir-lang/noir#8355)
feat: SSA pass impact report
(noir-lang/noir#8393)
chore: bump external pinned commits
(noir-lang/noir#8433)
chore: separate benchmarking from github actions more
(noir-lang/noir#7943)
chore(fuzz): Break up the AST fuzzer `compare` module
(noir-lang/noir#8431)
chore(fuzz): Rename `init_vs_final` to `min_vs_full`
(noir-lang/noir#8430)
fix!: error on tuple mismatch
(noir-lang/noir#8424)
chore: bump external pinned commits
(noir-lang/noir#8429)
chore(acir): Test whether the predicate has an effect on slice
intrinsics (noir-lang/noir#8421)
feat(ssa): Mark transitively dead parameters during DIE
(noir-lang/noir#8254)
fix(ssa_gen): Do not code gen fetching of empty arrays when initializing
the data bus (noir-lang/noir#8426)
chore: remove `.aztec-sync-commit`
(noir-lang/noir#8415)
chore(test): Add more unit tests for
`inline_functions_with_at_most_one_instruction`
(noir-lang/noir#8418)
chore: add minor docs for interpreter
(noir-lang/noir#8397)
fix: print slice composite types surrounded by parentheses
(noir-lang/noir#8412)
feat: Skip SSA passes that contain any of the given messages
(noir-lang/noir#8416)
fix: disable range constraints using the predicate
(noir-lang/noir#8396)
chore: bumping external libraries
(noir-lang/noir#8406)
chore: redo typo PR by shystrui1199
(noir-lang/noir#8405)
feat(test): add `nargo_fuzz_target`
(noir-lang/noir#8308)
fix: allow names to collide in the values/types namespaces
(noir-lang/noir#8286)
fix: Fix sequencing of side-effects in lvalue
(noir-lang/noir#8384)
feat(greybox_fuzzer): Maximum executions parameter added
(noir-lang/noir#8390)
fix: warn on and discard unreachable statements after break and continue
(noir-lang/noir#8382)
fix: add handling for u128 infix ops in interpreter
(noir-lang/noir#8392)
chore: move acirgen tests into separate file
(noir-lang/noir#8376)
feat(fuzz): initial version of comptime vs brillig target for AST fuzzer
(noir-lang/noir#8335)
chore: apply lints to `ast_fuzzer`
(noir-lang/noir#8386)
chore: add note on AI generated PRs in `CONTRIBUTING.md`
(noir-lang/noir#8385)
chore: document flattening pass
(noir-lang/noir#8312)
fix: comptime shift-right overflow is zero
(noir-lang/noir#8380)
feat: let static_assert accept any type for its message
(noir-lang/noir#8322)
fix(expand): output safety comment before statements
(noir-lang/noir#8378)
chore: avoid need to rebuild after running tests
(noir-lang/noir#8379)
chore: bump dependencies (noir-lang/noir#8372)
chore: Add GITHUB_TOKEN to cross build
(noir-lang/noir#8370)
chore: redo typo PR by GarmashAlex
(noir-lang/noir#8364)
chore: remove unsafe code from greybox fuzzer
(noir-lang/noir#8315)
feat: add `--fuzz-timeout` to `nargo test` options
(noir-lang/noir#8326)
chore: bump external pinned commits
(noir-lang/noir#8334)
fix(expand): try to use "Self" in function calls
(noir-lang/noir#8353)
fix: Fix evaluation order of assignments with side-effects in their rhs
(noir-lang/noir#8342)
fix: let comptime Field value carry the field's sign
(noir-lang/noir#8343)
fix: Ordering of items in callstacks
(noir-lang/noir#8338)
chore: add snapshosts for nargo expand tests
(noir-lang/noir#8318)
fix(ownership): Clone global arrays
(noir-lang/noir#8328)
chore: Replace all SSA interpreter panics with error variants
(noir-lang/noir#8311)
feat: Metamorphic AST fuzzing
(noir-lang/noir#8299)
fix: fix some Display implementations for AST nodes
(noir-lang/noir#8316)
chore: remove leftover file
(noir-lang/noir#8313)
fix: uses non-zero points with ec-add-unsafe
(noir-lang/noir#8248)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 12, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: always type-check turbofish, and error when it's not allowed
(noir-lang/noir#8437)
chore: Release Noir(1.0.0-beta.5)
(noir-lang/noir#7955)
feat(greybox_fuzzer): Parallel fuzz tests
(noir-lang/noir#8432)
fix(ssa): Mislabeled instructions with side effects in
EnableSideEffectsIf removal pass
(noir-lang/noir#8355)
feat: SSA pass impact report
(noir-lang/noir#8393)
chore: bump external pinned commits
(noir-lang/noir#8433)
chore: separate benchmarking from github actions more
(noir-lang/noir#7943)
chore(fuzz): Break up the AST fuzzer `compare` module
(noir-lang/noir#8431)
chore(fuzz): Rename `init_vs_final` to `min_vs_full`
(noir-lang/noir#8430)
fix!: error on tuple mismatch
(noir-lang/noir#8424)
chore: bump external pinned commits
(noir-lang/noir#8429)
chore(acir): Test whether the predicate has an effect on slice
intrinsics (noir-lang/noir#8421)
feat(ssa): Mark transitively dead parameters during DIE
(noir-lang/noir#8254)
fix(ssa_gen): Do not code gen fetching of empty arrays when initializing
the data bus (noir-lang/noir#8426)
chore: remove `.aztec-sync-commit`
(noir-lang/noir#8415)
chore(test): Add more unit tests for
`inline_functions_with_at_most_one_instruction`
(noir-lang/noir#8418)
chore: add minor docs for interpreter
(noir-lang/noir#8397)
fix: print slice composite types surrounded by parentheses
(noir-lang/noir#8412)
feat: Skip SSA passes that contain any of the given messages
(noir-lang/noir#8416)
fix: disable range constraints using the predicate
(noir-lang/noir#8396)
chore: bumping external libraries
(noir-lang/noir#8406)
chore: redo typo PR by shystrui1199
(noir-lang/noir#8405)
feat(test): add `nargo_fuzz_target`
(noir-lang/noir#8308)
fix: allow names to collide in the values/types namespaces
(noir-lang/noir#8286)
fix: Fix sequencing of side-effects in lvalue
(noir-lang/noir#8384)
feat(greybox_fuzzer): Maximum executions parameter added
(noir-lang/noir#8390)
fix: warn on and discard unreachable statements after break and continue
(noir-lang/noir#8382)
fix: add handling for u128 infix ops in interpreter
(noir-lang/noir#8392)
chore: move acirgen tests into separate file
(noir-lang/noir#8376)
feat(fuzz): initial version of comptime vs brillig target for AST fuzzer
(noir-lang/noir#8335)
chore: apply lints to `ast_fuzzer`
(noir-lang/noir#8386)
chore: add note on AI generated PRs in `CONTRIBUTING.md`
(noir-lang/noir#8385)
chore: document flattening pass
(noir-lang/noir#8312)
fix: comptime shift-right overflow is zero
(noir-lang/noir#8380)
feat: let static_assert accept any type for its message
(noir-lang/noir#8322)
fix(expand): output safety comment before statements
(noir-lang/noir#8378)
chore: avoid need to rebuild after running tests
(noir-lang/noir#8379)
chore: bump dependencies (noir-lang/noir#8372)
chore: Add GITHUB_TOKEN to cross build
(noir-lang/noir#8370)
chore: redo typo PR by GarmashAlex
(noir-lang/noir#8364)
chore: remove unsafe code from greybox fuzzer
(noir-lang/noir#8315)
feat: add `--fuzz-timeout` to `nargo test` options
(noir-lang/noir#8326)
chore: bump external pinned commits
(noir-lang/noir#8334)
fix(expand): try to use "Self" in function calls
(noir-lang/noir#8353)
fix: Fix evaluation order of assignments with side-effects in their rhs
(noir-lang/noir#8342)
fix: let comptime Field value carry the field's sign
(noir-lang/noir#8343)
fix: Ordering of items in callstacks
(noir-lang/noir#8338)
chore: add snapshosts for nargo expand tests
(noir-lang/noir#8318)
fix(ownership): Clone global arrays
(noir-lang/noir#8328)
chore: Replace all SSA interpreter panics with error variants
(noir-lang/noir#8311)
feat: Metamorphic AST fuzzing
(noir-lang/noir#8299)
fix: fix some Display implementations for AST nodes
(noir-lang/noir#8316)
chore: remove leftover file
(noir-lang/noir#8313)
fix: uses non-zero points with ec-add-unsafe
(noir-lang/noir#8248)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nargo expand places safety comment on same line as expression

3 participants