1 files changed, 49 insertions, 37 deletions

diff --git a/src/iter.rs b/src/iter.rs
index 4b6210e..7f7ce55 100644
--- a/src/iter.rs
+++ b/src/iter.rs
@@ -1,18 +1,21 @@
-//! Iterating over set flag values.
+/*!
+Yield the bits of a source flags value in a set of contained flags values.
+*/
 
-use crate::{Flags, Flag};
+use crate::{Flag, Flags};
 
-/// An iterator over a set of flags.
-///
-/// Any bits that don't correspond to a valid flag will be yielded
-/// as a final item from the iterator.
+/**
+An iterator over flags values.
+
+This iterator will yield flags values for contained, defined flags first, with any remaining bits yielded
+as a final flags value.
+*/
 pub struct Iter<B: 'static> {
     inner: IterNames<B>,
     done: bool,
 }
 
 impl<B: Flags> Iter<B> {
-    /// Create a new iterator over the given set of flags.
     pub(crate) fn new(flags: &B) -> Self {
         Iter {
             inner: IterNames::new(flags),
@@ -22,10 +25,11 @@ impl<B: Flags> Iter<B> {
 }
 
 impl<B: 'static> Iter<B> {
+    // Used by the `bitflags` macro
     #[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,
         }
     }
@@ -33,18 +37,18 @@ impl<B: 'static> Iter<B> {
 
 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
                 }
@@ -54,80 +58,88 @@ impl<B: Flags> Iterator for Iter<B> {
     }
 }
 
-/// An iterator over a set of flags and their names.
-///
-/// Any bits that don't correspond to a valid flag will be ignored.
+/**
+An iterator over flags values.
+
+This iterator only yields flags values for contained, defined, named flags. Any remaining bits
+won't be yielded, but can be found with the [`IterNames::remaining`] method.
+*/
 pub struct IterNames<B: 'static> {
     flags: &'static [Flag<B>],
     idx: usize,
     source: B,
-    state: B,
+    remaining: B,
 }
 
 impl<B: Flags> IterNames<B> {
-    /// Create a new iterator over the given set of flags.
     pub(crate) fn new(flags: &B) -> Self {
         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> {
+    // Used by the bitflags macro
     #[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,
         }
     }
 
-    /// Get the remaining (unyielded) flags.
+    /// Get a flags value of any remaining bits that haven't been yielded yet.
     ///
     /// Once the iterator has finished, this method can be used to
     /// check whether or not there are any bits that didn't correspond
-    /// to a valid flag remaining.
+    /// to a contained, defined, named 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;
 
+            // Skip unnamed flags
+            if flag.name().is_empty() {
+                continue;
+            }
+
             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
     }
 }