From d8ea0ba667d4e0ca1bf57bf1a51979dab102da49 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Sat, 23 Oct 2021 22:30:16 +1000 Subject: [PATCH 1/7] add a hidden trait to discourage manual impls of BitFlags --- src/bitflags_trait.rs | 5 +- src/lib.rs | 7 ++ tests/compile-fail/cfg/multi.rs | 25 +++++++ tests/compile-fail/cfg/multi.stderr.beta | 35 ++++++++++ .../non_integer_base/all_defined.stderr.beta | 4 +- tests/compile-fail/trait/custom_impl.rs | 65 +++++++++++++++++++ .../trait/custom_impl.stderr.beta | 11 ++++ .../visibility/private_field.stderr.beta | 2 +- tests/compile-pass/cfg/nested-value.rs | 23 +++++++ tests/compile-pass/trait/precedence.rs | 22 +++++++ 10 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 tests/compile-fail/cfg/multi.rs create mode 100644 tests/compile-fail/cfg/multi.stderr.beta create mode 100644 tests/compile-fail/trait/custom_impl.rs create mode 100644 tests/compile-fail/trait/custom_impl.stderr.beta create mode 100644 tests/compile-pass/cfg/nested-value.rs create mode 100644 tests/compile-pass/trait/precedence.rs diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index 0b8facbb..c12c3242 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -1,6 +1,9 @@ use core as _core; -pub trait BitFlags { +#[doc(hidden)] +pub trait ImplementedByBitFlagsMacro {} + +pub trait BitFlags: ImplementedByBitFlagsMacro { type Bits; /// Returns an empty set of flags. fn empty() -> Self; diff --git a/src/lib.rs b/src/lib.rs index 75108972..617afea0 100644 --- a/src/lib.rs +++ b/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 diff --git a/tests/compile-fail/cfg/multi.rs b/tests/compile-fail/cfg/multi.rs new file mode 100644 index 00000000..461db914 --- /dev/null +++ b/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()); + } +} diff --git a/tests/compile-fail/cfg/multi.stderr.beta b/tests/compile-fail/cfg/multi.stderr.beta new file mode 100644 index 00000000..75960547 --- /dev/null +++ b/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) diff --git a/tests/compile-fail/non_integer_base/all_defined.stderr.beta b/tests/compile-fail/non_integer_base/all_defined.stderr.beta index 1f0fb5cf..78bb2a51 100644 --- a/tests/compile-fail/non_integer_base/all_defined.stderr.beta +++ b/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) diff --git a/tests/compile-fail/trait/custom_impl.rs b/tests/compile-fail/trait/custom_impl.rs new file mode 100644 index 00000000..66875898 --- /dev/null +++ b/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 { + 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() { } diff --git a/tests/compile-fail/trait/custom_impl.stderr.beta b/tests/compile-fail/trait/custom_impl.stderr.beta new file mode 100644 index 00000000..8053fdc6 --- /dev/null +++ b/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:6:21 + | +6 | pub trait BitFlags: ImplementedByBitFlagsMacro { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` diff --git a/tests/compile-fail/visibility/private_field.stderr.beta b/tests/compile-fail/visibility/private_field.stderr.beta index 58a04660..e1146989 100644 --- a/tests/compile-fail/visibility/private_field.stderr.beta +++ b/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(); - | ^^ + | ++ diff --git a/tests/compile-pass/cfg/nested-value.rs b/tests/compile-pass/cfg/nested-value.rs new file mode 100644 index 00000000..21ae278e --- /dev/null +++ b/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()); + } +} diff --git a/tests/compile-pass/trait/precedence.rs b/tests/compile-pass/trait/precedence.rs new file mode 100644 index 00000000..f61db85d --- /dev/null +++ b/tests/compile-pass/trait/precedence.rs @@ -0,0 +1,22 @@ +use bitflags::{bitflags, BitFlags}; + +bitflags! { + struct Flags: u32 { + const A = 0b00000001; + } +} + +impl From for Flags { + fn from(v: u32) -> Flags { + Flags::from_bits_truncate(v) + } +} + +fn all_from_trait() { + let _ = F::all(); +} + +fn main() { + all_from_trait::(); + let _ = Flags::all(); +} From f0a2bf65d95e196a6f44e48af5ee160675784817 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Sun, 24 Oct 2021 06:21:42 +1000 Subject: [PATCH 2/7] add a stub doc to BitFlags --- src/bitflags_trait.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index c12c3242..2e5f5966 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -3,6 +3,10 @@ use core as _core; #[doc(hidden)] pub trait ImplementedByBitFlagsMacro {} +/// A flags enum. +/// +/// Manual implementations of this trait are not supported. +/// It should only be automatically implemented through the `bitflags!` macro. pub trait BitFlags: ImplementedByBitFlagsMacro { type Bits; /// Returns an empty set of flags. From 9e770dd52cd20d26c18f8a07ad6d82452bbb61f2 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Mon, 25 Oct 2021 09:04:06 +1000 Subject: [PATCH 3/7] support TRYBUILD=overwrite in our beta test scheme --- .../trait/custom_impl.stderr.beta | 18 ++++---- tests/compile.rs | 45 +++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/tests/compile-fail/trait/custom_impl.stderr.beta b/tests/compile-fail/trait/custom_impl.stderr.beta index 8053fdc6..02b2b91b 100644 --- a/tests/compile-fail/trait/custom_impl.stderr.beta +++ b/tests/compile-fail/trait/custom_impl.stderr.beta @@ -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` - | + --> $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:6:21 - | -6 | pub trait BitFlags: ImplementedByBitFlagsMacro { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` + --> $DIR/bitflags_trait.rs:10:21 + | +10 | pub trait BitFlags: ImplementedByBitFlagsMacro { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` diff --git a/tests/compile.rs b/tests/compile.rs index ed02d01e..f3ee9122 100644 --- a/tests/compile.rs +++ b/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) -> 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) -> 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) -> io::Result<()> { + fs::remove_file(path)?; + + Ok(()) +} + +#[rustversion::not(beta)] +fn remove_beta_stderr(_: impl AsRef) -> io::Result<()> { + Ok(()) +} + #[rustversion::beta] fn rename_beta_stderr(from: impl AsRef, to: impl AsRef) -> io::Result<()> { fs::copy(from, to)?; From df35be860bddb83bd497cdc8f81b7b3e6589d843 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Mon, 25 Oct 2021 09:05:20 +1000 Subject: [PATCH 4/7] update docs for BitFlags Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com> --- src/bitflags_trait.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index 2e5f5966..944cb589 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -3,10 +3,9 @@ use core as _core; #[doc(hidden)] pub trait ImplementedByBitFlagsMacro {} -/// A flags enum. -/// -/// Manual implementations of this trait are not supported. -/// It should only be automatically implemented through the `bitflags!` macro. +/// 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. From 368822a5a6ed817a6a3dce7f7711b1cd0d48c1ef Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Mon, 25 Oct 2021 09:05:43 +1000 Subject: [PATCH 5/7] Remove unnecessary core use statement Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com> --- src/bitflags_trait.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bitflags_trait.rs b/src/bitflags_trait.rs index 944cb589..3331ed27 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -1,5 +1,3 @@ -use core as _core; - #[doc(hidden)] pub trait ImplementedByBitFlagsMacro {} From b924d4c6487041ae491b860b6b8e21739e9602c5 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Mon, 25 Oct 2021 09:06:38 +1000 Subject: [PATCH 6/7] update expected output for custom impl --- .../compile-fail/trait/custom_impl.stderr.beta | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/compile-fail/trait/custom_impl.stderr.beta b/tests/compile-fail/trait/custom_impl.stderr.beta index 02b2b91b..7f3b4b1e 100644 --- a/tests/compile-fail/trait/custom_impl.stderr.beta +++ b/tests/compile-fail/trait/custom_impl.stderr.beta @@ -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` - | + --> $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:10:21 - | -10 | pub trait BitFlags: ImplementedByBitFlagsMacro { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` + --> $DIR/bitflags_trait.rs:7:21 + | +7 | pub trait BitFlags: ImplementedByBitFlagsMacro { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BitFlags` From faa453b13b44dc8e9a48262b09632ef32e9fccc0 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Tue, 26 Oct 2021 07:24:37 +1000 Subject: [PATCH 7/7] no need to use our re-export of core in the trait definition --- 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 3331ed27..0ffee465 100644 --- a/src/bitflags_trait.rs +++ b/src/bitflags_trait.rs @@ -14,7 +14,7 @@ pub trait BitFlags: ImplementedByBitFlagsMacro { 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 + fn from_bits(bits: Self::Bits) -> Option where Self: Sized; /// Convert from underlying bit representation, dropping any bits /// that do not correspond to flags.