Skip to content

RFC: enable derive(From) for single-field structs #3809

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Kobzol
Copy link

@Kobzol Kobzol commented May 6, 2025

Previously discussed as Pre-RFC on IRLO.

Rendered

@jieyouxu jieyouxu added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 6, 2025
@clarfonthey
Copy link

Fully in support of this. A few things that probably should be elaborated:

  • Generic single fields
  • Transparent structs with extra zero-size fields like PhantomData

The transparent case, IMHO, is probably best left as a future extension, but it's an interesting case where Default impls on the extra fields would not be needed.

@Kobzol
Copy link
Author

Kobzol commented May 6, 2025

Fully in support of this. A few things that probably should be elaborated:

  • Generic single fields
  • Transparent structs with extra zero-size fields like PhantomData

The transparent case, IMHO, is probably best left as a future extension, but it's an interesting case where Default impls on the extra fields would not be needed.

PhantomData implements Default unconditionally for any T, so that would be fine. I agree it's best to leave it for "later" though.

Good point with the generics, I'll add a mention to the RFC.

@kennytm
Copy link
Member

kennytm commented May 7, 2025

"For later" - perhaps it could recognize #3681 fields and automatically skip them as well.

#[derive(From)]
struct Foo {
    a: usize, // no #[from] needed, because all other fields are explicitly default'ed
    b: ZstTag = ZstTag,
    c: &'static str = "localhost",
}

// generates

impl From<usize> for Foo {
    fn from(a: usize) -> Self {
        Self { a, .. }
    }
}

OTOH we may still want the #[from] in the above case for uniformity if we want to support #[derive(From)] on all fields being defaulted

#[derive(From, Default)]
struct Foo2 {
    #[from] // <-- perhaps still want this
    a: u128 = 1,
    b: u16 = 8080,
}

Co-authored-by: Jake Goulding <[email protected]>
@nielsle
Copy link

nielsle commented May 9, 2025

The crate named derive-new creates a new constructor, and it has several fancy options.

But perhaps this functionality belongs in a third party crate rather than the standard library.


We could make `#[derive(From)]` generate both directions, but that would make it impossible to only ask for the "basic" `From` direction without some additional syntax.

A better alternative might be to support generating the other direction in the future through something like `#[derive(Into)]`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth noting here that impl From<Newtype> for Inner and impl Into<Inner> for Newtype have slightly different semantics. Using derive(Into) to mean impl From could be confusing for generic types where there is a coherence issue, even if it does imply an Into impl.

(I've had similar issues with the derive_more version.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the derive-macro's name does not necessarily correspond to the impl'ed trait name anymore since #3621 (currently named #[derive(CoercePointee)], which actually does impl CoerceUnsized + DispatchFromDyn.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The #[derive(Into)] example is kind of hand-waving, I'm not sure if it's actually a good idea. I would like to avoid doing that in this RFC though, as that sounds like a separate can of worms, I explicitly tried to keep this RFC as simple as possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the derive-macro's name does not necessarily correspond to the impl'ed trait name anymore since #3621 (currently named #[derive(CoercePointee)], which actually does impl CoerceUnsized + DispatchFromDyn.)

Understood! But my comment was more about the confusing semantics, than the exact name of the impl'd trait.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, for the impl From<NewType> for Inner case, I think I would prefer something like impl_from!(Newtype -> Inner) or a more generic impl_from!(Newtype -> Inner = |source| source.0) or impl_from!(Newtype -> Inner = self.0) (restricted to using fields of NewType)

Although it is a little annoying, to have to repeat the type of Inner.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But my comment was more about the confusing semantics

I don't think there is any name more appropriate than #[derive(Into)] especially if this RFC is using #[derive(From)]. The final effect you get is still having an impl Into<Inner> for Self effectively, through the intermediate impl From<Self> for Inner + the blanket impl.

See JelteF/derive_more#13 for a brief discussion how derive_more still chooses to name it #[derive(Into)].

@Kobzol
Copy link
Author

Kobzol commented May 10, 2025

The crate named derive-new creates a new constructor, and it has several fancy options.

But perhaps this functionality belongs in a third party crate rather than the standard library.

While new constructors are pretty common, I would say that it's too opinionated for it to be included in the stdlib, as it's not a standard (library) trait like From is. In any case, that would be a separate RFC :)

Co-authored-by: teor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants