From f9fd9a7359b35511dc8c93e716db90ae3f95b041 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Mon, 17 Feb 2025 14:19:38 +0100 Subject: [PATCH 1/3] Remove evil impls --- src/lib.rs | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 578d7c0..b33066a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1227,10 +1227,7 @@ impl Num for OrderedFloat { /// ``` /// use ordered_float::NotNan; /// -/// let mut v = [ -/// NotNan::new(2.0).unwrap(), -/// NotNan::new(1.0).unwrap(), -/// ]; +/// let mut v = [NotNan::new(2.0).unwrap(), NotNan::new(1.0).unwrap()]; /// v.sort(); /// assert_eq!(v, [1.0, 2.0]); /// ``` @@ -1270,7 +1267,6 @@ impl Num for OrderedFloat { /// [transmute](core::mem::transmute) or pointer casts to convert between any type `T` and /// `NotNan`, as long as this does not create a NaN value. /// However, consider using [`bytemuck`] as a safe alternative if possible. -/// #[cfg_attr( not(feature = "bytemuck"), doc = "[`bytemuck`]: https://docs.rs/bytemuck/1/" @@ -1384,9 +1380,10 @@ impl NotNan { /// Note: For the reverse conversion (from `NotNan` to `NotNan`), you can use /// `.into()`. pub fn as_f32(self) -> NotNan { - // This is not destroying invariants, as it is a pure rounding operation. The only two special - // cases are where f32 would be overflowing, then the operation yields Infinity, or where - // the input is already NaN, in which case the invariant is already broken elsewhere. + // This is not destroying invariants, as it is a pure rounding operation. The only two + // special cases are where f32 would be overflowing, then the operation yields + // Infinity, or where the input is already NaN, in which case the invariant is + // already broken elsewhere. NotNan(self.0 as f32) } } @@ -1473,14 +1470,14 @@ impl PartialEq for NotNan { /// Adds a float directly. /// /// Panics if the provided value is NaN or the computation results in NaN -impl Add for NotNan { +/*impl Add for NotNan { type Output = Self; #[inline] fn add(self, other: T) -> Self { NotNan::new(self.0 + other).expect("Addition resulted in NaN") } -} +}*/ /// Adds a float directly. /// @@ -1501,26 +1498,26 @@ impl<'a, T: FloatCore + Sum + 'a> Sum<&'a NotNan> for NotNan { /// Subtracts a float directly. /// /// Panics if the provided value is NaN or the computation results in NaN -impl Sub for NotNan { +/*impl Sub for NotNan { type Output = Self; #[inline] fn sub(self, other: T) -> Self { NotNan::new(self.0 - other).expect("Subtraction resulted in NaN") } -} +}*/ /// Multiplies a float directly. /// /// Panics if the provided value is NaN or the computation results in NaN -impl Mul for NotNan { +/*impl Mul for NotNan { type Output = Self; #[inline] fn mul(self, other: T) -> Self { NotNan::new(self.0 * other).expect("Multiplication resulted in NaN") } -} +}*/ impl Product for NotNan { fn product>>(iter: I) -> Self { @@ -1535,6 +1532,7 @@ impl<'a, T: FloatCore + Product + 'a> Product<&'a NotNan> for NotNan { } } +/* /// Divides a float directly. /// /// Panics if the provided value is NaN or the computation results in NaN @@ -1557,7 +1555,7 @@ impl Rem for NotNan { fn rem(self, other: T) -> Self { NotNan::new(self.0 % other).expect("Rem resulted in NaN") } -} +}*/ macro_rules! impl_not_nan_binop { ($imp:ident, $method:ident, $assign_imp:ident, $assign_method:ident) => { @@ -1566,25 +1564,26 @@ macro_rules! impl_not_nan_binop { #[inline] fn $method(self, other: Self) -> Self { - self.$method(other.0) + NotNan::new(self.0.$method(other.0)) + .expect("Operation on two NotNan resulted in NaN") } } - impl $imp<&T> for NotNan { + /*impl $imp<&T> for NotNan { type Output = NotNan; #[inline] fn $method(self, other: &T) -> Self::Output { self.$method(*other) } - } + }*/ impl $imp<&Self> for NotNan { type Output = NotNan; #[inline] fn $method(self, other: &Self) -> Self::Output { - self.$method(other.0) + self.$method(*other) } } @@ -1593,7 +1592,7 @@ macro_rules! impl_not_nan_binop { #[inline] fn $method(self, other: Self) -> Self::Output { - (*self).$method(other.0) + (*self).$method(*other) } } @@ -1602,11 +1601,11 @@ macro_rules! impl_not_nan_binop { #[inline] fn $method(self, other: NotNan) -> Self::Output { - (*self).$method(other.0) + (*self).$method(other) } } - impl $imp for &NotNan { + /*impl $imp for &NotNan { type Output = NotNan; #[inline] @@ -1636,19 +1635,19 @@ macro_rules! impl_not_nan_binop { fn $assign_method(&mut self, other: &T) { *self = (*self).$method(*other); } - } + }*/ impl $assign_imp for NotNan { #[inline] fn $assign_method(&mut self, other: Self) { - (*self).$assign_method(other.0); + *self = (*self).$method(other); } } impl $assign_imp<&Self> for NotNan { #[inline] fn $assign_method(&mut self, other: &Self) { - (*self).$assign_method(other.0); + *self = (*self).$method(*other); } } }; From 8ff0bb1cf5a0f288e99b51a5b5fc9b194c38072d Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Mon, 17 Feb 2025 14:32:16 +0100 Subject: [PATCH 2/3] Re-add NotNan x PossiblyNan operations (now returning PossiblyNan) --- src/lib.rs | 84 +++++++++++++++++++++++------------------------------- 1 file changed, 36 insertions(+), 48 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b33066a..a56bc1f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1469,15 +1469,15 @@ impl PartialEq for NotNan { /// Adds a float directly. /// -/// Panics if the provided value is NaN or the computation results in NaN -/*impl Add for NotNan { - type Output = Self; +/// This returns a `T` and not a `NotNan` because if the added value is NaN, this will be NaN +impl Add for NotNan { + type Output = T; #[inline] - fn add(self, other: T) -> Self { - NotNan::new(self.0 + other).expect("Addition resulted in NaN") + fn add(self, other: T) -> Self::Output { + self.0 + other } -}*/ +} /// Adds a float directly. /// @@ -1497,27 +1497,29 @@ impl<'a, T: FloatCore + Sum + 'a> Sum<&'a NotNan> for NotNan { /// Subtracts a float directly. /// -/// Panics if the provided value is NaN or the computation results in NaN -/*impl Sub for NotNan { - type Output = Self; +/// This returns a `T` and not a `NotNan` because if the substracted value is NaN, this will be +/// NaN +impl Sub for NotNan { + type Output = T; #[inline] - fn sub(self, other: T) -> Self { - NotNan::new(self.0 - other).expect("Subtraction resulted in NaN") + fn sub(self, other: T) -> Self::Output { + self.0 - other } -}*/ +} /// Multiplies a float directly. /// -/// Panics if the provided value is NaN or the computation results in NaN -/*impl Mul for NotNan { - type Output = Self; +/// This returns a `T` and not a `NotNan` because if the multiplied value is NaN, this will be +/// NaN +impl Mul for NotNan { + type Output = T; #[inline] - fn mul(self, other: T) -> Self { - NotNan::new(self.0 * other).expect("Multiplication resulted in NaN") + fn mul(self, other: T) -> Self::Output { + self.0 * other } -}*/ +} impl Product for NotNan { fn product>>(iter: I) -> Self { @@ -1532,30 +1534,30 @@ impl<'a, T: FloatCore + Product + 'a> Product<&'a NotNan> for NotNan { } } -/* /// Divides a float directly. /// -/// Panics if the provided value is NaN or the computation results in NaN +/// This returns a `T` and not a `NotNan` because if the divided-by value is NaN, this will be +/// NaN impl Div for NotNan { - type Output = Self; + type Output = T; #[inline] - fn div(self, other: T) -> Self { - NotNan::new(self.0 / other).expect("Division resulted in NaN") + fn div(self, other: T) -> Self::Output { + self.0 / other } } /// Calculates `%` with a float directly. /// -/// Panics if the provided value is NaN or the computation results in NaN +/// This returns a `T` and not a `NotNan` because if the RHS is NaN, this will be NaN impl Rem for NotNan { - type Output = Self; + type Output = T; #[inline] - fn rem(self, other: T) -> Self { - NotNan::new(self.0 % other).expect("Rem resulted in NaN") + fn rem(self, other: T) -> Self::Output { + self.0 % other } -}*/ +} macro_rules! impl_not_nan_binop { ($imp:ident, $method:ident, $assign_imp:ident, $assign_method:ident) => { @@ -1569,14 +1571,14 @@ macro_rules! impl_not_nan_binop { } } - /*impl $imp<&T> for NotNan { - type Output = NotNan; + impl $imp<&T> for NotNan { + type Output = T; #[inline] fn $method(self, other: &T) -> Self::Output { self.$method(*other) } - }*/ + } impl $imp<&Self> for NotNan { type Output = NotNan; @@ -1605,8 +1607,8 @@ macro_rules! impl_not_nan_binop { } } - /*impl $imp for &NotNan { - type Output = NotNan; + impl $imp for &NotNan { + type Output = T; #[inline] fn $method(self, other: T) -> Self::Output { @@ -1615,7 +1617,7 @@ macro_rules! impl_not_nan_binop { } impl $imp<&T> for &NotNan { - type Output = NotNan; + type Output = T; #[inline] fn $method(self, other: &T) -> Self::Output { @@ -1623,20 +1625,6 @@ macro_rules! impl_not_nan_binop { } } - impl $assign_imp for NotNan { - #[inline] - fn $assign_method(&mut self, other: T) { - *self = (*self).$method(other); - } - } - - impl $assign_imp<&T> for NotNan { - #[inline] - fn $assign_method(&mut self, other: &T) { - *self = (*self).$method(*other); - } - }*/ - impl $assign_imp for NotNan { #[inline] fn $assign_method(&mut self, other: Self) { From 86f0b797389737e04765c391d20d0417d462984f Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Mon, 17 Feb 2025 14:40:53 +0100 Subject: [PATCH 3/3] Adapt tests --- tests/test.rs | 172 ++++++++------------------------------------------ 1 file changed, 28 insertions(+), 144 deletions(-) diff --git a/tests/test.rs b/tests/test.rs index 0484f78..3efa017 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -257,22 +257,22 @@ fn not_nan32_fail_when_constructing_with_nan() { #[test] fn not_nan32_calculate_correctly() { assert_eq!(*(not_nan(5.0f32) + not_nan(4.0f32)), 5.0f32 + 4.0f32); - assert_eq!(*(not_nan(5.0f32) + 4.0f32), 5.0f32 + 4.0f32); + assert_eq!(not_nan(5.0f32) + 4.0f32, 5.0f32 + 4.0f32); assert_eq!(*(not_nan(5.0f32) - not_nan(4.0f32)), 5.0f32 - 4.0f32); - assert_eq!(*(not_nan(5.0f32) - 4.0f32), 5.0f32 - 4.0f32); + assert_eq!(not_nan(5.0f32) - 4.0f32, 5.0f32 - 4.0f32); assert_eq!(*(not_nan(5.0f32) * not_nan(4.0f32)), 5.0f32 * 4.0f32); - assert_eq!(*(not_nan(5.0f32) * 4.0f32), 5.0f32 * 4.0f32); + assert_eq!(not_nan(5.0f32) * 4.0f32, 5.0f32 * 4.0f32); assert_eq!(*(not_nan(8.0f32) / not_nan(4.0f32)), 8.0f32 / 4.0f32); - assert_eq!(*(not_nan(8.0f32) / 4.0f32), 8.0f32 / 4.0f32); + assert_eq!(not_nan(8.0f32) / 4.0f32, 8.0f32 / 4.0f32); assert_eq!(*(not_nan(8.0f32) % not_nan(4.0f32)), 8.0f32 % 4.0f32); - assert_eq!(*(not_nan(8.0f32) % 4.0f32), 8.0f32 % 4.0f32); + assert_eq!(not_nan(8.0f32) % 4.0f32, 8.0f32 % 4.0f32); assert_eq!(*(-not_nan(1.0f32)), -1.0f32); - assert!(panic::catch_unwind(|| not_nan(0.0f32) + f32::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f32) - f32::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f32) * f32::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f32) / f32::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f32) % f32::NAN).is_err()); + assert!(f32::is_nan(not_nan(0.0f32) + f32::NAN)); + assert!(f32::is_nan(not_nan(0.0f32) - f32::NAN)); + assert!(f32::is_nan(not_nan(0.0f32) * f32::NAN)); + assert!(f32::is_nan(not_nan(0.0f32) / f32::NAN)); + assert!(f32::is_nan(not_nan(0.0f32) % f32::NAN)); let mut number = not_nan(5.0f32); number += not_nan(4.0f32); @@ -285,44 +285,6 @@ fn not_nan32_calculate_correctly() { assert_eq!(*number, 5.0f32); number %= not_nan(4.0f32); assert_eq!(*number, 1.0f32); - - number = not_nan(5.0f32); - number += 4.0f32; - assert_eq!(*number, 9.0f32); - number -= 4.0f32; - assert_eq!(*number, 5.0f32); - number *= 4.0f32; - assert_eq!(*number, 20.0f32); - number /= 4.0f32; - assert_eq!(*number, 5.0f32); - number %= 4.0f32; - assert_eq!(*number, 1.0f32); - - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f32); - tmp += f32::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f32); - tmp -= f32::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f32); - tmp *= f32::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f32); - tmp /= f32::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f32); - tmp %= f32::NAN; - }) - .is_err()); } #[test] @@ -341,22 +303,22 @@ fn not_nan64_fail_when_constructing_with_nan() { #[test] fn not_nan64_calculate_correctly() { assert_eq!(*(not_nan(5.0f64) + not_nan(4.0f64)), 5.0f64 + 4.0f64); - assert_eq!(*(not_nan(5.0f64) + 4.0f64), 5.0f64 + 4.0f64); + assert_eq!(not_nan(5.0f64) + 4.0f64, 5.0f64 + 4.0f64); assert_eq!(*(not_nan(5.0f64) - not_nan(4.0f64)), 5.0f64 - 4.0f64); - assert_eq!(*(not_nan(5.0f64) - 4.0f64), 5.0f64 - 4.0f64); + assert_eq!(not_nan(5.0f64) - 4.0f64, 5.0f64 - 4.0f64); assert_eq!(*(not_nan(5.0f64) * not_nan(4.0f64)), 5.0f64 * 4.0f64); - assert_eq!(*(not_nan(5.0f64) * 4.0f64), 5.0f64 * 4.0f64); + assert_eq!(not_nan(5.0f64) * 4.0f64, 5.0f64 * 4.0f64); assert_eq!(*(not_nan(8.0f64) / not_nan(4.0f64)), 8.0f64 / 4.0f64); - assert_eq!(*(not_nan(8.0f64) / 4.0f64), 8.0f64 / 4.0f64); + assert_eq!(not_nan(8.0f64) / 4.0f64, 8.0f64 / 4.0f64); assert_eq!(*(not_nan(8.0f64) % not_nan(4.0f64)), 8.0f64 % 4.0f64); - assert_eq!(*(not_nan(8.0f64) % 4.0f64), 8.0f64 % 4.0f64); + assert_eq!(not_nan(8.0f64) % 4.0f64, 8.0f64 % 4.0f64); assert_eq!(*(-not_nan(1.0f64)), -1.0f64); - assert!(panic::catch_unwind(|| not_nan(0.0f64) + f64::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f64) - f64::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f64) * f64::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f64) / f64::NAN).is_err()); - assert!(panic::catch_unwind(|| not_nan(0.0f64) % f64::NAN).is_err()); + assert!(f64::is_nan(not_nan(0.0f64) + f64::NAN)); + assert!(f64::is_nan(not_nan(0.0f64) - f64::NAN)); + assert!(f64::is_nan(not_nan(0.0f64) * f64::NAN)); + assert!(f64::is_nan(not_nan(0.0f64) / f64::NAN)); + assert!(f64::is_nan(not_nan(0.0f64) % f64::NAN)); let mut number = not_nan(5.0f64); number += not_nan(4.0f64); @@ -369,44 +331,6 @@ fn not_nan64_calculate_correctly() { assert_eq!(*number, 5.0f64); number %= not_nan(4.0f64); assert_eq!(*number, 1.0f64); - - number = not_nan(5.0f64); - number += 4.0f64; - assert_eq!(*number, 9.0f64); - number -= 4.0f64; - assert_eq!(*number, 5.0f64); - number *= 4.0f64; - assert_eq!(*number, 20.0f64); - number /= 4.0f64; - assert_eq!(*number, 5.0f64); - number %= 4.0f64; - assert_eq!(*number, 1.0f64); - - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f64); - tmp += f64::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f64); - tmp -= f64::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f64); - tmp *= f64::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f64); - tmp /= f64::NAN; - }) - .is_err()); - assert!(panic::catch_unwind(|| { - let mut tmp = not_nan(0.0f64); - tmp %= f64::NAN; - }) - .is_err()); } #[test] @@ -578,7 +502,7 @@ fn hash_is_good_for_fractional_numbers() { fn test_add_fails_on_nan() { let a = not_nan(f32::INFINITY); let b = not_nan(f32::NEG_INFINITY); - let _c = a + b; + let _c: NotNan = a + b; } #[test] @@ -586,7 +510,7 @@ fn test_add_fails_on_nan() { fn test_add_fails_on_nan_ref() { let a = not_nan(f32::INFINITY); let b = not_nan(f32::NEG_INFINITY); - let _c = a + &b; + let _c: NotNan = a + &b; } #[test] @@ -594,31 +518,7 @@ fn test_add_fails_on_nan_ref() { fn test_add_fails_on_nan_ref_ref() { let a = not_nan(f32::INFINITY); let b = not_nan(f32::NEG_INFINITY); - let _c = &a + &b; -} - -#[test] -#[should_panic] -fn test_add_fails_on_nan_t_ref() { - let a = not_nan(f32::INFINITY); - let b = f32::NEG_INFINITY; - let _c = a + &b; -} - -#[test] -#[should_panic] -fn test_add_fails_on_nan_ref_t_ref() { - let a = not_nan(f32::INFINITY); - let b = f32::NEG_INFINITY; - let _c = &a + &b; -} - -#[test] -#[should_panic] -fn test_add_fails_on_nan_ref_t() { - let a = not_nan(f32::INFINITY); - let b = f32::NEG_INFINITY; - let _c = &a + b; + let _c: NotNan = &a + &b; } #[test] @@ -629,22 +529,6 @@ fn test_add_assign_fails_on_nan_ref() { a += &b; } -#[test] -#[should_panic] -fn test_add_assign_fails_on_nan_t_ref() { - let mut a = not_nan(f32::INFINITY); - let b = f32::NEG_INFINITY; - a += &b; -} - -#[test] -#[should_panic] -fn test_add_assign_fails_on_nan_t() { - let mut a = not_nan(f32::INFINITY); - let b = f32::NEG_INFINITY; - a += b; -} - #[test] fn add() { assert_eq!(not_nan(0.0) + not_nan(0.0), 0.0); @@ -729,11 +613,11 @@ fn not_nan_panic_safety() { num }; - assert!(!catch_op(not_nan(f32::INFINITY), |a| *a += f32::NEG_INFINITY).is_nan()); - assert!(!catch_op(not_nan(f32::INFINITY), |a| *a -= f32::INFINITY).is_nan()); - assert!(!catch_op(not_nan(0.0), |a| *a *= f32::INFINITY).is_nan()); - assert!(!catch_op(not_nan(0.0), |a| *a /= 0.0).is_nan()); - assert!(!catch_op(not_nan(0.0), |a| *a %= 0.0).is_nan()); + assert!(!catch_op(not_nan(f32::INFINITY), |a| *a += not_nan(f32::NEG_INFINITY)).is_nan()); + assert!(!catch_op(not_nan(f32::INFINITY), |a| *a -= not_nan(f32::INFINITY)).is_nan()); + assert!(!catch_op(not_nan(0.0), |a| *a *= not_nan(f32::INFINITY)).is_nan()); + assert!(!catch_op(not_nan(0.0), |a| *a /= not_nan(0.0)).is_nan()); + assert!(!catch_op(not_nan(0.0), |a| *a %= not_nan(0.0)).is_nan()); } #[test]