Add attributes to std::bitflags, use bitflags for std::io::FilePermission #13897
Conversation
|
/cc @bjz |
| #[macro_export] | ||
| macro_rules! bitflags( | ||
| ($BitFlags:ident: $T:ty { | ||
| ($(#[$attr:meta])* flags $BitFlags:ident: $T:ty { |
There was a problem hiding this comment.
Maybe this flags keyword should be struct?
There was a problem hiding this comment.
That's a good idea, since the documentation already reveals that the generated type is a struct.
There was a problem hiding this comment.
Hmm, on second thought, I'm not so sure about this -- it might be confusing. Consider:
bitflags!(
struct Flags: u32 {
FlagA = 1,
FlagB = 2
}
)
The problem is that the stuff inside { ... } doesn't look very struct-like. (If anything, it looks more like our current enums.) So having a distinct pseudo keyword like flags may be the best approach.
There was a problem hiding this comment.
That's a good point, flags sounds good to me.
|
Also, can you re-word the commits that have breaking changes to follow these guidelines? It basically just needs to include "[breaking-change]" somewhere. |
|
@alexcrichton This should be good to go. |
|
Looks great. 👍 |
|
It looks like this also closes #2623, could you add that to the commit message? |
|
@alexcrichton I don't think that's right: #2623 is also specifically talking about the fact that the exposed API is in Unix style, and does not currently translate to/make sense for Windows. So I think there's more work to be done to close that issue, potentially changing the API again... |
|
Oh hm, I see, I will comment on that issue. |
The `std::bitflags::bitflags!` macro did not provide support for
adding attributes to the generated structure or flags, due to
limitations in the parser for macros. This patch works around the
parser limitations by requiring a `flags` keyword in the overall
`bitflags!` invocation, and a `static` keyword for each flag:
bitflags!(
#[deriving(Hash)]
#[doc="Three flags"]
flags Flags: u32 {
#[doc="The first flag"]
static FlagA = 0x00000001,
static FlagB = 0x00000010,
static FlagC = 0x00000100
}
)
The intent of `std::bitflags` is to allow building type-safe wrappers around C-style flags APIs. But in addition to construction these flags from the Rust side, we need a way to convert them from the C side. This patch adds a `from_bits` function, which is unsafe since the bits in question may not represent a valid combination of flags.
This patch changes `std::io::FilePermissions` from an exposed `u32` representation to a typesafe representation (that only allows valid flag combinations) using the `std::bitflags`, thus ensuring a greater degree of safety on the Rust side. Despite the change to the type, most code should continue to work as-is, sincde the new type provides bit operations in the style of C flags. To get at the underlying integer representation, use the `bits` method; to (unsafely) convert to `FilePermissions`, use `FilePermissions::from_bits`. Closes rust-lang#6085. [breaking-change]
The `std::bitflags::bitflags!` macro did not provide support for
adding attributes to the generates structure, due to limitations in
the parser for macros. This patch works around the parser limitations
by requiring a `flags` keyword in the `bitflags!` invocations:
bitflags!(
#[deriving(Hash)]
#[doc="Three flags"]
flags Flags: u32 {
FlagA = 0x00000001,
FlagB = 0x00000010,
FlagC = 0x00000100
}
)
The intent of `std::bitflags` is to allow building type-safe wrappers
around C-style flags APIs. But in addition to construction these flags
from the Rust side, we need a way to convert them from the C
side. This patch adds a `from_bits` function, which is unsafe since
the bits in question may not represent a valid combination of flags.
Finally, this patch changes `std::io::FilePermissions` from an exposed
`u32` representation to a typesafe representation (that only allows valid
flag combinations) using the `std::bitflags`.
Closes #6085.
|
🍬 |
| @@ -114,6 +127,12 @@ macro_rules! bitflags( | |||
| self.bits | |||
| } | |||
|
|
|||
| /// Convert from underlying bit representation. Unsafe because the | |||
| /// bits are not guaranteed to represent valid flags. | |||
| pub unsafe fn from_bits(bits: $T) -> $BitFlags { | |||
There was a problem hiding this comment.
Couldn't there be a safe version of this that just makes sure it's a valid combination of bits?
Previously, the `from_bits` function in the `std::bitflags::bitflags` macro was marked as unsafe, as it did not check that the bits being converted actually corresponded to flags. This patch changes the function to check against the full set of possible flags and return an `Option` which is `None` if a non-flag bit is set. It also adds a `from_bits_truncate` function which simply zeros any bits not corresponding to a flag. This addresses the concern raised in rust-lang#13897
Previously, the `from_bits` function in the `std::bitflags::bitflags` macro was marked as unsafe, as it did not check that the bits being converted actually corresponded to flags. This patch changes the function to check against the full set of possible flags and return an `Option` which is `None` if a non-flag bit is set. It also adds a `from_bits_truncate` function which simply zeros any bits not corresponding to a flag. This addresses the concern raised in rust-lang#13897
fix(ty): should query impls in nearest block fix rust-lang/rust-analyzer#13895
The
std::bitflags::bitflags!macro did not provide support foradding attributes to the generates structure, due to limitations in
the parser for macros. This patch works around the parser limitations
by requiring a
flagskeyword in thebitflags!invocations:The intent of
std::bitflagsis to allow building type-safe wrappersaround C-style flags APIs. But in addition to construction these flags
from the Rust side, we need a way to convert them from the C
side. This patch adds a
from_bitsfunction, which is unsafe sincethe bits in question may not represent a valid combination of flags.
Finally, this patch changes
std::io::FilePermissionsfrom an exposedu32representation to a typesafe representation (that only allows validflag combinations) using the
std::bitflags.Closes #6085.