From 8b10fe14edd76e362b2e129f3be4969f37ac09e7 Mon Sep 17 00:00:00 2001 From: Arturo Castro Date: Tue, 19 Apr 2022 11:53:17 +0200 Subject: [PATCH 1/6] from_bits_(truncate) fail with composite flags When a bitflags type contains composite flags like: ```rs bitflags! { struct Flags: u8 { const A = 0b00000001; const BC = 0b00000110; } } ``` from_bits and from_bits_truncate would not work as expected allowing flags that are not declared. Fixes #275 --- src/lib.rs | 63 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fe52de57..0e311d8f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -559,18 +559,41 @@ macro_rules! __impl_bitflags { /// representation contains bits that do not correspond to a flag. #[inline] pub const fn from_bits(bits: $T) -> $crate::_core::option::Option { - if (bits & !Self::all().bits()) == 0 { - $crate::_core::option::Option::Some(Self { bits }) - } else { - $crate::_core::option::Option::None + if bits == 0 { + return Some(Self{ bits }) } + + $( + #[allow(unused_doc_comments, unused_attributes)] + $(#[$attr $($args)*])* + if bits & Self::$Flag.bits == Self::$Flag.bits { + return Some(Self{ bits }) + } + )* + + None } /// Convert from underlying bit representation, dropping any bits /// that do not correspond to flags. #[inline] pub const fn from_bits_truncate(bits: $T) -> Self { - Self { bits: bits & Self::all().bits } + if bits == 0 { + return Self{ bits } + } + + #[allow(unused_mut)] + let mut truncated = 0; + + $( + #[allow(unused_doc_comments, unused_attributes)] + $(#[$attr $($args)*])* + if bits & Self::$Flag.bits == Self::$Flag.bits { + truncated |= Self::$Flag.bits + } + )* + + Self { bits: truncated } } /// Convert from underlying bit representation, preserving all @@ -1798,3 +1821,33 @@ mod tests { } } } + + + #[test] + fn test_from_bits_edge_cases() { + bitflags! { + struct Flags: u8 { + const A = 0b00000001; + const BC = 0b00000110; + } + } + + + let flags = Flags::from_bits(0b00000100); + assert!(flags.is_none()); + } + + #[test] + fn test_from_bits_truncate_edge_cases() { + bitflags! { + struct Flags: u8 { + const A = 0b00000001; + const BC = 0b00000110; + } + } + + + let flags = Flags::from_bits_truncate(0b00000100); + assert!(flags.is_empty()); + } +} \ No newline at end of file From 8a5eb88dbd884e7bbd24dc1022da26d0e3423f17 Mon Sep 17 00:00:00 2001 From: Arturo Castro Date: Tue, 19 Apr 2022 12:30:44 +0200 Subject: [PATCH 2/6] fix previous commit extra brace commited accidentally due to partial staging --- src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0e311d8f..46542b05 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1820,8 +1820,6 @@ mod tests { const D = 8; } } -} - #[test] fn test_from_bits_edge_cases() { From 44c7063fe4c40ffed0d7e5d0dbae4a32ed91022b Mon Sep 17 00:00:00 2001 From: Arturo Castro Date: Tue, 19 Apr 2022 12:36:35 +0200 Subject: [PATCH 3/6] serde::Serialize -> serde_derive::Serialize serde:: doesn't work on tests on CI --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 46542b05..a5cbf53f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1812,7 +1812,7 @@ mod tests { } bitflags! { - #[derive(serde::Serialize, serde::Deserialize)] + #[derive(serde_derive::Serialize, serde_derive::Deserialize)] struct SerdeFlags: u32 { const A = 1; const B = 2; From 00fddb89cb80fec095b3341b602a6584a6a3cec9 Mon Sep 17 00:00:00 2001 From: Arturo Castro Date: Wed, 20 Apr 2022 13:46:21 +0200 Subject: [PATCH 4/6] fix fail tests for changes to fix the from_bits functions --- .../non_integer_base/all_defined.stderr.beta | 55 ++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) 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 6fada425..ba986b5c 100644 --- a/tests/compile-fail/non_integer_base/all_defined.stderr.beta +++ b/tests/compile-fail/non_integer_base/all_defined.stderr.beta @@ -49,8 +49,59 @@ error[E0308]: mismatched types = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) help: try wrapping the expression in `MyInt` | -562 | if (bits & !Self::all().bits()) == MyInt(0) { - | ++++++ + +562 | if bits == MyInt(0) { + | ++++++ + + +error[E0308]: mismatched types + --> $DIR/all_defined.rs:115:1 + | +115 | / bitflags! { +116 | | struct Flags128: MyInt { +117 | | const A = MyInt(0b0000_0001u8); +118 | | const B = MyInt(0b0000_0010u8); +119 | | const C = MyInt(0b0000_0100u8); +120 | | } +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) +help: try wrapping the expression in `MyInt` + | +581 | if bits == MyInt(0) { + | ++++++ + + +error[E0277]: no implementation for `{integer} |= MyInt` + --> $DIR/all_defined.rs:115:1 + | +115 | / bitflags! { +116 | | struct Flags128: MyInt { +117 | | const A = MyInt(0b0000_0001u8); +118 | | const B = MyInt(0b0000_0010u8); +119 | | const C = MyInt(0b0000_0100u8); +120 | | } +121 | | } + | |_^ no implementation for `{integer} |= MyInt` + | + = help: the trait `BitOrAssign` is not implemented for `{integer}` + = 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 + | +115 | / bitflags! { +116 | | struct Flags128: MyInt { +117 | | const A = MyInt(0b0000_0001u8); +118 | | const B = MyInt(0b0000_0010u8); +119 | | const C = MyInt(0b0000_0100u8); +120 | | } +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) +help: try wrapping the expression in `MyInt` + | +596 | Self { bits: MyInt(truncated) } + | ++++++ + error[E0308]: mismatched types --> $DIR/all_defined.rs:115:1 From 7253391691d01e9709a24616c32c2f0bb10767f1 Mon Sep 17 00:00:00 2001 From: Arturo Castro Date: Tue, 26 Apr 2022 11:34:55 +0200 Subject: [PATCH 5/6] fix from_bits was still wrong for some edge cases --- src/lib.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a5cbf53f..a354d133 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -559,19 +559,12 @@ macro_rules! __impl_bitflags { /// representation contains bits that do not correspond to a flag. #[inline] pub const fn from_bits(bits: $T) -> $crate::_core::option::Option { - if bits == 0 { - return Some(Self{ bits }) + let truncated = Self::from_bits_truncate(bits).bits; + if truncated == bits { + Some(Self{ bits }) + } else { + None } - - $( - #[allow(unused_doc_comments, unused_attributes)] - $(#[$attr $($args)*])* - if bits & Self::$Flag.bits == Self::$Flag.bits { - return Some(Self{ bits }) - } - )* - - None } /// Convert from underlying bit representation, dropping any bits @@ -1832,7 +1825,9 @@ mod tests { let flags = Flags::from_bits(0b00000100); - assert!(flags.is_none()); + assert_eq!(flags, None); + let flags = Flags::from_bits(0b00000101); + assert_eq!(flags, None); } #[test] @@ -1846,6 +1841,8 @@ mod tests { let flags = Flags::from_bits_truncate(0b00000100); - assert!(flags.is_empty()); + assert_eq!(flags, Flags::empty()); + let flags = Flags::from_bits_truncate(0b00000101); + assert_eq!(flags, Flags::A); } } \ No newline at end of file From ea776b76b38e9c3dd2ba4271aef4270eeb6ad226 Mon Sep 17 00:00:00 2001 From: Arturo Castro Date: Tue, 26 Apr 2022 11:38:59 +0200 Subject: [PATCH 6/6] fix should fail output changed on last commit --- .../non_integer_base/all_defined.stderr.beta | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) 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 ba986b5c..0607f234 100644 --- a/tests/compile-fail/non_integer_base/all_defined.stderr.beta +++ b/tests/compile-fail/non_integer_base/all_defined.stderr.beta @@ -49,25 +49,7 @@ error[E0308]: mismatched types = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) help: try wrapping the expression in `MyInt` | -562 | if bits == MyInt(0) { - | ++++++ + - -error[E0308]: mismatched types - --> $DIR/all_defined.rs:115:1 - | -115 | / bitflags! { -116 | | struct Flags128: MyInt { -117 | | const A = MyInt(0b0000_0001u8); -118 | | const B = MyInt(0b0000_0010u8); -119 | | const C = MyInt(0b0000_0100u8); -120 | | } -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) -help: try wrapping the expression in `MyInt` - | -581 | if bits == MyInt(0) { +574 | if bits == MyInt(0) { | ++++++ + error[E0277]: no implementation for `{integer} |= MyInt` @@ -100,7 +82,7 @@ error[E0308]: mismatched types = note: this error originates in the macro `__impl_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info) help: try wrapping the expression in `MyInt` | -596 | Self { bits: MyInt(truncated) } +589 | Self { bits: MyInt(truncated) } | ++++++ + error[E0308]: mismatched types