Considerations for 2.0

[KodrAus]

Before we actually release a build with #220 I think it’s a good time to consider any breaking changes that could work their way into a 2.0 release. Without much public surface area I think the risk of multiple bitflags versions in a dependency graph is low.

Some I had in mind:

• Reconsider making from_bits_unchecked unsafe. The idea was to make it unsafe to construct flags using bits that don’t correspond to flags specified. That way an implementor could consider using things like unreachable_unchecked in code that matches on bits. In hindsight this just seems like something people work around rather than make use of. Since we generate types in the scope of the bitflags! invocation there’s already nothing stopping someone from manually stuffing extra bits in a flags type. So we can’t rely on from_bits_unchecked in bitflags code either.
• Consider not deriving any traits by default. Ship an optional proc macro #[bitflags_derive] that can be used to add them. That way callers can decide on their own equality and debug implementations if they want.
• Consider changing the serialization implementation to use a text format for human-readable formats and raw bits for others.
• Anything else?

[konsumlamm]

There are some issues that I'd like to see getting fixed, but I'm not sure if that would be a breaking change:

• Constants cannot be cfg-gated multiple times #137
• Auto generate values when no values are specified #236

[joshtriplett]

I'd love to have a well-documented way to construct bitflags in a constant. Right now, I use a macro wrapped around TheBitflagsType::from_bits_truncate(TheBitflagsType::Flag1.bits() | TheBitflagsType::Flag2.bits() | ... ).

[roblabla]

Reconsider making from_bits_unchecked unsafe. The idea was to make it unsafe to construct flags using bits that don’t correspond to flags specified. That way an implementor could consider using things like unreachable_unchecked in code that matches on bits. In hindsight this just seems like something people work around rather than make use of. Since we generate types in the scope of the bitflags! invocation there’s already nothing stopping someone from manually stuffing extra bits in a flags type. So we can’t rely on from_bits_unchecked in bitflags code either.

In fact, this issue has been a hindrance for me for a long time. I often want to use bitflags when doing FFI wrappers, where I don't necessarily know all the possible flags, but want to keep unknown bits around so I can forward them, serialize them to disk, etc... Or file format parsers where I similarly can't know all the possible flags, but want to make sure I keep the originals around. Having the function marked unsafe is super annoying, especially since there's no mentions of the invariants that need to be upheld.

Renaming from_bits_unchecked to from_bits_unknown, making it safe, and documenting the impact it has would make this library much more useful to me!

[KodrAus]

Renaming from_bits_unchecked to from_bits_unknown, making it safe, and documenting the impact it has would make this library much more useful to me!

This at least is something we could do now without any breakage. We could just deprecate from_bits_unchecked. If we did decide to pursue a 2.0 as the first release with a BitFlags trait then we could create a 1.x branch for these non-breaking things.

[KodrAus]

I'd love to have a well-documented way to construct bitflags in a constant. Right now, I use a macro

That’s probably the most ergonomic looking way until impl const lands. There are some nice const methods if you don’t mind chaining:

TheBitflagsType::Flag1.union(TheBitflagsType::Flag2).union(…)

A general lack of nice examples is another issue 🙂

[Dushistov]

It would be nice to have some meta information for FFI. Not only the way to iterate, but also names.
So you can import them info build.rs and generate C language header file with proper names and values.

[bunnie]

Hi...a colleague pointed me here because I had tried to use bitflags before and I failed to use it well. I can very well believe it's entirely because I am failing to grasp how to use bitflags, but one core use case I could not figure out how to map well into bitflags is the case of individual binary features heterogenously mixed with fields meant to represent an integer number.

For example, I have (had?) a CODEC where there could be a configuration word something a bit like this:

pub const LM49352_OUTPUT_OPTIONS: u8 = 0x14;
bitflags! {
    pub struct OutputOptions: u8 {
        const LR_HP_LEVEL_0DB    = 0b0000_0000;
        const LR_HP_LEVEL_N1P5DB = 0b0000_0010;
        const LR_HP_LEVEL_N3DB   = 0b0000_0100;
        const LR_HP_LEVEL_N6DB   = 0b0000_0110;
        const LR_HP_LEVEL_N9DB   = 0b0000_1000;
        const LR_HP_LEVEL_N12DB  = 0b0000_1010;
        const LR_HP_LEVEL_N15DB  = 0b0000_1100;
        const LR_HP_LEVEL_N18DB  = 0b0000_1110;

        const AUX_0DB            = 0b0000_0000;
        const AUX_N6DB           = 0b0001_0000;

        const AUX_EARPEACE       = 0b0000_0000;
        const AUX_LINEOUT        = 0b0010_0000;

        const LS_LEVEL_0DB       = 0b0000_0000;
        const LS_LEVEL_4DB       = 0b0100_0000;
        const LS_LEVEL_8DB       = 0b1100_0000;
        const LS_LEVEL_12DB      = 0b1100_0000;
    }
}

I could completely see that maybe I missed an obvious feature of bitflags that would have allowed me to define say, bits 1-3 as some integer value that could be masked and shifted into the right spot, but I couldn't seem to find any mention of that in the docs.

When we had to port to a new codec, I tried playing around with using bitflags as some way to just keep a mask and a bit offset, but it felt like a lot of overhead compared to just defining a couple consts and doing shifts and adds, because I was ultimately taking everything out of a bitflag and casting it to an int and then doing the operation, instead of taking advantage of the neat operator overloading that bitflags has built in. But maybe that's because I'm an old school C programmer who just finds that kind of mask-and-shift idiom very natural, and closures to be clumsy...

Anyways, a colleague had pointed me to this thread since it looked like feedback was solicited on bitflags, and I thought I'd share a use case that I found a bit hard to be covered by the API. Maybe my use case should just be out of scope, because (for example) extending bitflags to take a closure that could map integer fields onto something else (a bit like the PAC crate does) probably looks pretty syntactically cool and powerful but it's also very non-obvious to me that it would compile down to what you're looking for, e.g. just an integer that you're looking to jam into a register.

fwiw, as a matter of preference I also did not find the PAC crate to be very ergonomic because a lot of times hardware implementations did not really want to map nicely onto its abstractions and I was constantly finding myself going to godbolt to debug/confirm that my code was generating what I intended it to....99% of the time it's great but I think it's not a great sign for an API if you have to keep a godbolt instance around to make sure it's generating the intended code without accidental extra complexity or misunderstandings because of ambiguities in the syntax. In other words, I'm not advocating something as complicated as PAC, either, as a solution to this problem.

[roblabla]

@bunnie What you want looks more akin to bitfields than bitflags. The way I do this is by combining bitfield and bitflags crate:

#[repr(u8)]
enum LrHpLevel {
    0DB, N1P5DB, N3DB, // ...
}

#[repr(u8)]
enum LsLevel {
   0DB, 4DB // ...
}

bitflags! {
    pub struct AuxFlags: u8 {
        const AUX_0DB            = 0b00;
        const AUX_N6DB           = 0b01;
        const AUX_EARPEACE       = 0b00;
        const AUX_LINEOUT        = 0b10;
    }
}

bitfield!{
    pub struct OutputOptions(u8);
    impl Debug;
    u8, from into LrHpLevel, lr_hp_level, set_lr_hp_level: 3, 1;
    u8, from into AuxFlags, aux_flags, set_aux_flags: 5, 4;
    u8, from into LsLevel, ls_level, set_ls_level, 7, 6;
}

What might be a good idea is making sure those two crates interoperate better, and maybe provide some documentation, as it is a rather common use-case in the embedded world.

Also, eventually I hope bitfields will be handled in the language proper (see rust-lang/rfcs#314) in which case bitflags should hopefully work as-is.

[bunnie]

Ahah! The feature I was looking for was hiding in another crate, in plain sight, all along. thanks!

[KodrAus]

@roblabla that could make a great example in bitflags if you're happy for it to be included in a (currently non-existing) /examples directory.

[CasperN]

I'd like Bitflags to also work for unrecognized flags without that being unsafe (#228 )

[KodrAus]

I've created a https://github.com/bitflags/bitflags/tree/release/1.x branch that non-breaking changes can continue to work their way into so main will now be the head of 2.0.

So here's what I'm thinking as a plan for 2.0 right now:

Replacing from_bits_unchecked with a safe from_bits_preserve

The from_bits_unchecked method is a hassle for users. They don't want to write unsafe because it's not even really possible for them to uphold whatever invariants unchecked needs to preserve. We should deprecate it in 1.x since its unsafety is meaningless and replace it with a safe from_bits_preserve that has the exact same implementation.

Bits<T> and removing automatic trait impls

I think we should remove the following automatic impls:

• PartialOrd / Ord (Allow other semantics of PartialOrd #155)
• Debug (No way to override trait implementations generated by bitflags! macro #218) (Add a way to not implement the Debug trait #168)

and change the shape of structs generated by bitflags from:

struct MyFlags {
    bits: $bits_ty,
}

to:

struct MyFlags(Bits<$bits_ty>);

where Bits<T> looks like:

#[repr(transparent)]
pub struct Bits<T>(T);

and have roughly the same public API as a generated bitflags! type (including implementing the BitFlags trait).

This Bits type will always implement derivable traits (including optional external ones like Arbitrary (#248) and Serialize / Deserialize (#147)). That way you can still write:

bitflags! {
    #[derive(Serialize, Arbitrary, PartialOrd, Ord)]
    pub struct MyFlags: u8 { .. }
}

and get something workable. If you want to implement traits manually you can use the .bits() method on your generated flags to access those underlying bits directly (as $bits_ty, not Bits<$bits_ty>).

When new requests to add trait implementations come in, we add them to Bits<T>, so end-users can then choose to derive them.

Changing semantics

I haven't audited this yet, but any trait impls that operate on flags types should treat unrecognized flags as if they are recognized. So when performing bitwise operations or in FromIterator we ensure we operate on the underlying bits type.

Unanswered questions

• Should we also remove an automatic impl for PartialEq and Eq? They don't have an alternative implementation that I can see, but users might expect to combine PartialEq / Eq with PartialOrd / Ord.
• Go even further and remove any trait impls that are derivable. If you can derive it yourself then we don’t implement it.
• Will the scheme with Bits<T> fall down when there are traits that need some knowledge of the valid flags? We could generate a bespoke Bits type for each flags type to carry trait impls. We could do this using a private internal trait that lets us generate a type in a const _: () { .. } block and refer to it by associated type.

[KodrAus]

I've started sketching out an alternative implementation for a 2.x in #264. It allows us to add #[derive] support for external libraries like arbitrary without potentially breaking callers, but comes with some potential breakage:

bitflags! {
+   #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    struct Flags: u32 {
        const A = 0b00000001;
        const B = 0b00000010;
        const C = 0b00000100;
-       const ABC = Flags::A.bits | Flags::B.bits | Flags::C.bits;
+       const ABC = Flags::A.bits() | Flags::B.bits() | Flags::C.bits();
    }
}

If you want to derive Serialize, then you'd add a serde feature to bitflags, then add it. This is different from today, but would be consistent with any other traits we support, like Arbitrary.

This gives you a chance to change the way you implement traits like Debug and PartialOrd in the same way you would for any other type, but does require a stack of #[derive]s to be equivalent to 1.x types.

How do people feel about this? I don't want to create churn that doesn't seem worthwhile, but if it's palatable then it gives us a way to naturally tweak trait implementations. There's a middle-ground here where we would include #[derive]s for some of these types, like Clone, Copy, and Hash, and exclude others that you may want an alternative implementation for like PartialOrd:

bitflags! {
+   #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
    struct Flags: u32 {
        const A = 0b00000001;
        const B = 0b00000010;
        const C = 0b00000100;
-       const ABC = Flags::A.bits | Flags::B.bits | Flags::C.bits;
+       const ABC = Flags::A.bits() | Flags::B.bits() | Flags::C.bits();
    }
}

What do you all think?

[Rua]

For my own use case, making it safe to include unknown bits in the flags could actually introduce UB. It relies on not receiving any unrecognised bits, because it needs to check the validity of each bit before passing it on to a foreign API (Vulkan). Unrecognised bits could potentially violate the API if left, and if the user could introduce an unchecked bit safely, then that could lead to UB in safe code, which goes against what Rust tries to do.

Having no guarantee that the bitflags values received by functions are all valid in safe code, would mean that every Rust function has to run every bitflags type it receives through from_bits_truncate to ensure validity. But that would also remove the ability by the user to include unrecognised bits that are ok to use, if the user knows what they are doing, which is the whole point of unsafe.

So, please keep from_bits_unchecked unsafe, or at the very least do not introduce a safe way for the user to specify unknown bits without a way to disable it.

[roblabla]

Maybe an attribute could be given like #[allow_unknown_bits] or something.

[GilShoshan94]

I'd love to have a well-documented way to construct bitflags in a constant. Right now, I use a macro

That’s probably the most ergonomic looking way until impl const lands. There are some nice const methods if you don’t mind chaining:

TheBitflagsType::Flag1.union(TheBitflagsType::Flag2).union(…)

A general lack of nice examples is another issue 🙂

Hi, I am pretty new to Rust and could not find if what I am about to propose is possible.
While a lot of methods are cons such as union and intersection and we can chain them, it is nicer and more readable to use operators.

const SPECIFIC_FLAG: TheBitflagsType = TheBitflagsType::Flag1 | TheBitflagsType::Flag2).union | … ;

Is nicer and more intuitive than:

const SPECIFIC_FLAG: TheBitflagsType = TheBitflagsType::Flag1.union(TheBitflagsType::Flag2).union(…);

So my question and suggestion is:
Could the operators be const function ?
I looked into the code and for the Bitwise OR | operator, it seems that currently the code is (line 726 in lib.rs):

impl $crate::_core::ops::BitOr for $BitFlags {
            type Output = Self;

            /// Returns the union of the two sets of flags.
            #[inline]
            fn bitor(self, other: $BitFlags) -> Self {
                Self { bits: self.bits | other.bits }
            }
        }

Could changing

fn bitor(self, other: $BitFlags) -> Self {

to

const fn bitor(self, other: $BitFlags) -> Self {

work and make it useable in constant ?
Same for the other operators (!, &, ^).

[roblabla]

So my question and suggestion is:
Could the operators be const function ?

It is currently impossible to use traits from const functions, including the operator trait, in stable rust. The operators on integers/floats/etc... are special cased to work in const context.

So, no, unfortunately, that is not currently possible.

[GilShoshan94]

@roblabla
Thank you for the explanation, I didn't know.
Hope it will come in stable Rust.

[KodrAus]

For my own use case, making it safe to include unknown bits in the flags could actually introduce UB.

@Rua is there a repository somewhere with your use-case I could take a look at?

[djdisodo]

can you allow to add attribute for field bits

[KodrAus]

Hi @djdisodo 👋 Could you elaborate a bit on what you mean? Which attributes would you like to add to the bits field?

[djdisodo]

@KodrAus any attribute, just like current bitflags macro handles attributes that will be applied to expanded struct
($outer)

i'm planning to use helper attribute to the field

[KodrAus]

@djdisodo Hmm, the plan for 2.0 at this stage is to change the type of that field to make it anonymous so that we have some control over how that field implements other traits. You probably won't be able to meaningfully add arbitrary attributes to it. If there's a library with a trait that you want to implement then we can consider adding a feature flag for it.

[coderedart]

just here to express support for serializing to text in human readable formats like json.

[reitermarkus]

Somewhat related to

Reconsider making from_bits_unchecked unsafe.

I'd like to have something like

fn bits_mut(&mut self) -> &mut u16;

Currently I am using an enum to switch between &mut u16 and &mut MyBitFlags.

[KodrAus]

Hey all! 👋

Things have been moving slowly but surely here, and now have #282 that splits responsibilities between the user-facing API and the bitflags internal API. This means users are responsible for exposing more on their generated flags types, but in bitflags we can make #[derive]s on ecosystem traits possible to opt-in to.

In 1.x you'd write:

bitflags! {
    pub struct Flags: u32 {
        const A = 0b00000001;
        const B = 0b00000010;
        const C = 0b00000100;
        const ABC = Flags::A.bits | Flags::B.bits | Flags::C.bits;
    }
}

In 2.x you'd write:

bitflags! {
    #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    pub struct Flags: u32 {
        const A = 0b00000001;
        const B = 0b00000010;
        const C = 0b00000100;
        const ABC = Flags::A.bits() | Flags::B.bits() | Flags::C.bits();
    }
}

to get an equivalent flags type. You can still generate flags without the #[derive] block just fine:

bitflags! {
    pub struct Flags: u32 {
        const A = 0b00000001;
        const B = 0b00000010;
        const C = 0b00000100;
        const ABC = Flags::A.bits() | Flags::B.bits() | Flags::C.bits();
    }
}

and you'll get some set-operators, but none of the traits you'd normally derive, just as if you were writing a plain-old-struct. If you didn't want your flags type to derive PartialOrd, you simply don't add it to your #[derive], and can implement it manually instead if you'd like.

This new approach gives us more flexibility, but it comes at a cost: you have to do more work when generating flags types. I'm conscious of how easy it can be to trade ergonomics for flexibility to the point that nobody finds upgrading worthwhile. So I'd love to hear from anyone out there with flags types they'd now need to start peppering #[derive]s all over whether or not that effort seems worthwhile for them.

There's a sliding scale of how far down this path we want to go. I've opted for "any trait that can be #[derive]'d is not derived for you". That seemed the easiest to reason about, but previously we included a lot of traits.

[CasperN]

I don't think adding derive statements are that annoying but if its an issue, you could introduce a second macro, e.g. bitflags_extended!, for power users, and keep bitflags! for providing ergonomic defaults.

[KodrAus]

I've merged #282 which does most of the internal refactoring to let us separate the API bitflags owns from the API end-users own. Along with that I've changed unsafe fn from_bits_unchecked to fn from_bits_retain. It's functionally the same, but no longer unsafe.

The last thing we need to do before being able to put out a pre-release is make the iterators we generate for enabled flags a concrete type instead of impl Iterator.

[KodrAus]

I've just published a 2.0.0-rc.1 version of bitflags that includes the following changes:

• There's a BitFlags trait that makes it possible to work with flags types generically.
• You can iterate over set flags and their names, as well as get a flag value from its name. These should support cases where you want to serialize/deserialize flags as strings.
• The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
• You now need to add more #[derive]s so you get better control over the semantics of formatting and equality.
• We now have a private type to add manual trait implementations like Arbitrary to that users can then opt-in to through #[derive]s.

I think this is an exciting release and am keen for any feedback on how you all find it!

[coderedart]

You can iterate over set flags and their names, as well as get a flag value from its name. These should support cases where you want to serialize/deserialize flags as strings.

does that mean serializing to json or other text formats will just serialize the number representing bits rather than text names?

gfx-rs/wgpu#2629

[KodrAus]

@coderedart If you previously derived serde traits then adding a #[serde(transparent)] attribute will guarantee your flags will serialize exactly the same as they did before (which is the bits as an integer).

We don’t have a parser for the Flag1 | Flag2 format used in formatting yet, but do now generate the boilerplate to parse the stringified names of individual flags to their values for you. So we could write a fully generic parser based on the BitFlags trait that let you serialize using a textual representation of your flags instead of the integer.

[KodrAus]

Since we do already require the #[serde(transparent)] attribute on existing flags types I think we could consider using a text representation for human-readable flags by default and let you use #[serde(with)] to maintain the old behavior.

[coderedart]

instead of Flag1 | Flat2 etc.., isn't it simpler to just separate them by a comma? Flag1,Flag2.

or even a sequence ["Flag1", "Flag2"]. anything of these is fine as a default, and as you already said, people can use the #[serde(with)] attribute for their custom use cases.

As this is a breaking semver anyway, its the best time to adopt such a change (or explicitly reject it).

[KodrAus]

It would be easier to use a sequence for sure. I think a reason to have a canonical string format is that it would open up support for opting in to FromStr, Display, etc and target the lowest common datatype that you’d expect a format or schema to support.

[KodrAus]

In #297 I've started work on a text parser for flags formatted using bar-separated-names, like:

A | B | 0x8f

That PR also updates our default serde implementation to use formatted strings for human-readable formats and the underlying integer type directly for binary formats. There were a few things I wanted to call out and get some input on:

• Are we comfortable changing the default serialization format in this bump? It's a major version change so I think if there's any time to do it it would be then, but may break already serialized data.
• If you format Flags::empty() with a default derived Debug impl, you'll get Flags(0x0). The 0x0 is a placeholder for an empty set of flags, because we parse hex numbers, so 0x0 will be naturally parsed as empty. The alternative is to simply produce an empty string, but I felt like it might be surprising for a format to not write any output at all. We could split Debug and Display here, since the Display impl on an empty string is itself an empty string so there's precedent there. Do you have any preferences on one approach or another?

[coderedart]

1. I prefer not changing the default. making it opt-in would ensure easier transition for existing crates. and users like me who need readable flags, will manually opt into this change. Although, we should mention in README and top level docs that this option exists, so that new users can use this instead of wasting time writing their own serialization functions.
2. I think empty string makes sense. if there's no flags set, then there's no data to be serialized.

[KodrAus]

Missing the fact that your serialization defaults will change when you bump the library version is definitely a concern for me.

My own opinion is that we should change the default while we have some reasonable chance to do so, call it out explicitly in the release notes along with a demonstration of how to keep the existing behavior if you rely on it. We’re moving from an unconsidered default of serializing to a struct with an integer field to a considered (and I think entirely superior) one of serializing to a formatted string for end-users or the underlying bits directly for compact formats. #298 is an example of where the current default is simply unsuitable for compact users and #147 is an example for end-users.

[nim65s]

Hi,

As a new user, I can say I was really puzzled when my embedded systems stop working when I added some bitflags in my messages. It didn't took long to find that the deserialization was raising errors, and then it didn't took long to find that only the messages with bitflags were raising errors, and then it didn't took long to workaround conversions from and to u8s, but overall it was not the best experience.

So obviously I'm biased, but even if I understand that changing the default is not an easy decision to take, I'd be in favor of doing it 😅

[KodrAus]

I've opened #299 to push a new RC release that includes the change to the default serialization format. It will now use a formatted string for human-readable formats and the underlying bits
type for compact formats.

To keep the old behavior, see the bitflags-serde-legacy library.

[KodrAus]

If everything looks good, I'll leave this release candidate around for a few weeks and publish it as a stable 2.0.0 with no other changes on March 1st 🙂

[KodrAus]

2.0.0 should be up on crates.io shortly, so I think we can finally call this issue closed 🎉 Thanks to everyone who contributed their input to this release, I hope this foundation will help the library best serve its users for a long time to come.