Skip to content

chore(fuzz): Capture comptime print output#8635

Merged
TomAFrench merged 13 commits intomasterfrom
af/8606-capture-comptime-print
May 27, 2025
Merged

chore(fuzz): Capture comptime print output#8635
TomAFrench merged 13 commits intomasterfrom
af/8606-capture-comptime-print

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented May 22, 2025

Description

Problem*

Resolves #8606
Fixes #8658

Needs AztecProtocol/aztec-packages#14507, #8660 and #8665 to be merged to pass CI.

Summary*

Adds an interpreter_output to Context which is accessed by the comptime Interpreter through the Elaborator to do the printing. The field holds a trait object which implements std::io::Write; we use dynamic dispatch to access it for writing.

In the fuzz test tests the content goes in a byte vector and is used to compare the output from the interpreter with the real execution.

Fixes the comptime interpreter to display Field with hexadecimal format.

Adds the ability to the AST printer to show the type in let bindings to combat the ambiguity between the generated AST, which carries type information, and the printed source, where the frontend has to infer it from literals. This manifested itself as a value being printed as a number in the test and a field in nargo, because the test knew the type was u32, but that did not appear in the printed source, which was parsed for the comptime snippet.

Additional Context

I started off by adding a generic W: std::io::Write, and borrowing a mutable reference to it through the Elaborator, but the number of places I had to add it was huge relative to how little I expect comptime printing to be used. I thought the performance impact of dynamic dispatch should be negligible, as this is most likely used for debugging, and this way the code churn is kept to a minimum.

I ended up using Rc<RefCell<dyn std::io::Write>> so that the we can share the ownership over something writeable between the test and the Context without losing type information, which is what would happen if we used Box and passed it over to Context, then took it back.

Discrepancies

Run the test with the following command:

cargo test -p noir_ast_fuzzer_fuzz comptime_vs_brillig

The first test failure shows our formatting is different:

thread 'targets::comptime_vs_brillig::tests::fuzz_with_arbtest' panicked at tooling/ast_fuzzer/fuzz/src/targets/mod.rs:46:18:
called `Result::unwrap()` on an `Err` value: programs disagree on printed output:
---
[278759899407742179878585973846349345507, 172403831841634215884590204055802967033]
106748601

--- != ---
[0xd1b7342960b48d8b50bde6d8d02a72e3, 0x81b3c52d9a442b048766f07cd2d173f9]
106748601

---

This was fixed by changing the formatting of `Field` in the interpreter, and then by including the type information in `let`.


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.

@aakoshh aakoshh changed the title Af/8606 capture comptime print chore(fuzz): Capture comptime print output May 22, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 4647a8a Previous: 176cfd4 Ratio
rollup-merge 0.004 s 0.003 s 1.33

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@aakoshh aakoshh requested a review from a team May 23, 2025 11:19
@aakoshh aakoshh marked this pull request as ready for review May 23, 2025 11:19
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: d66b8d3 Previous: d3cb2b2 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 2 s 1 s 2
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib 2 s 1 s 2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
In preparation for noir-lang/noir#8635 where the
format of `Field` printed at `comptime` will change from numeric to
hexadecimal, to match the behaviour of the runtime. A slight
inconvenience is that `let x = 0;` seems to be inferred to be `Field` by
the compiler, which would show up as `0x00`, which in this case
disrupted the macro, as what has been rendered as `arg0` became
`arg0x00`.
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@TomAFrench TomAFrench added this pull request to the merge queue May 27, 2025
Merged via the queue into master with commit 6af0859 May 27, 2025
120 checks passed
@TomAFrench TomAFrench deleted the af/8606-capture-comptime-print branch May 27, 2025 16:43
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jun 1, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(fmt): correctly format mixed secondary attributes and doc comments
(noir-lang/noir#8735)
fix!: require types for trait impl associated constants
(noir-lang/noir#8734)
fix!: Prevent returning references from if expressions
(noir-lang/noir#8731)
fix: cast signed to u1 follow-up
(noir-lang/noir#8730)
fix: cast signed to u1 (noir-lang/noir#8720)
fix: do not mutate arrays later copied inside other arrays
(noir-lang/noir#8701)
chore: box `AsTraitPath` and `TypePath` in `ExpressionKind`
(noir-lang/noir#8716)
fix: general solution for accessing associated constants
(noir-lang/noir#8417)
fix(ssa): Validate checked signed add/sub is followed by a truncate
(noir-lang/noir#8706)
chore: add pre- and post-check on `array_set` optimization pass
(noir-lang/noir#8714)
fix: merge expr bindings with instantiations bindings during
monomorphization (noir-lang/noir#8713)
fix: better way to do LSP file overrides
(noir-lang/noir#8702)
fix(fmt): correct indentation when formatting long struct patterns
(noir-lang/noir#8711)
fix(fuzz): Prevent breaking/continuing out from let blocks in the AST
fuzzer (noir-lang/noir#8708)
chore: remove override for zero length inputs
(noir-lang/noir#8709)
feat: Replace converging jmpif with empty block destinations with a
single jmp (noir-lang/noir#8613)
feat(cli): Add `nargo interpret` command
(noir-lang/noir#8700)
chore: more 1-tuple printing fixes
(noir-lang/noir#8699)
chore(fuzz): Fuzz the SSA parser
(noir-lang/noir#8647)
fix: (SSA parser) translate blocks in logical order rather than syntax
order (noir-lang/noir#8668)
fix(SSA): show and parse range_check's assert_message
(noir-lang/noir#8652)
chore: Show the step number in the SSA message
(noir-lang/noir#8698)
chore(docs): Add pointers to tests
(noir-lang/noir#8695)
chore(fuzz): Capture comptime print output
(noir-lang/noir#8635)
fix: nargo expand reexports correctly implemented
(noir-lang/noir#8693)
feat(cli): Show multiple SSA passes
(noir-lang/noir#8692)
chore(test): Add regression test for defunctionalize
(noir-lang/noir#8691)
chore: add an assertion when parsing SSA that all functions are
well-formed (noir-lang/noir#8671)
feat!: prevent compiling blake3 hashes which barretenberg cannot prove
(noir-lang/noir#8690)
fix(ssa): Remove the array cache from the function inserter
(noir-lang/noir#8607)
chore: bump external pinned commits
(noir-lang/noir#8684)
chore: reenable fuzzing tests in CI
(noir-lang/noir#8688)
fix: Handle `&mut function` in defunctionalize
(noir-lang/noir#8665)
fix(defunctionalize): Higher order functions (HOF) dynamic dispatch and
HOFs in arrays (noir-lang/noir#8672)
chore(ci): avoid running fuzzer tests in the merge queue
(noir-lang/noir#8664)
fix: Make casts in `comptime` consistent with runtime casts
(noir-lang/noir#8669)
fix: relax connectedness requirement on purity analysis pass
(noir-lang/noir#8667)
fix: always use `u32` for indexing arrays in SSA
(noir-lang/noir#8633)
chore: explicitly pull from `next` branch in aztec-packages
(noir-lang/noir#8660)
chore(fuzz): Build the dictionary from the SSA
(noir-lang/noir#8591)
chore(docs): Remove old versioned docs
(noir-lang/noir#8061)
chore: bump external pinned commits
(noir-lang/noir#8634)
fix(SSA): don't use string literal if byte is "form feed" ('\f')
(noir-lang/noir#8653)
chore(defunctionalize): Add regression test for missing lambda variants
panic (noir-lang/noir#8642)
chore(ci): Do not run ast_fuzzer orig vs. morph in ci
(noir-lang/noir#8646)
fix: disable underflow fix for fields
(noir-lang/noir#8631)
fix: revert "fix: error on unused generic in trait impl
(noir-lang/noir#8395)"
(noir-lang/noir#8636)
fix(ssa interpreter): Default to zero when we have an overflowing shl
(noir-lang/noir#8638)
fix: ensure that purity analysis pass explores all functions
(noir-lang/noir#8452)
feat: acir_formal_proofs (noir-lang/noir#8140)
fix: error on unused generic in trait impl
(noir-lang/noir#8395)
fix(expand): use re-exports for non-visibile items
(noir-lang/noir#8374)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Ary Borenszweig <asterite@gmail.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.

Comptime vs Runtime printing of Fields is different Capture prints at compile time

2 participants