-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Rename ItemKind::Ty
to ItemKind::TyAlias
#63213
Conversation
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 we should rename ImplItemKind::Type
to ::Ty(pe)Alias
also in that case.
Strongly agree with this change; r=me when-green |
r? @Centril |
@bors r=Centril |
📌 Commit da36977b4fda3ef3b89004d9f375e72b33e2b09c has been approved by |
☔ The latest upstream changes (presumably #63180) made this pull request unmergeable. Please resolve the merge conflicts. |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
da36977
to
fd819d0
Compare
@bors r=Centril |
📌 Commit fd819d0 has been approved by |
Rename `ItemKind::Ty` to `ItemKind::TyAlias` The current name is not entirely clear without context and `TyAlias` is consistent with `ItemKind::TraitAlias`.
☀️ Test successful - checks-azure |
Rustup to rust-lang/rust#63213 changelog: none
No, that's an associated type... |
@@ -1837,7 +1837,7 @@ pub enum ImplItemKind { | |||
/// A method implementation with the given signature and body. | |||
Method(MethodSig, BodyId), | |||
/// An associated type. | |||
Type(P<Ty>), | |||
TyAlias(P<Ty>), |
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.
Basically I think this should be reverted. Also, as per some issue, Method
should be Fn
.
The full name is "associated type alias/synonym". |
@Centril I've never heard "associated type alias" in the context of Rust, and also this PR only changes the |
Ah woops; we should change the As for "in the context of Rust", that's probably because it is just shorter to use the abbreviation. Folks in the Haskell / C++ communities usually don't say the full thing either. Another suggestion for you if you don't like |
I like |
Let's also rename @eddyb I'll happily review that PR :) |
I prefer
That was an oversight 👍 The intention was that they were all made consistent. |
The current name is not entirely clear without context and
TyAlias
is consistent withItemKind::TraitAlias
.