-
Notifications
You must be signed in to change notification settings - Fork 144
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
Iterator over all the enabled options #278
Conversation
This adds a new function that returns an iterator over all the enabled flags. This implementation takes into account combined flags like the ones described in: bitflags#204 (comment) It uses an `impl Iterator` as return type because using an specific Iter type implies using const generics which are not supported in earlier versions of rust and adding a const to the trait in order to express the correct dimension of the array as can be seen in the last solution proposed in bitflags#204. This superseeds bitflags#204
Ah this is an interesting approach 🤔 Thanks for exploring this @arturoc! I'd love to see some test cases that demonstrate how it works, and ideally we'd be able to replace our debug implementation with it (that might require an implementation that yielded |
Ah yes I forgot to bring the tests from the previous implementation, just pushed them in a new commit |
Note how in the first test the flags appear in the wrong order due to the optimization to not go through all the options in every iteration. Removing that would show the flags in the correct order but would be slower for bitflags with a lot of options. I can try to do some benchmarks to understand if it's worth |
Just figured a better way to optimize the iterator method so it preserves the order of the flags and the options array is a const now. |
struct Flags: u32 { | ||
const ONE = 0b001; | ||
const TWO = 0b010; | ||
const THREE = 0b100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add a cfg'd-out flag to this as well just to make sure we handle those? Something like:
#[cfg(windows)]
const FOUR_WIN = 0b1000;
#[cfg(unix)]
const FOUR_UNIX = 0b10000;
and then use those matching cfgs to check that we iterate over those flags when they're available, but don't when they're not?
Thanks for working on this @arturoc! It's a nice simple approach you've ended up with. I like it! |
Thanks! have added a test for cfg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done! This looks good to me.
Adds a new function that returns an iterator over all the enabled flags.
This implementation takes into account combined flags like the ones described in: #204 (comment)
It uses an
impl Iterator
as return type because using an specific Iter type implies using const generics which are not supported in earlier versions of rust and adding a const to the trait in order to express the correct dimension of the array as can be seen in the last solution proposed in #204.Superseeds #204