From b9621b596db7990f41c6622fc51545f8f5a8fcc4 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sun, 29 May 2022 14:25:33 -0700 Subject: [PATCH] feat(bitfield): packing spec type safety This branch changes bitfield packing specs to ensure that a field of a given bitfield type may not be used to pack into/from a bitfield of another type. Signed-off-by: Eliza Weisman --- bitfield/src/bitfield.rs | 60 +++++++++++++++++++++++++++++----------- bitfield/src/pack.rs | 50 ++++++++++++++++----------------- 2 files changed, 69 insertions(+), 41 deletions(-) diff --git a/bitfield/src/bitfield.rs b/bitfield/src/bitfield.rs index 8e83edb8..ce007e71 100644 --- a/bitfield/src/bitfield.rs +++ b/bitfield/src/bitfield.rs @@ -181,6 +181,34 @@ /// assert_eq!(formatted, expected); /// ``` /// +/// Packing specs from one bitfield type may *not* be used with a different +/// bitfield type's `get`, `set`, or `with` methods. For example, the following +/// is a type error: +/// +/// ```compile_fail +/// use mycelium_bitfield::bitfield; +/// +/// bitfield! { +/// struct Bitfield1 { +/// pub const FOO: bool; +/// pub const BAR: bool; +/// pub const BAZ = 6; +/// } +/// } +/// +/// bitfield! { +/// struct Bitfield2 { +/// pub const ALICE = 2; +/// pub const BOB = 4; +/// pub const CHARLIE = 2; +/// } +/// } +/// +/// +/// // This is a *type error*, because `Bitfield2`'s field `ALICE` cannot be +/// // used with a `Bitfield2` value: +/// let bits = Bitfield1::new().with(Bitfield2::ALICE, 0b11); +/// ``` /// [`fmt::Debug`]: core::fmt::Debug /// [`fmt::Display`]: core::fmt::Display /// [`fmt::Binary`]: core::fmt::Binary @@ -227,7 +255,7 @@ macro_rules! bitfield { )+ } - const FIELDS: &'static [(&'static str, $crate::bitfield! { @t $T, $T })] = &[$( + const FIELDS: &'static [(&'static str, $crate::bitfield! { @t $T, $T, Self })] = &[$( (stringify!($Field), Self::$Field.typed()) ),+]; @@ -243,7 +271,7 @@ macro_rules! bitfield { /// Packs the bit representation of `value` into `self` at the bit /// range designated by `field`, returning a new bitfield. - $vis fn with(self, field: $crate::bitfield! { @t $T, T }, value: T) -> Self + $vis fn with(self, field: $crate::bitfield! { @t $T, T, Self }, value: T) -> Self where T: $crate::FromBits<$T>, { @@ -253,7 +281,7 @@ macro_rules! bitfield { /// Packs the bit representation of `value` into `self` at the range /// designated by `field`, mutating `self` in place. - $vis fn set(&mut self, field: $crate::bitfield! { @t $T, T }, value: T) -> &mut Self + $vis fn set(&mut self, field: $crate::bitfield! { @t $T, T, Self }, value: T) -> &mut Self where T: $crate::FromBits<$T>, { @@ -269,7 +297,7 @@ macro_rules! bitfield { /// This method panics if `self` does not contain a valid bit /// pattern for a `T`-typed value, as determined by `T`'s /// [`mycelium_bitfield::FromBits::try_from_bits`] implementation. - $vis fn get(self, field: $crate::bitfield! { @t $T, T }) -> T + $vis fn get(self, field: $crate::bitfield! { @t $T, T, Self }) -> T where T: $crate::FromBits<$T>, { @@ -286,7 +314,7 @@ macro_rules! bitfield { /// - `Err(T::Error)` if `src` does not contain a valid bit /// pattern for a `T`-typed value, as determined by `T`'s /// [`mycelium_bitfield::FromBits::try_from_bits`] implementation. - $vis fn try_get(self, field: $crate::bitfield! { @t $T, T }) -> Result + $vis fn try_get(self, field: $crate::bitfield! { @t $T, T, Self }) -> Result where T: $crate::FromBits<$T>, { @@ -297,7 +325,7 @@ macro_rules! bitfield { /// /// This is intended to be used in unit tests. $vis fn assert_valid() { - <$crate::bitfield! { @t $T, $T }>::assert_all_valid(&Self::FIELDS); + <$crate::bitfield! { @t $T, $T, Self }>::assert_all_valid(&Self::FIELDS); } } @@ -420,7 +448,7 @@ macro_rules! bitfield { $($rest:tt)* ) => { $(#[$meta])* - $vis const $Field: $crate::bitfield!{ @t $T, $T } = Self::$Prev.next($value); + $vis const $Field: $crate::bitfield!{ @t $T, $T, Self } = Self::$Prev.next($value); $crate::bitfield!{ @field<$T>, prev: $Field: $($rest)* } }; @@ -430,7 +458,7 @@ macro_rules! bitfield { $($rest:tt)* ) => { $(#[$meta])* - $vis const $Field: $crate::bitfield!{ @t $T, $Val } = Self::$Prev.then::<$Val>(); + $vis const $Field: $crate::bitfield!{ @t $T, $Val, Self } = Self::$Prev.then::<$Val>(); $crate::bitfield!{ @field<$T>, prev: $Field: $($rest)* } }; @@ -441,7 +469,7 @@ macro_rules! bitfield { $($rest:tt)* ) => { $(#[$meta])* - $vis const $Field: $crate::bitfield!{ @t $T, $T } = <$crate::bitfield!{ @t $T, $T }>::least_significant($value); + $vis const $Field: $crate::bitfield!{ @t $T, $T, Self } = <$crate::bitfield!{ @t $T, $T, () }>::least_significant($value).typed(); $crate::bitfield!{ @field<$T>, prev: $Field: $($rest)* } }; @@ -451,7 +479,7 @@ macro_rules! bitfield { $($rest:tt)* ) => { $(#[$meta])* - $vis const $Field: $crate::bitfield!{ @t $T, $Val } = <$crate::bitfield!{ @t $T, $Val } >::first(); + $vis const $Field: $crate::bitfield!{ @t $T, $Val, Self } = <$crate::bitfield!{ @t $T, $Val, Self } >::first(); $crate::bitfield!{ @field<$T>, prev: $Field: $($rest)* } }; @@ -528,12 +556,12 @@ macro_rules! bitfield { // $crate::bitfield! { @process_derives $vis struct $Name<$T> { $Next, $($Before),* } { $($rest)* } } // }; - (@t usize, $V:ty) => { $crate::PackUsize<$V> }; - (@t u64, $V:ty) => { $crate::Pack64<$V> }; - (@t u32, $V:ty) => { $crate::Pack32<$V> }; - (@t u16, $V:ty) => { $crate::Pack16<$V> }; - (@t u8, $V:ty) => { $crate::Pack8<$V> }; - (@t $T:ty, $V:ty) => { compile_error!(concat!("unsupported bitfield type `", stringify!($T), "`; expected one of `usize`, `u64`, `u32`, `u16`, or `u8`")) } + (@t usize, $V:ty, $F:ty) => { $crate::PackUsize<$V, $F> }; + (@t u64, $V:ty, $F:ty) => { $crate::Pack64<$V, $F> }; + (@t u32, $V:ty, $F:ty) => { $crate::Pack32<$V, $F> }; + (@t u16, $V:ty, $F:ty) => { $crate::Pack16<$V, $F> }; + (@t u8, $V:ty, $F:ty) => { $crate::Pack8<$V, $F> }; + (@t $T:ty, $V:ty, $F:ty) => { compile_error!(concat!("unsupported bitfield type `", stringify!($T), "`; expected one of `usize`, `u64`, `u32`, `u16`, or `u8`")) } } #[cfg(test)] diff --git a/bitfield/src/pack.rs b/bitfield/src/pack.rs index 36b3a093..b04f30fe 100644 --- a/bitfield/src/pack.rs +++ b/bitfield/src/pack.rs @@ -15,10 +15,10 @@ macro_rules! make_packers { stringify!($Bits), "`] values." )] - pub struct $Pack { + pub struct $Pack { mask: $Bits, shift: u32, - _dst_ty: PhantomData, + _dst_ty: PhantomData, } #[doc = concat!( @@ -38,7 +38,7 @@ macro_rules! make_packers { dst_shr: $Bits, } - impl $Pack { + impl $Pack<$Bits> { #[doc = concat!( "Wrap a [`", stringify!($Bits), @@ -133,7 +133,7 @@ macro_rules! make_packers { } } - impl $Pack { + impl $Pack { // XXX(eliza): why is this always `u32`? ask the stdlib i guess... const SIZE_BITS: u32 = <$Bits>::MAX.leading_ones(); @@ -152,7 +152,7 @@ macro_rules! make_packers { } #[doc(hidden)] - pub const fn typed(self) -> $Pack + pub const fn typed(self) -> $Pack where T2: FromBits<$Bits> { @@ -215,7 +215,7 @@ macro_rules! make_packers { base } - pub const fn then(&self) -> $Pack + pub const fn then(&self) -> $Pack where T2: FromBits<$Bits> { @@ -224,7 +224,7 @@ macro_rules! make_packers { /// Returns a packer for packing a value into the next more-significant /// `n` from `self`. - pub const fn next(&self, n: u32) -> $Pack { + pub const fn next(&self, n: u32) -> $Pack<$Bits, F> { let shift = self.shift_next(); let mask = Self::mk_mask(n) << shift; $Pack { mask, shift, _dst_ty: core::marker::PhantomData, } @@ -232,7 +232,7 @@ macro_rules! make_packers { /// Returns a packer for packing a value into all the remaining /// more-significant bits after `self`. - pub const fn remaining(&self) -> $Pack { + pub const fn remaining(&self) -> $Pack<$Bits, F> { let shift = self.shift_next(); let n = Self::SIZE_BITS - shift; let mask = Self::mk_mask(n) << shift; @@ -405,14 +405,14 @@ macro_rules! make_packers { } } - impl $Pack + impl $Pack where T: FromBits<$Bits>, { /// Returns a packing spec for packing a `T`-typed value in the /// first [`T::BITS`](FromBits::BITS) least-significant bits. pub const fn first() -> Self { - $Pack::least_significant(T::BITS).typed() + $Pack::<$Bits, ()>::least_significant(T::BITS).typed() } /// Returns a pair type for packing bits from the range @@ -422,7 +422,7 @@ macro_rules! make_packers { /// The packing pair can be used to pack bits from one location /// into another location, and vice versa. pub const fn pair_at(&self, at: u32) -> $Pair { - let dst = $Pack::starting_at(at, self.bits()).typed(); + let dst = $Pack::<$Bits, ()>::starting_at(at, self.bits()).typed(); let at = at.saturating_sub(1); // TODO(eliza): validate that `at + self.bits() < N_BITS` in // const fn somehow lol @@ -435,7 +435,7 @@ macro_rules! make_packers { (0, (self.shift - at) as $Bits) }; $Pair { - src: *self, + src: self.typed(), dst, dst_shl, dst_shr, @@ -521,7 +521,7 @@ macro_rules! make_packers { } } - impl Clone for $Pack { + impl Clone for $Pack { fn clone(&self) -> Self { Self { mask: self.mask, @@ -531,9 +531,9 @@ macro_rules! make_packers { } } - impl Copy for $Pack {} + impl Copy for $Pack {} - impl fmt::Debug for $Pack { + impl fmt::Debug for $Pack { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct(stringify!($Pack)) .field("mask", &format_args!("{:#b}", self.mask)) @@ -543,7 +543,7 @@ macro_rules! make_packers { } } - impl fmt::UpperHex for $Pack { + impl fmt::UpperHex for $Pack { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct(stringify!($Pack)) .field("mask", &format_args!("{:#X}", self.mask)) @@ -553,7 +553,7 @@ macro_rules! make_packers { } } - impl fmt::LowerHex for $Pack { + impl fmt::LowerHex for $Pack { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct(stringify!($Pack)) .field("mask", &format_args!("{:#x}", self.mask)) @@ -563,7 +563,7 @@ macro_rules! make_packers { } } - impl fmt::Binary for $Pack { + impl fmt::Binary for $Pack { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct(stringify!($Pack)) .field("mask", &format_args!("{:#b}", self.mask)) @@ -579,28 +579,28 @@ macro_rules! make_packers { } } - impl PartialEq<$Pack> for $Pack { + impl PartialEq<$Pack> for $Pack { #[inline] - fn eq(&self, other: &$Pack) -> bool { + fn eq(&self, other: &$Pack) -> bool { self.mask == other.mask && self.shift == other.shift } } - impl PartialEq<&'_ $Pack> for $Pack { + impl PartialEq<&'_ $Pack> for $Pack { #[inline] - fn eq(&self, other: &&'_ $Pack) -> bool { + fn eq(&self, other: &&'_ $Pack) -> bool { self.eq(*other) } } - impl PartialEq<$Pack> for &'_ $Pack { + impl PartialEq<$Pack> for &'_ $Pack { #[inline] - fn eq(&self, other: &$Pack) -> bool { + fn eq(&self, other: &$Pack) -> bool { (*self).eq(other) } } - impl Eq for $Pack {} + impl Eq for $Pack {} // === packing type ===