Auto merge of #42431 - nagisa:core-float-2, r=alexcrichton

Fix NaN handling in is_sign_negative/positive This would be my proposed fix for the #42425 provided we decide it is indeed a problem. Note this would technically be a breaking change to a stable API. We might want to consider deprecating these methods and adding new ones.
rust-lang · Jun 28, 2017 · 88c3242 · 88c3242
2 parents 4079e61 + 3b9fe77
commit 88c3242
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 46 deletions.
diff --git a/src/libcore/num/f32.rs b/src/libcore/num/f32.rs
@@ -205,18 +205,25 @@ impl Float for f32 {
         }
     }
 
-    /// Returns `true` if `self` is positive, including `+0.0` and
-    /// `Float::infinity()`.
+    /// Returns `true` if and only if `self` has a positive sign, including `+0.0`, `NaN`s with
+    /// positive sign bit and positive infinity.
     #[inline]
     fn is_sign_positive(self) -> bool {
-        self > 0.0 || (1.0 / self) == INFINITY
+        !self.is_sign_negative()
     }
 
-    /// Returns `true` if `self` is negative, including `-0.0` and
-    /// `Float::neg_infinity()`.
+    /// Returns `true` if and only if `self` has a negative sign, including `-0.0`, `NaN`s with
+    /// negative sign bit and negative infinity.
     #[inline]
     fn is_sign_negative(self) -> bool {
-        self < 0.0 || (1.0 / self) == NEG_INFINITY
+        // IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
+        // applies to zeros and NaNs as well.
+        #[repr(C)]
+        union F32Bytes {
+            f: f32,
+            b: u32
+        }
+        unsafe { F32Bytes { f: self }.b & 0x8000_0000 != 0 }
     }
 
     /// Returns the reciprocal (multiplicative inverse) of the number.

diff --git a/src/libcore/num/f64.rs b/src/libcore/num/f64.rs
@@ -205,18 +205,23 @@ impl Float for f64 {
         }
     }
 
-    /// Returns `true` if `self` is positive, including `+0.0` and
-    /// `Float::infinity()`.
+    /// Returns `true` if and only if `self` has a positive sign, including `+0.0`, `NaN`s with
+    /// positive sign bit and positive infinity.
     #[inline]
     fn is_sign_positive(self) -> bool {
-        self > 0.0 || (1.0 / self) == INFINITY
+        !self.is_sign_negative()
     }
 
-    /// Returns `true` if `self` is negative, including `-0.0` and
-    /// `Float::neg_infinity()`.
+    /// Returns `true` if and only if `self` has a negative sign, including `-0.0`, `NaN`s with
+    /// negative sign bit and negative infinity.
     #[inline]
     fn is_sign_negative(self) -> bool {
-        self < 0.0 || (1.0 / self) == NEG_INFINITY
+        #[repr(C)]
+        union F64Bytes {
+            f: f64,
+            b: u64
+        }
+        unsafe { F64Bytes { f: self }.b & 0x8000_0000_0000_0000 != 0 }
     }
 
     /// Returns the reciprocal (multiplicative inverse) of the number.

diff --git a/src/libstd/f32.rs b/src/libstd/f32.rs
@@ -363,39 +363,29 @@ impl f32 {
     #[inline]
     pub fn signum(self) -> f32 { num::Float::signum(self) }
 
-    /// Returns `true` if `self`'s sign bit is positive, including
-    /// `+0.0` and `INFINITY`.
+    /// Returns `true` if and only if `self` has a positive sign, including `+0.0`, `NaN`s with
+    /// positive sign bit and positive infinity.
     ///
     /// ```
-    /// use std::f32;
-    ///
-    /// let nan = f32::NAN;
     /// let f = 7.0_f32;
     /// let g = -7.0_f32;
     ///
     /// assert!(f.is_sign_positive());
     /// assert!(!g.is_sign_positive());
-    /// // Requires both tests to determine if is `NaN`
-    /// assert!(!nan.is_sign_positive() && !nan.is_sign_negative());
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     #[inline]
     pub fn is_sign_positive(self) -> bool { num::Float::is_sign_positive(self) }
 
-    /// Returns `true` if `self`'s sign is negative, including `-0.0`
-    /// and `NEG_INFINITY`.
+    /// Returns `true` if and only if `self` has a negative sign, including `-0.0`, `NaN`s with
+    /// negative sign bit and negative infinity.
     ///
     /// ```
-    /// use std::f32;
-    ///
-    /// let nan = f32::NAN;
     /// let f = 7.0f32;
     /// let g = -7.0f32;
     ///
     /// assert!(!f.is_sign_negative());
     /// assert!(g.is_sign_negative());
-    /// // Requires both tests to determine if is `NaN`.
-    /// assert!(!nan.is_sign_positive() && !nan.is_sign_negative());
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     #[inline]
@@ -1184,7 +1174,7 @@ mod tests {
         assert!(!nan.is_infinite());
         assert!(!nan.is_finite());
         assert!(!nan.is_normal());
-        assert!(!nan.is_sign_positive());
+        assert!(nan.is_sign_positive());
         assert!(!nan.is_sign_negative());
         assert_eq!(Fp::Nan, nan.classify());
     }
@@ -1428,7 +1418,8 @@ mod tests {
         assert!(!(-1f32).is_sign_positive());
         assert!(!NEG_INFINITY.is_sign_positive());
         assert!(!(1f32/NEG_INFINITY).is_sign_positive());
-        assert!(!NAN.is_sign_positive());
+        assert!(NAN.is_sign_positive());
+        assert!(!(-NAN).is_sign_positive());
     }
 
     #[test]
@@ -1441,6 +1432,7 @@ mod tests {
         assert!(NEG_INFINITY.is_sign_negative());
         assert!((1f32/NEG_INFINITY).is_sign_negative());
         assert!(!NAN.is_sign_negative());
+        assert!((-NAN).is_sign_negative());
     }
 
     #[test]

diff --git a/src/libstd/f64.rs b/src/libstd/f64.rs
@@ -301,21 +301,15 @@ impl f64 {
     #[inline]
     pub fn signum(self) -> f64 { num::Float::signum(self) }
 
-    /// Returns `true` if `self`'s sign bit is positive, including
-    /// `+0.0` and `INFINITY`.
+    /// Returns `true` if and only if `self` has a positive sign, including `+0.0`, `NaN`s with
+    /// positive sign bit and positive infinity.
     ///
     /// ```
-    /// use std::f64;
-    ///
-    /// let nan: f64 = f64::NAN;
-    ///
     /// let f = 7.0_f64;
     /// let g = -7.0_f64;
     ///
     /// assert!(f.is_sign_positive());
     /// assert!(!g.is_sign_positive());
-    /// // Requires both tests to determine if is `NaN`
-    /// assert!(!nan.is_sign_positive() && !nan.is_sign_negative());
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     #[inline]
@@ -326,21 +320,15 @@ impl f64 {
     #[inline]
     pub fn is_positive(self) -> bool { num::Float::is_sign_positive(self) }
 
-    /// Returns `true` if `self`'s sign is negative, including `-0.0`
-    /// and `NEG_INFINITY`.
+    /// Returns `true` if and only if `self` has a negative sign, including `-0.0`, `NaN`s with
+    /// negative sign bit and negative infinity.
     ///
     /// ```
-    /// use std::f64;
-    ///
-    /// let nan = f64::NAN;
-    ///
     /// let f = 7.0_f64;
     /// let g = -7.0_f64;
     ///
     /// assert!(!f.is_sign_negative());
     /// assert!(g.is_sign_negative());
-    /// // Requires both tests to determine if is `NaN`.
-    /// assert!(!nan.is_sign_positive() && !nan.is_sign_negative());
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     #[inline]
@@ -1101,7 +1089,7 @@ mod tests {
         assert!(!nan.is_infinite());
         assert!(!nan.is_finite());
         assert!(!nan.is_normal());
-        assert!(!nan.is_sign_positive());
+        assert!(nan.is_sign_positive());
         assert!(!nan.is_sign_negative());
         assert_eq!(Fp::Nan, nan.classify());
     }
@@ -1347,7 +1335,8 @@ mod tests {
         assert!(!(-1f64).is_sign_positive());
         assert!(!NEG_INFINITY.is_sign_positive());
         assert!(!(1f64/NEG_INFINITY).is_sign_positive());
-        assert!(!NAN.is_sign_positive());
+        assert!(NAN.is_sign_positive());
+        assert!(!(-NAN).is_sign_positive());
     }
 
     #[test]
@@ -1360,6 +1349,7 @@ mod tests {
         assert!(NEG_INFINITY.is_sign_negative());
         assert!((1f64/NEG_INFINITY).is_sign_negative());
         assert!(!NAN.is_sign_negative());
+        assert!((-NAN).is_sign_negative());
     }
 
     #[test]