-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Implement RFC 2338, "Type alias enum variants" #56225
Conversation
The tricky part of handling type parameters is still to be done, but basic cases are working already. Any advice on this expanding on your previous comment would be appreciated, @petrochenkov. (Hope you don't mind I r?ed you, as well.) |
This comment has been minimized.
This comment has been minimized.
Everything about generic arguments should be in |
c9d9926
to
45f338d
Compare
This comment has been minimized.
This comment has been minimized.
45f338d
to
dc46591
Compare
This comment has been minimized.
This comment has been minimized.
dc46591
to
f19488f
Compare
This comment has been minimized.
This comment has been minimized.
f19488f
to
f75e0a4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
7bb6ccd
to
36b6e33
Compare
This comment has been minimized.
This comment has been minimized.
Big push coming up... I should be able to finish this tomorrow. |
36b6e33
to
b3f6ace
Compare
Implement RFC 2338, "Type alias enum variants" This PR implements [RFC 2338](rust-lang/rfcs#2338), allowing one to write code like the following. ```rust #![feature(type_alias_enum_variants)] enum Foo { Bar(i32), Baz { i: i32 }, } type Alias = Foo; fn main() { let t = Alias::Bar(0); let t = Alias::Baz { i: 0 }; match t { Alias::Bar(_i) => {} Alias::Baz { i: _i } => {} } } ``` Since `Self` can be considered a type alias in this context, it also enables using `Self::Variant` as both a constructor and pattern. Fixes issues #56199 and #56611. N.B., after discussing the syntax for type arguments on enum variants with @petrochenkov and @eddyb (there are also a few comments on the [tracking issue](#49683)), the consensus seems to be treat the syntax as follows, which ought to be backwards-compatible. ```rust Option::<u8>::None; // OK Option::None::<u8>; // OK, but lint in near future (hard error next edition?) Alias::<u8>::None; // OK Alias::None::<u8>; // Error ``` I do not know if this will need an FCP, but let's start one if so.
☀️ Test successful - status-appveyor, status-travis |
📣 Toolstate changed by #56225! Tested on commit 5918318. 💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). |
Tested on commit rust-lang/rust@5918318. Direct link to PR: <rust-lang/rust#56225> 💔 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 @Xanewok, @rust-lang/infra). 💔 rls on linux: test-pass → build-fail (cc @nrc @Xanewok, @rust-lang/infra).
item.name -> item.ident.name
rustup rust-lang/rust#56225 item.name -> item.ident.name
Changes: ```` UI test cleanup: Extract iter_skip_next from methods.rs Update test output after rebase Remove false negatives from known problems Implement use_self for tuple structs Document known problems rustup rust-lang#56225 Remove unnecessary `use` statements after `cargo fix` Apply cargo fix --edition-idioms fixes Use match ergonomics for booleans lint Use match ergonomics for block_in_if_condition lint Use match ergonomics for bit_mask lint Use match ergonomics for attrs lint Use match ergonomics for assign_ops lint Use match ergonomics for artithmetic lint Use match ergonomics for approx_const lint Remove crate:: prefixes from crate paths Support array indexing expressions in unused write to a constant Mark writes to constants as side-effect-less Update README local run command to remove syspath Remove unsafe from consts clippy lints Fix formatting Merge new_without_default_derive into new_without_default Only print out question_mark lint when it actually triggered Add failing test Reinserted commata Recomend `.as_ref()?` in certain situations Deduplicate some code? ````
submodules: update clippy from f7bdf50 to 39bd844 Fixes clippy toolstate Changes: ```` UI test cleanup: Extract iter_skip_next from methods.rs Update test output after rebase Remove false negatives from known problems Implement use_self for tuple structs Document known problems rustup #56225 Remove unnecessary `use` statements after `cargo fix` Apply cargo fix --edition-idioms fixes Use match ergonomics for booleans lint Use match ergonomics for block_in_if_condition lint Use match ergonomics for bit_mask lint Use match ergonomics for attrs lint Use match ergonomics for assign_ops lint Use match ergonomics for artithmetic lint Use match ergonomics for approx_const lint Remove crate:: prefixes from crate paths Support array indexing expressions in unused write to a constant Mark writes to constants as side-effect-less Update README local run command to remove syspath Remove unsafe from consts clippy lints Fix formatting Merge new_without_default_derive into new_without_default Only print out question_mark lint when it actually triggered Add failing test Reinserted commata Recomend `.as_ref()?` in certain situations Deduplicate some code? ```` r? @oli-obk or anyone else
Changes: ```` UI test cleanup: Extract iter_skip_next from methods.rs Update test output after rebase Remove false negatives from known problems Implement use_self for tuple structs Document known problems rustup rust-lang#56225 Remove unnecessary `use` statements after `cargo fix` Apply cargo fix --edition-idioms fixes Use match ergonomics for booleans lint Use match ergonomics for block_in_if_condition lint Use match ergonomics for bit_mask lint Use match ergonomics for attrs lint Use match ergonomics for assign_ops lint Use match ergonomics for artithmetic lint Use match ergonomics for approx_const lint Remove crate:: prefixes from crate paths Support array indexing expressions in unused write to a constant Mark writes to constants as side-effect-less Update README local run command to remove syspath Remove unsafe from consts clippy lints Fix formatting Merge new_without_default_derive into new_without_default Only print out question_mark lint when it actually triggered Add failing test Reinserted commata Recomend `.as_ref()?` in certain situations Deduplicate some code? ````
[WIP] High priority resolutions for associated variants cc #56225 (comment)
High priority resolutions for associated variants In rust-lang#56225 variants were assigned lowest priority during name resolution to avoid crater run and potential breakage. This PR changes the rules to give variants highest priority instead. Some motivation: - If variants (and their constructors) are treated as associated items, then they are obviously *inherent* associated items since they don't come from traits. - Inherent associated items have higher priority during resolution than associated items from traits. - The reason is that there is a way to disambiguate in favor of trait items (`<Type as Trait>::Ambiguous`), but there's no way to disambiguate in favor of inherent items, so they became unusable in case of ambiguities if they have low priority. - It's technically problematic to fallback from associated types to anything until lazy normalization (?) is implemented. Crater found some regressions from this change, but they are all in type positions, e.g. ```rust fn f() -> Self::Ambiguos { ... } // Variant `Ambiguous` or associated type `Ambiguous`? ``` , so variants are not usable there right now, but they may become usable in the future if rust-lang/rfcs#2593 is accepted. This PR keeps code like this successfully resolving, but introduces a future-compatibility lint `ambiguous_associated_items` that recommends rewriting it as `<Self as Trait>::Ambiguous`.
High priority resolutions for associated variants In rust-lang#56225 variants were assigned lowest priority during name resolution to avoid crater run and potential breakage. This PR changes the rules to give variants highest priority instead. Some motivation: - If variants (and their constructors) are treated as associated items, then they are obviously *inherent* associated items since they don't come from traits. - Inherent associated items have higher priority during resolution than associated items from traits. - The reason is that there is a way to disambiguate in favor of trait items (`<Type as Trait>::Ambiguous`), but there's no way to disambiguate in favor of inherent items, so they became unusable in case of ambiguities if they have low priority. - It's technically problematic to fallback from associated types to anything until lazy normalization (?) is implemented. Crater found some regressions from this change, but they are all in type positions, e.g. ```rust fn f() -> Self::Ambiguos { ... } // Variant `Ambiguous` or associated type `Ambiguous`? ``` , so variants are not usable there right now, but they may become usable in the future if rust-lang/rfcs#2593 is accepted. This PR keeps code like this successfully resolving, but introduces a future-compatibility lint `ambiguous_associated_items` that recommends rewriting it as `<Self as Trait>::Ambiguous`.
High priority resolutions for associated variants In rust-lang#56225 variants were assigned lowest priority during name resolution to avoid crater run and potential breakage. This PR changes the rules to give variants highest priority instead. Some motivation: - If variants (and their constructors) are treated as associated items, then they are obviously *inherent* associated items since they don't come from traits. - Inherent associated items have higher priority during resolution than associated items from traits. - The reason is that there is a way to disambiguate in favor of trait items (`<Type as Trait>::Ambiguous`), but there's no way to disambiguate in favor of inherent items, so they became unusable in case of ambiguities if they have low priority. - It's technically problematic to fallback from associated types to anything until lazy normalization (?) is implemented. Crater found some regressions from this change, but they are all in type positions, e.g. ```rust fn f() -> Self::Ambiguos { ... } // Variant `Ambiguous` or associated type `Ambiguous`? ``` , so variants are not usable there right now, but they may become usable in the future if rust-lang/rfcs#2593 is accepted. This PR keeps code like this successfully resolving, but introduces a future-compatibility lint `ambiguous_associated_items` that recommends rewriting it as `<Self as Trait>::Ambiguous`.
High priority resolutions for associated variants In rust-lang#56225 variants were assigned lowest priority during name resolution to avoid crater run and potential breakage. This PR changes the rules to give variants highest priority instead. Some motivation: - If variants (and their constructors) are treated as associated items, then they are obviously *inherent* associated items since they don't come from traits. - Inherent associated items have higher priority during resolution than associated items from traits. - The reason is that there is a way to disambiguate in favor of trait items (`<Type as Trait>::Ambiguous`), but there's no way to disambiguate in favor of inherent items, so they became unusable in case of ambiguities if they have low priority. - It's technically problematic to fallback from associated types to anything until lazy normalization (?) is implemented. Crater found some regressions from this change, but they are all in type positions, e.g. ```rust fn f() -> Self::Ambiguos { ... } // Variant `Ambiguous` or associated type `Ambiguous`? ``` , so variants are not usable there right now, but they may become usable in the future if rust-lang/rfcs#2593 is accepted. This PR keeps code like this successfully resolving, but introduces a future-compatibility lint `ambiguous_associated_items` that recommends rewriting it as `<Self as Trait>::Ambiguous`.
High priority resolutions for associated variants In rust-lang#56225 variants were assigned lowest priority during name resolution to avoid crater run and potential breakage. This PR changes the rules to give variants highest priority instead. Some motivation: - If variants (and their constructors) are treated as associated items, then they are obviously *inherent* associated items since they don't come from traits. - Inherent associated items have higher priority during resolution than associated items from traits. - The reason is that there is a way to disambiguate in favor of trait items (`<Type as Trait>::Ambiguous`), but there's no way to disambiguate in favor of inherent items, so they became unusable in case of ambiguities if they have low priority. - It's technically problematic to fallback from associated types to anything until lazy normalization (?) is implemented. Crater found some regressions from this change, but they are all in type positions, e.g. ```rust fn f() -> Self::Ambiguos { ... } // Variant `Ambiguous` or associated type `Ambiguous`? ``` , so variants are not usable there right now, but they may become usable in the future if rust-lang/rfcs#2593 is accepted. This PR keeps code like this successfully resolving, but introduces a future-compatibility lint `ambiguous_associated_items` that recommends rewriting it as `<Self as Trait>::Ambiguous`.
High priority resolutions for associated variants In rust-lang#56225 variants were assigned lowest priority during name resolution to avoid crater run and potential breakage. This PR changes the rules to give variants highest priority instead. Some motivation: - If variants (and their constructors) are treated as associated items, then they are obviously *inherent* associated items since they don't come from traits. - Inherent associated items have higher priority during resolution than associated items from traits. - The reason is that there is a way to disambiguate in favor of trait items (`<Type as Trait>::Ambiguous`), but there's no way to disambiguate in favor of inherent items, so they became unusable in case of ambiguities if they have low priority. - It's technically problematic to fallback from associated types to anything until lazy normalization (?) is implemented. Crater found some regressions from this change, but they are all in type positions, e.g. ```rust fn f() -> Self::Ambiguos { ... } // Variant `Ambiguous` or associated type `Ambiguous`? ``` , so variants are not usable there right now, but they may become usable in the future if rust-lang/rfcs#2593 is accepted. This PR keeps code like this successfully resolving, but introduces a future-compatibility lint `ambiguous_associated_items` that recommends rewriting it as `<Self as Trait>::Ambiguous`.
Changes: ```` UI test cleanup: Extract iter_skip_next from methods.rs Update test output after rebase Remove false negatives from known problems Implement use_self for tuple structs Document known problems rustup rust-lang/rust#56225 Remove unnecessary `use` statements after `cargo fix` Apply cargo fix --edition-idioms fixes Use match ergonomics for booleans lint Use match ergonomics for block_in_if_condition lint Use match ergonomics for bit_mask lint Use match ergonomics for attrs lint Use match ergonomics for assign_ops lint Use match ergonomics for artithmetic lint Use match ergonomics for approx_const lint Remove crate:: prefixes from crate paths Support array indexing expressions in unused write to a constant Mark writes to constants as side-effect-less Update README local run command to remove syspath Remove unsafe from consts clippy lints Fix formatting Merge new_without_default_derive into new_without_default Only print out question_mark lint when it actually triggered Add failing test Reinserted commata Recomend `.as_ref()?` in certain situations Deduplicate some code? ````
This PR implements RFC 2338, allowing one to write code like the following.
Since
Self
can be considered a type alias in this context, it also enables usingSelf::Variant
as both a constructor and pattern.Fixes issues #56199 and #56611.
N.B., after discussing the syntax for type arguments on enum variants with @petrochenkov and @eddyb (there are also a few comments on the tracking issue), the consensus seems to be treat the syntax as follows, which ought to be backwards-compatible.
I do not know if this will need an FCP, but let's start one if so.