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

Closed
opened 2024-01-25 02:12:41 -06:00 by glandium · 7 comments
glandium commented 2024-01-25 02:12:41 -06:00 (Migrated from github.com)

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.

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.
KodrAus commented 2024-02-01 16:05:56 -06:00 (Migrated from github.com)

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 {
    ...
  }
}
Hi @glandium :wave: 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: ```rust #[derive(bitflags::Debug, Foo, Bar)] pub struct Qux(u32); bitflags! { impl Qux: u32 { ... } } ```
dtolnay commented 2024-02-01 16:21:34 -06:00 (Migrated from github.com)

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 {...}
// }
It wouldn't need to be a proc macro, it should be possible to make this work: ```rust #[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 commented 2024-02-01 16:46:59 -06:00 (Migrated from github.com)

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.

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 commented 2024-02-04 15:58:24 -06:00 (Migrated from github.com)

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.

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 commented 2024-02-04 16:39:31 -06:00 (Migrated from github.com)

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.

> 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 commented 2024-02-04 18:23:20 -06:00 (Migrated from github.com)

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.

`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`.
KodrAus commented 2025-05-14 16:20:50 -05:00 (Migrated from github.com)

In the time since this issue was opened we've changed a few things to make working with custom derives a bit nicer. You can now declare your flags type as a regular struct, and then implement flags functionality on it separately:

// Define a flags type outside of the `bitflags` macro as a newtype
// It can accept custom derives for libraries `bitflags` doesn't support natively
#[derive(zerocopy::IntoBytes, zerocopy::FromBytes, zerocopy::KnownLayout, zerocopy::Immutable)]
#[repr(transparent)]
pub struct ManualFlags(u32);

// Next: use `impl Flags` instead of `struct Flags`
bitflags! {
    impl ManualFlags: u32 {
        const A = 0b00000001;
        const B = 0b00000010;
        const C = 0b00000100;
        const ABC = Self::A.bits() | Self::B.bits() | Self::C.bits();
    }
}

I've also created a proc macro library called bitflags_derive that is able to generate better flags-aware implementations of traits without needing direct support in bitflags for them:

#[macro_use]
extern crate bitflags;

#[macro_use]
extern crate bitflags_derive;

bitflags! {
    #[derive(FlagsDisplay, FlagsFromStr)]
    struct MyFlags: u8 {
        const A = 1;
        const B = 1 << 1;
        const C = 1 << 2;
    }
}

let flags = "A | B".parse::<MyFlags>()?;

assert_eq!("A | B", flags.to_string());

This is the place to add new flags-aware custom derive support going forwards.

In the time since this issue was opened we've changed a few things to make working with custom derives a bit nicer. You can now declare your flags type as a regular struct, and then implement flags functionality on it separately: ```rust // Define a flags type outside of the `bitflags` macro as a newtype // It can accept custom derives for libraries `bitflags` doesn't support natively #[derive(zerocopy::IntoBytes, zerocopy::FromBytes, zerocopy::KnownLayout, zerocopy::Immutable)] #[repr(transparent)] pub struct ManualFlags(u32); // Next: use `impl Flags` instead of `struct Flags` bitflags! { impl ManualFlags: u32 { const A = 0b00000001; const B = 0b00000010; const C = 0b00000100; const ABC = Self::A.bits() | Self::B.bits() | Self::C.bits(); } } ``` I've also created a proc macro library called [`bitflags_derive`](https://github.com/bitflags/bitflags-derive) that is able to generate better flags-aware implementations of traits without needing direct support in `bitflags` for them: ```rust #[macro_use] extern crate bitflags; #[macro_use] extern crate bitflags_derive; bitflags! { #[derive(FlagsDisplay, FlagsFromStr)] struct MyFlags: u8 { const A = 1; const B = 1 << 1; const C = 1 << 2; } } let flags = "A | B".parse::<MyFlags>()?; assert_eq!("A | B", flags.to_string()); ``` This is the place to add new flags-aware custom derive support going forwards.
Sign in to join this conversation.
No labels
bug
good first issue
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-bitflags/bitflags#395
No description provided.