Implement RFC 2302: tuple_struct_self_ctor#53751
Implement RFC 2302: tuple_struct_self_ctor#53751bors merged 2 commits intorust-lang:masterfrom F001:tuple-struct-self-ctor
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Centril
left a comment
There was a problem hiding this comment.
Thank you for doing this! More tests would be nice :)
src/doc/unstable-book/src/language-features/tuple-struct-self-ctor.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Include tests for:
Selfas a function passed somewhereSelfas a unit struct value wrt. pattern matching and construction.
There was a problem hiding this comment.
Include a test where all of the above works through type aliases.
There was a problem hiding this comment.
Type alias is not working. At present, even this case can not work:
struct S(i32);
type T = S;
fn main() {
let x = T(1);
}I think type alias is unrelated to this feature. We should solve this in a separate PR.
There was a problem hiding this comment.
The test would rather be (and is part of the RFC):
struct S(i32);
type T = S;
impl T {
fn stuff() {
let Self(x) = match Self(0) { Self(x) => Self(x) };
let opt: Option<Self> = Some(0).map(Self);
}
}For named field structs, this already works today:
pub struct S { f: i32 }
pub type T = S;
impl T {
pub fn stuff() {
let Self { f: _x } = match (Self { f: 0 }) { Self { f } => Self { f } };
}
}There was a problem hiding this comment.
Thanks, test case is added.
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
Constructors are already to this table in build_reduced_graph.rs, so this seems to be duplicate work.
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
With https://github.com/rust-lang/rust/pull/53751/files?w=1#r214416891 in mind, this field looks like an exact copy of struct_constructors field above.
There was a problem hiding this comment.
Still needs tests that it works through type aliases such as with:
struct S(i32);
type T = S;
impl T {
fn stuff() {
let Self(x) = match Self(0) { Self(x) => Self(x) };
let opt: Option<Self> = Some(0).map(Self);
}
}There was a problem hiding this comment.
Thanks. Test case is added, see ST6.
src/libsyntax/feature_gate.rs
Outdated
There was a problem hiding this comment.
Unit struct constructors are not feature gated.
A better place to place the feature gate is probably lowering.rs.
Every time we construct hir::Path { with a single Self segment and Def::StructCtor definition we can report a feature error.
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
Feature gates very rarely enable features, they usually report an error when some feature is already used, so this condition is not necessary.
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
This won't work for type aliases in impl.
struct S(u8);
type Alias = S;
impl Alias { ... Self(10) ... }There was a problem hiding this comment.
Thanks. Test case is added.
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
Ideally, Self value "rib" should be added in all cases when Self type rib is added, and not only for methods in impls.
|
I'm pretty sure this implementation also loses generic arguments from impl. impl S<u8, u16> {
...
Self(x) // Will probably start inferring `T` and `U` for `S<T, U>` instead of using `u8` and `u16`
...
} |
|
Implementation strategy that supports type aliases and works correctly with generic parameters should likely mirror what is currently done with I'd recommend to search for |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@petrochenkov A new variant |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
src/librustc_typeck/check/mod.rs
Outdated
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
src/librustc/hir/lowering.rs
Outdated
There was a problem hiding this comment.
Nit: this is not about tuple structs, unit structs have Self constructors too, so the wording could be tweaked to "Self struct constructors are unstable" without mentioning tuples.
There was a problem hiding this comment.
The feature name tuple_struct_self_ctor contains "tuple" too.
There was a problem hiding this comment.
The original feature name came from the RFC. Now it is renamed to self_struct_ctor. And related test files are renamed.
src/librustc_resolve/lib.rs
Outdated
There was a problem hiding this comment.
This check is unnecessary, you can always add the self constructor rib.
Whether the type is a struct or not needs to be checked in typeck anyway, and you'll get better diagnostics if Self referring to e.g. enums is reported there.
|
🌲 The tree is currently closed for pull requests below priority 1, this pull request will be tested once the tree is reopened |
…rkor Implement RFC 2302: tuple_struct_self_ctor Tracking issue: #51994
|
💥 Test timed out |
|
@bors retry |
…rkor Implement RFC 2302: tuple_struct_self_ctor Tracking issue: #51994
|
☀️ Test successful - status-appveyor, status-travis |
|
📣 Toolstate changed by #53751! Tested on commit 6ff0b2e. 💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). |
Tested on commit rust-lang/rust@6ff0b2e. Direct link to PR: <rust-lang/rust#53751> 💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). 💔 rls on windows: test-pass → build-fail (cc @nrc, @rust-lang/infra). 💔 rls on linux: test-pass → build-fail (cc @nrc, @rust-lang/infra).
And clippy caused RLS to fail as usual. |
Avoid panic when matching function call Fix rust-lang#55718 This bug is introduced by rust-lang#53751. The original code checked `Def::AssociatedConst(..) | Def::Method(..)` before `pat_ty.no_bound_vars().expect("expected fn type")`. But somehow I exchanged the sequence carelessly. Sorry about that. r? @petrochenkov
Tracking issue: #51994