-
Notifications
You must be signed in to change notification settings - Fork 13k
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
[breaking batch] Rename and refactor ast::Pat_ variants #31581
Conversation
r=me for this, but I'd like someone else to sign off on the naming scheme too. |
982c2e5
to
bf07420
Compare
☔ The latest upstream changes (presumably #31524) made this pull request unmergeable. Please resolve the merge conflicts. |
bf07420
to
9348abe
Compare
Rebased. |
/// A unit struct/variant pattern. | ||
/// Some const patterns are also represented by `PatKind::UnitStruct` due to parser limitations. | ||
/// Some unit patterns are represented by `PatKind::Ident` due to the same parser limitations. | ||
UnitStruct(Path), |
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.
I think this should be just Path
.
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.
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.
Associated constants and top-level constants are different though.
LGTM except for the |
9348abe
to
9f414a4
Compare
Updated. |
Ping @Manishearth |
📌 Commit 9f414a4 has been approved by |
cc #31487 (comment) plugin-[breaking-change] The first commit renames `ast::Pat_` to `ast::PatKind` and uses its variants in enum qualified form. I've also taken the opportunity and renamed `PatKind::Region` into `PatKind::Ref`. The second commit splits `PatKind::Enum` into `PatKind::TupleStruct` and `PatKind::UnitStruct`. So, pattern kinds now correspond to their struct/variant kinds - `Struct`, `TupleStruct` and `UnitStruct`. @nikomatsakis @nrc @arielb1 Are you okay with this naming scheme? An alternative possible naming scheme is `PatKind::StructVariant`, `PatKind::TupleVariant`, `PatKind::UnitVariant` (it's probably closer to the common use, but I like it less). I intend to apply these changes to HIR later, they should not necessarily go in the same nightly with #31487 r? @Manishearth
⌛ Testing commit 9f414a4 with merge 9d98390... |
And split `PatKind::Enum` into `PatKind::TupleStruct` and `PatKind::Path`. This is the HIR part of #31581. This is also kind of a preparation for rust-lang/rfcs#1492. r? @eddyb
And split `PatKind::Enum` into `PatKind::TupleStruct` and `PatKind::Path`. This is the HIR part of rust-lang/rust#31581. This is also kind of a preparation for rust-lang/rfcs#1492. r? @eddyb
cc #31487 (comment)
plugin-[breaking-change]
The first commit renames
ast::Pat_
toast::PatKind
and uses its variants in enum qualified form. I've also taken the opportunity and renamedPatKind::Region
intoPatKind::Ref
.The second commit splits
PatKind::Enum
intoPatKind::TupleStruct
andPatKind::UnitStruct
.So, pattern kinds now correspond to their struct/variant kinds -
Struct
,TupleStruct
andUnitStruct
.@nikomatsakis @nrc @arielb1 Are you okay with this naming scheme?
An alternative possible naming scheme is
PatKind::StructVariant
,PatKind::TupleVariant
,PatKind::UnitVariant
(it's probably closer to the common use, but I like it less).I intend to apply these changes to HIR later, they should not necessarily go in the same nightly with #31487
r? @Manishearth