mir_build: Split MatchPairTree into testable and or-pattern variants - #158551
Closed
Zalathar wants to merge 2 commits into
Closed
mir_build: Split MatchPairTree into testable and or-pattern variants#158551Zalathar wants to merge 2 commits into
MatchPairTree into testable and or-pattern variants#158551Zalathar wants to merge 2 commits into
Conversation
This creates a clearer distinction between non-or-pattern nodes that can be directly tested, and or-pattern nodes that require or-expansion before they can participate in tests. Storing testable nodes and or-pattern nodes in separate lists means that there is no need to uphold a sorting invariant, and avoids the need for assertions and unwraps in various helper functions that expect to only deal with one or the other. There should be no change to the resulting MIR.
Now that or-patterns have their own separate variant, testable match-pairs are always guaranteed to have a place.
Collaborator
|
Some changes occurred in match lowering cc @Nadrieril |
rustbot
added
S-waiting-on-review
Collaborator
|
|
Nadrieril
reviewed
Jun 29, 2026
Comment on lines
+1136
to
+1144
| #[derive(Debug)] | ||
| struct MatchPairsQueue<'tcx> { | ||
| /// Match pairs that can be "tested" directly, because they are not or-patterns. | ||
| testable_match_pairs: Vec<TestableMatchPairTree<'tcx>>, | ||
| /// Or-patterns, which must be expanded before their subpatterns can participate in tests. | ||
| /// These should only be processed after `testable_match_pairs` is empty | ||
| /// (see [`Self::starts_with_or_pattern`]). | ||
| or_match_pairs: Vec<OrMatchPairTree<'tcx>>, | ||
| } |
Member
There was a problem hiding this comment.
Unfortunately I don't think that's the right approach: we will need to support having or-patterns not sorted to the end in order to fix #158387. Also, it's not a "must": sorting or-patterns at the end is purely an optimization (well, except the union case), so forgetting the invariant isn't a huge issue.
Member
Author
There was a problem hiding this comment.
Hmm yeah, I can appreciate not wanting to further entrench the or-pattern reordering behaviour.
Member
Author
|
The future of this PR is a bit murky and it’s currently not a high priority for me, so: @rustbot author |
rustbot
added
S-waiting-on-author
Member
Author
|
Closing this, as I now have some different ideas about separating or-patterns and testable patterns. |
rustbot
removed
the
S-waiting-on-author
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
3 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.
This creates a clearer distinction between non-or-pattern nodes that can be directly tested, and or-pattern nodes that require or-expansion before they can participate in tests.
Storing testable nodes and or-pattern nodes in separate lists means that there is no need to uphold a sorting invariant, and avoids the need for assertions and unwraps in various helper functions that expect to only deal with one or the other.
There should be no change to the resulting MIR.
r? Nadrieril