Fix macro_metavar_expr_concat behavior with nested repetitions #150026
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
Delta17920
force-pushed
the
fix/150002-macro-concat-bug
branch
from
d913071 to
0f793cc
Compare
|
@rustbot reroll |
|
r? compiler |
| let mut matched = matched_from_ident(dcx, *ident, tscx.interp)?; | ||
| let mut repeats_iter = tscx.repeats.iter(); | ||
|
|
||
| loop { |
There was a problem hiding this comment.
This kind of loop seems to be similar to what's already implemented in lookup_cur_matched. I don't quite understand why we need to do this logic again here. Maybe that function is not applicable, but I expect there already being code for getting the right variable to repetition conversion somewhere else. That's (to my admittedly slightly limited understanding) what lookup_cur_matched tries to do. Can you explain why this logic needs to be redone here?
There was a problem hiding this comment.
Thanks for the suggestion. You are right my implementation handles the descent manually, but since I'm just looking for the specific match at the current depth (and erroring otherwise), lookup_cur_matched covers this exact use case. I'll update it to use that existing function to avoid duplication
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
Delta17920
force-pushed
the
fix/150002-macro-concat-bug
branch
from
0f793cc to
3566b67
Compare
|
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. |
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
@jdonszelmann any other changes? |
|
I'll get to it. My throughput is a bit lower atm because of holidays |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…-bug, r=jdonszelmann
Fix macro_metavar_expr_concat behavior with nested repetitions
**The Bug**: The `${concat(...)}` expression was using the wrong loop index when inside nested repetitions (like optional groups), causing it to get "stuck" on the first element and generate duplicate code.
**The Fix**: Updated `metavar_expr_concat` in `transcribe.rs` to correctly search the repetition stack (`tscx.repeats`) for the target variable instead of blindly using the last index.
**Tests**:
Added `tests/ui/macros/concat-nested-repetition.rs.`
Fixes rust-lang#150002
Rollup of 4 pull requests Successful merges: - #150026 (Fix macro_metavar_expr_concat behavior with nested repetitions) - #150521 (resolve: Rename "name bindings" to "name declarations") - #150704 (MGCA: Const constructors support) - #150728 (Cleanup some ui tests for const-traits) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 4 pull requests Successful merges: - #150026 (Fix macro_metavar_expr_concat behavior with nested repetitions) - #150521 (resolve: Rename "name bindings" to "name declarations") - #150704 (MGCA: Const constructors support) - #150728 (Cleanup some ui tests for const-traits) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #150026 - Delta17920:fix/150002-macro-concat-bug, r=jdonszelmann Fix macro_metavar_expr_concat behavior with nested repetitions **The Bug**: The `${concat(...)}` expression was using the wrong loop index when inside nested repetitions (like optional groups), causing it to get "stuck" on the first element and generate duplicate code. **The Fix**: Updated `metavar_expr_concat` in `transcribe.rs` to correctly search the repetition stack (`tscx.repeats`) for the target variable instead of blindly using the last index. **Tests**: Added `tests/ui/macros/concat-nested-repetition.rs.` Fixes #150002
7 participants
The Bug: The
${concat(...)}expression was using the wrong loop index when inside nested repetitions (like optional groups), causing it to get "stuck" on the first element and generate duplicate code.The Fix: Updated
metavar_expr_concatintranscribe.rsto correctly search the repetition stack (tscx.repeats) for the target variable instead of blindly using the last index.Tests:
Added
tests/ui/macros/concat-nested-repetition.rs.Fixes #150002