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

Please allow to use the "internal" derives in the "custom-derive" case #395

Open
glandium opened this issue Jan 25, 2024 · 6 comments
Open

Comments

@glandium
Copy link

In bitflags 1, it was possible to do something like

bitflags!{
  #[derive(Debug, Foo, Bar)]
  pub struct Qux : u32 {
    ...
  }
}

With bitflags 2, the equivalent is:

#[derive(Debug, Foo, Bar)]
pub struct Qux(u32);
bitflags! {
  impl Qux: u32 {
    ...
  }
}

Unfortunately, that means Debug is derived normally, not using the nicer implementation that bitflags provides. One can implement it manually, but that quickly becomes cumbersome. It would be more useful if bitflags allowed to use some or all of its internal derives.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 31, 2024
…es. r=gfx-reviewers,emilio,ErichDonGubler

bitflags 2 has a shortcoming with using custom derives: you can't use
custom derives (for e.g. MallocSizeOf) at the same time as bitflags's for
the derives it supports.
See bitflags/bitflags#395

Differential Revision: https://phabricator.services.mozilla.com/D199941
moz-gfx pushed a commit to moz-gfx/webrender that referenced this issue Jan 31, 2024
…es. r=gfx-reviewers,emilio,ErichDonGubler

bitflags 2 has a shortcoming with using custom derives: you can't use
custom derives (for e.g. MallocSizeOf) at the same time as bitflags's for
the derives it supports.
See bitflags/bitflags#395

Differential Revision: https://phabricator.services.mozilla.com/D199941

[ghsync] From https://hg.mozilla.org/mozilla-central/rev/45bd6cc1bf7490f80630afce7550706e9b064f81
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Feb 1, 2024
…es. r=gfx-reviewers,emilio,ErichDonGubler

bitflags 2 has a shortcoming with using custom derives: you can't use
custom derives (for e.g. MallocSizeOf) at the same time as bitflags's for
the derives it supports.
See bitflags/bitflags#395

Differential Revision: https://phabricator.services.mozilla.com/D199941

UltraBlame original commit: 45bd6cc1bf7490f80630afce7550706e9b064f81
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Feb 1, 2024
…es. r=gfx-reviewers,emilio,ErichDonGubler

bitflags 2 has a shortcoming with using custom derives: you can't use
custom derives (for e.g. MallocSizeOf) at the same time as bitflags's for
the derives it supports.
See bitflags/bitflags#395

Differential Revision: https://phabricator.services.mozilla.com/D199941

UltraBlame original commit: 45bd6cc1bf7490f80630afce7550706e9b064f81
ErichDonGubler pushed a commit to ErichDonGubler/firefox that referenced this issue Feb 1, 2024
…es. r=gfx-reviewers,emilio,ErichDonGubler

bitflags 2 has a shortcoming with using custom derives: you can't use
custom derives (for e.g. MallocSizeOf) at the same time as bitflags's for
the derives it supports.
See bitflags/bitflags#395

Differential Revision: https://phabricator.services.mozilla.com/D199941
@KodrAus
Copy link
Member

KodrAus commented Feb 1, 2024

Hi @glandium 👋

It looks like you're using the impl-style bitflags! because you also need support for other custom derives? It seems like there's more we can do to make that pattern nicer. Perhaps a proc-macro library that leveraged the Flags trait to offer its own custom-derives for Debug etc:

#[derive(bitflags::Debug, Foo, Bar)]
pub struct Qux(u32);
bitflags! {
  impl Qux: u32 {
    ...
  }
}

@dtolnay
Copy link
Contributor

dtolnay commented Feb 1, 2024

It wouldn't need to be a proc macro, it should be possible to make this work:

#[derive(....)]
pub struct Qux(u32);

bitflags! {
    #[derive(Debug)}
    impl Qux: u32 {
        ...
    }
}

// produces the same Debug impl that this would produce:
// bitflags! {
//     #[derive(Debug)]
//     struct Qux: u32 {...}
// }

@glandium
Copy link
Author

glandium commented Feb 1, 2024

Both would work, although the proc-macro would feel better because it groups the derive in a single place, but I can understand that it's more stuff to compile.

@KodrAus
Copy link
Member

KodrAus commented Feb 4, 2024

My preference would be for a proc-macro, starting in an external library with the option to include in bitflags through a feature, but would never want to pull any proc-macros in to bitflags by-default. The reason I'd like to avoid macro_rules! for this is the macro logic in bitflags! is already quite complicated, and I'd like to avoid introducing any magic attributes wherever possible. The bitflags! { struct Qux: u32 { .. } } case currently just uses a newtype to make plain #[derive()]s work, since many custom-derives treat newtypes differently.

@glandium
Copy link
Author

glandium commented Feb 4, 2024

The bitflags! { struct Qux: u32 { .. } } case currently just uses a newtype to make plain #[derive()]s work

That's what bitflags 1 was doing. bitflags 2 is different in that matter, which is the reason for this issue in the first place. Specifically, it uses a newtype of an internal type that #[derive()]s can't be applied to.

@KodrAus
Copy link
Member

KodrAus commented Feb 5, 2024

bitflags 1.x used a struct like struct Flags { bits: u32 }, so you could #[derive] many things, but they would be based on that un-semantic u32 so probably not what you actually want to implement. bitflags 2.x uses a newtype like struct Flags(FlagsInner) where FlagsInner manually implements traits semantically so you can add #[derive]s on Flags and get something closer to what you want to implement, but has the (significant) drawback that only traits implemented by FlagsInner can be #[derive]'d on Flags.

gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Feb 8, 2024
…es. r=gfx-reviewers,emilio,ErichDonGubler

bitflags 2 has a shortcoming with using custom derives: you can't use
custom derives (for e.g. MallocSizeOf) at the same time as bitflags's for
the derives it supports.
See bitflags/bitflags#395

Differential Revision: https://phabricator.services.mozilla.com/D199941

UltraBlame original commit: 45bd6cc1bf7490f80630afce7550706e9b064f81
github-actions bot pushed a commit to Loirooriol/stylo that referenced this issue Mar 17, 2024
…es. r=gfx-reviewers,emilio,ErichDonGubler

bitflags 2 has a shortcoming with using custom derives: you can't use
custom derives (for e.g. MallocSizeOf) at the same time as bitflags's for
the derives it supports.
See bitflags/bitflags#395

Differential Revision: https://phabricator.services.mozilla.com/D199941
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants