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

Use bitflags!() to replace flag.rs #96

Closed
andelf opened this issue May 6, 2014 · 3 comments · Fixed by #103
Closed

Use bitflags!() to replace flag.rs #96

andelf opened this issue May 6, 2014 · 3 comments · Fixed by #103

Comments

@andelf
Copy link
Contributor

andelf commented May 6, 2014

Rust now provides standard implementation of bitflag : rust-lang/rust#13072

@jcmoyer

@jcmoyer
Copy link
Contributor

jcmoyer commented May 6, 2014

I'm glad that there's finally a stdlib alternative! Some things I'm concerned about, though:

  1. bitflags! doesn't generate implementations for Shl and Shr. We'd still need to implement those ourselves, which implies that the flag_type! macro would have to stay around for a little longer anyways.
  2. Likewise, it doesn't implement Not to negate the contained value.
  3. The Flags trait I implemented for flag_type! is there to provide a consistent, easy to remember way of computing the none/all bitsets so you don't need to use the awkward flag typename.
  4. There's not an easy way to implement the inverse of empty for bitflags! because of (2). Apparently we could have a second impl $BitFlags and add the all function there.

If we want to keep all the functionality of flag_type! but still use bitflags! we'd still have to wrap the use of bitflags! inside of flag_type! and implement the extra traits for it.

@jcmoyer
Copy link
Contributor

jcmoyer commented May 8, 2014

On second thought I think we can drop Shl and Shr since these operations don't make sense for a set. I've submitted a PR to rust to implement Not and the inverse of empty (all). If it lands I'll happily tear out flag.rs since the main use cases will be covered yet again. This will introduce breaking changes since we lose the Flags trait.

@jcmoyer
Copy link
Contributor

jcmoyer commented May 15, 2014

I'll start working on this since rust-lang/rust#14009 has landed.

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 a pull request may close this issue.

2 participants