Skip to content

rustc_expand: improve diagnostics for non-repeatable metavars#152679

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Unique-Usman:ua/decmacrounrepeatable
Feb 25, 2026
Merged

rustc_expand: improve diagnostics for non-repeatable metavars#152679
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Unique-Usman:ua/decmacrounrepeatable

Conversation

@Unique-Usman
Copy link
Contributor

@Unique-Usman Unique-Usman commented Feb 15, 2026

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 15, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 15, 2026

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 68 candidates
  • Random selection from 14 candidates

@Unique-Usman
Copy link
Contributor Author

r? @estebank

@rustbot rustbot assigned estebank and unassigned BoxyUwU Feb 15, 2026
@Unique-Usman
Copy link
Contributor Author

This is rebased on top of ->791ee34

@rust-log-analyzer

This comment has been minimized.

@Unique-Usman Unique-Usman force-pushed the ua/decmacrounrepeatable branch from 0fbe120 to 81f68bb Compare February 15, 2026 21:33
@rust-bors

This comment has been minimized.

@Unique-Usman Unique-Usman force-pushed the ua/decmacrounrepeatable branch from 81f68bb to 60060dd Compare February 20, 2026 14:59
@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Unique-Usman Unique-Usman force-pushed the ua/decmacrounrepeatable branch from 9704509 to d4e8858 Compare February 23, 2026 20:17
@rust-log-analyzer

This comment has been minimized.

Co-authored-by: Esteban Küber <esteban@kuber.com.ar>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
@Unique-Usman
Copy link
Contributor Author

@estebank - this is ready to land. Thank you for the review.

@estebank
Copy link
Contributor

@bors r+

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 24, 2026

📌 Commit 93b9973 has been approved by estebank

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 24, 2026
@Unique-Usman
Copy link
Contributor Author

this #47452 should be closed now ?

@estebank
Copy link
Contributor

Updated the description so that when the PR merges the ticket gets autoclosed.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 24, 2026
…le, r=estebank

rustc_expand: improve diagnostics for non-repeatable metavars

Fix rust-lang#47452.
rust-bors bot pushed a commit that referenced this pull request Feb 25, 2026
Rollup of 8 pull requests

Successful merges:

 - #149169 (ptr::replace: make calls on ZST null ptr not UB)
 - #150562 (Fix doc link used in suggestion for pinning self)
 - #152679 (rustc_expand: improve diagnostics for non-repeatable metavars)
 - #153017 (Implement debuginfo for unsafe binder types)
 - #152868 (delete some very old trivial `Box` tests)
 - #152922 (rustc_public: Make fields that shouldn't be exposed visible only in `rustc_public`)
 - #153029 (Rename `rustc::pass_by_value` lint as `rustc::disallowed_pass_by_ref`.)
 - #153051 (Migration of `LintDiagnostic` - part 3)
rust-bors bot pushed a commit that referenced this pull request Feb 25, 2026
Rollup of 12 pull requests

Successful merges:

 - #149169 (ptr::replace: make calls on ZST null ptr not UB)
 - #150562 (Fix doc link used in suggestion for pinning self)
 - #152418 (`BTreeMap::merge` optimized)
 - #152679 (rustc_expand: improve diagnostics for non-repeatable metavars)
 - #152952 (mGCA: improve ogca diagnostic message )
 - #152977 (Fix relative path handling for --extern-html-root-url)
 - #153017 (Implement debuginfo for unsafe binder types)
 - #152868 (delete some very old trivial `Box` tests)
 - #152922 (rustc_public: Make fields that shouldn't be exposed visible only in `rustc_public`)
 - #153032 (Fix attribute parser and kind names.)
 - #153051 (Migration of `LintDiagnostic` - part 3)
 - #153060 (Give a better error when updating a submodule fails)
@rust-bors rust-bors bot merged commit 35391c8 into rust-lang:main Feb 25, 2026
11 checks passed
@rustbot rustbot added this to the 1.95.0 milestone Feb 25, 2026
rust-timer added a commit that referenced this pull request Feb 25, 2026
Rollup merge of #152679 - Unique-Usman:ua/decmacrounrepeatable, r=estebank

rustc_expand: improve diagnostics for non-repeatable metavars

Fix #47452.
@Kobzol
Copy link
Member

Kobzol commented Feb 25, 2026

@rust-timer build 2f7d51a

For #153074.

@rust-timer
Copy link
Collaborator

Missing artifact for sha 2f7d51a313739380dbe5fa7a71454821a3a8df04 (https://ci-artifacts.rust-lang.org/rustc-builds/2f7d51a313739380dbe5fa7a71454821a3a8df04/rustc-nightly-x86_64-unknown-linux-gnu.tar.xz); not built yet, try again later.

@JonathanBrouwer
Copy link
Contributor

@rust-timer build 2f7d51a

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2f7d51a): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
2.0% [0.2%, 4.9%] 43
Regressions ❌
(secondary)
0.9% [0.2%, 3.3%] 25
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.0% [0.2%, 4.9%] 43

Max RSS (memory usage)

Results (primary 1.7%, secondary -4.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.7% [1.7%, 1.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.0% [-4.0%, -4.0%] 1
All ❌✅ (primary) 1.7% [1.7%, 1.7%] 1

Cycles

Results (primary 2.2%, secondary -1.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.2% [2.0%, 2.5%] 5
Regressions ❌
(secondary)
2.0% [2.0%, 2.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.4% [-3.9%, -2.8%] 2
All ❌✅ (primary) 2.2% [2.0%, 2.5%] 5

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 482.035s -> 479.929s (-0.44%)
Artifact size: 397.81 MiB -> 397.73 MiB (-0.02%)

@rustbot rustbot added the perf-regression Performance regression. label Feb 25, 2026
@JonathanBrouwer
Copy link
Contributor

Oof, that's a significant regression :c
What is the best way forward? Revert?

@Kobzol
Copy link
Member

Kobzol commented Feb 25, 2026

Seems like there might be room for improvement here, e.g. merge the two Vecs together, do not always fill them, use an arena or something, or others. But given the scale of the regression, I'd suggest reverting it first.

@JonathanBrouwer
Copy link
Contributor

Created a revert PR here: #153095

@estebank
Copy link
Contributor

Yes, lets revert and clean up.

Comment on lines +407 to +420
let mut bindings = vec![];
for rule in rules {
let MacroRule::Func { lhs, .. } = rule else { continue };
for param in lhs {
let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
bindings.push(*bind);
}
}

let mut matched_rule_bindings = vec![];
for param in lhs {
let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
matched_rule_bindings.push(*bind);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Without closely looking at the lifetimes, it might be that we can pass lhs and rules to from_tts and move this logic to the error code path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macro_rules: "no syntax variables matched as repeating at this depth" fires before "unknown macro variable"

8 participants