From c88e58f37efd32bdb7d15a039109d3b8ac48cf11 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Thu, 2 Feb 2023 14:52:38 +1000 Subject: [PATCH 01/16] sketch out FromStr impls for flags types --- Cargo.toml | 1 + src/external.rs | 12 ++--- src/external/serde_support.rs | 94 ++++++++++++++++++++++++----------- src/internal.rs | 37 ++++++++++++++ src/lib.rs | 7 ++- src/parser.rs | 40 +++++++++++++++ 6 files changed, 151 insertions(+), 40 deletions(-) create mode 100644 src/parser.rs diff --git a/Cargo.toml b/Cargo.toml index 04941307..632db9e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ serde_derive = "1.0" serde_json = "1.0" [features] +std = [] example_generated = [] rustc-dep-of-std = ["core", "compiler_builtins"] diff --git a/src/external.rs b/src/external.rs index 3076710f..2f15deb2 100644 --- a/src/external.rs +++ b/src/external.rs @@ -48,9 +48,9 @@ macro_rules! __impl_external_bitflags_serde { &self, serializer: S, ) -> $crate::__private::core::result::Result { - $crate::__private::serde_support::serialize_bits_default( + $crate::__private::serde_support::serialize_bits_default::<$InternalBitFlags, $T, S>( $crate::__private::core::stringify!($InternalBitFlags), - &self.bits, + &self, serializer, ) } @@ -60,14 +60,10 @@ macro_rules! __impl_external_bitflags_serde { fn deserialize>( deserializer: D, ) -> $crate::__private::core::result::Result { - let bits = $crate::__private::serde_support::deserialize_bits_default( + $crate::__private::serde_support::deserialize_bits_default::<$InternalBitFlags, $T, D>( $crate::__private::core::stringify!($InternalBitFlags), deserializer, - )?; - - $crate::__private::core::result::Result::Ok($InternalBitFlags::from_bits_retain( - bits, - )) + ) } } }; diff --git a/src/external/serde_support.rs b/src/external/serde_support.rs index 5d30d1c3..563983a0 100644 --- a/src/external/serde_support.rs +++ b/src/external/serde_support.rs @@ -1,56 +1,90 @@ -use core::fmt; +use core::{fmt, str}; use serde::{ de::{Error, MapAccess, Visitor}, ser::SerializeStruct, Deserialize, Deserializer, Serialize, Serializer, }; -// These methods are compatible with the result of `#[derive(Serialize, Deserialize)]` on bitflags `1.0` types - -pub fn serialize_bits_default( +pub fn serialize_bits_default, B: Serialize, S: Serializer>( name: &'static str, - bits: &B, + flags: &T, serializer: S, ) -> Result { - let mut serialize_struct = serializer.serialize_struct(name, 1)?; - serialize_struct.serialize_field("bits", bits)?; - serialize_struct.end() + if serializer.is_human_readable() { + serializer.collect_str(flags) + } else { + let mut serialize_struct = serializer.serialize_struct(name, 1)?; + serialize_struct.serialize_field("bits", flags.as_ref())?; + serialize_struct.end() + } } -pub fn deserialize_bits_default<'de, B: Deserialize<'de>, D: Deserializer<'de>>( +pub fn deserialize_bits_default< + 'de, + T: str::FromStr + From, + B: Deserialize<'de>, + D: Deserializer<'de>, +>( name: &'static str, deserializer: D, -) -> Result { - struct BitsVisitor(core::marker::PhantomData); - - impl<'de, T: Deserialize<'de>> Visitor<'de> for BitsVisitor { - type Value = T; +) -> Result +where + ::Err: fmt::Display, +{ + if deserializer.is_human_readable() { + struct FlagsVisitor(core::marker::PhantomData); + + impl<'de, T: str::FromStr> Visitor<'de> for FlagsVisitor + where + ::Err: fmt::Display, + { + type Value = T; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a string value of `|` separated flags") + } - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a primitive bitflags value wrapped in a struct") + fn visit_str(self, flags: &str) -> Result { + flags.parse().map_err(|e| E::custom(e)) + } } - fn visit_map>(self, mut map: A) -> Result { - let mut bits = None; + deserializer.deserialize_str(FlagsVisitor(Default::default())) + } else { + struct BitsVisitor(core::marker::PhantomData); - while let Some(key) = map.next_key()? { - match key { - "bits" => { - if bits.is_some() { - return Err(Error::duplicate_field("bits")); - } + impl<'de, T: Deserialize<'de>> Visitor<'de> for BitsVisitor { + type Value = T; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a primitive bitflags value wrapped in a struct") + } - bits = Some(map.next_value()?); + fn visit_map>(self, mut map: A) -> Result { + let mut bits = None; + + while let Some(key) = map.next_key()? { + match key { + "bits" => { + if bits.is_some() { + return Err(Error::duplicate_field("bits")); + } + + bits = Some(map.next_value()?); + } + v => return Err(Error::unknown_field(v, &["bits"])), } - v => return Err(Error::unknown_field(v, &["bits"])), } - } - bits.ok_or_else(|| Error::missing_field("bits")) + bits.ok_or_else(|| Error::missing_field("bits")) + } } - } - deserializer.deserialize_struct(name, &["bits"], BitsVisitor(Default::default())) + let bits: B = + deserializer.deserialize_struct(name, &["bits"], BitsVisitor(Default::default()))?; + + Ok(bits.into()) + } } #[cfg(test)] diff --git a/src/internal.rs b/src/internal.rs index fa63b8e1..ae425778 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -57,6 +57,12 @@ macro_rules! __impl_internal_bitflags { } impl $crate::__private::core::fmt::Debug for $InternalBitFlags { + fn fmt(&self, f: &mut $crate::__private::core::fmt::Formatter) -> $crate::__private::core::fmt::Result { + $crate::__private::core::fmt::Display::fmt(self, f) + } + } + + impl $crate::__private::core::fmt::Display for $InternalBitFlags { fn fmt(&self, f: &mut $crate::__private::core::fmt::Formatter) -> $crate::__private::core::fmt::Result { // Iterate over the valid flags let mut first = true; @@ -88,6 +94,25 @@ macro_rules! __impl_internal_bitflags { } } + // The impl for `FromStr` should mirror what's produced by `Display` + impl $crate::__private::core::str::FromStr for $InternalBitFlags { + type Err = $crate::ParseError; + + fn from_str(s: &str) -> $crate::__private::core::result::Result { + let mut parsed_flags = Self::empty(); + + for flag in s.split('|') { + let flag = flag.trim(); + + let parsed_flag = Self::from_name(flag).ok_or_else(|| $crate::ParseError::invalid_flag(flag))?; + + parsed_flags.insert(parsed_flag); + } + + $crate::__private::core::result::Result::Ok(parsed_flags) + } + } + impl $crate::__private::core::fmt::Binary for $InternalBitFlags { fn fmt(&self, f: &mut $crate::__private::core::fmt::Formatter) -> $crate::__private::core::fmt::Result { $crate::__private::core::fmt::Binary::fmt(&self.bits(), f) @@ -267,6 +292,18 @@ macro_rules! __impl_internal_bitflags { } } + impl $crate::__private::core::convert::AsRef<$T> for $InternalBitFlags { + fn as_ref(&self) -> &$T { + &self.bits + } + } + + impl $crate::__private::core::convert::From<$T> for $InternalBitFlags { + fn from(bits: $T) -> Self { + Self::from_bits_retain(bits) + } + } + impl $crate::__private::core::iter::Iterator for $Iter { type Item = $BitFlags; diff --git a/src/lib.rs b/src/lib.rs index 85aec33f..d409b6a8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -338,15 +338,18 @@ //! assert_eq!(2, count_unset_flags(&Flags::B)); //! ``` -#![cfg_attr(not(test), no_std)] +#![cfg_attr(not(any(feature = "std", test)), no_std)] #![doc(html_root_url = "https://docs.rs/bitflags/2.0.0-rc.1")] #![forbid(unsafe_code)] #[doc(inline)] pub use traits::BitFlags; +mod parser; mod traits; +pub use parser::*; + #[doc(hidden)] pub mod __private { pub use crate::{external::*, traits::*}; @@ -360,7 +363,7 @@ pub mod __private { /* How does the bitflags crate work? -This library generates `struct`s in the end-user's crate with a bunch of constants on it that represent flags. +This library generates a `struct` in the end-user's crate with a bunch of constants on it that represent flags. The difference between `bitflags` and a lot of other libraries is that we don't actually control the generated `struct` in the end. It's part of the end-user's crate, so it belongs to them. That makes it difficult to extend `bitflags` with new functionality because we could end up breaking valid code that was already written. diff --git a/src/parser.rs b/src/parser.rs new file mode 100644 index 00000000..b80e8324 --- /dev/null +++ b/src/parser.rs @@ -0,0 +1,40 @@ +use core::fmt; + +#[derive(Debug)] +pub struct ParseError { + #[cfg(feature = "std")] + invalid: String, +} + +impl ParseError { + pub fn invalid_flag(flag: impl fmt::Display) -> Self { + #[cfg(feature = "std")] + { + ParseError { + invalid: flag.to_string(), + } + } + #[cfg(not(feature = "std"))] + { + let _ = flag; + + ParseError {} + } + } +} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "failed to parse a set of bitflags")?; + + #[cfg(feature = "std")] + { + write!(f, ": unrecognized flag `{}`", self.invalid)?; + } + + Ok(()) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ParseError {} From 3d9a7af23e5fe30d27bbe81421a63b5424265fbe Mon Sep 17 00:00:00 2001 From: KodrAus Date: Thu, 2 Feb 2023 17:27:03 +1000 Subject: [PATCH 02/16] work on serde formats and parsing --- src/external.rs | 2 - src/external/serde_support.rs | 130 ++++++++++++++++++++++++++++++---- src/internal.rs | 10 ++- src/lib.rs | 7 ++ src/parser.rs | 80 ++++++++++++++++----- 5 files changed, 196 insertions(+), 33 deletions(-) diff --git a/src/external.rs b/src/external.rs index 2f15deb2..4b696acb 100644 --- a/src/external.rs +++ b/src/external.rs @@ -49,7 +49,6 @@ macro_rules! __impl_external_bitflags_serde { serializer: S, ) -> $crate::__private::core::result::Result { $crate::__private::serde_support::serialize_bits_default::<$InternalBitFlags, $T, S>( - $crate::__private::core::stringify!($InternalBitFlags), &self, serializer, ) @@ -61,7 +60,6 @@ macro_rules! __impl_external_bitflags_serde { deserializer: D, ) -> $crate::__private::core::result::Result { $crate::__private::serde_support::deserialize_bits_default::<$InternalBitFlags, $T, D>( - $crate::__private::core::stringify!($InternalBitFlags), deserializer, ) } diff --git a/src/external/serde_support.rs b/src/external/serde_support.rs index 563983a0..089a0ffb 100644 --- a/src/external/serde_support.rs +++ b/src/external/serde_support.rs @@ -1,21 +1,17 @@ use core::{fmt, str}; use serde::{ - de::{Error, MapAccess, Visitor}, - ser::SerializeStruct, + de::{Error, Visitor}, Deserialize, Deserializer, Serialize, Serializer, }; pub fn serialize_bits_default, B: Serialize, S: Serializer>( - name: &'static str, flags: &T, serializer: S, ) -> Result { if serializer.is_human_readable() { serializer.collect_str(flags) } else { - let mut serialize_struct = serializer.serialize_struct(name, 1)?; - serialize_struct.serialize_field("bits", flags.as_ref())?; - serialize_struct.end() + flags.as_ref().serialize(serializer) } } @@ -25,7 +21,6 @@ pub fn deserialize_bits_default< B: Deserialize<'de>, D: Deserializer<'de>, >( - name: &'static str, deserializer: D, ) -> Result where @@ -51,6 +46,69 @@ where deserializer.deserialize_str(FlagsVisitor(Default::default())) } else { + let bits = B::deserialize(deserializer)?; + + Ok(bits.into()) + } +} + +pub mod legacy_format { + //! Generic implementations of `serde::Serialize` and `serde::Deserialize` for flags types + //! that's compatible with `#[derive(Serialize, Deserialize)]` on types generated by + //! `bitflags` `1.x`. + //! + //! # Using this module + //! + //! When upgrading from `bitflags` `1.x`, replace your `#[derive(Serialize, Deserialize)]` + //! with the following manual implementations: + //! + //! + //! ``` + //! bitflags! { + //! // #[derive(Serialize, Deserialize)] + //! struct SerdeLegacyFlags: u32 { + //! const A = 1; + //! const B = 2; + //! const C = 4; + //! const D = 8; + //! } + //! } + //! + //! impl serde::Serialize for SerdeLegacyFlags { + //! fn serialize(&self, serializer: S) -> Result { + //! bitflags::serde_support::legacy_format::serialize(self, serializer) + //! } + //! } + //! + //! impl<'de> serde::Deserialize<'de> for SerdeLegacyFlags { + //! fn deserialize>(deserializer: D) -> Result { + //! bitflags::serde_support::legacy_format::deserialize(deserializer) + //! } + //! } + //! ``` + + use core::{fmt, any::type_name}; + use serde::{Serialize, Serializer, Deserialize, Deserializer, ser::SerializeStruct, de::{Error, Visitor, MapAccess}}; + + use crate::BitFlags; + + /// Serialize a flags type equivalently to how `#[derive(Serialize)]` on a flags type + /// from `bitflags` `1.x` would. + pub fn serialize(flags: &T, serializer: S) -> Result + where + ::Bits: Serialize, + { + let mut serialize_struct = serializer.serialize_struct(type_name::(), 1)?; + serialize_struct.serialize_field("bits", &flags.bits())?; + serialize_struct.end() + } + + /// Deserialize a flags type equivalently to how `#[derive(Deserialize)]` on a flags type + /// from `bitflags` `1.x` would. + pub fn deserialize<'de, T: BitFlags, D: Deserializer<'de>>(deserializer: D) -> Result + where + ::Bits: Deserialize<'de>, + { struct BitsVisitor(core::marker::PhantomData); impl<'de, T: Deserialize<'de>> Visitor<'de> for BitsVisitor { @@ -80,10 +138,9 @@ where } } - let bits: B = - deserializer.deserialize_struct(name, &["bits"], BitsVisitor(Default::default()))?; + let bits = deserializer.deserialize_struct(type_name::(), &["bits"], BitsVisitor(Default::default()))?; - Ok(bits.into()) + Ok(T::from_bits_retain(bits)) } } @@ -99,18 +156,39 @@ mod tests { } } + bitflags! { + struct SerdeLegacyFlags: u32 { + const A = 1; + const B = 2; + const C = 4; + const D = 8; + } + } + + impl serde::Serialize for SerdeLegacyFlags { + fn serialize(&self, serializer: S) -> Result { + crate::serde_support::legacy_format::serialize(self, serializer) + } + } + + impl<'de> serde::Deserialize<'de> for SerdeLegacyFlags { + fn deserialize>(deserializer: D) -> Result { + crate::serde_support::legacy_format::deserialize(deserializer) + } + } + #[test] fn test_serde_bitflags_default_serialize() { let flags = SerdeFlags::A | SerdeFlags::B; let serialized = serde_json::to_string(&flags).unwrap(); - assert_eq!(serialized, r#"{"bits":3}"#); + assert_eq!(serialized, r#""A | B""#); } #[test] fn test_serde_bitflags_default_deserialize() { - let deserialized: SerdeFlags = serde_json::from_str(r#"{"bits":12}"#).unwrap(); + let deserialized: SerdeFlags = serde_json::from_str(r#""C | D""#).unwrap(); let expected = SerdeFlags::C | SerdeFlags::D; @@ -126,4 +204,32 @@ mod tests { assert_eq!(deserialized.bits(), flags.bits()); } + + #[test] + fn test_serde_bitflags_legacy_serialize() { + let flags = SerdeLegacyFlags::A | SerdeLegacyFlags::B; + + let serialized = serde_json::to_string(&flags).unwrap(); + + assert_eq!(serialized, r#"{"bits":3}"#); + } + + #[test] + fn test_serde_bitflags_legacy_deserialize() { + let deserialized: SerdeLegacyFlags = serde_json::from_str(r#"{"bits":12}"#).unwrap(); + + let expected = SerdeLegacyFlags::C | SerdeLegacyFlags::D; + + assert_eq!(deserialized.bits(), expected.bits()); + } + + #[test] + fn test_serde_bitflags_legacy_roundtrip() { + let flags = SerdeLegacyFlags::A | SerdeLegacyFlags::B; + + let deserialized: SerdeLegacyFlags = + serde_json::from_str(&serde_json::to_string(&flags).unwrap()).unwrap(); + + assert_eq!(deserialized.bits(), flags.bits()); + } } diff --git a/src/internal.rs b/src/internal.rs index ae425778..acac09a5 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -104,7 +104,15 @@ macro_rules! __impl_internal_bitflags { for flag in s.split('|') { let flag = flag.trim(); - let parsed_flag = Self::from_name(flag).ok_or_else(|| $crate::ParseError::invalid_flag(flag))?; + let parsed_flag = if flag.starts_with("0x") { + // Parse a flag formatted as hex + let flag = &flag[2..]; + let bits = <$T>::from_str_radix(flag, 16).map_err(|_| $crate::ParseError::invalid_hex_flag(flag))?; + + Self::from_bits_retain(bits) + } else { + Self::from_name(flag).ok_or_else(|| $crate::ParseError::invalid_named_flag(flag))? + }; parsed_flags.insert(parsed_flag); } diff --git a/src/lib.rs b/src/lib.rs index d409b6a8..ee548176 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -360,6 +360,13 @@ pub mod __private { pub use serde; } +#[cfg(feature = "serde")] +pub mod serde_support { + //! Utilities for integrating `serde` with generated flags types. + + pub use crate::external::serde_support::legacy_format; +} + /* How does the bitflags crate work? diff --git a/src/parser.rs b/src/parser.rs index b80e8324..f7cc82df 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,35 +1,79 @@ use core::fmt; #[derive(Debug)] -pub struct ParseError { - #[cfg(feature = "std")] - invalid: String, +pub struct ParseError(ParseErrorKind); + +#[derive(Debug)] +enum ParseErrorKind { + InvalidNamedFlag { + #[cfg(not(feature = "std"))] + got: (), + #[cfg(feature = "std")] + got: String, + }, + InvalidHexFlag { + #[cfg(not(feature = "std"))] + got: (), + #[cfg(feature = "std")] + got: String, + } } impl ParseError { - pub fn invalid_flag(flag: impl fmt::Display) -> Self { - #[cfg(feature = "std")] - { - ParseError { - invalid: flag.to_string(), + pub fn invalid_hex_flag(flag: impl fmt::Display) -> Self { + let _flag = flag; + + let got = { + #[cfg(feature = "std")] + { + _flag.to_string() } - } - #[cfg(not(feature = "std"))] - { - let _ = flag; + }; - ParseError {} - } + ParseError(ParseErrorKind::InvalidHexFlag { + got, + }) + } + + pub fn invalid_named_flag(flag: impl fmt::Display) -> Self { + let _flag = flag; + + let got = { + #[cfg(feature = "std")] + { + _flag.to_string() + } + }; + + ParseError(ParseErrorKind::InvalidNamedFlag { + got, + }) } } impl fmt::Display for ParseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "failed to parse a set of bitflags")?; + match self.0 { + ParseErrorKind::InvalidNamedFlag { got } => { + let _got = got; - #[cfg(feature = "std")] - { - write!(f, ": unrecognized flag `{}`", self.invalid)?; + write!(f, "unrecognized named flag")?; + + #[cfg(feature = "std")] + { + write!(f, " `{}`", _got)?; + } + } + ParseErrorKind::InvalidHexFlag { got } => { + let _got = got; + + write!(f, "invalid hex flag")?; + + #[cfg(feature = "std")] + { + write!(f, " `{}`", _got)?; + } + } } Ok(()) From e075f4db97489644c56d4b1cec8f885dfb193e82 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Thu, 2 Feb 2023 20:34:55 +1000 Subject: [PATCH 03/16] move serde compat into an example --- examples/fmt.rs | 49 ++++++++++++ examples/serde_compat.rs | 125 +++++++++++++++++++++++++++++ src/{parser.rs => error.rs} | 12 +-- src/external/serde_support.rs | 145 +--------------------------------- src/internal.rs | 4 - src/lib.rs | 17 ++-- 6 files changed, 187 insertions(+), 165 deletions(-) create mode 100644 examples/fmt.rs create mode 100644 examples/serde_compat.rs rename src/{parser.rs => error.rs} (89%) diff --git a/examples/fmt.rs b/examples/fmt.rs new file mode 100644 index 00000000..84457642 --- /dev/null +++ b/examples/fmt.rs @@ -0,0 +1,49 @@ +//! An example of implementing Rust's standard formatting and parsing traits for flags types. + +use core::{fmt, str}; + +fn main() -> Result<(), bitflags::ParseError> { + bitflags::bitflags! { + // You can `#[derive]` the `Debug` trait, but implementing it manually + // can produce output like `A | B` instead of `Flags(A | B)`. + // #[derive(Debug)] + #[derive(PartialEq, Eq)] + pub struct Flags: u32 { + const A = 1; + const B = 2; + const C = 4; + const D = 8; + } + } + + impl fmt::Debug for Flags { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(&self.0, f) + } + } + + impl fmt::Display for Flags { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } + } + + impl str::FromStr for Flags { + type Err = bitflags::ParseError; + + fn from_str(flags: &str) -> Result { + Ok(Self(flags.parse()?)) + } + } + + let flags = Flags::A | Flags::B; + + println!("{}", flags); + + let formatted = flags.to_string(); + let parsed: Flags = formatted.parse()?; + + assert_eq!(flags, parsed); + + Ok(()) +} diff --git a/examples/serde_compat.rs b/examples/serde_compat.rs new file mode 100644 index 00000000..6029aad0 --- /dev/null +++ b/examples/serde_compat.rs @@ -0,0 +1,125 @@ +//! An example of implementing `serde::Serialize` and `serde::Deserialize` equivalently to how +//! `#[derive(Serialize, Deserialize)]` would on `bitflags` `1.x` types. + +#[cfg(feature = "serde")] +fn main() { + bitflags::bitflags! { + // Removed: `serde` traits from the `#[derive]` attribute + // #[derive(Serialize, Deserialize)] + #[derive(Debug, PartialEq, Eq)] + pub struct Flags: u32 { + const A = 1; + const B = 2; + const C = 4; + const D = 8; + } + } + + // Added: Manual `Serialize` and `Deserialize` implementations based on a generic impl + impl serde::Serialize for Flags { + fn serialize(&self, serializer: S) -> Result { + legacy_format::serialize(self, serializer) + } + } + + impl<'de> serde::Deserialize<'de> for Flags { + fn deserialize>(deserializer: D) -> Result { + legacy_format::deserialize(deserializer) + } + } + + pub mod legacy_format { + //! This module is a generic implementation of `Serialize` and `Deserialize` that can be used by + //! any flags type generated by `bitflags!`. + //! + //! Don't be intimidated by the amount of `serde` code in here! It boils down to serializing and deserializing + //! a struct with a single `bits` field. It may be converted into a library at some point, but is also suitable + //! to copy into your own project if you need it. + + use core::{any::type_name, fmt}; + use serde::{ + de::{Error, MapAccess, Visitor}, + ser::SerializeStruct, + Deserialize, Deserializer, Serialize, Serializer, + }; + + use bitflags::BitFlags; + + /// Serialize a flags type equivalently to how `#[derive(Serialize)]` on a flags type + /// from `bitflags` `1.x` would. + pub fn serialize( + flags: &T, + serializer: S, + ) -> Result + where + ::Bits: Serialize, + { + let mut serialize_struct = serializer.serialize_struct(type_name::(), 1)?; + serialize_struct.serialize_field("bits", &flags.bits())?; + serialize_struct.end() + } + + /// Deserialize a flags type equivalently to how `#[derive(Deserialize)]` on a flags type + /// from `bitflags` `1.x` would. + pub fn deserialize<'de, T: BitFlags, D: Deserializer<'de>>( + deserializer: D, + ) -> Result + where + ::Bits: Deserialize<'de>, + { + struct BitsVisitor(core::marker::PhantomData); + + impl<'de, T: Deserialize<'de>> Visitor<'de> for BitsVisitor { + type Value = T; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a primitive bitflags value wrapped in a struct") + } + + fn visit_map>(self, mut map: A) -> Result { + let mut bits = None; + + while let Some(key) = map.next_key()? { + match key { + "bits" => { + if bits.is_some() { + return Err(Error::duplicate_field("bits")); + } + + bits = Some(map.next_value()?); + } + v => return Err(Error::unknown_field(v, &["bits"])), + } + } + + bits.ok_or_else(|| Error::missing_field("bits")) + } + } + + let bits = deserializer.deserialize_struct( + type_name::(), + &["bits"], + BitsVisitor(Default::default()), + )?; + + Ok(T::from_bits_retain(bits)) + } + } + + let flags = Flags::A | Flags::B; + + let serialized = serde_json::to_string(&flags).unwrap(); + + println!("{:?} -> {}", flags, serialized); + + assert_eq!(serialized, r#"{"bits":3}"#); + + let deserialized: Flags = serde_json::from_str(&serialized).unwrap(); + + println!("{} -> {:?}", serialized, flags); + + assert_eq!(deserialized, flags); +} + +#[cfg(not(feature = "serde"))] +fn main() {} diff --git a/src/parser.rs b/src/error.rs similarity index 89% rename from src/parser.rs rename to src/error.rs index f7cc82df..e1c87512 100644 --- a/src/parser.rs +++ b/src/error.rs @@ -16,7 +16,7 @@ enum ParseErrorKind { got: (), #[cfg(feature = "std")] got: String, - } + }, } impl ParseError { @@ -30,9 +30,7 @@ impl ParseError { } }; - ParseError(ParseErrorKind::InvalidHexFlag { - got, - }) + ParseError(ParseErrorKind::InvalidHexFlag { got }) } pub fn invalid_named_flag(flag: impl fmt::Display) -> Self { @@ -45,9 +43,7 @@ impl ParseError { } }; - ParseError(ParseErrorKind::InvalidNamedFlag { - got, - }) + ParseError(ParseErrorKind::InvalidNamedFlag { got }) } } @@ -66,7 +62,7 @@ impl fmt::Display for ParseError { } ParseErrorKind::InvalidHexFlag { got } => { let _got = got; - + write!(f, "invalid hex flag")?; #[cfg(feature = "std")] diff --git a/src/external/serde_support.rs b/src/external/serde_support.rs index 089a0ffb..fea662b1 100644 --- a/src/external/serde_support.rs +++ b/src/external/serde_support.rs @@ -9,8 +9,10 @@ pub fn serialize_bits_default, B: Serialize, S: Seria serializer: S, ) -> Result { if serializer.is_human_readable() { + // Serialize human-readable flags as a string like `"A | B"` serializer.collect_str(flags) } else { + // Serialize non-human-readable flags directly as the underlying bits flags.as_ref().serialize(serializer) } } @@ -27,6 +29,7 @@ where ::Err: fmt::Display, { if deserializer.is_human_readable() { + // Deserialize human-readable flags by parsing them from strings like `"A | B"` struct FlagsVisitor(core::marker::PhantomData); impl<'de, T: str::FromStr> Visitor<'de> for FlagsVisitor @@ -46,104 +49,13 @@ where deserializer.deserialize_str(FlagsVisitor(Default::default())) } else { + // Deserialize non-human-readable flags directly from the underlying bits let bits = B::deserialize(deserializer)?; Ok(bits.into()) } } -pub mod legacy_format { - //! Generic implementations of `serde::Serialize` and `serde::Deserialize` for flags types - //! that's compatible with `#[derive(Serialize, Deserialize)]` on types generated by - //! `bitflags` `1.x`. - //! - //! # Using this module - //! - //! When upgrading from `bitflags` `1.x`, replace your `#[derive(Serialize, Deserialize)]` - //! with the following manual implementations: - //! - //! - //! ``` - //! bitflags! { - //! // #[derive(Serialize, Deserialize)] - //! struct SerdeLegacyFlags: u32 { - //! const A = 1; - //! const B = 2; - //! const C = 4; - //! const D = 8; - //! } - //! } - //! - //! impl serde::Serialize for SerdeLegacyFlags { - //! fn serialize(&self, serializer: S) -> Result { - //! bitflags::serde_support::legacy_format::serialize(self, serializer) - //! } - //! } - //! - //! impl<'de> serde::Deserialize<'de> for SerdeLegacyFlags { - //! fn deserialize>(deserializer: D) -> Result { - //! bitflags::serde_support::legacy_format::deserialize(deserializer) - //! } - //! } - //! ``` - - use core::{fmt, any::type_name}; - use serde::{Serialize, Serializer, Deserialize, Deserializer, ser::SerializeStruct, de::{Error, Visitor, MapAccess}}; - - use crate::BitFlags; - - /// Serialize a flags type equivalently to how `#[derive(Serialize)]` on a flags type - /// from `bitflags` `1.x` would. - pub fn serialize(flags: &T, serializer: S) -> Result - where - ::Bits: Serialize, - { - let mut serialize_struct = serializer.serialize_struct(type_name::(), 1)?; - serialize_struct.serialize_field("bits", &flags.bits())?; - serialize_struct.end() - } - - /// Deserialize a flags type equivalently to how `#[derive(Deserialize)]` on a flags type - /// from `bitflags` `1.x` would. - pub fn deserialize<'de, T: BitFlags, D: Deserializer<'de>>(deserializer: D) -> Result - where - ::Bits: Deserialize<'de>, - { - struct BitsVisitor(core::marker::PhantomData); - - impl<'de, T: Deserialize<'de>> Visitor<'de> for BitsVisitor { - type Value = T; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a primitive bitflags value wrapped in a struct") - } - - fn visit_map>(self, mut map: A) -> Result { - let mut bits = None; - - while let Some(key) = map.next_key()? { - match key { - "bits" => { - if bits.is_some() { - return Err(Error::duplicate_field("bits")); - } - - bits = Some(map.next_value()?); - } - v => return Err(Error::unknown_field(v, &["bits"])), - } - } - - bits.ok_or_else(|| Error::missing_field("bits")) - } - } - - let bits = deserializer.deserialize_struct(type_name::(), &["bits"], BitsVisitor(Default::default()))?; - - Ok(T::from_bits_retain(bits)) - } -} - #[cfg(test)] mod tests { bitflags! { @@ -156,27 +68,6 @@ mod tests { } } - bitflags! { - struct SerdeLegacyFlags: u32 { - const A = 1; - const B = 2; - const C = 4; - const D = 8; - } - } - - impl serde::Serialize for SerdeLegacyFlags { - fn serialize(&self, serializer: S) -> Result { - crate::serde_support::legacy_format::serialize(self, serializer) - } - } - - impl<'de> serde::Deserialize<'de> for SerdeLegacyFlags { - fn deserialize>(deserializer: D) -> Result { - crate::serde_support::legacy_format::deserialize(deserializer) - } - } - #[test] fn test_serde_bitflags_default_serialize() { let flags = SerdeFlags::A | SerdeFlags::B; @@ -204,32 +95,4 @@ mod tests { assert_eq!(deserialized.bits(), flags.bits()); } - - #[test] - fn test_serde_bitflags_legacy_serialize() { - let flags = SerdeLegacyFlags::A | SerdeLegacyFlags::B; - - let serialized = serde_json::to_string(&flags).unwrap(); - - assert_eq!(serialized, r#"{"bits":3}"#); - } - - #[test] - fn test_serde_bitflags_legacy_deserialize() { - let deserialized: SerdeLegacyFlags = serde_json::from_str(r#"{"bits":12}"#).unwrap(); - - let expected = SerdeLegacyFlags::C | SerdeLegacyFlags::D; - - assert_eq!(deserialized.bits(), expected.bits()); - } - - #[test] - fn test_serde_bitflags_legacy_roundtrip() { - let flags = SerdeLegacyFlags::A | SerdeLegacyFlags::B; - - let deserialized: SerdeLegacyFlags = - serde_json::from_str(&serde_json::to_string(&flags).unwrap()).unwrap(); - - assert_eq!(deserialized.bits(), flags.bits()); - } } diff --git a/src/internal.rs b/src/internal.rs index acac09a5..f8077b7e 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -86,10 +86,6 @@ macro_rules! __impl_internal_bitflags { $crate::__private::core::write!(f, "{:#x}", extra_bits)?; } - if first { - f.write_str("empty")?; - } - $crate::__private::core::fmt::Result::Ok(()) } } diff --git a/src/lib.rs b/src/lib.rs index ee548176..edbb8a6b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -345,10 +345,10 @@ #[doc(inline)] pub use traits::BitFlags; -mod parser; +mod error; mod traits; -pub use parser::*; +pub use error::*; #[doc(hidden)] pub mod __private { @@ -360,13 +360,6 @@ pub mod __private { pub use serde; } -#[cfg(feature = "serde")] -pub mod serde_support { - //! Utilities for integrating `serde` with generated flags types. - - pub use crate::external::serde_support::legacy_format; -} - /* How does the bitflags crate work? @@ -1116,7 +1109,7 @@ mod tests { #[test] fn test_debug() { assert_eq!(format!("{:?}", Flags::A | Flags::B), "Flags(A | B)"); - assert_eq!(format!("{:?}", Flags::empty()), "Flags(empty)"); + assert_eq!(format!("{:?}", Flags::empty()), "Flags()"); assert_eq!(format!("{:?}", Flags::ABC), "Flags(A | B | C)"); let extra = Flags::from_bits_retain(0xb8); @@ -1129,7 +1122,7 @@ mod tests { "Flags(A | B | C | ABC | 0xb8)" ); - assert_eq!(format!("{:?}", EmptyFlags::empty()), "EmptyFlags(empty)"); + assert_eq!(format!("{:?}", EmptyFlags::empty()), "EmptyFlags()"); } #[test] @@ -1293,7 +1286,7 @@ mod tests { assert!(Flags::SOME.contains(Flags::NONE)); assert!(Flags::NONE.is_empty()); - assert_eq!(format!("{:?}", Flags::empty()), "Flags(empty)"); + assert_eq!(format!("{:?}", Flags::empty()), "Flags()"); assert_eq!(format!("{:?}", Flags::SOME), "Flags(NONE | SOME)"); } From eae62a14ff77fe4e84169808dfcb185e9266bb1c Mon Sep 17 00:00:00 2001 From: KodrAus Date: Thu, 2 Feb 2023 22:08:22 +1000 Subject: [PATCH 04/16] avoid producing empty strings for empty flags --- src/internal.rs | 13 +++++++++++++ src/lib.rs | 47 ++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/internal.rs b/src/internal.rs index f8077b7e..c986ff46 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -82,10 +82,17 @@ macro_rules! __impl_internal_bitflags { if !first { f.write_str(" | ")?; } + first = false; $crate::__private::core::write!(f, "{:#x}", extra_bits)?; } + // If no flags are set then write the empty flags to avoid producing + // an empty string + if first { + $crate::__private::core::write!(f, "{:#x}", <$T as $crate::__private::Bits>::EMPTY)?; + } + $crate::__private::core::fmt::Result::Ok(()) } } @@ -95,8 +102,14 @@ macro_rules! __impl_internal_bitflags { type Err = $crate::ParseError; fn from_str(s: &str) -> $crate::__private::core::result::Result { + let s = s.trim(); + let mut parsed_flags = Self::empty(); + if s.is_empty() { + return Ok(parsed_flags); + } + for flag in s.split('|') { let flag = flag.trim(); diff --git a/src/lib.rs b/src/lib.rs index edbb8a6b..2fa7de60 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -558,8 +558,12 @@ pub mod example_generated; #[cfg(test)] mod tests { - use std::collections::hash_map::DefaultHasher; - use std::hash::{Hash, Hasher}; + use std::{ + collections::hash_map::DefaultHasher, + fmt, + hash::{Hash, Hasher}, + str, + }; bitflags! { #[doc = "> The first principle is that you must not fool yourself — and"] @@ -599,6 +603,20 @@ mod tests { } } + impl fmt::Display for Flags { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } + } + + impl str::FromStr for Flags { + type Err = crate::ParseError; + + fn from_str(flags: &str) -> Result { + Ok(Self(flags.parse()?)) + } + } + bitflags! { #[derive(Clone, Copy, Default, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] struct EmptyFlags: u32 { @@ -1109,7 +1127,7 @@ mod tests { #[test] fn test_debug() { assert_eq!(format!("{:?}", Flags::A | Flags::B), "Flags(A | B)"); - assert_eq!(format!("{:?}", Flags::empty()), "Flags()"); + assert_eq!(format!("{:?}", Flags::empty()), "Flags(0x0)"); assert_eq!(format!("{:?}", Flags::ABC), "Flags(A | B | C)"); let extra = Flags::from_bits_retain(0xb8); @@ -1122,7 +1140,27 @@ mod tests { "Flags(A | B | C | ABC | 0xb8)" ); - assert_eq!(format!("{:?}", EmptyFlags::empty()), "EmptyFlags()"); + assert_eq!(format!("{:?}", EmptyFlags::empty()), "EmptyFlags(0x0)"); + } + + #[test] + fn test_display_from_str() { + fn format_parse_case(flags: Flags) { + assert_eq!(flags, { + match flags.to_string().parse::() { + Ok(flags) => flags, + Err(e) => panic!("failed to parse `{}`: {}", flags, e), + } + }); + } + + format_parse_case(Flags::empty()); + format_parse_case(Flags::A); + format_parse_case(Flags::A | Flags::B); + format_parse_case(Flags::ABC); + format_parse_case(Flags::all()); + format_parse_case(Flags::from_bits_retain(0xb8)); + format_parse_case(Flags::from_bits_retain(0x20)); } #[test] @@ -1286,7 +1324,6 @@ mod tests { assert!(Flags::SOME.contains(Flags::NONE)); assert!(Flags::NONE.is_empty()); - assert_eq!(format!("{:?}", Flags::empty()), "Flags()"); assert_eq!(format!("{:?}", Flags::SOME), "Flags(NONE | SOME)"); } From b34d987218789594f9512ef27e15850542e8bf5c Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 08:02:55 +1000 Subject: [PATCH 05/16] add some more tests for parsing and formatting --- Cargo.toml | 1 + src/error.rs | 8 ++++ src/external.rs | 1 + src/external/serde_support.rs | 35 ++++++++++++++-- src/internal.rs | 37 ++++++++++++++--- src/lib.rs | 75 ++++++++++++++++++++++++++++------- 6 files changed, 133 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 632db9e0..05c469e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ trybuild = "1.0" rustversion = "1.0" serde_derive = "1.0" serde_json = "1.0" +serde_test = "1.0" [features] std = [] diff --git a/src/error.rs b/src/error.rs index e1c87512..b1db461a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,6 +5,7 @@ pub struct ParseError(ParseErrorKind); #[derive(Debug)] enum ParseErrorKind { + EmptyFlag, InvalidNamedFlag { #[cfg(not(feature = "std"))] got: (), @@ -45,6 +46,10 @@ impl ParseError { ParseError(ParseErrorKind::InvalidNamedFlag { got }) } + + pub fn empty_flag() -> Self { + ParseError(ParseErrorKind::EmptyFlag) + } } impl fmt::Display for ParseError { @@ -70,6 +75,9 @@ impl fmt::Display for ParseError { write!(f, " `{}`", _got)?; } } + ParseErrorKind::EmptyFlag => { + write!(f, "encountered empty flag")?; + } } Ok(()) diff --git a/src/external.rs b/src/external.rs index 4b696acb..91ad9de1 100644 --- a/src/external.rs +++ b/src/external.rs @@ -50,6 +50,7 @@ macro_rules! __impl_external_bitflags_serde { ) -> $crate::__private::core::result::Result { $crate::__private::serde_support::serialize_bits_default::<$InternalBitFlags, $T, S>( &self, + self.is_empty(), serializer, ) } diff --git a/src/external/serde_support.rs b/src/external/serde_support.rs index fea662b1..4a164786 100644 --- a/src/external/serde_support.rs +++ b/src/external/serde_support.rs @@ -6,13 +6,22 @@ use serde::{ pub fn serialize_bits_default, B: Serialize, S: Serializer>( flags: &T, + is_empty: bool, serializer: S, ) -> Result { + // Serialize human-readable flags as a string like `"A | B"` if serializer.is_human_readable() { - // Serialize human-readable flags as a string like `"A | B"` - serializer.collect_str(flags) - } else { - // Serialize non-human-readable flags directly as the underlying bits + // Special case for empty flags through `serde` + // Rather than serializing as `0x0`, serialize as + // an empty string + if is_empty { + serializer.serialize_str("") + } else { + serializer.collect_str(flags) + } + } + // Serialize non-human-readable flags directly as the underlying bits + else { flags.as_ref().serialize(serializer) } } @@ -68,6 +77,15 @@ mod tests { } } + #[test] + fn test_serde_bitflags_default_serialize_empty() { + let flags = SerdeFlags::empty(); + + let serialized = serde_json::to_string(&flags).unwrap(); + + assert_eq!(serialized, r#""""#); + } + #[test] fn test_serde_bitflags_default_serialize() { let flags = SerdeFlags::A | SerdeFlags::B; @@ -77,6 +95,15 @@ mod tests { assert_eq!(serialized, r#""A | B""#); } + #[test] + fn test_serde_bitflags_default_deserialize_empty() { + let deserialized: SerdeFlags = serde_json::from_str(r#""""#).unwrap(); + + let expected = SerdeFlags::empty(); + + assert_eq!(deserialized.bits(), expected.bits()); + } + #[test] fn test_serde_bitflags_default_deserialize() { let deserialized: SerdeFlags = serde_json::from_str(r#""C | D""#).unwrap(); diff --git a/src/internal.rs b/src/internal.rs index c986ff46..ee965594 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -64,6 +64,14 @@ macro_rules! __impl_internal_bitflags { impl $crate::__private::core::fmt::Display for $InternalBitFlags { fn fmt(&self, f: &mut $crate::__private::core::fmt::Formatter) -> $crate::__private::core::fmt::Result { + // A formatter for bitflags that produces text output like: + // + // A | B | 0xf6 + // + // The names of set flags are written in a bar-separated-format, + // followed by a hex number of any remaining bits that are set + // but don't correspond to any flags. + // Iterate over the valid flags let mut first = true; for (name, _) in self.iter_names() { @@ -87,8 +95,14 @@ macro_rules! __impl_internal_bitflags { $crate::__private::core::write!(f, "{:#x}", extra_bits)?; } - // If no flags are set then write the empty flags to avoid producing - // an empty string + // If no flags are set then write an empty hex flag to avoid + // writing an empty string. In some contexts, like serialization, + // an empty string is preferrable, but it may be unexpected in + // others for a format not to produce any output. + // + // We can remove this `0x0` and remain compatible with `FromStr`, + // because an empty string will still parse to an empty set of flags, + // just like `0x0` does. if first { $crate::__private::core::write!(f, "{:#x}", <$T as $crate::__private::Bits>::EMPTY)?; } @@ -97,7 +111,7 @@ macro_rules! __impl_internal_bitflags { } } - // The impl for `FromStr` should mirror what's produced by `Display` + // The impl for `FromStr` should parse anything produced by `Display` impl $crate::__private::core::str::FromStr for $InternalBitFlags { type Err = $crate::ParseError; @@ -106,20 +120,31 @@ macro_rules! __impl_internal_bitflags { let mut parsed_flags = Self::empty(); + // If the input is empty then return an empty set of flags if s.is_empty() { - return Ok(parsed_flags); + return $crate::__private::core::result::Result::Ok(parsed_flags); } for flag in s.split('|') { let flag = flag.trim(); + // If the flag is empty then we've got missing input + if flag.is_empty() { + return $crate::__private::core::result::Result::Err($crate::ParseError::empty_flag()); + } + + // If the flag starts with `0x` then it's a hex number + // Parse it directly to the underlying bits type let parsed_flag = if flag.starts_with("0x") { - // Parse a flag formatted as hex let flag = &flag[2..]; let bits = <$T>::from_str_radix(flag, 16).map_err(|_| $crate::ParseError::invalid_hex_flag(flag))?; Self::from_bits_retain(bits) - } else { + } + // Otherwise the flag is a name + // The generated flags type will determine whether + // or not it's a valid identifier + else { Self::from_name(flag).ok_or_else(|| $crate::ParseError::invalid_named_flag(flag))? }; diff --git a/src/lib.rs b/src/lib.rs index 2fa7de60..f6d1a799 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -603,13 +603,17 @@ mod tests { } } - impl fmt::Display for Flags { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Display::fmt(&self.0, f) + bitflags! { + #[derive(Debug, PartialEq, Eq)] + struct FmtFlags: u16 { + const 고양이 = 0b0000_0001; + const 개 = 0b0000_0010; + const 물고기 = 0b0000_0100; + const 물고기_고양이 = Self::고양이.bits() | Self::물고기.bits(); } } - impl str::FromStr for Flags { + impl str::FromStr for FmtFlags { type Err = crate::ParseError; fn from_str(flags: &str) -> Result { @@ -617,6 +621,12 @@ mod tests { } } + impl fmt::Display for FmtFlags { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } + } + bitflags! { #[derive(Clone, Copy, Default, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] struct EmptyFlags: u32 { @@ -1144,23 +1154,60 @@ mod tests { } #[test] - fn test_display_from_str() { - fn format_parse_case(flags: Flags) { + fn test_display_from_str_roundtrip() { + fn format_parse_case(flags: FmtFlags) { assert_eq!(flags, { - match flags.to_string().parse::() { + match flags.to_string().parse::() { Ok(flags) => flags, Err(e) => panic!("failed to parse `{}`: {}", flags, e), } }); } - format_parse_case(Flags::empty()); - format_parse_case(Flags::A); - format_parse_case(Flags::A | Flags::B); - format_parse_case(Flags::ABC); - format_parse_case(Flags::all()); - format_parse_case(Flags::from_bits_retain(0xb8)); - format_parse_case(Flags::from_bits_retain(0x20)); + fn parse_case(expected: FmtFlags, flags: &str) { + assert_eq!(expected, flags.parse::().unwrap()); + } + + format_parse_case(FmtFlags::empty()); + format_parse_case(FmtFlags::all()); + format_parse_case(FmtFlags::고양이); + format_parse_case(FmtFlags::고양이 | FmtFlags::개); + format_parse_case(FmtFlags::물고기_고양이); + format_parse_case(FmtFlags::from_bits_retain(0xb8)); + format_parse_case(FmtFlags::from_bits_retain(0x20)); + + parse_case(FmtFlags::empty(), ""); + parse_case(FmtFlags::empty(), " \r\n\t"); + parse_case(FmtFlags::empty(), "0x0"); + parse_case(FmtFlags::empty(), "0x0"); + + parse_case(FmtFlags::고양이, "고양이"); + parse_case(FmtFlags::고양이, " 고양이 "); + parse_case(FmtFlags::고양이, "고양이 | 고양이 | 고양이"); + parse_case(FmtFlags::고양이, "0x01"); + + parse_case(FmtFlags::고양이 | FmtFlags::개, "고양이 | 개"); + parse_case(FmtFlags::고양이 | FmtFlags::개, "고양이|개"); + parse_case(FmtFlags::고양이 | FmtFlags::개, "\n고양이|개 "); + + parse_case(FmtFlags::고양이 | FmtFlags::물고기, "물고기_고양이"); + } + + #[test] + fn test_from_str_err() { + fn parse_case(pat: &str, flags: &str) { + let err = flags.parse::().unwrap_err().to_string(); + assert!(err.contains(pat), "`{}` not found in error `{}`", pat, err); + } + + parse_case("empty flag", "|"); + parse_case("empty flag", "|||"); + parse_case("empty flag", "고양이 |"); + parse_case("unrecognized named flag", "NOT_A_FLAG"); + parse_case("unrecognized named flag", "고양이 개"); + parse_case("unrecognized named flag", "고양이 | NOT_A_FLAG"); + parse_case("invalid hex flag", "0xhi"); + parse_case("invalid hex flag", "고양이 | 0xhi"); } #[test] From f825817bdb4fa30415ea86c19f2d473ef5de634e Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 08:11:13 +1000 Subject: [PATCH 06/16] use serde_test for testing derived serde impls --- .github/workflows/rust.yml | 2 +- examples/serde.rs | 36 +++++++++++++++++++++++++ src/external/serde_support.rs | 49 ++++++----------------------------- 3 files changed, 45 insertions(+), 42 deletions(-) create mode 100644 examples/serde.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f00ed6c2..f924b12d 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -50,7 +50,7 @@ jobs: run: cargo install cargo-hack - name: Powerset - run: cargo hack test --feature-powerset --lib --optional-deps "serde" --depth 3 --skip rustc-dep-of-std + run: cargo hack test --feature-powerset --lib --optional-deps "std serde" --depth 3 --skip rustc-dep-of-std - name: Docs run: cargo doc --features example_generated diff --git a/examples/serde.rs b/examples/serde.rs new file mode 100644 index 00000000..22eae2db --- /dev/null +++ b/examples/serde.rs @@ -0,0 +1,36 @@ +//! An example of implementing `serde::Serialize` and `serde::Deserialize`. +//! The `#[serde(transparent)]` attribute is recommended to serialize directly +//! to the underlying bits type without wrapping it in a `serde` newtype. + +#[cfg(feature = "serde")] +fn main() { + use serde_derive::*; + + bitflags::bitflags! { + #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] + #[serde(transparent)] + pub struct Flags: u32 { + const A = 1; + const B = 2; + const C = 4; + const D = 8; + } + } + + let flags = Flags::A | Flags::B; + + let serialized = serde_json::to_string(&flags).unwrap(); + + println!("{:?} -> {}", flags, serialized); + + assert_eq!(serialized, r#""A | B""#); + + let deserialized: Flags = serde_json::from_str(&serialized).unwrap(); + + println!("{} -> {:?}", serialized, flags); + + assert_eq!(deserialized, flags); +} + +#[cfg(not(feature = "serde"))] +fn main() {} diff --git a/src/external/serde_support.rs b/src/external/serde_support.rs index 4a164786..656a194e 100644 --- a/src/external/serde_support.rs +++ b/src/external/serde_support.rs @@ -67,8 +67,10 @@ where #[cfg(test)] mod tests { + use serde_test::{assert_tokens, Configure, Token::*}; bitflags! { - #[derive(serde_derive::Serialize, serde_derive::Deserialize)] + #[derive(serde_derive::Serialize, serde_derive::Deserialize, Debug, PartialEq, Eq)] + #[serde(transparent)] struct SerdeFlags: u32 { const A = 1; const B = 2; @@ -78,48 +80,13 @@ mod tests { } #[test] - fn test_serde_bitflags_default_serialize_empty() { - let flags = SerdeFlags::empty(); + fn test_serde_bitflags_default() { + assert_tokens(&SerdeFlags::empty().readable(), &[Str("")]); - let serialized = serde_json::to_string(&flags).unwrap(); + assert_tokens(&SerdeFlags::empty().compact(), &[U32(0)]); - assert_eq!(serialized, r#""""#); - } - - #[test] - fn test_serde_bitflags_default_serialize() { - let flags = SerdeFlags::A | SerdeFlags::B; - - let serialized = serde_json::to_string(&flags).unwrap(); - - assert_eq!(serialized, r#""A | B""#); - } - - #[test] - fn test_serde_bitflags_default_deserialize_empty() { - let deserialized: SerdeFlags = serde_json::from_str(r#""""#).unwrap(); - - let expected = SerdeFlags::empty(); - - assert_eq!(deserialized.bits(), expected.bits()); - } - - #[test] - fn test_serde_bitflags_default_deserialize() { - let deserialized: SerdeFlags = serde_json::from_str(r#""C | D""#).unwrap(); - - let expected = SerdeFlags::C | SerdeFlags::D; - - assert_eq!(deserialized.bits(), expected.bits()); - } - - #[test] - fn test_serde_bitflags_default_roundtrip() { - let flags = SerdeFlags::A | SerdeFlags::B; - - let deserialized: SerdeFlags = - serde_json::from_str(&serde_json::to_string(&flags).unwrap()).unwrap(); + assert_tokens(&(SerdeFlags::A | SerdeFlags::B).readable(), &[Str("A | B")]); - assert_eq!(deserialized.bits(), flags.bits()); + assert_tokens(&(SerdeFlags::A | SerdeFlags::B).compact(), &[U32(1 | 2)]); } } From 38e65c121e2292256b3e62570cfc698ca2b0894f Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 12:13:01 +1000 Subject: [PATCH 07/16] allow empty strings in the Display fmt but keep 0x0 in Debug for empty flags --- src/external.rs | 1 - src/external/serde_support.rs | 10 +--------- src/internal.rs | 27 +++++++++++++-------------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/external.rs b/src/external.rs index 91ad9de1..4b696acb 100644 --- a/src/external.rs +++ b/src/external.rs @@ -50,7 +50,6 @@ macro_rules! __impl_external_bitflags_serde { ) -> $crate::__private::core::result::Result { $crate::__private::serde_support::serialize_bits_default::<$InternalBitFlags, $T, S>( &self, - self.is_empty(), serializer, ) } diff --git a/src/external/serde_support.rs b/src/external/serde_support.rs index 656a194e..e1a460b9 100644 --- a/src/external/serde_support.rs +++ b/src/external/serde_support.rs @@ -6,19 +6,11 @@ use serde::{ pub fn serialize_bits_default, B: Serialize, S: Serializer>( flags: &T, - is_empty: bool, serializer: S, ) -> Result { // Serialize human-readable flags as a string like `"A | B"` if serializer.is_human_readable() { - // Special case for empty flags through `serde` - // Rather than serializing as `0x0`, serialize as - // an empty string - if is_empty { - serializer.serialize_str("") - } else { - serializer.collect_str(flags) - } + serializer.collect_str(flags) } // Serialize non-human-readable flags directly as the underlying bits else { diff --git a/src/internal.rs b/src/internal.rs index ee965594..bb31fdc7 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -58,7 +58,19 @@ macro_rules! __impl_internal_bitflags { impl $crate::__private::core::fmt::Debug for $InternalBitFlags { fn fmt(&self, f: &mut $crate::__private::core::fmt::Formatter) -> $crate::__private::core::fmt::Result { - $crate::__private::core::fmt::Display::fmt(self, f) + if self.is_empty() { + // If no flags are set then write an empty hex flag to avoid + // writing an empty string. In some contexts, like serialization, + // an empty string is preferrable, but it may be unexpected in + // others for a format not to produce any output. + // + // We can remove this `0x0` and remain compatible with `FromStr`, + // because an empty string will still parse to an empty set of flags, + // just like `0x0` does. + $crate::__private::core::write!(f, "{:#x}", <$T as $crate::__private::Bits>::EMPTY) + } else { + $crate::__private::core::fmt::Display::fmt(self, f) + } } } @@ -91,22 +103,9 @@ macro_rules! __impl_internal_bitflags { f.write_str(" | ")?; } - first = false; $crate::__private::core::write!(f, "{:#x}", extra_bits)?; } - // If no flags are set then write an empty hex flag to avoid - // writing an empty string. In some contexts, like serialization, - // an empty string is preferrable, but it may be unexpected in - // others for a format not to produce any output. - // - // We can remove this `0x0` and remain compatible with `FromStr`, - // because an empty string will still parse to an empty set of flags, - // just like `0x0` does. - if first { - $crate::__private::core::write!(f, "{:#x}", <$T as $crate::__private::Bits>::EMPTY)?; - } - $crate::__private::core::fmt::Result::Ok(()) } } From c63de2c361d61b271351e28442b6672ca76a21a7 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 12:15:08 +1000 Subject: [PATCH 08/16] add some docs to the ParseError type --- src/error.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/error.rs b/src/error.rs index b1db461a..9c97fb43 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,6 @@ use core::fmt; +/// An error encountered while parsing flags from text. #[derive(Debug)] pub struct ParseError(ParseErrorKind); @@ -21,6 +22,7 @@ enum ParseErrorKind { } impl ParseError { + /// An invalid hex flag was encountered. pub fn invalid_hex_flag(flag: impl fmt::Display) -> Self { let _flag = flag; @@ -34,6 +36,7 @@ impl ParseError { ParseError(ParseErrorKind::InvalidHexFlag { got }) } + /// A named flag that doesn't correspond to any on the flags type was encountered. pub fn invalid_named_flag(flag: impl fmt::Display) -> Self { let _flag = flag; @@ -47,6 +50,7 @@ impl ParseError { ParseError(ParseErrorKind::InvalidNamedFlag { got }) } + /// A hex or named flag wasn't found between separators. pub fn empty_flag() -> Self { ParseError(ParseErrorKind::EmptyFlag) } From 00df59e2216e3f78630fa81f3d10e2cfbe2e0ff9 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 12:47:14 +1000 Subject: [PATCH 09/16] add some docs on the text grammar --- src/lib.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index f6d1a799..7effd409 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -154,6 +154,9 @@ //! - `LowerHex` and `UpperHex`. //! - `Octal`. //! +//! Also see the _Debug and Display_ section for details about standard text +//! representations for flags types. +//! //! ## Operators //! //! The following operator traits are implemented for the generated `struct`s: @@ -267,7 +270,20 @@ //! //! ## `Debug` and `Display` //! -//! The `Debug` trait can be derived for a reasonable implementation. +//! The `Debug` trait can be derived for a reasonable implementation. This library defines a standard +//! text-based representation for flags that generated flags types can use. It uses the following +//! whitespace and case insensitive grammar: +//! +//! - _Flags:_ (_Flag_)`|`* +//! - _Flag:_ _Identifier_ | _HexNumber_ +//! - _Identifier:_ Any Rust identifier +//! - _HexNumber_: `0x`([0-9a-zA-Z])* +//! +//! As an example, this is how `Flags::A | Flags::B | 0x0c` can be represented as text: +//! +//! ```text +//! A | B | 0x0c +//! ``` //! //! ## `PartialEq` and `PartialOrd` //! From f8605164f033a24bf0906ea43cb052a774c6f564 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 14:43:30 +1000 Subject: [PATCH 10/16] just rely on rustup in CI --- .github/workflows/rust.yml | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f924b12d..3059fe8f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -40,11 +40,7 @@ jobs: uses: actions/checkout@v2 - name: Install Rust Toolchain - uses: actions-rs/toolchain@v1 - with: - override: true - profile: minimal - toolchain: ${{ matrix.channel }}-${{ matrix.rust_target }} + run: rustup default ${{ matrix.channel }}-${{ matrix.rust_target }} - name: Install cargo-hack run: cargo install cargo-hack @@ -66,12 +62,9 @@ jobs: uses: actions/checkout@v2 - name: Install Rust toolchain - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: nightly - target: thumbv6m-none-eabi - override: true + run: | + - rustup default nightly + - rustup target add thumbv6m-none-eabi - name: Default features uses: actions-rs/cargo@v1 From 2233facd0fddd2f2abb25fef9e80ed074bd89fc8 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 14:44:41 +1000 Subject: [PATCH 11/16] yaml --- .github/workflows/rust.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3059fe8f..ca2832b7 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -63,8 +63,8 @@ jobs: - name: Install Rust toolchain run: | - - rustup default nightly - - rustup target add thumbv6m-none-eabi + rustup default nightly + rustup target add thumbv6m-none-eabi - name: Default features uses: actions-rs/cargo@v1 From e7afc931d17c933cc8823e334e1a8fb1953ed6ac Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 3 Feb 2023 14:57:19 +1000 Subject: [PATCH 12/16] fix up by-ref for fields when std is enabled --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 9c97fb43..11121150 100644 --- a/src/error.rs +++ b/src/error.rs @@ -58,7 +58,7 @@ impl ParseError { impl fmt::Display for ParseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self.0 { + match &self.0 { ParseErrorKind::InvalidNamedFlag { got } => { let _got = got; From 575236cb243171f3095bf26e958a83465f35e251 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Wed, 8 Feb 2023 12:26:46 +1000 Subject: [PATCH 13/16] shift ParseError into parser module so it can be built out --- examples/fmt.rs | 4 ++-- src/internal.rs | 8 ++++---- src/lib.rs | 6 ++---- src/{error.rs => parser.rs} | 2 ++ 4 files changed, 10 insertions(+), 10 deletions(-) rename src/{error.rs => parser.rs} (98%) diff --git a/examples/fmt.rs b/examples/fmt.rs index 84457642..3bb9b8c4 100644 --- a/examples/fmt.rs +++ b/examples/fmt.rs @@ -2,7 +2,7 @@ use core::{fmt, str}; -fn main() -> Result<(), bitflags::ParseError> { +fn main() -> Result<(), bitflags::parser::ParseError> { bitflags::bitflags! { // You can `#[derive]` the `Debug` trait, but implementing it manually // can produce output like `A | B` instead of `Flags(A | B)`. @@ -29,7 +29,7 @@ fn main() -> Result<(), bitflags::ParseError> { } impl str::FromStr for Flags { - type Err = bitflags::ParseError; + type Err = bitflags::parser::ParseError; fn from_str(flags: &str) -> Result { Ok(Self(flags.parse()?)) diff --git a/src/internal.rs b/src/internal.rs index bb31fdc7..272b08eb 100644 --- a/src/internal.rs +++ b/src/internal.rs @@ -112,7 +112,7 @@ macro_rules! __impl_internal_bitflags { // The impl for `FromStr` should parse anything produced by `Display` impl $crate::__private::core::str::FromStr for $InternalBitFlags { - type Err = $crate::ParseError; + type Err = $crate::parser::ParseError; fn from_str(s: &str) -> $crate::__private::core::result::Result { let s = s.trim(); @@ -129,14 +129,14 @@ macro_rules! __impl_internal_bitflags { // If the flag is empty then we've got missing input if flag.is_empty() { - return $crate::__private::core::result::Result::Err($crate::ParseError::empty_flag()); + return $crate::__private::core::result::Result::Err($crate::parser::ParseError::empty_flag()); } // If the flag starts with `0x` then it's a hex number // Parse it directly to the underlying bits type let parsed_flag = if flag.starts_with("0x") { let flag = &flag[2..]; - let bits = <$T>::from_str_radix(flag, 16).map_err(|_| $crate::ParseError::invalid_hex_flag(flag))?; + let bits = <$T>::from_str_radix(flag, 16).map_err(|_| $crate::parser::ParseError::invalid_hex_flag(flag))?; Self::from_bits_retain(bits) } @@ -144,7 +144,7 @@ macro_rules! __impl_internal_bitflags { // The generated flags type will determine whether // or not it's a valid identifier else { - Self::from_name(flag).ok_or_else(|| $crate::ParseError::invalid_named_flag(flag))? + Self::from_name(flag).ok_or_else(|| $crate::parser::ParseError::invalid_named_flag(flag))? }; parsed_flags.insert(parsed_flag); diff --git a/src/lib.rs b/src/lib.rs index 7effd409..dc1fc55b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -361,11 +361,9 @@ #[doc(inline)] pub use traits::BitFlags; -mod error; +pub mod parser; mod traits; -pub use error::*; - #[doc(hidden)] pub mod __private { pub use crate::{external::*, traits::*}; @@ -630,7 +628,7 @@ mod tests { } impl str::FromStr for FmtFlags { - type Err = crate::ParseError; + type Err = crate::parser::ParseError; fn from_str(flags: &str) -> Result { Ok(Self(flags.parse()?)) diff --git a/src/error.rs b/src/parser.rs similarity index 98% rename from src/error.rs rename to src/parser.rs index 11121150..fc74fc28 100644 --- a/src/error.rs +++ b/src/parser.rs @@ -1,3 +1,5 @@ +//! Parsing flags from text. + use core::fmt; /// An error encountered while parsing flags from text. From 514952c3d60cb6658f40bb8d43a6e22ea3465415 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Wed, 8 Feb 2023 13:30:36 +1000 Subject: [PATCH 14/16] remove the serde legacy example --- examples/serde_compat.rs | 125 --------------------------------------- 1 file changed, 125 deletions(-) delete mode 100644 examples/serde_compat.rs diff --git a/examples/serde_compat.rs b/examples/serde_compat.rs deleted file mode 100644 index 6029aad0..00000000 --- a/examples/serde_compat.rs +++ /dev/null @@ -1,125 +0,0 @@ -//! An example of implementing `serde::Serialize` and `serde::Deserialize` equivalently to how -//! `#[derive(Serialize, Deserialize)]` would on `bitflags` `1.x` types. - -#[cfg(feature = "serde")] -fn main() { - bitflags::bitflags! { - // Removed: `serde` traits from the `#[derive]` attribute - // #[derive(Serialize, Deserialize)] - #[derive(Debug, PartialEq, Eq)] - pub struct Flags: u32 { - const A = 1; - const B = 2; - const C = 4; - const D = 8; - } - } - - // Added: Manual `Serialize` and `Deserialize` implementations based on a generic impl - impl serde::Serialize for Flags { - fn serialize(&self, serializer: S) -> Result { - legacy_format::serialize(self, serializer) - } - } - - impl<'de> serde::Deserialize<'de> for Flags { - fn deserialize>(deserializer: D) -> Result { - legacy_format::deserialize(deserializer) - } - } - - pub mod legacy_format { - //! This module is a generic implementation of `Serialize` and `Deserialize` that can be used by - //! any flags type generated by `bitflags!`. - //! - //! Don't be intimidated by the amount of `serde` code in here! It boils down to serializing and deserializing - //! a struct with a single `bits` field. It may be converted into a library at some point, but is also suitable - //! to copy into your own project if you need it. - - use core::{any::type_name, fmt}; - use serde::{ - de::{Error, MapAccess, Visitor}, - ser::SerializeStruct, - Deserialize, Deserializer, Serialize, Serializer, - }; - - use bitflags::BitFlags; - - /// Serialize a flags type equivalently to how `#[derive(Serialize)]` on a flags type - /// from `bitflags` `1.x` would. - pub fn serialize( - flags: &T, - serializer: S, - ) -> Result - where - ::Bits: Serialize, - { - let mut serialize_struct = serializer.serialize_struct(type_name::(), 1)?; - serialize_struct.serialize_field("bits", &flags.bits())?; - serialize_struct.end() - } - - /// Deserialize a flags type equivalently to how `#[derive(Deserialize)]` on a flags type - /// from `bitflags` `1.x` would. - pub fn deserialize<'de, T: BitFlags, D: Deserializer<'de>>( - deserializer: D, - ) -> Result - where - ::Bits: Deserialize<'de>, - { - struct BitsVisitor(core::marker::PhantomData); - - impl<'de, T: Deserialize<'de>> Visitor<'de> for BitsVisitor { - type Value = T; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a primitive bitflags value wrapped in a struct") - } - - fn visit_map>(self, mut map: A) -> Result { - let mut bits = None; - - while let Some(key) = map.next_key()? { - match key { - "bits" => { - if bits.is_some() { - return Err(Error::duplicate_field("bits")); - } - - bits = Some(map.next_value()?); - } - v => return Err(Error::unknown_field(v, &["bits"])), - } - } - - bits.ok_or_else(|| Error::missing_field("bits")) - } - } - - let bits = deserializer.deserialize_struct( - type_name::(), - &["bits"], - BitsVisitor(Default::default()), - )?; - - Ok(T::from_bits_retain(bits)) - } - } - - let flags = Flags::A | Flags::B; - - let serialized = serde_json::to_string(&flags).unwrap(); - - println!("{:?} -> {}", flags, serialized); - - assert_eq!(serialized, r#"{"bits":3}"#); - - let deserialized: Flags = serde_json::from_str(&serialized).unwrap(); - - println!("{} -> {:?}", serialized, flags); - - assert_eq!(deserialized, flags); -} - -#[cfg(not(feature = "serde"))] -fn main() {} From 3d4777af478738a1c074c543c9455270acf559ac Mon Sep 17 00:00:00 2001 From: KodrAus Date: Wed, 8 Feb 2023 16:52:06 +1000 Subject: [PATCH 15/16] move text grammar to the parser module --- src/lib.rs | 15 ++------------- src/parser.rs | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index dc1fc55b..69da4fd6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -271,19 +271,8 @@ //! ## `Debug` and `Display` //! //! The `Debug` trait can be derived for a reasonable implementation. This library defines a standard -//! text-based representation for flags that generated flags types can use. It uses the following -//! whitespace and case insensitive grammar: -//! -//! - _Flags:_ (_Flag_)`|`* -//! - _Flag:_ _Identifier_ | _HexNumber_ -//! - _Identifier:_ Any Rust identifier -//! - _HexNumber_: `0x`([0-9a-zA-Z])* -//! -//! As an example, this is how `Flags::A | Flags::B | 0x0c` can be represented as text: -//! -//! ```text -//! A | B | 0x0c -//! ``` +//! text-based representation for flags that generated flags types can use. For details on the exact +//! grammar, see the [`parser`] module. //! //! ## `PartialEq` and `PartialOrd` //! diff --git a/src/parser.rs b/src/parser.rs index fc74fc28..4c54c843 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,4 +1,18 @@ //! Parsing flags from text. +//! +//! `bitflags` defines the following whitespace-insensitive grammar for flags formatted +//! as text: +//! +//! - _Flags:_ (_Flag_)`|`* +//! - _Flag:_ _Identifier_ | _HexNumber_ +//! - _Identifier:_ Any Rust identifier +//! - _HexNumber_: `0x`([0-9a-zA-Z])* +//! +//! As an example, this is how `Flags::A | Flags::B | 0x0c` can be represented as text: +//! +//! ```text +//! A | B | 0x0c +//! ``` use core::fmt; From bdd5f42740eba9c59eee271a3ef6d3af7f8900ad Mon Sep 17 00:00:00 2001 From: KodrAus Date: Wed, 8 Feb 2023 17:23:16 +1000 Subject: [PATCH 16/16] add some basic parsing and formatting benches --- .github/workflows/rust.yml | 16 +++++++ benches/parse.rs | 96 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 benches/parse.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index ca2832b7..9fd66642 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -54,6 +54,22 @@ jobs: - name: Smoke test run: cargo run --manifest-path tests/smoke-test/Cargo.toml + benches: + name: Benches + runs-on: ubuntu-latest + steps: + - name: Checkout sources + uses: actions/checkout@v2 + + - name: Install Rust toolchain + run: rustup default nightly + + - name: Default features + uses: actions-rs/cargo@v1 + with: + command: bench + args: --no-run + embedded: name: Build (embedded) runs-on: ubuntu-latest diff --git a/benches/parse.rs b/benches/parse.rs new file mode 100644 index 00000000..caa92034 --- /dev/null +++ b/benches/parse.rs @@ -0,0 +1,96 @@ +#![feature(test)] + +extern crate test; + +use std::{ + fmt::{self, Display}, + str::FromStr, +}; + +bitflags::bitflags! { + struct Flags10: u32 { + const A = 0b0000_0000_0000_0001; + const B = 0b0000_0000_0000_0010; + const C = 0b0000_0000_0000_0100; + const D = 0b0000_0000_0000_1000; + const E = 0b0000_0000_0001_0000; + const F = 0b0000_0000_0010_0000; + const G = 0b0000_0000_0100_0000; + const H = 0b0000_0000_1000_0000; + const I = 0b0000_0001_0000_0000; + const J = 0b0000_0010_0000_0000; + } +} + +impl FromStr for Flags10 { + type Err = bitflags::parser::ParseError; + + fn from_str(flags: &str) -> Result { + Ok(Flags10(flags.parse()?)) + } +} + +impl Display for Flags10 { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Display::fmt(&self.0, f) + } +} + +#[bench] +fn format_flags_1_present(b: &mut test::Bencher) { + b.iter(|| Flags10::J.to_string()) +} + +#[bench] +fn format_flags_5_present(b: &mut test::Bencher) { + b.iter(|| (Flags10::F | Flags10::G | Flags10::H | Flags10::I | Flags10::J).to_string()) +} + +#[bench] +fn format_flags_10_present(b: &mut test::Bencher) { + b.iter(|| { + (Flags10::A + | Flags10::B + | Flags10::C + | Flags10::D + | Flags10::E + | Flags10::F + | Flags10::G + | Flags10::H + | Flags10::I + | Flags10::J) + .to_string() + }) +} + +#[bench] +fn parse_flags_1_10(b: &mut test::Bencher) { + b.iter(|| { + let flags: Flags10 = "J".parse().unwrap(); + flags + }) +} + +#[bench] +fn parse_flags_5_10(b: &mut test::Bencher) { + b.iter(|| { + let flags: Flags10 = "F | G | H | I | J".parse().unwrap(); + flags + }) +} + +#[bench] +fn parse_flags_10_10(b: &mut test::Bencher) { + b.iter(|| { + let flags: Flags10 = "A | B | C | D | E | F | G | H | I | J".parse().unwrap(); + flags + }) +} + +#[bench] +fn parse_flags_1_10_hex(b: &mut test::Bencher) { + b.iter(|| { + let flags: Flags10 = "0xFF".parse().unwrap(); + flags + }) +}