-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Support Self
in struct expressions and patterns
#37035
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.
LGTM, certainly something I'd like to use myself.
cc @rust-lang/lang Any objections?
@@ -802,6 +802,34 @@ impl<'tcx> fmt::Display for ty::TraitRef<'tcx> { | |||
} | |||
} | |||
|
|||
impl<'tcx> ty::TypeVariants<'tcx> { | |||
pub fn descr(&self) -> &'static str { |
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.
Don't we have something like this already?
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'm not entirely sure, I've searched a bit, but found only type pretty printing (in the same ppaux.rs), but no short descriptions.
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.
That's because it's hidden away in some error reporting logic.
if ty.is_fn() { | ||
// Tuple variants have fn type even in type namespace, | ||
// extract true variant type from it. | ||
ty = tcx.no_late_bound_regions(&ty.fn_ret()).unwrap(); |
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.
Oops, this is no longer necessary, need to remove.
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.
Due to the "actual variant definition" vs "variant constructor" split?
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.
No, due to tcx.parent_def_id(did)
a few lines above.
Only type arguments are taken from the variant (from base_segments.last()
), ast_path_to_ty
works with the enum after that and returns enum's type, which cannot be a function.
b251cbb
to
4f96b8a
Compare
Can we have a test to make sure this fails? I'd write it myself but not sure how. struct Foo<A> { inner: A };
trait Bar { fn bar(); }
impl Bar for Foo<i32> {
fn bar() {
Self { inner: 1.5f32 };
}
} |
4f96b8a
to
9d63af0
Compare
@Ericson2314 |
Thanks! |
This feels a little weird to me since |
I've heard something about this being true before 1.0, but now paths in struct expressions/patterns are types, possibly with provided type arguments, not just type constructors. struct S<T, U = u16> {
a: T,
b: U,
}
type Alias<T> = S<T>;
fn main() {
// Everything below is legal
let s = S::<u8, u8> { a: 0, b: 1 }; // desugared into S::<u8, u8> { a: 0, b: 1 };
let s = S::<u8> { a: 0, b: 1 }; // desugared into S::<u8, u16> { a: 0, b: 1 };
let s = S { a: 0, b: 1 }; // desugared into S::<_, _> { a: 0, b: 1 };
let s = Alias::<u16> { a: 0, b: 1 }; // desugared into S::<u16, u16> { a: 0, b: 1 };
} So, |
@petrochenkov huh, ok, then I feel much better about this, thanks for the explanation! Does this mean that if the type params or lifetime params of the values for fields disagree with the params of |
@Ericson2314 asked for the same test a few messages above and I added compile-fail/struct-path-self-type-mismatch.rs |
@nrc not knowing that feature prior to this also led me to think of the test case. :) |
@petrochenkov sweet, thanks! |
What about the common case of constructors with generics? struct Foo<T> { ... }
impl<T> Foo<T> {
fn new<U>(u: U) -> Foo<U> {
Self { ... } // is this Foo<T> or Foo<U>?
}
} |
@aturon I think you might need a About [root@li1424-173 rust]# build/x86_64-unknown-linux-gnu/stage1/bin/rustc - <<<"struct Foo<T> { inner: T } impl<T> Foo<T> { fn new<U>(u: U) -> Foo<U> { Self { inner: u } } } fn main() {}" │··
error[E0308]: mismatched types │··
--> <anon>:1:87 │··
| │··
1 | struct Foo<T> { inner: T } impl<T> Foo<T> { fn new<U>(u: U) -> Foo<U> { Self { inner: u } } } fn main() {} │··
| ^ expected type parameter, found a different type parameter │··
| │··
= note: expected type `T` │··
= note: found type `U` │··
│··
error[E0308]: mismatched types │··
--> <anon>:1:73 │··
| │··
1 | struct Foo<T> { inner: T } impl<T> Foo<T> { fn new<U>(u: U) -> Foo<U> { Self { inner: u } } } fn main() {} │··
| ^^^^^^^^^^^^^^^^^ expected type parameter, found a different type parameter │··
| │··
= note: expected type `Foo<U>` │··
= note: found type `Foo<T>` │··
│··
error: aborting due to 2 previous errors @petrochenkov can we get this to work for tuple structs as well? [root@li1424-173 rust]# build/x86_64-unknown-linux-gnu/stage1/bin/rustc - <<<"struct Foo<T>(T); impl<T> Foo<T> { fn new<U>(u: U) -> Foo<U> { Self(u) } } fn main() {}" │··
error[E0425]: unresolved name `Self` │··
--> <anon>:1:64 │··
| │··
1 | struct Foo<T>(T); impl<T> Foo<T> { fn new<U>(u: U) -> Foo<U> { Self(u) } } fn main() {} │··
| ^^^^ unresolved name │··
│··
error: aborting due to previous error Note that |
Let's try that again, with a team: @rfcbot fcp merge |
@durka Type aliases also don't work for tuple structs - but you can have a function with the same name. |
Oh, I didn't know that type aliases don't work for tuple structs. And you don't even get a warning, just an error when you try to use the alias. That's super weird, but yeah orthogonal to this PR. |
FCP proposed with disposition to merge. Review requested from: No concerns currently listed. |
I feel like I've run into cases like this from time to time, though probably in all cases it suffices to make a separate In any case, I think the proposed behavior for this PR is fine there. |
AIUI, this is a case of the case I was asking about and your example should give a type error since |
9d63af0
to
077eed2
Compare
Added this example #37035 (comment) to the tests. |
All relevant subteam members have reviewed. No concerns remain. |
It has been one week since all blocks to the FCP were resolved. |
OK, seems like we're all on board with letting this change go forward. |
📌 Commit 077eed2 has been approved by |
☔ The latest upstream changes (presumably #36695) made this pull request unmergeable. Please resolve the merge conflicts. |
Diagnostics for struct path resolution errors in resolve and typeck are unified. Self type is treated as a type alias in few places (not reachable yet). Unsafe cell is seen in constants even through type aliases. All checks for struct paths in typeck work on type level.
077eed2
to
8a38928
Compare
@bors r=eddyb |
📌 Commit 8a38928 has been approved by |
Support `Self` in struct expressions and patterns Struct expressions and patterns generally support type aliases `Alias { field: 10 }` i.e. they already have to work with `ty::Ty` to do their job. `Self` is a type alias (when it's not a type parameter) => struct expressions and patterns should support `Self`. Typical example: ``` impl MyStruct { fn new() -> Self { Self { a: 10, b: "Hello" } } } ``` The first commit does some preparations and cleanups, see the commit message for details. This also fixes couple of bugs related to aliases in struct paths (fixes #36286). EDIT: Since struct expressions and patterns always work with `ty::Ty` now, associated paths in them are also supported. If associated type `A::B` successfully resolves to a struct (or union) type, then `A::B { /* fields */ }` is a valid expression/pattern. This will become more important when enum variants are treated as [associated items](#26264 (comment)). r? @eddyb
Discussing in the @rust-lang/core team meeting, we realized that this probably ought to have been feature-gated! |
Even though it's small, it seems like feature-gating (and moving to stabilize in next cycle) would be more proper. |
👍 for feature-gating. |
Feature gate Self and associated types in struct expressions and patterns cc rust-lang#37544 Fixes rust-lang#37035 (comment) r? @nikomatsakis
Struct expressions and patterns generally support type aliases
Alias { field: 10 }
i.e. they already have to work withty::Ty
to do their job.Self
is a type alias (when it's not a type parameter) => struct expressions and patterns should supportSelf
.Typical example:
The first commit does some preparations and cleanups, see the commit message for details.
This also fixes couple of bugs related to aliases in struct paths (fixes #36286).
EDIT:
Since struct expressions and patterns always work with
ty::Ty
now, associated paths in them are also supported. If associated typeA::B
successfully resolves to a struct (or union) type, thenA::B { /* fields */ }
is a valid expression/pattern. This will become more important when enum variants are treated as associated items.r? @eddyb