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

Make Debug fancy again and handle #[cfg] properly. #14

Merged
merged 1 commit into from Jun 29, 2015

Conversation

chris-morgan
Copy link
Contributor

Debug was implemented using the nice readable flag names in #7, but the flag names were removed in #9 because it used $Flags which broke trivial #[cfg]-based removal of flags. Changing .contains($Flag) to .contains($BitFlags { bits: $value }) instead (akin to what .all() did) would have fixed that issue, allowing a friendly Debug again, but then I found a further improvement to make, because the #[cfg] behaviour was still off, as a flag’s value was still written out regardless of the #[cfg] attribute, so if it contained stuff that didn’t exist bitflags! would still blow up. This is inconsistent with how such things work in Rust proper, so I’ve fixed it.

The fix is rather convoluted, involving a dummy module, a bunch of dummy constants, a nested function and glob imports, but it works, avoiding referring to $value successfully. This change causes this to work:

bitflags! {
    flags Foo: u32 {
        #[cfg(a)]
        const A = 0b1,
        #[cfg(a)]
        const B = A.bits,  // or anything else that is `#[cfg(a)]`
    }
}

(Formerly all() was still trying to use A.bits when cfg(not(a)) and thus A wasn’t defined.)

This change allows #[cfg] on a flag to work like it should, allowing it to depend on things that don’t exist unless the conditions are met. Sure, that seems a pretty rare case for flags, but it’s real. Beyond the style of example above, my imagined scenario is a -sys crate having declared certain flag values, but only when certain features are enabled, and so when defining the bitflags in another crate (for the -sys crate doesn’t use bitflags) you can only use those values with the appropriate features.

`Debug` was implemented using the nice readable flag names in bitflags#7, but
the flag names were removed in bitflags#9 because it used `$Flags` which broke
trivial `#[cfg]`-based removal of flags. Changing `.contains($Flag)` to
`.contains($BitFlags { bits: $value })` instead (akin to what `.all()`
did) would have fixed that issue, allowing a friendly `Debug` again, but
then I found a further improvement to make, because the `#[cfg]`
behaviour was still off, as a flag’s value was still written out
regardless of the `#[cfg]` attribute, so if *it* contained stuff that
didn’t exist `bitflags!` would still blow up. This is inconsistent with
how such things work in Rust proper, so I’ve fixed it.

The fix is rather convoluted, involving a dummy module, a bunch of dummy
constants, a nested function and glob imports, but it works, avoiding
referring to `$value` successfully. This change causes this to work:

```rust
bitflags! {
    flags Foo: u32 {
        #[cfg(a)]
        const A = 0b1,
        #[cfg(a)]
        const B = A.bits,  // or anything else that is `#[cfg(a)]`
    }
}
```

(Formerly `all()` was still trying to use `A.bits` when `cfg(not(a))`
and thus `A` wasn’t defined.)

This change allows `#[cfg]` on a flag to work like it should, allowing
it to depend on things that don’t exist unless the conditions are met.
Sure, that seems a pretty rare case for flags, but it’s real. Beyond
the style of example above, my imagined scenario is a -sys crate
having declared certain flag values, but only when certain features are
enabled, and so when defining the bitflags in another crate (for the
-sys crate doesn’t use bitflags) you can only use those values with the
appropriate features.
@alexcrichton alexcrichton merged commit 4f12093 into bitflags:master Jun 29, 2015
@alexcrichton
Copy link
Contributor

Thanks!

@tamird tamird mentioned this pull request Nov 1, 2015
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

Successfully merging this pull request may close these issues.

None yet

2 participants