Skip to content
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

Merged
merged 2 commits into from
Feb 14, 2016

Conversation

petrochenkov
Copy link
Contributor

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

@Manishearth
Copy link
Member

r=me for this, but I'd like someone else to sign off on the naming scheme too.

@bors
Copy link
Contributor

bors commented Feb 13, 2016

☔ The latest upstream changes (presumably #31524) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Contributor Author

Rebased.

@Manishearth
Copy link
Member

@nrc @eddyb can you approve the change with variants here?

/// 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),
Copy link
Member

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.

Copy link
Contributor Author

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.

Yeah, probably, for AST and for HIR for the time being.
I'd like to have something like this in the HIR eventually, when name resolution is performed before lowering to HIR:
pat

Copy link
Member

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.

@eddyb
Copy link
Member

eddyb commented Feb 13, 2016

LGTM except for the UnitStruct name.

@petrochenkov
Copy link
Contributor Author

Updated.

@petrochenkov
Copy link
Contributor Author

Ping @Manishearth

@Manishearth
Copy link
Member

@bors r+

cc @erickt

@bors
Copy link
Contributor

bors commented Feb 13, 2016

📌 Commit 9f414a4 has been approved by Manishearth

bors added a commit that referenced this pull request Feb 14, 2016
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
@bors
Copy link
Contributor

bors commented Feb 14, 2016

⌛ Testing commit 9f414a4 with merge 9d98390...

@bors bors merged commit 9f414a4 into rust-lang:master Feb 14, 2016
bors added a commit that referenced this pull request Feb 15, 2016
bors added a commit that referenced this pull request Feb 15, 2016
bors added a commit that referenced this pull request Feb 17, 2016
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
@petrochenkov petrochenkov deleted the patrefact branch September 21, 2016 19:50
critiqjo pushed a commit to critiqjo/rustdoc that referenced this pull request Dec 16, 2016
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants