chore: use human-readable bytecode in snapshots#8164
Conversation
...compile_success_empty/arithmetic_generics/execute__tests__force_brillig_false_inliner_0.snap
Show resolved
Hide resolved
|
@noir-lang/core Can I get a review on this as it's going to otherwise constantly pick up conflicts. |
There was a problem hiding this comment.
PRs with minor actual additions/deletions can now have massive diffs (e.g. #8355 and #8328) due to snapshots. This looks like it is going to blow-up the diff from snapshots potentially quite dramatically?
Could we consider a strategy that avoids having to manually commit snapshots on each PR before moving forward with this change? In general I find the current snapshot flow a bit cumbersome (e.g. the common conflicts on PRs that alter a lot of snapshots) and we could reduce some friction here while reducing the diff sizes.
vezenovm
left a comment
vezenovm
left a comment
There was a problem hiding this comment.
Discussed offline, even with a different snapshot workflow we will still want this format. We can work on updating the snapshot workflow separately.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ 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: faee2b8 | Previous: 6accfc6 | Ratio |
|---|---|---|---|
rollup-merge |
0.004 s |
0.003 s |
1.33 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: faee2b8 | Previous: 6accfc6 | Ratio |
|---|---|---|---|
private-kernel-reset |
7.548 s |
6.254 s |
1.21 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix: sign extend in signed cast (noir-lang/noir#8264) chore(fuzz): Do not use zero length types in the main input output (noir-lang/noir#8465) chore: fix visibility issues in test suite (noir-lang/noir#8454) chore: blackbox functions for ssa intepreter (noir-lang/noir#8375) feat: improve bitshift codegen (noir-lang/noir#8442) fix(ssa): Mark mutually recursive simple functions (noir-lang/noir#8447) fix: Fix nested trait dispatch with associated types (noir-lang/noir#8440) chore: carry visibilities in monomorphized AST (noir-lang/noir#8439) chore(tests): Add regression for now passing test (noir-lang/noir#8441) chore: use human-readable bytecode in snapshots (noir-lang/noir#8164) chore: bump external pinned commits (noir-lang/noir#8445) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
2 participants
Description
Problem*
Comes from discussion with @michaeljklein
Summary*
This PR replaces the
bytecodefield in the snapshots with a human readable display of the ACIR program to make it easier to parse changes to the snapshot.Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.