-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fully implement or-pattern parsing #63693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
5299d8a
parser: extract `ban_unexpected_or_or`.
Centril 1ba7550
parser: type alias `type Expected = Option<&'static str>;`.
Centril 0bbea47
parser: refactor `parse_pat_with_or` + use it in [p0, p1, ..] pats.
Centril 30b841d
parser: improve or-patterns recovery.
Centril d34ee76
parser: move `multiple-pattern-typo` -> `or-patterns` directory.
Centril 5f57fee
parser: `multiple-pattern-typo`: cover more or-pattern places.
Centril f852c7c
parser: simplify `parse_pat_with_or`.
Centril 21d9b85
parser: extract `maybe_recover_unexpected_comma`.
Centril a4a34ab
parser: integrate `maybe_recover_unexpected_comma` in `parse_pat_with…
Centril 7b59b4f
parser: extract `eat_or_separator`.
Centril dc5bbaf
parser: improve `parse_pat_with_or` docs.
Centril 6498959
parser: use `eat_or_separator` for leading vert.
Centril 39f5e5b
parser: move `maybe_recover_unexpected_comma` to a more appropriate p…
Centril 8f6a0cd
parser: document `ban_unexpected_or_or`.
Centril b7178ef
parser: `parse_pats` -> `parse_top_pat{_unpack}`.
Centril 92d66a1
parser: document `parse_pat`.
Centril 95792b4
parser: `let` stmts & `for` exprs: allow or-patterns.
Centril 5f6bec8
parser: drive-by: simplify `parse_arg_general`.
Centril a9af18b
move `ui/or-pattern-mismatch` -> `ui/or-patterns/`.
Centril f35432e
or-patterns: add syntactic tests.
Centril d3b3bce
move `feature-gate-or_patterns.*` -> `ui/or-patterns/`
Centril 1ffea18
or-patterns: harden feature gating tests.
Centril b205055
parser: better recovery for || in inner pats.
Centril b2966e6
parser: bool -> GateOr.
Centril a9ef859
parser: bool -> TopLevel.
Centril 3a40542
parse_top_pat: silence leading vert gating sometimes.
Centril e374772
parser: extract recover_inner_leading_vert.
Centril 0ab8430
parser: reword || recovery.
Centril 1202cb0
parser: simplify parse_pat_with_or_{inner}
Centril 083963e
parser: 'while parsing this or-pattern...'
Centril 1caaa40
parser: gracefully handle `fn foo(A | B: type)`.
Centril 6a73199
or_patterns: add run-rustfix tests.
Centril acb1130
parser: TopLevel -> RecoverComma.
Centril 2bd27fb
parser: fix span for leading vert.
Centril File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would this just be a singular
pat
?Foo(A | B)
will now be a single pattern that contains an or-pattern, but woudn't we still want to parseFoo(A) | Bar(B)
as multiple patterns?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having
arm.pat: P<Pat>
instead ofarm.pats: Vec<P<Pat>>
simplifies the overall structure of the compiler by using a uniform mechanism for or-patterns instead of having a distinction between or-patterns at the top-level and in nested positions. As an example, you can see that the parserpat.rs
parser has, in this PR, a hack to unpack an or-pattern at the top level. There are many other places (e.g. in resolve, save-analysis, etc.) in which it would simplify (and reduce duplication of logic) to have one uniform mechanism. So we should representp1 | p2
at the top of a match arm simply asPatKind::Or
.Edit: As a further consideration, using
Vec<P<Pat>>
pessimizes what I think is the most common case of an arm with justFoo =>
. Moreover,let
now permits or-patterns so there's no good reason to have a top-level distinction sometimes and sometimes not. In terms of preventing bugs, it is also better to have one mechanism since you can ensure that way that the semantics at the top and inner levels do not diverge.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it! This makes a lot of sense and I'd love to see this happen.The lowering in MIR will require quite a bit of work and thought in order to get to this point, but AFAIK unifying the two could also greatly simplify
librustc_mir/build/matches
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh absolutely. I think basically we want to work from both sides and meet in the middle. That is, we fix the AST and add a hack to lowering. Then we fix lowering + HIR and add a hack to HAIR. Eventually we get down to MIR and the hack is no more.