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

Allow safe conversion from Bitmask to/from underlying type for serialization/other purposes #2

Open
ForgottenMaster opened this issue Nov 21, 2018 · 0 comments

Comments

@ForgottenMaster
Copy link
Owner

We provide type safe conversions:

Bit => underlying enum class value
This is safe because each Bit is guaranteed to map exactly to the underlying enum class.

Bit => Bitmask
This is safe because each Bit is guaranteed to be safely promotable to a Bitmask with a single bit set in the underlying type.

However, for purposes of serialization and other reasons, the caller will often want to get the underlying value from the Bitmask, or to construct a Bitmask from the underlying type.

However, we cannot just give them a std::uint8_t (or std::uint16_t, etc.) because the caller will assume they can construct a Bitmask from a std::uintx_t as well. While this is safe for the specific case where we're passing back a value we got from the getter, in the general case this is not type-safe.

Somebody could easily, if we were to accept a uint8_t, pass a uint8_t with more bits set than we actually know how to deal with.

In order to be more explicit and leverage the type system, we should instead retrieve and accept a std::bitset where N is the count of bits we're actually using (number of variants passed to the construction macro).

This way, we cannot pass, for example a 7 bit set to a Bitmask that only uses 4 possible bits and is more explicit so the user isn't surprised when they accidentally set bit 7 and it gets ignored in the Bitmask.

However, std::bitset isn't usable in a constexpr context as it is now. While the constructor is constexpr, it seems that the conversions back to integral types are not, so will need a new/similar struct that is constexpr compatible.

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

1 participant