Specify the behavior of bitflags

[KodrAus]

Read the draft here

---

First, I'd like to extend a huge and humble thank you to all bitflags users. We've had a few tricky releases this year and the results of a few bugs along the way will have sent some of you on long and arduous debugging journeys. I appreciate all the feedback and understanding you've provided along the way.

A specification for bitflags

After a lot of back-and-forth with users (such as in #363) I think I've settled on what is fundamentally not right with bitflags today; it has no solid foundation to build off. It started as a bit of a scrappy but handy macro for reducing a lot of boilerplate and has evolved into something rather more complex. Without any solid foundation much of that evolution has been ad-hoc and resulted in inconsistencies in behavior. That makes it difficult to predict and communicate what bitflags does and why in many cases.

This PR aims to address that by introducing something of a formal specification for bitflags. It defines the bitflags "domain" and fully specifies the behavior of the code it generates in a way that is consistent and predictable.

What it means for you

This specification doesn't make any breaking changes to bitflags. It'll continue to work as it currently does, but its behavior is now made more obvious.

At some point in the future (there's no hurry on this) we may do a 3.0.0 that makes operators like |, &, and ^ truncate like ! does, but if you use bitflags_external you won't need to make any changes to your code when that happens.

[joshtriplett]

You make a recommendation for how to handle a bitflags type where you need to preserve all bits (define an ALL). Could you also make a recommendation for a bitflags type where you have a multi-bit bitfield that's intended as a numeric value? (For instance, "bits 4-6 of this type are a 3-bit value from 0 to 7".)

I'd love to have native support for that (define a numeric field across a range of bits, get functions to get and set that value as a number), but in the absence of native support, I'd like to see a recommendation in the spec and documentation for how to handle those. I'm guessing the answer is "define a THATFIELD_MASK that has all the bits in that field set".

[joshtriplett]

Also would you consider providing a version of the bitflags macro (with a different name) that doesn't require defining an ALL to avoid truncation? The advantages of that would be:

• More self-documenting code. If I have to define ALL I'd likely put a comment on it every time saying "define this so that bitflags preserves unknown bits". If there's a bitflags_preserve_unknown! macro, that's self-documenting.
• You don't have a potentially non-meaningful ALL field showing up in documentation, tab-completions, and iter_names.

Another possibility would be a wrapper, something like AllowUnknownBits<SomeBitflagsType>, with overridden operators that don't truncate, work with either the wrapped or unwrapped type, and return the same wrapper. That would be particularly self-documenting as a field within some struct, or in an FFI call.

[KodrAus]

Could you also make a recommendation for a bitflags type where you have a multi-bit bitfield that's intended as a numeric value?

I think this is a good use-case for bitfield libraries like bitfield or mycelium-bitfield. I don't think it's something bitflags will make a concerted effort to accommodate, but a function to treat a flag as an integer by shifting away trailing zeroes could make it possible. That does give me another point to add to the what isn't bitflags list though. This library is primarily for flags rather than fields.

Another possibility would be a wrapper, something like AllowUnknownBits

Ah that's a good alternative to consider. With the Flags trait we could define a generic Unknown<F: Flags>(F) that behaved that way. Unfortunately it wouldn't be able to inherit the flags of its wrappee. I should have made the Flag<B> type use the Bits trait the way the spec defines it, but it uses the flags type instead. It would remain spec-compliant if it simply defined a single ALL: !0 flag of its own anyways.

[KodrAus]

Giving this some more thought, I think a solution for unknown bits that applies at the definition would be best, because otherwise you have to audit all the places you use them.

I think based on this spec, we could consider defining an alternative macro that simply forwarded to the regular one, but included the const ALL = !0 flag for you, possibly using a well-defined name that is skipped when generating constants, so you don’t really see it.

I’ve been trying to avoid having to duplicate the logic of the bitflags! macro, or make it substantially more complicated through custom attributes or extra syntax.

[joshtriplett]

I think this is a good use-case for bitfield libraries like bitfield or mycelium-bitfield. I don't think it's something bitflags will make a concerted effort to accommodate, but a function to treat a flag as an integer by shifting away trailing zeroes could make it possible. That does give me another point to add to the what isn't bitflags list though. This library is primarily for flags rather than fields.

Valid. It'd be nice to have minimal support, but "use another library for that" is a fair response.

Ah that's a good alternative to consider. With the Flags trait we could define a generic Unknown<F: Flags>(F) that behaved that way. Unfortunately it wouldn't be able to inherit the flags of its wrappee. I should have made the Flag<B> type use the Bits trait the way the spec defines it, but it uses the flags type instead. It would remain spec-compliant if it simply defined a single ALL: !0 flag of its own anyways.

I think if the AllowUnknown wrapper (insert name bikeshedding here) has operators that work with both the wrapper type and the base type, and had a From impl, it wouldn't be necessary to inherit the flags of the base type. It'd be convenient enough to use, and the type would document whether any given user expected to handle or ignore/truncate unknown flags.

I think based on this spec, we could consider defining an alternative macro that simply forwarded to the regular one, but included the const ALL = !0 flag for you, possibly using a well-defined name that is skipped when generating constants, so you don’t really see it.

Fair enough; that does sound appealing.

Proposal: what if bitflags accepted const _ as something that extends the set of known bits without defining anything itself? Then the wrapper macro can use const _ = !0.

[KodrAus]

I think if the AllowUnknown wrapper (insert name bikeshedding here) has operators that work with both the wrapper type and the base type, and had a From impl, it wouldn't be necessary to inherit the flags of the base type.

Ah yes, I think this would be reasonable to do. The AllowUnknown type wouldn't itself be considered a flags type. Just a utility that overrides operators.

Proposal: what if bitflags accepted const _ as something that extends the set of known bits without defining anything itself? Then the wrapper macro can use const _ = !0.

I like this 👍 It has the benefit of being unsurprising syntactically and easy to detect in a macro_rules macro. We could allow flags to have empty names, and not use the names of empty flags in formatting.

[KodrAus]

There is a version of this spec we could ship without needing to make a major version bump:

1. Change the implementation of from_bits_truncate to do what this spec suggests and what negation currently does; just unset bits not corresponding to any flag
2. Change the spec so no operations truncate except for negation (that’s the big change that we’d need to ship a major version for). Suggest you use external_bitflags! if you want to guarantee that unknown bits are preserved in the results of operations.

That could let us defer any breaking version either indefinitely or at least for some time.

[KodrAus]

I've updated this spec so it no longer requires any breaking changes; there will be no 3.0.0 on the immediate horizon.