Merge pull request #261 from KodrAus/feat/bitflags-macro

Add a hidden trait to discourage manual impls of BitFlags

src/bitflags_trait.rs

@@ -1,6 +1,10 @@
-use core as _core;
+#[doc(hidden)]
+pub trait ImplementedByBitFlagsMacro {}
 
-pub trait BitFlags {
+/// A trait that is automatically implemented for all bitflags.
+///
+/// It should not be implemented manually.
+pub trait BitFlags: ImplementedByBitFlagsMacro {
     type Bits;
     /// Returns an empty set of flags.
     fn empty() -> Self;
@@ -10,7 +14,7 @@ pub trait BitFlags {
     fn bits(&self) -> Self::Bits;
     /// Convert from underlying bit representation, unless that
     /// representation contains bits that do not correspond to a flag.
-    fn from_bits(bits: Self::Bits) -> _core::option::Option<Self>
+    fn from_bits(bits: Self::Bits) -> Option<Self>
     where Self: Sized;
     /// Convert from underlying bit representation, dropping any bits
     /// that do not correspond to flags.

src/lib.rs

@@ -286,6 +286,11 @@ pub use bitflags_trait::BitFlags;
 
 mod bitflags_trait;
 
+#[doc(hidden)]
+pub mod __private {
+    pub use crate::bitflags_trait::ImplementedByBitFlagsMacro;
+}
+
 /// The macro used to generate the flag structure.
 ///
 /// See the [crate level docs](../bitflags/index.html) for complete documentation.
@@ -885,6 +890,8 @@ macro_rules! __impl_bitflags {
                 $BitFlags::set(self, other, value)
             }
         }
+
+        impl $crate::__private::ImplementedByBitFlagsMacro for $BitFlags {}
     };
 
     // Every attribute that the user writes on a const is applied to the

tests/compile-fail/cfg/multi.rs

@@ -0,0 +1,25 @@
+#[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());
+    }
+}

tests/compile-fail/cfg/multi.stderr.beta

@@ -0,0 +1,35 @@
+error[E0428]: the name `FOO` is defined multiple times
+  --> $DIR/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_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0428]: the name `FOO` is defined multiple times
+  --> $DIR/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)

tests/compile-fail/non_integer_base/all_defined.stderr.beta

@@ -10,7 +10,7 @@ error[E0308]: mismatched types
 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)
+    = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0308]: mismatched types
    --> $DIR/all_defined.rs:115:1
@@ -24,4 +24,4 @@ error[E0308]: mismatched types
 121 | | }
     | |_^ expected struct `MyInt`, found integer
     |
-    = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)
+    = note: this error originates in the macro `__impl_all_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)

tests/compile-fail/trait/custom_impl.rs

@@ -0,0 +1,65 @@
+use bitflags::BitFlags;
+
+pub struct BootlegFlags(u32);
+
+impl BitFlags for BootlegFlags {
+    type Bits = u32;
+
+    fn empty() -> Self {
+        unimplemented!()
+    }
+
+    fn all() -> Self {
+        unimplemented!()
+    }
+
+    fn bits(&self) -> u32 {
+        unimplemented!()
+    }
+
+    fn from_bits(_: u32) -> Option<BootlegFlags> {
+        unimplemented!()
+    }
+
+    fn from_bits_truncate(_: u32) -> BootlegFlags {
+        unimplemented!()
+    }
+
+    unsafe fn from_bits_unchecked(_: u32) -> BootlegFlags {
+        unimplemented!()
+    }
+
+    fn is_empty(&self) -> bool {
+        unimplemented!()
+    }
+
+    fn is_all(&self) -> bool {
+        unimplemented!()
+    }
+
+    fn intersects(&self, _: BootlegFlags) -> bool {
+        unimplemented!()
+    }
+
+    fn contains(&self, _: BootlegFlags) -> bool {
+        unimplemented!()
+    }
+
+    fn insert(&mut self, _: BootlegFlags) {
+        unimplemented!()
+    }
+
+    fn remove(&mut self, _: BootlegFlags) {
+        unimplemented!()
+    }
+
+    fn toggle(&mut self, _: BootlegFlags) {
+        unimplemented!()
+    }
+
+    fn set(&mut self, _: BootlegFlags, value: bool) {
+        unimplemented!()
+    }
+}
+
+fn main() { }

tests/compile-fail/trait/custom_impl.stderr.beta

@@ -0,0 +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`
+  |
+note: required by a bound in `BitFlags`
+ --> $DIR/bitflags_trait.rs:7:21
+  |
+7 | pub trait BitFlags: ImplementedByBitFlagsMacro {
+  |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags`

tests/compile-fail/visibility/private_field.stderr.beta

@@ -7,4 +7,4 @@ error[E0616]: field `bits` of struct `Flags1` is private
 help: a method `bits` also exists, call it with parentheses
    |
 12 |     let flag1 = example::Flags1::FLAG_A.bits();
-   |                                             ^^
+   |                                             ++

tests/compile-pass/cfg/nested-value.rs

@@ -0,0 +1,23 @@
+#[macro_use]
+extern crate bitflags;
+
+bitflags! {
+    pub struct Flags: u32 {
+        const FOO = {
+            #[cfg(target_os = "linux")] { 1 }
+            #[cfg(not(target_os = "linux"))] { 2 }
+        };
+    }
+}
+
+fn main() {
+    #[cfg(target_os = "linux")]
+    {
+        assert_eq!(1, Flags::FOO.bits());
+    }
+
+    #[cfg(not(target_os = "linux"))]
+    {
+        assert_eq!(2, Flags::FOO.bits());
+    }
+}

tests/compile-pass/trait/precedence.rs

@@ -0,0 +1,22 @@
+use bitflags::{bitflags, BitFlags};
+
+bitflags! {
+    struct Flags: u32 {
+        const A = 0b00000001;
+    }
+}
+
+impl From<u32> for Flags {
+    fn from(v: u32) -> Flags {
+        Flags::from_bits_truncate(v)
+    }
+}
+
+fn all_from_trait<F: BitFlags>() {
+    let _ = F::all();
+}
+
+fn main() {
+    all_from_trait::<Flags>();
+    let _ = Flags::all();
+}

tests/compile.rs

@@ -1,4 +1,5 @@
 use std::{
+    env,
     fs,
     ffi::OsStr,
     io,
@@ -12,7 +13,14 @@ fn fail() {
     prepare_stderr_files("tests/compile-fail").unwrap();
 
     let t = trybuild::TestCases::new();
+
     t.compile_fail("tests/compile-fail/**/*.rs");
+
+    // `trybuild` will run its tests on `drop`
+    // We want to get a chance to use its output first
+    drop(t);
+
+    overwrite_stderr_files("tests/compile-fail").unwrap();
 }
 
 #[test]
@@ -50,6 +58,43 @@ fn prepare_stderr_files(path: impl AsRef<Path>) -> io::Result<()> {
     Ok(())
 }
 
+// If we want to overwrite the expected compiler output then rename it
+// to use our `.stderr.beta` convention. Otherwise the renamed file won't
+// actually get picked up on the next run
+fn overwrite_stderr_files(path: impl AsRef<Path>) -> io::Result<()> {
+    if env::var("TRYBUILD").ok().filter(|o| o == "overwrite").is_some() {
+        for entry in WalkDir::new(path) {
+            let entry = entry?;
+
+            // Look for any `.stderr` files and rename them to `.stderr.beta`
+            // If there's an existing `.beta` file then we want to remove it
+            if entry.path().extension().and_then(OsStr::to_str) == Some("stderr") {
+                let renamed = entry.path().with_extension("stderr.beta");
+
+                if renamed.exists() {
+                    remove_beta_stderr(&renamed)?;
+                }
+
+                rename_beta_stderr(entry.path(), renamed)?;
+            }
+        }
+    }
+
+    Ok(())
+}
+
+#[rustversion::beta]
+fn remove_beta_stderr(path: impl AsRef<Path>) -> io::Result<()> {
+    fs::remove_file(path)?;
+
+    Ok(())
+}
+
+#[rustversion::not(beta)]
+fn remove_beta_stderr(_: impl AsRef<Path>) -> io::Result<()> {
+    Ok(())
+}
+
 #[rustversion::beta]
 fn rename_beta_stderr(from: impl AsRef<Path>, to: impl AsRef<Path>) -> io::Result<()> {
     fs::copy(from, to)?;