Add a bitflags! macro

[brendanzab]

The bitflags! macro generates a struct that holds a set of C-style bitmask flags. It is useful for creating typesafe wrappers for C APIs.

For example:

#[feature(phase)];
#[phase(syntax)] extern crate collections;

bitflags!(Flags: u32 {
    FlagA       = 0x00000001,
    FlagB       = 0x00000010,
    FlagC       = 0x00000100,
    FlagABC     = FlagA.bits
                | FlagB.bits
                | FlagC.bits
})

fn main() {
    let e1 = FlagA | FlagC;
    let e2 = FlagB | FlagC;
    assert!((e1 | e2) == FlagABC);   // union
    assert!((e1 & e2) == FlagC);     // intersection
    assert!((e1 - e2) == FlagA);     // set difference
}

[huonw]

Does this example actually pass tests?

Don't you need to import the macro with a #[phase(syntax)] extern crate collections;?

[brendanzab]

Weird, all the tests passed. Doesn't seem like it ran:

run doc-crate-collections [x86_64-apple-darwin]

running 2 tests
test hashmap::HashMap_0 ... ok
test lru_cache_0 ... ok

Maybe I should remove rust in ~~~rust?

Come to think of it, how would I do this test? I thought you couldn't do #[phase(syntax)] extern crate collections; from within a doc test? Also, the bitset! macro needs to be invoked at the item level.

[huonw]

Oh, yeah, maybe it's the rust or maybe its the ~~~ (most tests use

```

)

But if you put a fn main() {} in the test somewhere, rustdoc will no longer automatically insert the wrapping function main, so I think

#[feature(phase)];
#[phase(syntax)] extern crate collections;

/* bitset!() declaration */

fn main() {
    let e1 = FlagA | FlagC;
    // etc.
}

might work.

[huonw]

Oh, another naming thing: the word "bitset" sounds like it would have functionality pretty similar to our BitV type. And "bitset" is actually the name that e.g. Java and C++ use for the equivalent of our BitV type.

Maybe this could be called bitflags or something.

[brendanzab]

Rebased.

[brendanzab]

I added some documentation showing how bitflags can be extended with trait and type implementations.

[sfackler]

This isn't really needed since nothing else in libcollections uses bitflags!.

[brendanzab]

Oh, so that isn't needed for exporting macros?

[sfackler]

#[macro_escape] lets the definition escape to the next scope level (the crate root in this case). It's unrelated to #[macro_export].

[brendanzab]

Ahh, I knew about the escaping bit, but yeah, wasn't sure about the exporting.

[alexcrichton]

This seems pretty interesting, but is there a reason that the existing EnumSet collection doesn't suffice? Could EnumSet be improved to support this functionality? Right now we don't have a lot of apis that are macro-generated, and it would be nice to stay that way, but I may be missing something that this enables that you can't do with EnumSet

[brendanzab]

• One of the great things about bitflags is how nice the invocations look through the use of the bitwise operators. The API generated by bitflags! allows you to use the operators as in C code. EnumSet defines them but they are of little utility in practice.
• EnumSet works only works by casting to uints, rather than with other integer types.
• It is awkward to have yet another trait, especially one as limited in its utility as CLike.
• I don't like adding transmutes into enums if I can help it, but the Clike trait just goads me into it.

#[repr(u32)]
enum InitFlags {
    InitTimer               = 0x00000001,
    InitAudio               = 0x00000010,
    InitVideo               = 0x00000020,
    InitJoystick            = 0x00000200,
    InitHaptic              = 0x00001000,
    InitGamecontroller      = 0x00002000,
    InitEvents              = 0x00004000,
    InitNoparachute         = 0x00100000,
    InitEverything          = InitTimer as u32
                            | InitAudio as u32
                            | InitVideo as u32
                            | InitEvents as u32
                            | InitJoystick as u32
                            | InitHaptic as u32
                            | InitGamecontroller as u32,
}

impl CLike for InitFlags {
    fn to_uint(&self) -> uint {
        *self as uint
    }

    fn from_uint(v: uint) -> InitFlags {
        unsafe { cast::transmute(v as u32) }
    }
}

// ...

let mut flags = EnumSet::empty();
flags.add(sdl2::InitVideo);
flags.add(sdl2::InitEvents);
sdl2::init(flags)

bitflags!(InitFlags: Uint32 {
    InitTimer               = 0x00000001,
    InitAudio               = 0x00000010,
    InitVideo               = 0x00000020,
    InitJoystick            = 0x00000200,
    InitHaptic              = 0x00001000,
    InitGamecontroller      = 0x00002000,
    InitEvents              = 0x00004000,
    InitNoparachute         = 0x00100000,
    InitEverything          = InitTimer.bits
                            | InitAudio.bits
                            | InitVideo.bits
                            | InitEvents.bits
                            | InitJoystick.bits
                            | InitHaptic.bits
                            | InitGamecontroller.bits
})

// ...

sdl2::init(sdl2::InitVideo | sdl2::InitEvents)

[brendanzab]

I also think that the iterator might be broken, for example:

let mut flags = collections::EnumSet::empty();
flags.add(InitJoystick);
flags.add(InitVideo);
flags.add(InitHaptic);
for flag in flags.iter() {
    println!("{}", flag);
}

Prints:

InitEverything
InitVideo

Of course that does not mean that the implementation could not be fixed.

[alexcrichton]

Some of your concerns can be addressed by updating the EnumSet api. For example, it should take T: FromPrimitive + ToPrimitive rather than CLike which means you can use deriving. Other concerns could not be addressed easily, such as changing the underlying representation of the bit set from a 32-bit number to a 64-bit number (or similarly).

I'm worried about having two bit sets, and I'd like to reconcile the two of them before landing.

[brendanzab]

Removing the iterator, which seems to be broken, would remove the need for FromPrimitive. But I still find it ugly for the end user, having to jump through the hoops of using the add method. In its current state I wouldn't want to use the current EnumSet for one of my libraries. And the representation is a problem.

[brendanzab]

We could reccomend that library implementors implement specific operator overloads for convenience. For example you could have T | EnumSet<T> -> EnumSet<T>. But associativity would be 'surprising' for end users, for example EnumSet<T> | T would not work. So I would rather not do that.

[SiegeLord]

I think this is a great idea. I have actually came up with a eerily similar implementation for my own libraries: definition/usage.

My primary motivation was the bitwise operators interface, but it seems to me if I actually tried using EnumSet I'd have run into the remaining issues bjz mentioned.

[bvssvni]

EnumSet uses a phantom type to bind the usage, but since the generic constraint is decoupled from the type declaration, it becomes more confusing.

From a user-friendly perspective I think bitflags is better. It took me 3 seconds to understand how bitflags is used, but I am still pondering whether EnumSet doesn't have any caveats I am not aware about.

[brendanzab]

From a user-friendly perspective I think bitflags is better. It took me 3 seconds to understand how bitflags is used, but I am still pondering whether EnumSet doesn't have any caveats I am not aware about.

Was that from looking at the code, or my documentation? Macros can be pretty hard to document.

[bvssvni]

EnumSet lacks code example, which could clarify the usage quite a bit. I learned the usage of bitflags by looking at the example and the documentation gives better in-depth explanation. I like that you can specify the type and not rely on uint which EnumSet uses.

I think EnumSet is useful in cases where one can not alter the enum declaration. It can also be used to generate uint (with CLike) as a meta type for enum variants, which bitflags does not support.

It seems to me that the use cases does not completely overlap. Maybe we could have both?

[brendanzab]

I would just like to add a severe limitation to the bitflags! macro that I just thought of: the inability to add doc comments to the variants. :(

[pczarn]

I have basically a similar implementation as well (just implemented Sub and is_zero after I saw your code.) I think you should make negation possible.

Recursive macros can match the head/tail of the list of flags and optionally default to the value of the previous flag shifted by one.

[brendanzab]

Now that is cool! If only we could fix the documentation issue...

[aturon]

@bjz: this PR was discussed in the weekly meeting today, and consensus was that we should merge it, but as part of libstd. This is because we want to be able to provide type-safe APIs in libstd that are C-compatible. (Ultimately libstd and libcollections are likely to be refactored making this moot.)

We can improve the documentation aspect later on, once sufficient macro support is available.

Can you please update to move to libstd (and take care of any bitrot)? Then we will review and merge.

[brendanzab]

@aturon sure thing!

[brendanzab]

Rebased.

[alexcrichton]

Now that we can put documentation on macros themselves, I wonder if this would be best suited on the macro itself.

I suppose if you search for bitflags you'll probably hit the module before the macro, though.

[alexcrichton]

r=me with the updated examples (removing the extern crate collections)

[jcmoyer]

Is there any chance this will support Shl, Shr, and Not in the future? Also, an inverse of empty would be nice. I'd be happy to submit a PR for these since I'd like to replace my macro in rust-sdl2 with this one, but it's missing a few essential operations.

[thestinger]

@jcmoyer: I'm not sure that Shl and Shr make sense, since it's not an arithmetic type. It's already implementing subtraction as a set would rather than an integer. The set complement (Not) makes sense, but I'm not sure what it should do for signed types.

[jcmoyer]

@thestinger: I'd expect Not to invert all of the bits in the wrapped value regardless of it's signedness. Is there a reason why this wouldn't be valid for signed types? The macro is explicitly for dealing with numbers at the bit level where you typically don't care about this sort of thing.

As for Shl and Shr: shouldn't BitAnd and BitOr implementations be dropped for the same reason? If one operator is overloaded to do something odd, you can't expect any other operators to do "the right thing."

[thestinger]

BitAnd and BitOr do have a proper meaning on sets, and that applies to Not too.

My point was that since subtraction is performing a set difference, arithmetic/bitwise operators without a set equivalent don't make much sense.

[jcmoyer]

@thestinger: Makes sense. Thanks for clarifying.

[danthedeckie]

For future reference, this was later removed from std
231d9e7
and currently recommended to use the bitflags crate.