From 2b5c92027371de3fd1e1cea616038b0bc5b46d7a Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Fri, 29 Sep 2023 18:46:10 +0000 Subject: [PATCH] [byteorder] Implement `core::ops` traits Release 0.7.6 --- Cargo.toml | 8 +- src/byteorder.rs | 270 ++++++++++++++++++++++++++++++++++--- zerocopy-derive/Cargo.toml | 4 +- 3 files changed, 260 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7acd28389d..842b754709 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ [package] edition = "2021" name = "zerocopy" -version = "0.7.5" +version = "0.7.6" authors = ["Joshua Liebow-Feeser "] description = "Utilities for zero-copy parsing and serialization" license = "BSD-2-Clause" @@ -41,7 +41,7 @@ simd-nightly = ["simd"] __internal_use_only_features_that_work_on_stable = ["alloc", "derive", "simd"] [dependencies] -zerocopy-derive = { version = "=0.7.5", path = "zerocopy-derive", optional = true } +zerocopy-derive = { version = "=0.7.6", path = "zerocopy-derive", optional = true } [dependencies.byteorder] version = "1.3" @@ -52,7 +52,7 @@ optional = true # zerocopy-derive remain equal, even if the 'derive' feature isn't used. # See: https://github.com/matklad/macro-dep-test [target.'cfg(any())'.dependencies] -zerocopy-derive = { version = "=0.7.5", path = "zerocopy-derive" } +zerocopy-derive = { version = "=0.7.6", path = "zerocopy-derive" } [dev-dependencies] assert_matches = "1.5" @@ -67,4 +67,4 @@ testutil = { path = "testutil" } # CI test failures. trybuild = "=1.0.80" # In tests, unlike in production, zerocopy-derive is not optional -zerocopy-derive = { version = "=0.7.5", path = "zerocopy-derive" } +zerocopy-derive = { version = "=0.7.6", path = "zerocopy-derive" } diff --git a/src/byteorder.rs b/src/byteorder.rs index 01c3c4751f..76c375835f 100644 --- a/src/byteorder.rs +++ b/src/byteorder.rs @@ -93,13 +93,12 @@ macro_rules! impl_fmt_traits { impl_fmt_trait!($name, $native, Display); }; ($name:ident, $native:ident, "unsigned integer") => { - impl_fmt_traits!($name, $native, @all_traits); + impl_fmt_traits!($name, $native, @all_types); }; ($name:ident, $native:ident, "signed integer") => { - impl_fmt_traits!($name, $native, @all_traits); + impl_fmt_traits!($name, $native, @all_types); }; - - ($name:ident, $native:ident, @all_traits) => { + ($name:ident, $native:ident, @all_types) => { impl_fmt_trait!($name, $native, Display); impl_fmt_trait!($name, $native, Octal); impl_fmt_trait!($name, $native, LowerHex); @@ -108,6 +107,101 @@ macro_rules! impl_fmt_traits { }; } +macro_rules! impl_ops_traits { + ($name:ident, $native:ident, "floating point number") => { + impl_ops_traits!($name, $native, @all_types); + impl_ops_traits!($name, $native, @signed_integer_floating_point); + }; + ($name:ident, $native:ident, "unsigned integer") => { + impl_ops_traits!($name, $native, @signed_unsigned_integer); + impl_ops_traits!($name, $native, @all_types); + }; + ($name:ident, $native:ident, "signed integer") => { + impl_ops_traits!($name, $native, @signed_unsigned_integer); + impl_ops_traits!($name, $native, @signed_integer_floating_point); + impl_ops_traits!($name, $native, @all_types); + }; + ($name:ident, $native:ident, @signed_unsigned_integer) => { + impl_ops_traits!(@without_byteorder_swap $name, $native, BitAnd, bitand, BitAndAssign, bitand_assign); + impl_ops_traits!(@without_byteorder_swap $name, $native, BitOr, bitor, BitOrAssign, bitor_assign); + impl_ops_traits!(@without_byteorder_swap $name, $native, BitXor, bitxor, BitXorAssign, bitxor_assign); + impl_ops_traits!(@with_byteorder_swap $name, $native, Shl, shl, ShlAssign, shl_assign); + impl_ops_traits!(@with_byteorder_swap $name, $native, Shr, shr, ShrAssign, shr_assign); + + impl core::ops::Not for $name { + type Output = $name; + + #[inline(always)] + fn not(self) -> $name { + let self_native = $native::from_ne_bytes(self.0); + $name((!self_native).to_ne_bytes(), PhantomData) + } + } + }; + ($name:ident, $native:ident, @signed_integer_floating_point) => { + impl core::ops::Neg for $name { + type Output = $name; + + #[inline(always)] + fn neg(self) -> $name { + let self_native: $native = self.get(); + #[allow(clippy::arithmetic_side_effects)] + $name::::new(-self_native) + } + } + }; + ($name:ident, $native:ident, @all_types) => { + impl_ops_traits!(@with_byteorder_swap $name, $native, Add, add, AddAssign, add_assign); + impl_ops_traits!(@with_byteorder_swap $name, $native, Div, div, DivAssign, div_assign); + impl_ops_traits!(@with_byteorder_swap $name, $native, Mul, mul, MulAssign, mul_assign); + impl_ops_traits!(@with_byteorder_swap $name, $native, Rem, rem, RemAssign, rem_assign); + impl_ops_traits!(@with_byteorder_swap $name, $native, Sub, sub, SubAssign, sub_assign); + }; + (@with_byteorder_swap $name:ident, $native:ident, $trait:ident, $method:ident, $trait_assign:ident, $method_assign:ident) => { + impl core::ops::$trait for $name { + type Output = $name; + + #[inline(always)] + fn $method(self, rhs: $name) -> $name { + let self_native: $native = self.get(); + let rhs_native: $native = rhs.get(); + let result_native = core::ops::$trait::$method(self_native, rhs_native); + $name::::new(result_native) + } + } + + impl core::ops::$trait_assign for $name { + #[inline(always)] + fn $method_assign(&mut self, rhs: $name) { + *self = core::ops::$trait::$method(*self, rhs); + } + } + }; + // Implement traits in terms of the same trait on the native type, but + // without performing a byte order swap. This only works for bitwise + // operations like `&`, `|`, etc. + (@without_byteorder_swap $name:ident, $native:ident, $trait:ident, $method:ident, $trait_assign:ident, $method_assign:ident) => { + impl core::ops::$trait for $name { + type Output = $name; + + #[inline(always)] + fn $method(self, rhs: $name) -> $name { + let self_native = $native::from_ne_bytes(self.0); + let rhs_native = $native::from_ne_bytes(rhs.0); + let result_native = core::ops::$trait::$method(self_native, rhs_native); + $name(result_native.to_ne_bytes(), PhantomData) + } + } + + impl core::ops::$trait_assign for $name { + #[inline(always)] + fn $method_assign(&mut self, rhs: $name) { + *self = core::ops::$trait::$method(*self, rhs); + } + } + }; +} + macro_rules! doc_comment { ($x:expr, $($tt:tt)*) => { #[doc = $x] @@ -347,6 +441,7 @@ example of how it can be used for parsing UDP packets. } impl_fmt_traits!($name, $native, $number_kind); + impl_ops_traits!($name, $native, $number_kind); impl Debug for $name { #[inline] @@ -566,6 +661,14 @@ mod tests { } fn is_nan(self) -> bool; + + fn checked_add(self, rhs: Self) -> Option; + fn checked_div(self, rhs: Self) -> Option; + fn checked_mul(self, rhs: Self) -> Option; + fn checked_rem(self, rhs: Self) -> Option; + fn checked_sub(self, rhs: Self) -> Option; + fn checked_shl(self, rhs: Self) -> Option; + fn checked_shr(self, rhs: Self) -> Option; } trait ByteArray: @@ -618,7 +721,7 @@ mod tests { } macro_rules! impl_traits { - ($name:ident, $native:ident, $bytes:expr, $sign:ident $(, $is_nan:ident)?) => { + ($name:ident, $native:ident, $bytes:expr, $sign:ident $(, @$float:ident)?) => { impl Native for $native { // For some types, `0 as $native` is required (for example, when // `$native` is a floating-point type; `0` is an integer), but @@ -631,9 +734,7 @@ mod tests { type Distribution = Standard; const DIST: Standard = Standard; - fn is_nan(self) -> bool { - false $(|| self.$is_nan())? - } + impl_traits!(@float_dependent_methods $(@$float)?); } impl ByteOrderType for $name { @@ -665,6 +766,26 @@ mod tests { impl_byte_order_type_unsigned!($name, $sign); }; + (@float_dependent_methods) => { + fn is_nan(self) -> bool { false } + fn checked_add(self, rhs: Self) -> Option { self.checked_add(rhs) } + fn checked_div(self, rhs: Self) -> Option { self.checked_div(rhs) } + fn checked_mul(self, rhs: Self) -> Option { self.checked_mul(rhs) } + fn checked_rem(self, rhs: Self) -> Option { self.checked_rem(rhs) } + fn checked_sub(self, rhs: Self) -> Option { self.checked_sub(rhs) } + fn checked_shl(self, rhs: Self) -> Option { self.checked_shl(rhs.try_into().unwrap_or(u32::MAX)) } + fn checked_shr(self, rhs: Self) -> Option { self.checked_shr(rhs.try_into().unwrap_or(u32::MAX)) } + }; + (@float_dependent_methods @float) => { + fn is_nan(self) -> bool { self.is_nan() } + fn checked_add(self, rhs: Self) -> Option { Some(self + rhs) } + fn checked_div(self, rhs: Self) -> Option { Some(self / rhs) } + fn checked_mul(self, rhs: Self) -> Option { Some(self * rhs) } + fn checked_rem(self, rhs: Self) -> Option { Some(self % rhs) } + fn checked_sub(self, rhs: Self) -> Option { Some(self - rhs) } + fn checked_shl(self, _rhs: Self) -> Option { unimplemented!() } + fn checked_shr(self, _rhs: Self) -> Option { unimplemented!() } + }; } impl_traits!(U16, u16, 2, unsigned); @@ -675,30 +796,39 @@ mod tests { impl_traits!(I32, i32, 4, signed); impl_traits!(I64, i64, 8, signed); impl_traits!(I128, i128, 16, signed); - impl_traits!(F32, f32, 4, signed, is_nan); - impl_traits!(F64, f64, 8, signed, is_nan); + impl_traits!(F32, f32, 4, signed, @float); + impl_traits!(F64, f64, 8, signed, @float); - macro_rules! call_for_all_types { + macro_rules! call_for_unsigned_types { ($fn:ident, $byteorder:ident) => { $fn::>(); $fn::>(); $fn::>(); $fn::>(); + }; + } + + macro_rules! call_for_signed_types { + ($fn:ident, $byteorder:ident) => { $fn::>(); $fn::>(); $fn::>(); $fn::>(); + }; + } + + macro_rules! call_for_float_types { + ($fn:ident, $byteorder:ident) => { $fn::>(); $fn::>(); }; } - macro_rules! call_for_unsigned_types { + macro_rules! call_for_all_types { ($fn:ident, $byteorder:ident) => { - $fn::>(); - $fn::>(); - $fn::>(); - $fn::>(); + call_for_unsigned_types!($fn, $byteorder); + call_for_signed_types!($fn, $byteorder); + call_for_float_types!($fn, $byteorder); }; } @@ -834,6 +964,114 @@ mod tests { call_for_all_types!(test_non_native_endian, NonNativeEndian); } + #[cfg_attr(test, test)] + fn test_ops_impls() { + // Test implementations of traits in `core::ops`. Some of these are + // fairly banal, but some are optimized to perform the operation without + // swapping byte order (namely, bit-wise operations which are identical + // regardless of byte order). These are important to test, and while + // we're testing those anyway, it's trivial to test all of the impls. + + fn test(op: F, op_native: G, op_native_checked: Option) + where + T: ByteOrderType, + F: Fn(T, T) -> T, + G: Fn(T::Native, T::Native) -> T::Native, + H: Fn(T::Native, T::Native) -> Option, + { + let mut r = SmallRng::seed_from_u64(RNG_SEED); + for _ in 0..RAND_ITERS { + let n0 = T::Native::rand(&mut r); + let n1 = T::Native::rand(&mut r); + let t0 = T::new(n0); + let t1 = T::new(n1); + + // If this operation would overflow/underflow, skip it rather + // than attempt to catch and recover from panics. + if matches!(&op_native_checked, Some(checked) if checked(n0, n1).is_none()) { + continue; + } + + let n_res = op_native(n0, n1); + let t_res = op(t0, t1); + + // For `f32` and `f64`, NaN values are not considered equal to + // themselves. We store `Option`/`Option` and store + // NaN as `None` so they can still be compared. + let n_res = (!T::Native::is_nan(n_res)).then(|| n_res); + let t_res = (!T::Native::is_nan(t_res.get())).then(|| t_res.get()); + assert_eq!(n_res, t_res); + } + } + + macro_rules! test { + (@binary $trait:ident, $method:ident $([$checked_method:ident])?, $($call_for_macros:ident),*) => {{ + test!( + @inner $trait, + core::ops::$trait::$method, + core::ops::$trait::$method, + { + #[allow(unused_mut, unused_assignments)] + let mut op_native_checked = None:: Option>; + $( + op_native_checked = Some(T::Native::$checked_method); + )? + op_native_checked + }, + $($call_for_macros),* + ); + }}; + (@unary $trait:ident, $method:ident $([$checked_method:ident])?, $($call_for_macros:ident),*) => {{ + test!( + @inner $trait, + |slf, _rhs| core::ops::$trait::$method(slf), + |slf, _rhs| core::ops::$trait::$method(slf), + { + #[allow(unused_mut, unused_assignments)] + let mut op_native_checked = None:: Option>; + $( + op_native_checked = Some(|slf, _rhs| T::Native::$checked_method(slf)); + )? + op_native_checked + }, + $($call_for_macros),* + ); + }}; + (@inner $trait:ident, $op:expr, $op_native:expr, $op_native_checked:expr, $($call_for_macros:ident),*) => {{ + fn t>() + where + T::Native: core::ops::$trait, + { + test::( + $op, + $op_native, + $op_native_checked, + ); + } + + $( + $call_for_macros!(t, NativeEndian); + $call_for_macros!(t, NonNativeEndian); + )* + }}; + } + + test!(@binary Add, add[checked_add], call_for_all_types); + test!(@binary Div, div[checked_div], call_for_all_types); + test!(@binary Mul, mul[checked_mul], call_for_all_types); + test!(@binary Rem, rem[checked_rem], call_for_all_types); + test!(@binary Sub, sub[checked_sub], call_for_all_types); + + test!(@binary BitAnd, bitand, call_for_unsigned_types, call_for_signed_types); + test!(@binary BitOr, bitor, call_for_unsigned_types, call_for_signed_types); + test!(@binary BitXor, bitxor, call_for_unsigned_types, call_for_signed_types); + test!(@binary Shl, shl[checked_shl], call_for_unsigned_types, call_for_signed_types); + test!(@binary Shr, shr[checked_shr], call_for_unsigned_types, call_for_signed_types); + + test!(@unary Not, not, call_for_signed_types, call_for_unsigned_types); + test!(@unary Neg, neg, call_for_signed_types, call_for_float_types); + } + #[test] fn test_debug_impl() { // Ensure that Debug applies format options to the inner value. diff --git a/zerocopy-derive/Cargo.toml b/zerocopy-derive/Cargo.toml index fcabebbb34..19185dae13 100644 --- a/zerocopy-derive/Cargo.toml +++ b/zerocopy-derive/Cargo.toml @@ -5,7 +5,7 @@ [package] edition = "2021" name = "zerocopy-derive" -version = "0.7.5" +version = "0.7.6" authors = ["Joshua Liebow-Feeser "] description = "Custom derive for traits from the zerocopy crate" license = "BSD-2-Clause" @@ -30,4 +30,4 @@ testutil = { path = "../testutil" } # sometimes change the output format slightly, so a version mismatch can cause # CI test failures. trybuild = "=1.0.80" -zerocopy = { path = "../", features = ["default", "derive"] } \ No newline at end of file +zerocopy = { path = "../", features = ["default", "derive"] }