From 2397658dffd4a49a4352dda0359658068b2d24d3 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Tue, 3 May 2022 16:53:04 +1000 Subject: [PATCH 1/9] rework the way cfgs are handled --- src/bitflags_trait.rs | 38 +++++++++++++++ src/lib.rs | 59 ++++------------------- tests/compile-pass/cfg/redefined-value.rs | 24 +++++++++ 3 files changed, 71 insertions(+), 50 deletions(-) create mode 100644 tests/compile-pass/cfg/redefined-value.rs diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index 0ffee465..efeea2ea 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -1,3 +1,7 @@ +use core::ops::{ + BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Not, +}; + #[doc(hidden)] pub trait ImplementedByBitFlagsMacro {} @@ -6,6 +10,7 @@ pub trait ImplementedByBitFlagsMacro {} /// It should not be implemented manually. pub trait BitFlags: ImplementedByBitFlagsMacro { type Bits; + /// Returns an empty set of flags. fn empty() -> Self; /// Returns the set containing all flags. @@ -48,3 +53,36 @@ pub trait BitFlags: ImplementedByBitFlagsMacro { /// Inserts or removes the specified flags depending on the passed value. fn set(&mut self, other: Self, value: bool); } + +#[doc(hidden)] +pub trait Bits: BitAnd + BitAndAssign + BitOr + BitOrAssign + BitXor + BitXorAssign + Not + Sized { + /// The value of `Self` where no bits are set. + const EMPTY: Self; + + /// The value of `Self` where all bits are set. + const SATURATED: Self; +} + +macro_rules! impl_bits { + ($($u:ty, $i:ty,)*) => { + $( + impl Bits for $u { + const EMPTY: $u = 0; + const SATURATED: $u = <$u>::MAX; + } + + impl Bits for $i { + const EMPTY: $i = 0; + const SATURATED: $i = <$u>::MAX as $i; + } + )* + } +} + +impl_bits! { + u8, i8, + u16, i16, + u32, i32, + u64, i64, + u128, i128, +} diff --git a/src/lib.rs b/src/lib.rs index a2d3193b..6af095b8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -286,7 +286,10 @@ mod bitflags_trait; #[doc(hidden)] pub mod __private { pub use core; - pub use crate::bitflags_trait::ImplementedByBitFlagsMacro; + pub use crate::bitflags_trait::{ + ImplementedByBitFlagsMacro, + Bits, + }; } /// The macro used to generate the flag structure. @@ -389,45 +392,6 @@ macro_rules! bitflags { () => {}; } -// A helper macro to implement the `all` function. -#[macro_export(local_inner_macros)] -#[doc(hidden)] -macro_rules! __impl_all_bitflags { - ( - $BitFlags:ident: $T:ty { - $( - $(#[$attr:ident $($args:tt)*])* - $Flag:ident = $value:expr; - )+ - } - ) => { - // See `Debug::fmt` for why this approach is taken. - #[allow(non_snake_case)] - trait __BitFlags { - $( - #[allow(deprecated)] - const $Flag: $T = 0; - )+ - } - #[allow(non_snake_case)] - impl __BitFlags for $BitFlags { - $( - __impl_bitflags! { - #[allow(deprecated)] - $(? #[$attr $($args)*])* - const $Flag: $T = Self::$Flag.bits; - } - )+ - } - Self { bits: $(::$Flag)|+ } - }; - ( - $BitFlags:ident: $T:ty { } - ) => { - Self { bits: 0 } - }; -} - #[macro_export(local_inner_macros)] #[doc(hidden)] macro_rules! __impl_bitflags { @@ -505,20 +469,13 @@ macro_rules! __impl_bitflags { /// Returns an empty set of flags. #[inline] pub const fn empty() -> Self { - Self { bits: 0 } + Self { bits: <$T as $crate::__private::Bits>::EMPTY } } /// Returns the set containing all flags. #[inline] pub const fn all() -> Self { - __impl_all_bitflags! { - $BitFlags: $T { - $( - $(#[$attr $($args)*])* - $Flag = $value; - )* - } - } + Self::from_bits_truncate(<$T as $crate::__private::Bits>::SATURATED) } /// Returns the raw value of the flags currently stored. @@ -532,8 +489,9 @@ macro_rules! __impl_bitflags { #[inline] pub const fn from_bits(bits: $T) -> $crate::__private::core::option::Option { let truncated = Self::from_bits_truncate(bits).bits; + if truncated == bits { - $crate::__private::core::option::Option::Some(Self{ bits }) + $crate::__private::core::option::Option::Some(Self { bits }) } else { $crate::__private::core::option::Option::None } @@ -763,6 +721,7 @@ macro_rules! __impl_bitflags { .zip(OPTIONS_NAMES[start..NUM_FLAGS].iter().copied()) { start += 1; + if self.contains(flag) { self.remove(flag); diff --git a/tests/compile-pass/cfg/redefined-value.rs b/tests/compile-pass/cfg/redefined-value.rs new file mode 100644 index 00000000..6bb2b651 --- /dev/null +++ b/tests/compile-pass/cfg/redefined-value.rs @@ -0,0 +1,24 @@ +#[macro_use] +extern crate bitflags; + +bitflags! { + pub struct Flags: u32 { + #[cfg(target_os = "linux")] + const FOO = 1; + + #[cfg(not(target_os = "linux"))] + const FOO = 2; + } +} + +fn main() { + #[cfg(target_os = "linux")] + { + assert_eq!(1, Flags::FOO.bits()); + } + + #[cfg(not(target_os = "linux"))] + { + assert_eq!(2, Flags::FOO.bits()); + } +} From c37cfa1d5ca1b474a7d61a7ec734c048d2e0cefb Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Tue, 3 May 2022 18:00:49 +1000 Subject: [PATCH 2/9] use Bits constants in place of literals --- src/bitflags_trait.rs | 10 ++- src/lib.rs | 6 +- tests/compile-fail/cfg/multi.rs | 25 ------- tests/compile-fail/cfg/multi.stderr | 17 ----- .../non_integer_base/all_defined.stderr | 69 +++++-------------- tests/compile-fail/trait/custom_impl.stderr | 18 ++--- 6 files changed, 36 insertions(+), 109 deletions(-) delete mode 100644 tests/compile-fail/cfg/multi.rs delete mode 100644 tests/compile-fail/cfg/multi.stderr diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index efeea2ea..e00a149a 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -2,6 +2,7 @@ use core::ops::{ BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Not, }; +// Not re-exported #[doc(hidden)] pub trait ImplementedByBitFlagsMacro {} @@ -9,7 +10,7 @@ pub trait ImplementedByBitFlagsMacro {} /// /// It should not be implemented manually. pub trait BitFlags: ImplementedByBitFlagsMacro { - type Bits; + type Bits: Bits; /// Returns an empty set of flags. fn empty() -> Self; @@ -54,8 +55,10 @@ pub trait BitFlags: ImplementedByBitFlagsMacro { fn set(&mut self, other: Self, value: bool); } +pub trait Sealed {} + #[doc(hidden)] -pub trait Bits: BitAnd + BitAndAssign + BitOr + BitOrAssign + BitXor + BitXorAssign + Not + Sized { +pub trait Bits: Clone + Copy + BitAnd + BitAndAssign + BitOr + BitOrAssign + BitXor + BitXorAssign + Not + Sized + Sealed { /// The value of `Self` where no bits are set. const EMPTY: Self; @@ -75,6 +78,9 @@ macro_rules! impl_bits { const EMPTY: $i = 0; const SATURATED: $i = <$u>::MAX as $i; } + + impl Sealed for $u {} + impl Sealed for $i {} )* } } diff --git a/src/lib.rs b/src/lib.rs index 6af095b8..ef096808 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -419,7 +419,7 @@ macro_rules! __impl_bitflags { // Append any extra bits that correspond to flags to the end of the format let extra_bits = self.bits & !Self::all().bits(); - if extra_bits != 0 { + if extra_bits != <$T as $crate::__private::Bits>::EMPTY { if !first { f.write_str(" | ")?; } @@ -501,12 +501,12 @@ macro_rules! __impl_bitflags { /// that do not correspond to flags. #[inline] pub const fn from_bits_truncate(bits: $T) -> Self { - if bits == 0 { + if bits == <$T as $crate::__private::Bits>::EMPTY { return Self { bits } } #[allow(unused_mut)] - let mut truncated = 0; + let mut truncated = <$T as $crate::__private::Bits>::EMPTY; $( #[allow(unused_doc_comments, unused_attributes)] diff --git a/tests/compile-fail/cfg/multi.rs b/tests/compile-fail/cfg/multi.rs deleted file mode 100644 index 461db914..00000000 --- a/tests/compile-fail/cfg/multi.rs +++ /dev/null @@ -1,25 +0,0 @@ -#[macro_use] -extern crate bitflags; - -// NOTE: Ideally this would work, but our treatment of CFGs -// assumes flags may be missing but not redefined -bitflags! { - pub struct Flags: u32 { - #[cfg(target_os = "linux")] - const FOO = 1; - #[cfg(not(target_os = "linux"))] - const FOO = 2; - } -} - -fn main() { - #[cfg(target_os = "linux")] - { - assert_eq!(1, Flags::FOO.bits()); - } - - #[cfg(not(target_os = "linux"))] - { - assert_eq!(1, Flags::FOO.bits()); - } -} diff --git a/tests/compile-fail/cfg/multi.stderr b/tests/compile-fail/cfg/multi.stderr deleted file mode 100644 index be9ba21c..00000000 --- a/tests/compile-fail/cfg/multi.stderr +++ /dev/null @@ -1,17 +0,0 @@ -error[E0428]: the name `FOO` is defined multiple times - --> tests/compile-fail/cfg/multi.rs:6:1 - | -6 | / bitflags! { -7 | | pub struct Flags: u32 { -8 | | #[cfg(target_os = "linux")] -9 | | const FOO = 1; -... | -12 | | } -13 | | } - | | ^ - | | | - | |_`FOO` redefined here - | previous definition of the value `FOO` here - | - = note: `FOO` must be defined only once in the value namespace of this trait - = note: this error originates in the macro `__impl_all_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/compile-fail/non_integer_base/all_defined.stderr b/tests/compile-fail/non_integer_base/all_defined.stderr index fb9c2867..69d89dc5 100644 --- a/tests/compile-fail/non_integer_base/all_defined.stderr +++ b/tests/compile-fail/non_integer_base/all_defined.stderr @@ -1,22 +1,16 @@ -error[E0308]: mismatched types - --> tests/compile-fail/non_integer_base/all_defined.rs:115:1 +error[E0277]: the trait bound `MyInt: Bits` is not satisfied + --> tests/compile-fail/non_integer_base/all_defined.rs:116:22 | -115 | / bitflags! { -116 | | struct Flags128: MyInt { -117 | | const A = MyInt(0b0000_0001u8); -118 | | const B = MyInt(0b0000_0010u8); -119 | | const C = MyInt(0b0000_0100u8); -120 | | } -121 | | } - | |_^ expected struct `MyInt`, found integer +116 | struct Flags128: MyInt { + | ^^^^^ the trait `Bits` is not implemented for `MyInt` | - = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) -help: try wrapping the expression in `MyInt` +note: required by a bound in `bitflags::BitFlags::Bits` + --> src/bitflags_trait.rs | -458 | if extra_bits != MyInt(0) { - | ++++++ + + | type Bits: Bits; + | ^^^^ required by this bound in `bitflags::BitFlags::Bits` -error[E0308]: mismatched types +error[E0277]: the trait bound `MyInt: Bits` is not satisfied --> tests/compile-fail/non_integer_base/all_defined.rs:115:1 | 115 | / bitflags! { @@ -26,15 +20,11 @@ error[E0308]: mismatched types 119 | | const C = MyInt(0b0000_0100u8); 120 | | } 121 | | } - | |_^ expected struct `MyInt`, found integer + | |_^ the trait `Bits` is not implemented for `MyInt` | = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) -help: try wrapping the expression in `MyInt` - | -508 | Self { bits: MyInt(0) } - | ++++++ + -error[E0308]: mismatched types +error[E0277]: the trait bound `MyInt: Bits` is not satisfied --> tests/compile-fail/non_integer_base/all_defined.rs:115:1 | 115 | / bitflags! { @@ -44,15 +34,11 @@ error[E0308]: mismatched types 119 | | const C = MyInt(0b0000_0100u8); 120 | | } 121 | | } - | |_^ expected struct `MyInt`, found integer + | |_^ the trait `Bits` is not implemented for `MyInt` | = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) -help: try wrapping the expression in `MyInt` - | -546 | if bits == MyInt(0) { - | ++++++ + -error[E0277]: no implementation for `{integer} |= MyInt` +error[E0277]: the trait bound `MyInt: Bits` is not satisfied --> tests/compile-fail/non_integer_base/all_defined.rs:115:1 | 115 | / bitflags! { @@ -62,12 +48,11 @@ error[E0277]: no implementation for `{integer} |= MyInt` 119 | | const C = MyInt(0b0000_0100u8); 120 | | } 121 | | } - | |_^ no implementation for `{integer} |= MyInt` + | |_^ the trait `Bits` is not implemented for `MyInt` | - = help: the trait `BitOrAssign` is not implemented for `{integer}` = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) -error[E0308]: mismatched types +error[E0277]: the trait bound `MyInt: Bits` is not satisfied --> tests/compile-fail/non_integer_base/all_defined.rs:115:1 | 115 | / bitflags! { @@ -77,28 +62,6 @@ error[E0308]: mismatched types 119 | | const C = MyInt(0b0000_0100u8); 120 | | } 121 | | } - | |_^ expected struct `MyInt`, found integer + | |_^ the trait `Bits` is not implemented for `MyInt` | = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) -help: try wrapping the expression in `MyInt` - | -561 | Self { bits: MyInt(truncated) } - | ++++++ + - -error[E0308]: mismatched types - --> tests/compile-fail/non_integer_base/all_defined.rs:115:1 - | -115 | / bitflags! { -116 | | struct Flags128: MyInt { -117 | | const A = MyInt(0b0000_0001u8); -118 | | const B = MyInt(0b0000_0010u8); -119 | | const C = MyInt(0b0000_0100u8); -120 | | } -121 | | } - | |_^ expected struct `MyInt`, found integer - | - = note: this error originates in the macro `__impl_all_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) -help: try wrapping the expression in `MyInt` - | -409 | const $Flag: $T = MyInt(0); - | ++++++ + diff --git a/tests/compile-fail/trait/custom_impl.stderr b/tests/compile-fail/trait/custom_impl.stderr index 7f3b4b1e..3b862a9a 100644 --- a/tests/compile-fail/trait/custom_impl.stderr +++ b/tests/compile-fail/trait/custom_impl.stderr @@ -1,11 +1,11 @@ error[E0277]: the trait bound `BootlegFlags: ImplementedByBitFlagsMacro` is not satisfied - --> $DIR/custom_impl.rs:5:6 - | -5 | impl BitFlags for BootlegFlags { - | ^^^^^^^^ the trait `ImplementedByBitFlagsMacro` is not implemented for `BootlegFlags` - | + --> tests/compile-fail/trait/custom_impl.rs:5:6 + | +5 | impl BitFlags for BootlegFlags { + | ^^^^^^^^ the trait `ImplementedByBitFlagsMacro` is not implemented for `BootlegFlags` + | note: required by a bound in `BitFlags` - --> $DIR/bitflags_trait.rs:7:21 - | -7 | pub trait BitFlags: ImplementedByBitFlagsMacro { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` + --> src/bitflags_trait.rs + | + | pub trait BitFlags: ImplementedByBitFlagsMacro { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` From 00924d0fdb31cc0af940f1a7b4892724b750edc9 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Tue, 3 May 2022 18:01:16 +1000 Subject: [PATCH 3/9] run fmt --- src/bitflags_trait.rs | 21 ++++++++++++++++----- src/lib.rs | 21 +++++++++------------ 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index e00a149a..364ece01 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -1,6 +1,4 @@ -use core::ops::{ - BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Not, -}; +use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Not}; // Not re-exported #[doc(hidden)] @@ -21,7 +19,8 @@ pub trait BitFlags: ImplementedByBitFlagsMacro { /// Convert from underlying bit representation, unless that /// representation contains bits that do not correspond to a flag. fn from_bits(bits: Self::Bits) -> Option - where Self: Sized; + where + Self: Sized; /// Convert from underlying bit representation, dropping any bits /// that do not correspond to flags. fn from_bits_truncate(bits: Self::Bits) -> Self; @@ -58,7 +57,19 @@ pub trait BitFlags: ImplementedByBitFlagsMacro { pub trait Sealed {} #[doc(hidden)] -pub trait Bits: Clone + Copy + BitAnd + BitAndAssign + BitOr + BitOrAssign + BitXor + BitXorAssign + Not + Sized + Sealed { +pub trait Bits: + Clone + + Copy + + BitAnd + + BitAndAssign + + BitOr + + BitOrAssign + + BitXor + + BitXorAssign + + Not + + Sized + + Sealed +{ /// The value of `Self` where no bits are set. const EMPTY: Self; diff --git a/src/lib.rs b/src/lib.rs index ef096808..af01f7d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -285,11 +285,8 @@ mod bitflags_trait; #[doc(hidden)] pub mod __private { + pub use crate::bitflags_trait::{Bits, ImplementedByBitFlagsMacro}; pub use core; - pub use crate::bitflags_trait::{ - ImplementedByBitFlagsMacro, - Bits, - }; } /// The macro used to generate the flag structure. @@ -1312,7 +1309,10 @@ mod tests { assert_eq!(UNION, Flags::A | Flags::C); assert_eq!(DIFFERENCE, Flags::all() - Flags::A); assert_eq!(COMPLEMENT, !Flags::C); - assert_eq!(SYM_DIFFERENCE, (Flags::A | Flags::C) ^ (Flags::all() - Flags::A)); + assert_eq!( + SYM_DIFFERENCE, + (Flags::A | Flags::C) ^ (Flags::all() - Flags::A) + ); } #[test] @@ -1572,10 +1572,7 @@ mod tests { assert_eq!(format!("{:?}", extra), "0xb8"); assert_eq!(format!("{:?}", Flags::A | extra), "A | 0xb8"); - assert_eq!( - format!("{:?}", Flags::ABC | extra), - "A | B | C | 0xb8" - ); + assert_eq!(format!("{:?}", Flags::ABC | extra), "A | B | C | 0xb8"); assert_eq!(format!("{:?}", EmptyFlags::empty()), "(empty)"); } @@ -1789,7 +1786,8 @@ mod tests { fn test_serde_bitflags_roundtrip() { let flags = SerdeFlags::A | SerdeFlags::B; - let deserialized: SerdeFlags = serde_json::from_str(&serde_json::to_string(&flags).unwrap()).unwrap(); + let deserialized: SerdeFlags = + serde_json::from_str(&serde_json::to_string(&flags).unwrap()).unwrap(); assert_eq!(deserialized.bits, flags.bits); } @@ -1813,7 +1811,6 @@ mod tests { } } - let flags = Flags::from_bits(0b00000100); assert_eq!(flags, None); let flags = Flags::from_bits(0b00000101); @@ -1834,7 +1831,7 @@ mod tests { let flags = Flags::from_bits_truncate(0b00000101); assert_eq!(flags, Flags::A); } - + #[test] fn test_iter() { bitflags! { From 4c87d6b0dd7d66890333c773a313a94500efea56 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Tue, 3 May 2022 18:02:03 +1000 Subject: [PATCH 4/9] fix location of export comment --- src/bitflags_trait.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index 364ece01..dab43412 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -1,6 +1,5 @@ use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Not}; -// Not re-exported #[doc(hidden)] pub trait ImplementedByBitFlagsMacro {} @@ -54,6 +53,7 @@ pub trait BitFlags: ImplementedByBitFlagsMacro { fn set(&mut self, other: Self, value: bool); } +// Not re-exported pub trait Sealed {} #[doc(hidden)] From ab8ca303088695c3c9071f5e775df2d3fa997491 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Tue, 3 May 2022 20:30:29 +1000 Subject: [PATCH 5/9] update compile fail tests --- tests/compile-fail/trait/custom_impl.stderr | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/compile-fail/trait/custom_impl.stderr b/tests/compile-fail/trait/custom_impl.stderr index 3b862a9a..d7e8d494 100644 --- a/tests/compile-fail/trait/custom_impl.stderr +++ b/tests/compile-fail/trait/custom_impl.stderr @@ -1,11 +1,11 @@ error[E0277]: the trait bound `BootlegFlags: ImplementedByBitFlagsMacro` is not satisfied - --> tests/compile-fail/trait/custom_impl.rs:5:6 - | -5 | impl BitFlags for BootlegFlags { - | ^^^^^^^^ the trait `ImplementedByBitFlagsMacro` is not implemented for `BootlegFlags` - | + --> tests/compile-fail/trait/custom_impl.rs:5:6 + | +5 | impl BitFlags for BootlegFlags { + | ^^^^^^^^ the trait `ImplementedByBitFlagsMacro` is not implemented for `BootlegFlags` + | note: required by a bound in `BitFlags` - --> src/bitflags_trait.rs - | - | pub trait BitFlags: ImplementedByBitFlagsMacro { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` + --> src/bitflags_trait.rs + | + | pub trait BitFlags: ImplementedByBitFlagsMacro { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` From 13517007790914d31427735da7983e74d975fdde Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Wed, 4 May 2022 09:16:51 +1000 Subject: [PATCH 6/9] fix overlapping flags in the iterator --- src/lib.rs | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index af01f7d1..bd9da7dd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -674,7 +674,7 @@ macro_rules! __impl_bitflags { } /// Returns an iterator over set flags and their names. - pub fn iter(mut self) -> impl $crate::__private::core::iter::Iterator { + pub fn iter(self) -> impl $crate::__private::core::iter::Iterator { use $crate::__private::core::iter::Iterator as _; const NUM_FLAGS: usize = { @@ -709,9 +709,10 @@ macro_rules! __impl_bitflags { ]; let mut start = 0; + let mut state = self; $crate::__private::core::iter::from_fn(move || { - if self.is_empty() || NUM_FLAGS == 0 { + if state.is_empty() || NUM_FLAGS == 0 { $crate::__private::core::option::Option::None } else { for (flag, flag_name) in OPTIONS[start..NUM_FLAGS].iter().copied() @@ -719,8 +720,18 @@ macro_rules! __impl_bitflags { { start += 1; + // NOTE: We check whether the flag exists in self, but remove it from + // a different value. This ensure that overlapping flags are handled + // properly. Take the following example: + // + // const A: 0b00000001; + // const B: 0b00000101; + // + // Given the bits 0b00000101, both A and B are set. But if we removed A + // as we encountered it we'd be left with 0b00000100, which doesn't + // correspond to a valid flag on its own. if self.contains(flag) { - self.remove(flag); + state.remove(flag); return $crate::__private::core::option::Option::Some((flag_name, flag)) } @@ -1843,24 +1854,31 @@ mod tests { const FOUR_WIN = 0b1000; #[cfg(unix)] const FOUR_UNIX = 0b10000; + const FIVE = 0b01000100; } } let count = { #[cfg(any(unix, windows))] { - 4 + 5 } #[cfg(not(any(unix, windows)))] { - 3 + 4 } }; let flags = Flags::all(); assert_eq!(flags.iter().count(), count); + + for (_, flag) in flags.iter() { + assert!(flags.contains(flag)); + } + let mut iter = flags.iter(); + assert_eq!(iter.next().unwrap(), ("ONE", Flags::ONE)); assert_eq!(iter.next().unwrap(), ("TWO", Flags::TWO)); assert_eq!(iter.next().unwrap(), ("THREE", Flags::THREE)); @@ -1874,6 +1892,8 @@ mod tests { assert_eq!(iter.next().unwrap(), ("FOUR_WIN", Flags::FOUR_WIN)); } + assert_eq!(iter.next().unwrap(), ("FIVE", Flags::FIVE)); + assert_eq!(iter.next(), None); let flags = Flags::empty(); @@ -1881,7 +1901,9 @@ mod tests { let flags = Flags::ONE | Flags::THREE; assert_eq!(flags.iter().count(), 2); + let mut iter = flags.iter(); + assert_eq!(iter.next().unwrap(), ("ONE", Flags::ONE)); assert_eq!(iter.next().unwrap(), ("THREE", Flags::THREE)); assert_eq!(iter.next(), None); From 669b6ad6fd4820f2513eb6e0abe12d7a4d557263 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Wed, 4 May 2022 09:18:34 +1000 Subject: [PATCH 7/9] rename SATURATED to ALL --- src/bitflags_trait.rs | 10 +++++++--- src/lib.rs | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index dab43412..440d5274 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -56,6 +56,10 @@ pub trait BitFlags: ImplementedByBitFlagsMacro { // Not re-exported pub trait Sealed {} +/// A private trait that encodes the requirements of underlying bits types that can hold flags. +/// +/// This trait may be made public at some future point, but it presents a compatibility hazard +/// so is left internal for now. #[doc(hidden)] pub trait Bits: Clone @@ -74,7 +78,7 @@ pub trait Bits: const EMPTY: Self; /// The value of `Self` where all bits are set. - const SATURATED: Self; + const ALL: Self; } macro_rules! impl_bits { @@ -82,12 +86,12 @@ macro_rules! impl_bits { $( impl Bits for $u { const EMPTY: $u = 0; - const SATURATED: $u = <$u>::MAX; + const ALL: $u = <$u>::MAX; } impl Bits for $i { const EMPTY: $i = 0; - const SATURATED: $i = <$u>::MAX as $i; + const ALL: $i = <$u>::MAX as $i; } impl Sealed for $u {} diff --git a/src/lib.rs b/src/lib.rs index bd9da7dd..f5664b94 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -472,7 +472,7 @@ macro_rules! __impl_bitflags { /// Returns the set containing all flags. #[inline] pub const fn all() -> Self { - Self::from_bits_truncate(<$T as $crate::__private::Bits>::SATURATED) + Self::from_bits_truncate(<$T as $crate::__private::Bits>::ALL) } /// Returns the raw value of the flags currently stored. From 00bcfc8ffe63f802b720dbe0149ded3adad06c8c Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Wed, 4 May 2022 09:21:01 +1000 Subject: [PATCH 8/9] extra bits result in less optimal flags --- src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index f5664b94..f2546a5b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1579,11 +1579,16 @@ mod tests { assert_eq!(format!("{:?}", Flags::A | Flags::B), "A | B"); assert_eq!(format!("{:?}", Flags::empty()), "(empty)"); assert_eq!(format!("{:?}", Flags::ABC), "A | B | C"); + let extra = unsafe { Flags::from_bits_unchecked(0xb8) }; + assert_eq!(format!("{:?}", extra), "0xb8"); assert_eq!(format!("{:?}", Flags::A | extra), "A | 0xb8"); - assert_eq!(format!("{:?}", Flags::ABC | extra), "A | B | C | 0xb8"); + assert_eq!( + format!("{:?}", Flags::ABC | extra), + "A | B | C | ABC | 0xb8" + ); assert_eq!(format!("{:?}", EmptyFlags::empty()), "(empty)"); } From 8c67a430416c74470ff33fa408264094ed4ec43e Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Wed, 4 May 2022 10:47:27 +1000 Subject: [PATCH 9/9] fix emission of some expected warnings --- src/lib.rs | 15 ++++++++------- tests/compile-pass/deprecated.rs | 14 ++++++++++++++ tests/compile-pass/non_snake_case.rs | 13 +++++++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 tests/compile-pass/deprecated.rs create mode 100644 tests/compile-pass/non_snake_case.rs diff --git a/src/lib.rs b/src/lib.rs index f2546a5b..979ff918 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -456,7 +456,14 @@ macro_rules! __impl_bitflags { } } - #[allow(dead_code)] + #[allow( + dead_code, + deprecated, + unused_doc_comments, + unused_attributes, + unused_mut, + non_upper_case_globals + )] impl $BitFlags { $( $(#[$attr $($args)*])* @@ -502,11 +509,9 @@ macro_rules! __impl_bitflags { return Self { bits } } - #[allow(unused_mut)] let mut truncated = <$T as $crate::__private::Bits>::EMPTY; $( - #[allow(unused_doc_comments, unused_attributes)] $(#[$attr $($args)*])* if bits & Self::$Flag.bits == Self::$Flag.bits { truncated |= Self::$Flag.bits @@ -678,11 +683,9 @@ macro_rules! __impl_bitflags { use $crate::__private::core::iter::Iterator as _; const NUM_FLAGS: usize = { - #[allow(unused_mut)] let mut num_flags = 0; $( - #[allow(unused_doc_comments, unused_attributes)] $(#[$attr $($args)*])* { num_flags += 1; @@ -694,13 +697,11 @@ macro_rules! __impl_bitflags { const OPTIONS: [$BitFlags; NUM_FLAGS] = [ $( - #[allow(unused_doc_comments, unused_attributes)] $(#[$attr $($args)*])* $BitFlags::$Flag, )* ]; - #[allow(unused_doc_comments, unused_attributes)] const OPTIONS_NAMES: [&'static str; NUM_FLAGS] = [ $( $(#[$attr $($args)*])* diff --git a/tests/compile-pass/deprecated.rs b/tests/compile-pass/deprecated.rs new file mode 100644 index 00000000..167ce44c --- /dev/null +++ b/tests/compile-pass/deprecated.rs @@ -0,0 +1,14 @@ +#![deny(warnings)] + +#[macro_use] +extern crate bitflags; + +bitflags! { + pub struct Flags: u32 { + #[deprecated = "Use something else"] + const A = 0b00000001; + const B = 0b00000010; + } +} + +fn main() {} diff --git a/tests/compile-pass/non_snake_case.rs b/tests/compile-pass/non_snake_case.rs new file mode 100644 index 00000000..5a3b8103 --- /dev/null +++ b/tests/compile-pass/non_snake_case.rs @@ -0,0 +1,13 @@ +#![deny(warnings)] + +#[macro_use] +extern crate bitflags; + +bitflags! { + pub struct Flags: u32 { + const CamelCase = 0b00000001; + const B = 0b00000010; + } +} + +fn main() {}