Merge pull request #366 from KodrAus/feat/test-cleanups

Fix up incorrect sub assign behavior and other cleanups

examples/custom_bits_type.rs

@@ -1,6 +1,6 @@
 use std::ops::{BitAnd, BitOr, BitXor, Not};
 
-use bitflags::{Flags, Flag, Bits};
+use bitflags::{Bits, Flag, Flags};
 
 // Define a custom container that can be used in flags types
 // Note custom bits types can't be used in `bitflags!`
@@ -25,23 +25,35 @@ impl BitAnd for CustomBits {
     type Output = Self;
 
     fn bitand(self, other: Self) -> Self {
-        CustomBits([self.0[0] & other.0[0], self.0[1] & other.0[1], self.0[2] & other.0[2]])
+        CustomBits([
+            self.0[0] & other.0[0],
+            self.0[1] & other.0[1],
+            self.0[2] & other.0[2],
+        ])
     }
 }
 
 impl BitOr for CustomBits {
     type Output = Self;
 
     fn bitor(self, other: Self) -> Self {
-        CustomBits([self.0[0] | other.0[0], self.0[1] | other.0[1], self.0[2] | other.0[2]])
+        CustomBits([
+            self.0[0] | other.0[0],
+            self.0[1] | other.0[1],
+            self.0[2] | other.0[2],
+        ])
     }
 }
 
 impl BitXor for CustomBits {
     type Output = Self;
 
     fn bitxor(self, other: Self) -> Self {
-        CustomBits([self.0[0] & other.0[0], self.0[1] & other.0[1], self.0[2] & other.0[2]])
+        CustomBits([
+            self.0[0] & other.0[0],
+            self.0[1] & other.0[1],
+            self.0[2] & other.0[2],
+        ])
     }
 }
 

examples/macro_free.rs

@@ -4,7 +4,7 @@
 
 use std::{fmt, str};
 
-use bitflags::{Flags, Flag};
+use bitflags::{Flag, Flags};
 
 // First: Define your flags type. It just needs to be `Sized + 'static`.
 pub struct ManualFlags(u32);
@@ -54,5 +54,8 @@ impl fmt::Display for ManualFlags {
 }
 
 fn main() {
-    println!("{}", ManualFlags::A.union(ManualFlags::B).union(ManualFlags::C));
+    println!(
+        "{}",
+        ManualFlags::A.union(ManualFlags::B).union(ManualFlags::C)
+    );
 }

src/example_generated.rs

@@ -38,6 +38,10 @@ __impl_public_bitflags_forward! {
     Flags: u32, Field0
 }
 
+__impl_public_bitflags_ops! {
+    Flags
+}
+
 __impl_public_bitflags_iter! {
     Flags: u32, Flags
 }

src/external.rs

@@ -153,9 +153,7 @@ macro_rules! __impl_external_bitflags_serde {
             fn deserialize<D: $crate::__private::serde::Deserializer<'de>>(
                 deserializer: D,
             ) -> $crate::__private::core::result::Result<Self, D::Error> {
-                let flags: $PublicBitFlags = $crate::serde::deserialize(
-                    deserializer,
-                )?;
+                let flags: $PublicBitFlags = $crate::serde::deserialize(deserializer)?;
 
                 Ok(flags.0)
             }
@@ -235,20 +233,16 @@ macro_rules! __impl_external_bitflags_bytemuck {
     ) => {
         // SAFETY: $InternalBitFlags is guaranteed to have the same ABI as $T,
         // and $T implements Pod
-        unsafe impl $crate::__private::bytemuck::Pod for $InternalBitFlags
-        where
-            $T: $crate::__private::bytemuck::Pod,
+        unsafe impl $crate::__private::bytemuck::Pod for $InternalBitFlags where
+            $T: $crate::__private::bytemuck::Pod
         {
-
         }
 
         // SAFETY: $InternalBitFlags is guaranteed to have the same ABI as $T,
         // and $T implements Zeroable
-        unsafe impl $crate::__private::bytemuck::Zeroable for $InternalBitFlags
-        where
-            $T: $crate::__private::bytemuck::Zeroable,
+        unsafe impl $crate::__private::bytemuck::Zeroable for $InternalBitFlags where
+            $T: $crate::__private::bytemuck::Zeroable
         {
-
         }
     };
 }

src/external/arbitrary.rs

@@ -3,11 +3,9 @@
 use crate::Flags;
 
 /// Get a random known flags value.
-pub fn arbitrary<'a, B: Flags>(
-    u: &mut arbitrary::Unstructured<'a>,
-) -> arbitrary::Result<B>
+pub fn arbitrary<'a, B: Flags>(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<B>
 where
-    B::Bits: arbitrary::Arbitrary<'a>
+    B::Bits: arbitrary::Arbitrary<'a>,
 {
     B::from_bits(u.arbitrary()?).ok_or_else(|| arbitrary::Error::IncorrectFormat)
 }

src/external/bytemuck.rs

@@ -1,7 +1,7 @@
 #[cfg(test)]
 mod tests {
     use bytemuck::{Pod, Zeroable};
-    
+
     bitflags! {
         #[derive(Pod, Zeroable, Clone, Copy)]
         #[repr(transparent)]

src/external/serde.rs

@@ -1,17 +1,17 @@
 //! Specialized serialization for flags types using `serde`.
 
+use crate::{
+    parser::{self, ParseHex, WriteHex},
+    Flags,
+};
 use core::{fmt, str};
-use crate::{Flags, parser::{self, ParseHex, WriteHex}};
 use serde::{
     de::{Error, Visitor},
     Deserialize, Deserializer, Serialize, Serializer,
 };
 
 /// Serialize a set of flags as a human-readable string or their underlying bits.
-pub fn serialize<B: Flags, S: Serializer>(
-    flags: &B,
-    serializer: S,
-) -> Result<S::Ok, S::Error>
+pub fn serialize<B: Flags, S: Serializer>(flags: &B, serializer: S) -> Result<S::Ok, S::Error>
 where
     B::Bits: WriteHex + Serialize,
 {
@@ -26,13 +26,7 @@ where
 }
 
 /// Deserialize a set of flags from a human-readable string or their underlying bits.
-pub fn deserialize<
-    'de,
-    B: Flags,
-    D: Deserializer<'de>,
->(
-    deserializer: D,
-) -> Result<B, D::Error>
+pub fn deserialize<'de, B: Flags, D: Deserializer<'de>>(deserializer: D) -> Result<B, D::Error>
 where
     B::Bits: ParseHex + Deserialize<'de>,
 {

src/internal.rs

@@ -56,7 +56,7 @@ macro_rules! __impl_internal_bitflags {
                 if self.is_empty() {
                     // If no flags are set then write an empty hex flag to avoid
                     // writing an empty string. In some contexts, like serialization,
-                    // an empty string is preferrable, but it may be unexpected in
+                    // an empty string is preferable, but it may be unexpected in
                     // others for a format not to produce any output.
                     //
                     // We can remove this `0x0` and remain compatible with `FromStr`,
@@ -106,6 +106,10 @@ macro_rules! __impl_internal_bitflags {
             }
         }
 
+        __impl_public_bitflags_ops! {
+            $InternalBitFlags
+        }
+
         __impl_public_bitflags_iter! {
             $InternalBitFlags: $T, $PublicBitFlags
         }

src/iter.rs

@@ -1,6 +1,6 @@
 //! Iterating over set flag values.
 
-use crate::{Flags, Flag};
+use crate::{Flag, Flags};
 
 /// An iterator over a set of flags.
 ///
@@ -23,28 +23,28 @@ impl<B: Flags> Iter<B> {
 
 impl<B: 'static> Iter<B> {
     #[doc(hidden)]
-    pub const fn __private_const_new(flags: &'static [Flag<B>], source: B, state: B) -> Self {
+    pub const fn __private_const_new(flags: &'static [Flag<B>], source: B, remaining: B) -> Self {
         Iter {
-            inner: IterNames::__private_const_new(flags, source, state),
+            inner: IterNames::__private_const_new(flags, source, remaining),
             done: false,
         }
     }
 }
 
 impl<B: Flags> Iterator for Iter<B> {
     type Item = B;
-    
+
     fn next(&mut self) -> Option<Self::Item> {
         match self.inner.next() {
             Some((_, flag)) => Some(flag),
             None if !self.done => {
                 self.done = true;
-                
+
                 // After iterating through valid names, if there are any bits left over
                 // then return one final value that includes them. This makes `into_iter`
                 // and `from_iter` roundtrip
                 if !self.inner.remaining().is_empty() {
-                    Some(B::from_bits_retain(self.inner.state.bits()))
+                    Some(B::from_bits_retain(self.inner.remaining.bits()))
                 } else {
                     None
                 }
@@ -61,7 +61,7 @@ pub struct IterNames<B: 'static> {
     flags: &'static [Flag<B>],
     idx: usize,
     source: B,
-    state: B,
+    remaining: B,
 }
 
 impl<B: Flags> IterNames<B> {
@@ -70,19 +70,19 @@ impl<B: Flags> IterNames<B> {
         IterNames {
             flags: B::FLAGS,
             idx: 0,
-            state: B::from_bits_retain(flags.bits()),
+            remaining: B::from_bits_retain(flags.bits()),
             source: B::from_bits_retain(flags.bits()),
         }
     }
 }
 
 impl<B: 'static> IterNames<B> {
     #[doc(hidden)]
-    pub const fn __private_const_new(flags: &'static [Flag<B>], source: B, state: B) -> Self {
+    pub const fn __private_const_new(flags: &'static [Flag<B>], source: B, remaining: B) -> Self {
         IterNames {
             flags,
             idx: 0,
-            state,
+            remaining,
             source,
         }
     }
@@ -93,41 +93,41 @@ impl<B: 'static> IterNames<B> {
     /// check whether or not there are any bits that didn't correspond
     /// to a valid flag remaining.
     pub fn remaining(&self) -> &B {
-        &self.state
+        &self.remaining
     }
 }
 
 impl<B: Flags> Iterator for IterNames<B> {
     type Item = (&'static str, B);
-    
+
     fn next(&mut self) -> Option<Self::Item> {
         while let Some(flag) = self.flags.get(self.idx) {
             // Short-circuit if our state is empty
-            if self.state.is_empty() {
+            if self.remaining.is_empty() {
                 return None;
             }
 
             self.idx += 1;
 
             let bits = flag.value().bits();
 
-            // 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:
+            // If the flag is set in the original source _and_ it has bits that haven't
+            // been covered by a previous flag yet then yield it. These conditions cover
+            // two cases for multi-bit flags:
             //
-            // 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.source.contains(B::from_bits_retain(bits)) {
-                self.state.remove(B::from_bits_retain(bits));
+            // 1. When flags partially overlap, such as `0b00000001` and `0b00000101`, we'll
+            // yield both flags.
+            // 2. When flags fully overlap, such as in convenience flags that are a shorthand for others,
+            // we won't yield both flags.
+            if self.source.contains(B::from_bits_retain(bits))
+                && self.remaining.intersects(B::from_bits_retain(bits))
+            {
+                self.remaining.remove(B::from_bits_retain(bits));
 
                 return Some((flag.name(), B::from_bits_retain(bits)));
             }
         }
-        
+
         None
     }
 }