fix(semantic): expand a nested repetition over its own group of captures - #10281
Draft
orizi wants to merge 1 commit into
Conversation
A repetition is re-walked once per iteration of its parent, so the Repetition arm minted a fresh RepetitionId on every outer iteration. A nested repetition therefore had no stable identity: placeholder_to_rep_id ended up pointing at the last traversal's id, and expansion iterated the whole flat capture list on every outer iteration. m!(1 => [7], 2 => [9]) expanded to 1 + 7+9+2 + 7+9+0. Ids now come from the repetition's pattern site, so one id spans all iterations of the enclosing block, and each entry into a repetition records its match count in rep_group_lens. A repetition is entered once per iteration of its parent, so the parent's current index selects the group and the preceding group lengths give its offset into the flat captures - the leaf lookup is unchanged. is_macro_rule_match now returns the MatcherContext instead of a tuple of the fields expansion happens to need. The tuple silently dropped rep_depths when it was added, and dropped these two tables as well; returning the context is only safe now because the dead match-time repetition_indices loop, which left that map populated, is removed here - it was write-only, as the context was discarded before expansion. Still unhandled, and unchanged: a block whose only placeholder is bound to a deeper repetition, as in $($($x +)*)* 0, has nothing bound at its own level and re-walks the group. macro_rules! rejects that shape; making it an error is the follow-up, not making it iterate.
Collaborator
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
5 tasks
|
This change is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #10270.
A nested repetition now expands over its own group of captures rather than the whole flat list.
Captures are stored flat, one list per placeholder, so a nested repetition had no way to say "this iteration's slice". With more than one group at a level the inner block re-walked every capture on every outer step:
MatcherContextgainsrep_ids,rep_parentsandrep_group_lens. A repetition is entered once per iteration of its parent, so the parent's current index selects the group and the lengths of preceding groups give the offset.rep_idsis keyed by the pattern's syntax node rather than by traversal order — a nested repetition is re-walked on every outer iteration and would otherwise get a fresh id, and no stable identity, each time.Tests
Goldens in
expansion_test_data/inline_macros, all expected values cross-checked againstmacro_rules!under rustc:three_levelsstrengthened to repeat at every level (per review) —1 => [2 => (3, 4), 5 => (6, 7)], 8 => [9 => (10, 11)]unsorted_lhs— a pattern whose placeholders are not ordered by repetition depthmulti_vars— several placeholders bound at each of two depthsThe last three were requested on #10270 but cannot live there: that commit still walks the flat list, so they emit wrong output and landing them there would mean blessing it.