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

Add a check for if a bitmask is "full" (different to "all") #9

Closed
keithamus opened this issue Aug 12, 2023 · 2 comments
Closed

Add a check for if a bitmask is "full" (different to "all") #9

keithamus opened this issue Aug 12, 2023 · 2 comments

Comments

@keithamus
Copy link

Thanks for this great project! I've been using it for a project and it's come in very handy.

One gotcha I've discovered is that the ::all() method represents the maximum size of allowable bits given the size of the mask, which sounds reasonable in principle but in practice it limits it's usefulness unless the mask created makes use of all bits.

Consider the following struct:

#[bitmask(u8)]
pub enum Foo {
  A,
  B,
  C,
  D,
}

println!("{:#0b}", Foo::all()); // 0b11111111
println!("{:#0b}", Foo::A | Foo::B | Foo::C | Foo:D); // 0b1111

The result of this is that ::all() represents values that aren't used and could therefore be invalid, but also it produces a value which doesn't represent the sum of all masked values, which also means .is_all() is only useful for checking if the value is ::all(), rather than "full".

As far as I can see the only way to see if a bitmask is "full" is to | all variants together, which can get quite wordy (alternatively set up a const/method for this).

So I propose either:

  1. Change the semantics of :all() such that it returns the | of all variants. This could be done automatically, would have nice ergonomics, but would be a breaking change.
  2. Add the ability to configure the value of :all(), either by manually specifying the value returned or maybe a flag to opt-into the behaviour of returning the | of all variants.
  3. Add a new method, for example .is_full(), which makes the check. This would be different from :all(), which would mean the gotcha remains, but it would be the "easiest" change to implement IMO.

In addition to these, I think it could also be useful to add a .trunc() method which zeroes out any bits that aren't represented in enums, so that (given the example) calling Foo::from(255).trunc() would result in 0b1111, rather than 0b11111111.

@Lukas3674
Copy link
Owner

Hey, yes I am aware of the gotcha with ::all() and .is_all(), but haven't come around to changing it yet.
I like your proposition 3. a lot more than making breaking changes with ::all() and .is_all().
In my opinion adding ::full() and .is_full() should be easy to implement, same with the .trunc() function.
If you want to tackle the issue let me know, otherwise I'll see what I can do myself in the next couple of days.

Lukas3674 added a commit that referenced this issue Aug 13, 2023
Lukas3674 added a commit that referenced this issue Aug 13, 2023
@Lukas3674
Copy link
Owner

Thank you for the suggestion of .trunc() (I renamed it to .truncate(), because .intersects() and .contains() are also spelled out) and for the solution to ::all() and .is_all().

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
@keithamus @Lukas3674 and others