Skip to content
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

Tracking Issue for RFC 3107: derive_default_enum #87517

Closed
2 of 3 tasks
pnkfelix opened this issue Jul 27, 2021 · 31 comments
Closed
2 of 3 tasks

Tracking Issue for RFC 3107: derive_default_enum #87517

pnkfelix opened this issue Jul 27, 2021 · 31 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Jul 27, 2021

This is a tracking issue for the RFC 3107: "#[derive(Default)] on enums with a #[default] attribute" (rust-lang/rfcs#3107).
The feature gate for the issue is #![feature(derive_default_enum)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

None so far

Implementation history

@pnkfelix pnkfelix added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Jul 27, 2021
@ibraheemdev
Copy link
Member

Implementation PR was already created: #86735

@jhpratt
Copy link
Member

jhpratt commented Jul 28, 2021

Implementation has landed.

@jhpratt
Copy link
Member

jhpratt commented Aug 9, 2021

Actually, this is a duplicate of #86985. As the tracking issue in code links to this, I'll close that even though it was opened first.

Unresolved question for the record:

  • Should multiple #[default] attributes on the same variant be denied?

illicitonion added a commit to illicitonion/num_enum that referenced this issue Aug 14, 2021
rust-lang/rust#87517 started standardising
`#[default]` as an attribute on enum variants, support it compatibly
identically to `#[num_enum(default)]`.
illicitonion added a commit to illicitonion/num_enum that referenced this issue Aug 16, 2021
rust-lang/rust#87517 started standardising
`#[default]` as an attribute on enum variants, support it compatibly
identically to `#[num_enum(default)]`.
illicitonion added a commit to illicitonion/num_enum that referenced this issue Aug 16, 2021
rust-lang/rust#87517 started standardising
`#[default]` as an attribute on enum variants, support it compatibly
identically to `#[num_enum(default)]`.
@janriemer
Copy link
Contributor

Unresolved question for the record:

* Should multiple `#[default]` attributes on the same variant be denied?

@jhpratt @pnkfelix I've seen this in the Unresolved Questions section of the duplicate #86985, but it doesn't show up in this issue (only after three comments when @jhpratt points it out). I think, we should put it here in the Unresolved Questions section as well, so people see it at first glance and don't have to scroll three comments to see it (and therefore don't miss it that easily).
Thank you for considering this.

@jhpratt
Copy link
Member

jhpratt commented Nov 23, 2021

With #88681 being merged, I'm inclined to resolve the question of multiple #[default] attributes in favor of prohibiting them. This is the current behavior. There's not really a reason to have it warn instead of outright error in my opinion, and it could conflict if #[default = val] is permitted in the future.

@fbstj
Copy link
Contributor

fbstj commented Nov 23, 2021

https://github.com/rust-lang/rust/blob/master/src/test/ui/feature-gates/feature-gate-derive_default_enum.stderr still points at #86735. this file was the most "useful" result when I searched the feature gate on the repository/

@jhpratt

This comment was marked as resolved.

@rustbot rustbot added the S-tracking-needs-to-bake Status: The implementation is "complete" but it needs time to bake. label Dec 28, 2021
@jhpratt
Copy link
Member

jhpratt commented Jan 2, 2022

To match the actual implementation, the RFC text was updated to use the derive_default_enum feature name. If someone could update the title and text of this issue that would be great.

@lcnr lcnr changed the title Tracking Issue for RFC 3107: derive_enum_default Tracking Issue for RFC 3107: derive_default_enum Jan 3, 2022
@jhpratt
Copy link
Member

jhpratt commented Feb 22, 2022

How do people feel about stabilizing this? There was a minor concern from @Mark-Simulacrum in #92366, but that's a diagnostic that can be changed at any time. The implementation is straightforward, tested, used in rustc, and has remained unchanged for ~7 months.

@jhpratt
Copy link
Member

jhpratt commented Feb 28, 2022

Stabilization PR: #94457

@jhpratt
Copy link
Member

jhpratt commented Mar 7, 2022

Stabilization report

Summary

An attribute #[default], usable on enum unit variants, is introduced thereby allowing some enums to work with #[derive(Default)].

#[derive(Default)]
enum Padding {
    Space,
    Zero,
    #[default]
    None,
}

assert_eq!(Padding::default(), Padding::None);

The #[default] and #[non_exhaustive] attributes may not be used on the same variant.

Test cases

  • The feature gate itself is tested in feature-gate-deriving_default_enum.rs.
  • The basic, successful case is tested in deriving-default-enum.rs.
  • A number of edge cases are tested in macros-nonfatal-errors.rs:
    • #[default] on structs
    • #[default] on the enum as a whole
    • #[default] on the type (not the variant)
    • no declared default
    • multiple declared defaults
    • #[default] with a value
    • multiple #[default] attributes
    • #[default] on non-unit variant
    • #[default] on #[non_exhaustive] variant

Documentation

PR to update the reference will be made shortly.

Unresolved questions

There was one: whether multiple #[default] attributes should be accepted on a single variant (with no semantic effect). This was resolved in #92366 because #88681 implemented a similar check compiler-wide. Multiple #[default] attributes are not permitted.

@joshtriplett joshtriplett added T-lang Relevant to the language team, which will review and decide on the PR/issue. S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR and removed S-tracking-needs-to-bake Status: The implementation is "complete" but it needs time to bake. labels Mar 13, 2022
@joshtriplett
Copy link
Member

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Mar 13, 2022

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 13, 2022
@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 14, 2022
@nikomatsakis
Copy link
Contributor

No matter whose jurisdiction it is, this seems really useful, +1 from me. I would like a few more knobs on our derives.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 27, 2022
@rfcbot
Copy link

rfcbot commented Mar 27, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@clarfonthey
Copy link
Contributor

So, here's an interesting thing that I didn't see brought up, that I don't think should block stabilisation of the base feature, but should probably be mentioned.

What about unions?

#[derive(Default)]
pub union Thing {
    #[default]
    one: One,
    two: Two,
    three: Three,
}

We could easily make this create the below implementation:

impl Default for Thing {
    fn default() -> Thing {
        Thing { one: One::default() }
    }
}

@jhpratt
Copy link
Member

jhpratt commented Mar 29, 2022

That seems like a reasonable future possibility.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 6, 2022
@rfcbot
Copy link

rfcbot commented Apr 6, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 14, 2022
…um, r=davidtwco

Stabilize `derive_default_enum`

This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](rust-lang/rfcs#3107) and tracked in rust-lang#87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).

`@rustbot` label +S-waiting-on-review +T-lang
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 14, 2022
…um, r=davidtwco

Stabilize `derive_default_enum`

This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](rust-lang/rfcs#3107) and tracked in rust-lang#87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).

``@rustbot`` label +S-waiting-on-review +T-lang
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 14, 2022
…um, r=davidtwco

Stabilize `derive_default_enum`

This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](rust-lang/rfcs#3107) and tracked in rust-lang#87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).

```@rustbot``` label +S-waiting-on-review +T-lang
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 14, 2022
…um, r=davidtwco

Stabilize `derive_default_enum`

This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](rust-lang/rfcs#3107) and tracked in rust-lang#87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).

````@rustbot```` label +S-waiting-on-review +T-lang
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 15, 2022
…um, r=davidtwco

Stabilize `derive_default_enum`

This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](rust-lang/rfcs#3107) and tracked in rust-lang#87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).

`````@rustbot````` label +S-waiting-on-review +T-lang
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 15, 2022
…um, r=davidtwco

Stabilize `derive_default_enum`

This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](rust-lang/rfcs#3107) and tracked in rust-lang#87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).

``````@rustbot`````` label +S-waiting-on-review +T-lang
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 15, 2022
…um, r=davidtwco

Stabilize `derive_default_enum`

This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](rust-lang/rfcs#3107) and tracked in rust-lang#87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).

```````@rustbot``````` label +S-waiting-on-review +T-lang
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Apr 21, 2022
@JohnTitor
Copy link
Member

Triage: the stabilization PR (#94457) has landed 🎉 Closing as done.

@ilyvion
Copy link

ilyvion commented Jul 1, 2022

I don't know if this is the right place to ask, but why is it that

the #[default] attribute may only be used on unit enum variants

? I feel like a more natural behavior would be to behave like #[derive(Default)] behaves in every other context; which is that

#[derive(Default)]
struct Foo<T>(T);

will only be Default when T: Default and its value will be Foo(T::default()). That is to say,

#[derive(Default)]
pub enum Error<T> {
    #[default]
    Message(T),
    Other,
}

would only be Default when T: Default and its value would be Error::Message(T::default()).

@m-ou-se
Copy link
Member

m-ou-se commented Jul 1, 2022

@alexschrod How about these cases?

#[derive(Default)]
pub enum Either<A, B> {
    #[default]
    One(A),
    Two(B),
}

Does that require only A: Default? Or also B: Default?

How about this?

#[derive(Default)]
pub enum MaybeOption<T> {
    #[default]
    Yes(Option<T>),
    No,
}

Does that require T: Default or Option<T>: Default?

@ilyvion
Copy link

ilyvion commented Jul 1, 2022

It makes sense that for a struct all the generic types must be Default (at least given its current, somewhat unfortunate behavior, as I'll discuss farther down), but for an enum, it only makes sense to require the one(s) involved in the #[default] value to be Default. Which makes the answer obviously A only, in the first case. For the second case, the "ideal" answer is Option<T>, but since #[derive(Default)] seems to not have a lot of existing finesse with structs (struct Foo<T>(Option<T>) will end up with a "bad" T: Default bound, e.g.) one could carry that unfortunate legacy on with enums too, for the sake of feature parity.

@scottmcm
Copy link
Member

scottmcm commented Jul 1, 2022

but why is it that

Because derive(Debug) works totally differently from ever other derive when generic parameters are involved. (This was discussed in the FCP a bunch.)

Since it's not using the "we put the bound on all the generic parameters" approach, this is a great future-proofing until it's decided whether it should ever do anything else.

Maybe we'll one day get "perfect derive" (https://smallcultfollowing.com/babysteps//blog/2022/04/12/implied-bounds-and-perfect-derive/) that will give a different way to do this for everything, and that's when the rules will change.

@ilyvion
Copy link

ilyvion commented Jul 2, 2022

Thanks for the explanation, @scottmcm, it makes sense to me now that this first version is intentionally conservative with regard to a less problematic possible future improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests