Skip to content

fix: when macro parse error happens, discard warnings; also preserve unquoted token locations#8944

Merged
asterite merged 4 commits intomasterfrom
ab/failed-to-parse-macro-discard-warnings
Jun 17, 2025
Merged

fix: when macro parse error happens, discard warnings; also preserve unquoted token locations#8944
asterite merged 4 commits intomasterfrom
ab/failed-to-parse-macro-discard-warnings

Conversation

@asterite
Copy link
Collaborator

Description

Problem

No issue, just something confusing mentioned over Slack that could be improved.

Summary

Two things here:

  1. When a macro parse error happened we'd show the just first error. The issue was that in the list of "errors" warnings could exist too. So here we discard warnings and truly show the first error. (Maybe ideally we'd show all errors and warnings here but I'm not sure how we'd show that as a result of a macro expansion, so I did the minimum thing to improve the situation without introducing a large change).
  2. The location of the $expr token ended up being the location of the quote expression that held it, making errors point to that entire quote (as can be seen in the second commit). This was likely done before everything started holding a Location instead of a Span. The second improvement here is to preserve those token locations. The third commit shows how the error improves.

In the actual code with the confusing error message, this is what we initially saw in VSCode:

image

This is the warning caused by not being able to parse the macro.

Once that was fixed (first commit), we got this error:

image

It actually continues above and below that: it's the entire quote { ... } that has the error.

With the third commit we get this error:

image

with the error being "Expected value, found built-in type (resolved type)". So now it's much clearer where's the error.

Additional Context

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: e701428 Previous: 5071093 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 219 s 149 s 1.47
test_report_noir-lang_noir_bigcurve_ 293 s 239 s 1.23

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

CC: @TomAFrench

@TomAFrench
Copy link
Member

Looks like the snapshot is incorrect.

@asterite asterite enabled auto-merge June 17, 2025 15:30
@asterite asterite added this pull request to the merge queue Jun 17, 2025
Merged via the queue into master with commit d1e55d9 Jun 17, 2025
117 of 124 checks passed
@asterite asterite deleted the ab/failed-to-parse-macro-discard-warnings branch June 17, 2025 15:55
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jun 18, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(LSP): suggest generic type methods
(noir-lang/noir#8948)
fix: Fix if/match tracking in last uses pass
(noir-lang/noir#8935)
fix(ssa): Swap Brillig index shift and DIE in minimal pipeline
(noir-lang/noir#8946)
fix: when macro parse error happens, discard warnings; also preserve
unquoted token locations (noir-lang/noir#8944)
chore: disable noncritical workflow steps which fail on external prs
(noir-lang/noir#8939)
chore(test): Add `--minimal-ssa` to integration tests
(noir-lang/noir#8938)
fix(noirc_evaluator): U128 Binary::And simplification
(noir-lang/noir#8940)
chore: Improve access to data required to extract Noir
(noir-lang/noir#8793)
chore: bump external pinned commits
(noir-lang/noir#8920)
chore(fuzz): Compare print output on failed programs
(noir-lang/noir#8921)
chore(fuzz): Display nightly fuzz status badge
(noir-lang/noir#8932)
fix(fuzz): Consider values returned from Brillig to ACIR as dynamic
(noir-lang/noir#8931)
fix(fuzz): Fix env var name in fuzzing workflow
(noir-lang/noir#8929)
chore(fuzz): Add nightly fuzz workflow with 5 minute budget
(noir-lang/noir#8925)
fix(mem2reg): Keep last store for a used nested array
(noir-lang/noir#8917)
chore(fuzz): Increase the number of deterministic test cases
(noir-lang/noir#8872)
fix: Create calls to `apply` before function values are changed to
fields in defunctionalize (noir-lang/noir#8916)
chore: Add various regression tests for mutable references
(noir-lang/noir#8915)
fix: handle return_data in the interpreter SSA CLI
(noir-lang/noir#8914)
feat(fuzz): Generate references in the AST fuzzer
(noir-lang/noir#8728)
fix(fuzz): Use an inline block to circumvent negation with overflow
(noir-lang/noir#8911)
fix: Inline global arrays with functions at their call site
(noir-lang/noir#8905)
chore: Casting ACIR vs Brillig regression tests
(noir-lang/noir#8903)
fix(mem2reg): Keep last store for reference in array used only in an
array get (noir-lang/noir#8877)
fix: more SSA interpreter fixes
(noir-lang/noir#8904)
chore: Ban dynamic array indices with reference elements
(noir-lang/noir#8888)
chore(ssa_fuzzer): Add SSA validation to the SSA fuzzer
(noir-lang/noir#8901)
chore(fuzz): More metamorphic rules
(noir-lang/noir#8857)
fix: catch unbound type variables during frontend compilation
(noir-lang/noir#8686)
fix: preserve functions which are used in `array_set` instructions
(noir-lang/noir#8891)
fix: assorted SSA interpreter fixes
(noir-lang/noir#8893)
fix(ssa): Signed cast simplification
(noir-lang/noir#8862)
feat: simplify apply function cfg immediately
(noir-lang/noir#8895)
fix: do not hoist control dependent cast
(noir-lang/noir#8886)
fix(licm): Account for negative bounds when checking whether a loop
executes (noir-lang/noir#8889)
chore: Release Noir(1.0.0-beta.7)
(noir-lang/noir#8520)
chore: test noir-ecdsa library in CI
(noir-lang/noir#8859)
chore: Incorrect clippy warning blocking PRs
(noir-lang/noir#8861)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jul 16, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(LSP): suggest generic type methods
(noir-lang/noir#8948)
fix: Fix if/match tracking in last uses pass
(noir-lang/noir#8935)
fix(ssa): Swap Brillig index shift and DIE in minimal pipeline
(noir-lang/noir#8946)
fix: when macro parse error happens, discard warnings; also preserve
unquoted token locations (noir-lang/noir#8944)
chore: disable noncritical workflow steps which fail on external prs
(noir-lang/noir#8939)
chore(test): Add `--minimal-ssa` to integration tests
(noir-lang/noir#8938)
fix(noirc_evaluator): U128 Binary::And simplification
(noir-lang/noir#8940)
chore: Improve access to data required to extract Noir
(noir-lang/noir#8793)
chore: bump external pinned commits
(noir-lang/noir#8920)
chore(fuzz): Compare print output on failed programs
(noir-lang/noir#8921)
chore(fuzz): Display nightly fuzz status badge
(noir-lang/noir#8932)
fix(fuzz): Consider values returned from Brillig to ACIR as dynamic
(noir-lang/noir#8931)
fix(fuzz): Fix env var name in fuzzing workflow
(noir-lang/noir#8929)
chore(fuzz): Add nightly fuzz workflow with 5 minute budget
(noir-lang/noir#8925)
fix(mem2reg): Keep last store for a used nested array
(noir-lang/noir#8917)
chore(fuzz): Increase the number of deterministic test cases
(noir-lang/noir#8872)
fix: Create calls to `apply` before function values are changed to
fields in defunctionalize (noir-lang/noir#8916)
chore: Add various regression tests for mutable references
(noir-lang/noir#8915)
fix: handle return_data in the interpreter SSA CLI
(noir-lang/noir#8914)
feat(fuzz): Generate references in the AST fuzzer
(noir-lang/noir#8728)
fix(fuzz): Use an inline block to circumvent negation with overflow
(noir-lang/noir#8911)
fix: Inline global arrays with functions at their call site
(noir-lang/noir#8905)
chore: Casting ACIR vs Brillig regression tests
(noir-lang/noir#8903)
fix(mem2reg): Keep last store for reference in array used only in an
array get (noir-lang/noir#8877)
fix: more SSA interpreter fixes
(noir-lang/noir#8904)
chore: Ban dynamic array indices with reference elements
(noir-lang/noir#8888)
chore(ssa_fuzzer): Add SSA validation to the SSA fuzzer
(noir-lang/noir#8901)
chore(fuzz): More metamorphic rules
(noir-lang/noir#8857)
fix: catch unbound type variables during frontend compilation
(noir-lang/noir#8686)
fix: preserve functions which are used in `array_set` instructions
(noir-lang/noir#8891)
fix: assorted SSA interpreter fixes
(noir-lang/noir#8893)
fix(ssa): Signed cast simplification
(noir-lang/noir#8862)
feat: simplify apply function cfg immediately
(noir-lang/noir#8895)
fix: do not hoist control dependent cast
(noir-lang/noir#8886)
fix(licm): Account for negative bounds when checking whether a loop
executes (noir-lang/noir#8889)
chore: Release Noir(1.0.0-beta.7)
(noir-lang/noir#8520)
chore: test noir-ecdsa library in CI
(noir-lang/noir#8859)
chore: Incorrect clippy warning blocking PRs
(noir-lang/noir#8861)
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