Skip to content

fix: some nargo expand fixes#9324

Merged
asterite merged 8 commits intomasterfrom
ab/some-nargo-expand-fixes
Aug 4, 2025
Merged

fix: some nargo expand fixes#9324
asterite merged 8 commits intomasterfrom
ab/some-nargo-expand-fixes

Conversation

@asterite
Copy link
Collaborator

Description

Problem

For AztecProtocol/aztec-packages#15971

Summary

This mainly fixes an issue I knew about, but didn't consider it a priority to fix: calls generated by macros could refer to traits that are not in scope, if they are done using macro calls like get_trait_impl, then methods, etc. nargo expand will now check if the trait related to the call is in scope and, if not, use <Type as Trait>::method to call it.

While doing this I noticed that nargo expand would also not put unquote markers, so I fixed that too.

Additional Context

I think we should put a disclaimer somewhere that nargo expand is mostly a debugging tool and that it will try its best to produce code that compiles, but it won't always be possible (even if we fix all bugs). For example if a macro inserts a struct value, nargo expand will show it as Struct { field1: value1, field2: value2, ... } even though those fields might be private.

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.

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: 7938e46 Previous: d35a767 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 2 s 1 s 2

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

CC: @TomAFrench

@asterite asterite requested a review from a team July 25, 2025 14:27
@TomAFrench
Copy link
Member

Are there any ignored nargo expand tests which need to be removed from the list of tests in the build.rs file?

I think we should put a disclaimer somewhere that nargo expand is mostly a debugging tool and that it will try its best to produce code that compiles, but it won't always be possible (even if we fix all bugs). For example if a macro inserts a struct value, nargo expand will show it as Struct { field1: value1, field2: value2, ... } even though those fields might be private.

Agreed that we should highlight that this is done on a best-efforts basis.

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, only question is whether we can take some more tests off of the ignore list.

@asterite
Copy link
Collaborator Author

asterite commented Aug 4, 2025

@TomAFrench Good point. I changed the tests so we can now when they start passing and, indeed, some more tests passed.

@asterite asterite enabled auto-merge August 4, 2025 14:16
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 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 3523a83 Previous: 8146755 Ratio
rollup-block-root-single-tx 254 s 196 s 1.30

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

CC: @TomAFrench

@asterite asterite added this pull request to the merge queue Aug 4, 2025
Merged via the queue into master with commit 9b1b10f Aug 4, 2025
121 of 122 checks passed
@asterite asterite deleted the ab/some-nargo-expand-fixes branch August 4, 2025 14:58
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 11, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: Release Noir(1.0.0-beta.10)
(noir-lang/noir#9311)
feat(ssa_fuzzer): arrays support
(noir-lang/noir#9427)
chore: add link to issue on TODOs
(noir-lang/noir#8307)
fix(ownership): Increment reference count for nested array get in LHS
assignment (noir-lang/noir#9347)
chore: add some mem2reg unit tests
(noir-lang/noir#9405)
chore(as_slice_length): Various unit tests
(noir-lang/noir#9419)
chore(simplify_cfg): Additional unit tests
(noir-lang/noir#9426)
chore: create directory when writing witness artefact
(noir-lang/noir#9383)
fix: throw error if foreign call returns the wrong number of fields
(noir-lang/noir#9286)
chore: update ex in docs (noir-lang/noir#9385)
chore: add some `assert_constant` tests
(noir-lang/noir#9413)
chore: error on non constant inputs for Pedersen generators
(noir-lang/noir#9389)
chore(inlining): Unit tests for global values and conditional inlining
(noir-lang/noir#9411)
chore(ssa_fuzzer): refactor fuzzing modes + add fuzzing mode without DIE
pass (noir-lang/noir#9401)
chore(inlining): Various unit tests
(noir-lang/noir#9388)
feat(ssa_fuzzer): pushing generated program and witness to redis queue
(noir-lang/noir#9375)
fix(ssa_gen): Generate code for index before the collection
(noir-lang/noir#9332)
chore: Regression test for calling a mutable closure inside a mutable
closure (noir-lang/noir#9384)
fix: some nargo expand fixes
(noir-lang/noir#9324)
fix: disable comptime printing when requesting json output
(noir-lang/noir#9381)
chore: address TODO comments
(noir-lang/noir#9379)
feat: type alias for numeric generics
(noir-lang/noir#7583)
chore: bump external pinned commits
(noir-lang/noir#9291)
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.

2 participants