-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Self
in type definitions allowing enum List<T> { Nil, Cons(T, Box<Self>) }
#2300
Conversation
@rfcbot fcp merge To me, this is just a bug fix. I remember when it was really inconsistent where you could use As a note, we'll probably want to watch what error message you get if you try to use |
Team member @withoutboats has proposed to merge this. The next step is review by the rest of the tagged teams: Concerns:
Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
That was the fastest FCP I've ever seen 😆 |
I just want to point out that tuples also don't support |
A note on struct F(usize); impl F { fn x() { Self { 0: 1337 }; } } But I won't recommend anyone to write it that way =P If we can make |
This is certainly not a bug fix |
@petrochenkov Can you explain why it's a breaking change? Neither of the following compiles today: struct Foo<Self> { Variant(Self) }
struct Bar;
impl Bar {
fn bar() {
struct Baz(Self);
}
} |
@Fcpbot concern teaching when to use I fully approve of the technical change here. However, I have a concern about best practices for where to use |
@rfcbot concern teaching when to use Self vs naming the type |
@nrc I assume you are referring to the Migration advice section and specifically:
I understand your concern about the level of indirection and I think it's a valid one. However, I think that, and especially for longer and generic types, Also, often there won't be that many variants, so a reader can keep the With generic types, you also get a lot of nested #[derive(Debug)]
enum Expr<T: Ty> {
Lit(T::Repr),
Add(Box<Expr<Int>>, Box<Expr<Int>>),
If(Box<Expr<Bool>>, Box<Self>, Box<Self>),
} I myself prefer to write Syntax highlighting also has an easier time rendering However, the standard library, as far as I had time to check, seems not to agree with me. I checked a few These are my thoughts so far on the subject. If you believe these do not assuage your concerns and are not motivation enough, I am happy to move that section out of the RFC and propose the lint for |
Phew. It is great that it errors now, because technically it should not error. I think we should continue this tradition and keep items as "barriers" for new |
Here I'll try to audit items and item-likes with regards to possible meaning of
So, the found inconsistencies are:
Future compatibility: |
|
Ah, right, I was focused on name resolution too much. |
I have a concern about how this potentially interacts with nested and unnamed types. Suppose we end up merging #2102 , and for that matter future extensions to that to allow defining struct and union types inline inside other types that contain them. In that case, we'd have two questions:
|
@joshtriplett Thanks for bringing that to my attention! It seems to me that unnamed/nested types in structs and unions are somewhat similar to "structs" inside variants of enums as in EDIT: a third option could be to simply not allow |
@Centril |
@petrochenkov Right. Tho the third option could possibly not apply to enum variants (that is probably inconsistent?) and instead only to anonymous structs and unions. In any case, I'd prefer to go with option 2, "Self always applies to the top level" which is simple and easy to remember. |
Sadly we already let Self apply to the innermost level: https://play.rust-lang.org/?gist=19d4ca95e57936c4a78d948f66a7edc9&version=stable |
I don't think that one counts... ^,- In that case it is an item shadowing " |
Added notes about RFC 2102 to the reference level explanation. |
IMO, it's not very intuitive in case of complex type hierarchies with unnamed fields. I believe, the most reliable and forward compatible solution would be to disallow So, my opinion would be to preserve the behavior consistent over all sugar expansion patterns. |
@0x7CFE Please excuse my late reply =)
My intuition about this is that the type of unnamed structs and unions are not nameable, so I think that disallowing |
@rfcbot fcp concern I have a concern here. One thing we have talked about doing is permitting more kinds of items in the body of impls. In particular, I believe @aturon and I wanted to permit: impl Trait for Foo {
struct Bar { ... }
} as an equivalent to: struct _Bar { ... }
impl Trait for Foo {
type Bar = _Bar;
} But what would the meaning of This seems to be a direct consequence of trying to give (Note that |
This is true, and I'll include it in the drawbacks; I should have noted that enum JsonNode {
Null,
Bool(bool),
Number(f64),
String(String),
Array(Vec<Self>),
Map(HashMap<String, Self>),
} I don't think this is a problem because you are not searching for all positions frequently. My main conclusion from your experiment is that the migration advice should be removed, which I've now done. |
@rfcbot resolved teaching when to use Self vs naming the type |
So, I'm almost ready to mark my concern was resolved, but not quite: I would like an unresolved question to be added:
I realize that the "syntax" I semi-proposed earlier is only half the battle; the real question has to do with the confusion that will arise if we allow types to be directly declared within impls. For example, using the new syntax:
What type does that |
Also, why doesn't the RFC permit
|
@nikomatsakis |
@rfcbot resolve self-in-impl-vs-self-in-type |
I still have some reservations about the ever-expanding scope of |
I like declaring type directly within impls, but if you allow that then you might allow type aliases too, so
And maybe an analog of
|
@burdges Unfortunately, within an The analog to |
@rfcbot resolved self-in-impl-vs-self-in-type |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period is now complete. |
Huzzah! This RFC has been merged! Tracking issue: rust-lang/rust#49303 |
Rendered
Tracking issue
The special
Self
identifier is now permitted instruct
,enum
, andunion
type definitions. A simple example
struct
is:Thanks to @eddyb for giving me the idea, and among others to @est31 for reviewing.