Debug impl changes

enumflags/src/formatting.rs

@@ -0,0 +1,80 @@
+use core::fmt::{self, Debug, Binary};
+
+// Format an iterator of flags into "A | B | etc"
+pub struct FlagFormatter<I>(pub I);
+
+impl<T: Debug, I: Clone + Iterator<Item=T>> Debug for FlagFormatter<I> {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        let mut iter = self.0.clone();
+        if let Some(val) = iter.next() {
+            Debug::fmt(&val, fmt)?;
+            for val in iter {
+                fmt.write_str(" | ")?;
+                Debug::fmt(&val, fmt)?;
+            }
+            Ok(())
+        } else {
+            // convention would print "<empty>" or similar here, but this is an
+            // internal API that is never called that way, so just do nothing.
+            Ok(())
+        }
+    }
+}
+
+// A formatter that obeys format arguments but falls back to binary when
+// no explicit format is requested. Supports {:08?}, {:08x?}, etc.
+pub struct DebugBinaryFormatter<'a, F>(pub &'a F);
+
+impl<'a, F: Debug + Binary + 'a> Debug for DebugBinaryFormatter<'a, F> {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        // Check if {:x?} or {:X?} was used; this is determined via the
+        // discriminator of core::fmt::FlagV1::{DebugLowerHex, DebugUpperHex},
+        // which is not an accessible type: https://github.com/rust-lang/rust/blob/d65e272a9fe3e61aa5f229c5358e35a909435575/src/libcore/fmt/mod.rs#L306
+        // See also: https://github.com/rust-lang/rfcs/pull/2226
+        #[allow(deprecated)]
+        let format_hex = fmt.flags() >> 4;
+        let width = fmt.width().unwrap_or(0);
+
+        if format_hex & 1 != 0 { // FlagV1::DebugLowerHex
+            write!(fmt, "{:#0width$x?}", &self.0, width = width)
+        } else if format_hex & 2 != 0 { // FlagV1::DebugUpperHex
+            write!(fmt, "{:#0width$X?}", &self.0, width = width)
+        } else {
+            // Fall back to binary otheriwse
+            write!(fmt, "{:#0width$b}", &self.0, width = width)
+        }
+    }
+}
+
+#[test]
+fn flag_formatter() {
+    use core::iter;
+
+    macro_rules! assert_fmt {
+        ($fmt:expr, $expr:expr, $expected:expr) => {
+            assert_eq!(format!($fmt, FlagFormatter($expr)), $expected)
+        };
+    }
+
+    assert_fmt!("{:?}", iter::empty::<u8>(), "");
+    assert_fmt!("{:?}", iter::once(1), "1");
+    assert_fmt!("{:?}", [1, 2].iter(), "1 | 2");
+    assert_fmt!("{:?}", [1, 2, 10].iter(), "1 | 2 | 10");
+    assert_fmt!("{:02x?}", [1, 2, 10].iter(), "01 | 02 | 0a");
+    assert_fmt!("{:#04X?}", [1, 2, 10].iter(), "0x01 | 0x02 | 0x0A");
+}
+
+#[test]
+fn debug_binary_formatter() {
+    macro_rules! assert_fmt {
+        ($fmt:expr, $expr:expr, $expected:expr) => {
+            assert_eq!(format!($fmt, DebugBinaryFormatter(&$expr)), $expected)
+        };
+    }
+
+    assert_fmt!("{:?}", 10, "0b1010");
+    assert_fmt!("{:#?}", 10, "0b1010");
+    assert_fmt!("{:010?}", 10, "0b00001010");
+    assert_fmt!("{:010x?}", 10, "0x0000000a");
+    assert_fmt!("{:#010X?}", 10, "0x0000000A");
+}

enumflags/src/lib.rs

@@ -43,19 +43,24 @@
 //! }
 //! ```
 #![warn(missing_docs)]
-#![no_std]
+#![cfg_attr(not(test), no_std)]
 
+#[cfg(test)]
+extern crate core;
 use core::{fmt, cmp, ops};
 use core::iter::FromIterator;
 
 // Re-export libcore so the macro doesn't inject "extern crate" downstream.
 #[doc(hidden)]
 pub mod _internal {
     pub mod core {
-        pub use core::{convert, fmt, iter, option, ops};
+        pub use core::{convert, option, ops};
     }
 }
 
+// Internal debug formatting implementations
+mod formatting;
+
 /// Sealed trait
 mod details {
     use core::ops::{BitAnd, BitOr, BitXor, Not};
@@ -81,17 +86,6 @@ mod details {
 
 use details::BitFlagNum;
 
-/// A trait automatically implemented by `derive(EnumFlags)` on `T` to enable debug printing of
-/// `BitFlags<T>`. This is necessary because the names of the variants are needed.
-#[doc(hidden)]
-pub trait BitFlagsFmt
-where
-    Self: RawBitFlags,
-{
-    /// The implementation of Debug redirects here.
-    fn fmt(flags: BitFlags<Self>, f: &mut fmt::Formatter) -> fmt::Result;
-}
-
 /// A trait automatically implemented by `derive(EnumFlags)` to make the enum a valid type parameter
 /// for BitFlags.
 #[doc(hidden)]
@@ -107,6 +101,13 @@ pub trait RawBitFlags: Copy + Clone + 'static {
 
     /// Return a slice that contains each variant exactly one.
     fn flag_list() -> &'static [Self];
+
+    /// Return the name of the type for debug formatting purposes.
+    ///
+    /// This is typically `BitFlags<EnumName>`
+    fn bitflags_type_name() -> &'static str {
+        "BitFlags"
+    }
 }
 
 /// Represents a set of flags of some type `T`.
@@ -119,10 +120,76 @@ pub struct BitFlags<T: RawBitFlags> {
 
 impl<T> fmt::Debug for BitFlags<T>
 where
-    T: RawBitFlags + BitFlagsFmt,
+    T: RawBitFlags + fmt::Debug,
+    T::Type: fmt::Binary + fmt::Debug,
+{
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        let name = T::bitflags_type_name();
+        let bits = formatting::DebugBinaryFormatter(&self.val);
+        let iter = if !self.is_empty() {
+            let iter = T::flag_list().iter().filter(|&&flag| self.contains(flag));
+            Some(formatting::FlagFormatter(iter))
+        } else {
+            None
+        };
+
+        if !fmt.alternate() {
+            // Concise tuple formatting is a better default
+            let mut debug = fmt.debug_tuple(name);
+            debug.field(&bits);
+            if let Some(iter) = iter {
+                debug.field(&iter);
+            }
+            debug.finish()
+        } else {
+            // Pretty-printed tuples are ugly and hard to read, so use struct format
+            let mut debug = fmt.debug_struct(name);
+            debug.field("bits", &bits);
+            if let Some(iter) = iter {
+                debug.field("flags", &iter);
+            }
+            debug.finish()
+        }
+    }
+}
+
+impl<T> fmt::Binary for BitFlags<T>
+where
+    T: RawBitFlags,
+    T::Type: fmt::Binary,
+{
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Binary::fmt(&self.bits(), fmt)
+    }
+}
+
+impl<T> fmt::Octal for BitFlags<T>
+where
+    T: RawBitFlags,
+    T::Type: fmt::Octal,
+{
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Octal::fmt(&self.bits(), fmt)
+    }
+}
+
+impl<T> fmt::LowerHex for BitFlags<T>
+where
+    T: RawBitFlags,
+    T::Type: fmt::LowerHex,
+{
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        fmt::LowerHex::fmt(&self.bits(), fmt)
+    }
+}
+
+impl<T> fmt::UpperHex for BitFlags<T>
+where
+    T: RawBitFlags,
+    T::Type: fmt::UpperHex,
 {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        T::fmt(self.clone(), fmt)
+        fmt::UpperHex::fmt(&self.bits(), fmt)
     }
 }
 

enumflags_derive/src/lib.rs

@@ -79,7 +79,6 @@ fn extract_repr(attrs: &[syn::Attribute]) -> Option<syn::Ident> {
 fn gen_enumflags(ident: &Ident, item: &DeriveInput, data: &DataEnum) -> TokenStream {
     let span  = Span::call_site();
     let variants = data.variants.iter().map(|v| &v.ident);
-    let variants_names = variants.clone().map(ToString::to_string);
     let flag_values: Vec<_> = data.variants.iter()
         .map(|v| v.discriminant.as_ref().map(|d| fold_expr(&d.1)).expect("No discriminant")).collect();
     let variants_len = flag_values.len();
@@ -157,30 +156,6 @@ fn gen_enumflags(ident: &Ident, item: &DeriveInput, data: &DataEnum) -> TokenStr
                 }
             }
 
-            impl ::enumflags2::BitFlagsFmt for #ident {
-                fn fmt(flags: ::enumflags2::BitFlags<#ident>,
-                       fmt: &mut #std_path::fmt::Formatter)
-                       -> #std_path::fmt::Result {
-                    use ::enumflags2::RawBitFlags;
-                    use #std_path::iter::Iterator;
-                    const VARIANT_NAMES: [&'static str; #variants_len] = [#(#variants_names, )*];
-                    let mut vals =
-                        VARIANTS.iter().zip(VARIANT_NAMES.iter())
-                        .filter(|&(&val, _)| val as #ty & flags.bits() != 0)
-                        .map(|(_, name)| name);
-                    write!(fmt, "BitFlags<{}>(0b{:b}",
-                           stringify!(#ident),
-                           flags.bits())?;
-                    if let #std_path::option::Option::Some(val) = vals.next() {
-                        write!(fmt, ", {}", val)?;
-                        for val in vals {
-                            write!(fmt, " | {}", val)?;
-                        }
-                    }
-                    write!(fmt, ")")
-                }
-            }
-
             impl ::enumflags2::RawBitFlags for #ident {
                 type Type = #ty;
 
@@ -195,6 +170,10 @@ fn gen_enumflags(ident: &Ident, item: &DeriveInput, data: &DataEnum) -> TokenStr
                 fn flag_list() -> &'static [Self] {
                     &VARIANTS
                 }
+
+                fn bitflags_type_name() -> &'static str {
+                    concat!("BitFlags<", stringify!(#ident), ">")
+                }
             }
         }
     }

example/src/main.rs

@@ -2,7 +2,7 @@ extern crate enumflags2;
 #[macro_use]
 extern crate enumflags2_derive;
 use enumflags2::*;
-#[derive(EnumFlags, Copy, Clone)]
+#[derive(EnumFlags, Copy, Clone, Debug)]
 #[repr(u8)]
 pub enum Test {
     A = 0b0001,

test_suite/Cargo.toml

@@ -37,3 +37,8 @@ edition = "2018"
 name = "bitflags-test-no-implicit-prelude-2018"
 path = "tests/no_implicit_prelude_2018.rs"
 edition = "2018"
+
+[[test]]
+name = "bitflags-formatting"
+path = "tests/formatting.rs"
+edition = "2018"

test_suite/tests/formatting.rs

@@ -0,0 +1,74 @@
+use enumflags2_derive::EnumFlags;
+
+include!("../common.rs");
+
+#[test]
+fn debug_format() {
+    use enumflags2::BitFlags;
+
+    // Assert that our Debug output format meets expectations
+
+    assert_eq!(
+        format!("{:?}", BitFlags::<Test>::all()),
+        "BitFlags<Test>(0b1111, A | B | C | D)"
+    );
+
+    assert_eq!(
+        format!("{:?}", BitFlags::<Test>::empty()),
+        "BitFlags<Test>(0b0)"
+    );
+
+    assert_eq!(
+        format!("{:04x?}", BitFlags::<Test>::all()),
+        "BitFlags<Test>(0x0f, A | B | C | D)"
+    );
+
+    assert_eq!(
+        format!("{:04X?}", BitFlags::<Test>::all()),
+        "BitFlags<Test>(0x0F, A | B | C | D)"
+    );
+
+    // Also check alternate struct formatting
+
+    assert_eq!(
+        format!("{:#010?}", BitFlags::<Test>::all()),
+"BitFlags<Test> {
+    bits: 0b00001111,
+    flags: A | B | C | D,
+}"
+    );
+
+    assert_eq!(
+        format!("{:#?}", BitFlags::<Test>::empty()),
+"BitFlags<Test> {
+    bits: 0b0,
+}"
+    );
+}
+
+#[test]
+fn format() {
+    use enumflags2::BitFlags;
+
+    // Assert BitFlags<T> impls fmt::{Binary, Octal, LowerHex, UpperHex}
+
+    assert_eq!(
+        format!("{:b}", BitFlags::<Test>::all()),
+        "1111"
+    );
+
+    assert_eq!(
+        format!("{:o}", BitFlags::<Test>::all()),
+        "17"
+    );
+
+    assert_eq!(
+        format!("{:x}", BitFlags::<Test>::all()),
+        "f"
+    );
+
+    assert_eq!(
+        format!("{:#04X}", BitFlags::<Test>::all()),
+        "0x0F"
+    );
+}