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 support for iterating set bits in flags. #152

Closed
wants to merge 1 commit into from

Conversation

djg
Copy link

@djg djg commented Mar 29, 2018

This implements #28.

This is gated by feature = "iterator" because it adds a new trait and struct, as well as using num-traits crate to aid the generic implementation of struct BitFlagsIter.

I'm looking for feedback on the implementation of BitFlagsIter. I tried to implement this with out introducting the BitFlags trait, but I couldn't work out how to create a bitflags! struct with out the trait to enable conversion from raw bits to wrapped type.

I also tried implementing fn next() with out the help from num-traits but I had trouble getting Wrapping for Sub to resolve.

This needs documentation before it can land.

This is gated by feature = "iterator" because it adds a new trait and
struct, as well as using num-traits crate to aid the generic
implementation of BitFlagsIter.
@RReverser
Copy link

RReverser commented Apr 1, 2018

Maybe instead of separate BitFlagsIter implement iterator directly on generated bitflags structure or a simple newtype around it? Technically all you need for iteration is the current mask where at each time one significant bit is taken, emitted and cleared from the mask, so iterating over bitmask (which already implements Copy) would do just that. This should simplify implementation quite a bit.

@djg
Copy link
Author

djg commented Apr 4, 2018

@RReverser Thanks for your feedback. I'll have a look at simplifying.

With the lack of concat_idents! in macros I chose to implement the newtype wrapper via the generic BitFlagsIter, but then ran into issues trying to work on flags generically since there is currently no trait implemented which can be used to constrain T.

@dtolnay dtolnay added the wip label Apr 26, 2018
@djg
Copy link
Author

djg commented May 9, 2018

FYI: I'm still interested in this, I just haven't had time to get the addition to work as I'd hoped.

@Palladinium
Copy link

What work needs to still be done on this? This is a feature I'd like to see as well, and I'd be willing to finish it off to see it happen.

@Dushistov
Copy link

Dushistov commented Aug 15, 2019

@Palladinium

What work needs to still be done on this?
and I'd be willing to finish it off to see it happen

Stabilization of concat_idents can be solutions of all problems:

rust-lang/rust#29599

So it would be possible to introduce new type via:

struct concat_idents!($BitFlags, Iterator) {

Or if you can resolve problem with const fn inside traits: rust-lang/rust#57563 it would be possible to add something like

trait BigFlagsAllValues<T> {
   const fn values() -> &'static [T];
}
impl BitFlagsAllValues for $BitFlags 

@clouds56
Copy link

clouds56 commented Aug 1, 2020

According to the thread, concat_idents could be only used in places accept expr and things like

struct concat_idents!($BitFlags, Iterator) {

would never compiles.

Shall we look for some other solution?
The comment points out https://github.com/dtolnay/paste might be a possible solution.

I think the Iterator trait is so fundamental for bitflags.

@KodrAus
Copy link
Member

KodrAus commented May 4, 2022

In #278 we merged an implementation of Iterator for set flags and their names.

Thanks for originally pushing this along @djg!

@KodrAus KodrAus closed this May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants