Fix Half comparison (dotnet#39773)

* Fix Half comparison * Fixes the comparison to be correct when both numbers are negative * Add relevant tests * Add parentheses for clarity * Add tests validating comparison between negative numbers for Single/Double as well
Jacksondr5 · Aug 10, 2020 · 3b8110e · 3b8110e
1 parent 9d041a4
commit 3b8110e
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 2 deletions.
diff --git a/src/libraries/System.Private.CoreLib/src/System/Half.cs b/src/libraries/System.Private.CoreLib/src/System/Half.cs
@@ -116,7 +116,7 @@ private ushort Significand
                 // says they should be equal, even if the signs differ.
                 return leftIsNegative && !AreZero(left, right);
             }
-            return (short)(left._value) < (short)(right._value);
+            return (left._value < right._value) ^ leftIsNegative;
         }
 
         public static bool operator >(Half left, Half right)
@@ -141,7 +141,7 @@ private ushort Significand
                 // says they should be equal, even if the signs differ.
                 return leftIsNegative || AreZero(left, right);
             }
-            return (short)(left._value) <= (short)(right._value);
+            return (left._value <= right._value) ^ leftIsNegative;
         }
 
         public static bool operator >=(Half left, Half right)

diff --git a/src/libraries/System.Runtime/tests/System/DoubleTests.cs b/src/libraries/System.Runtime/tests/System/DoubleTests.cs
@@ -39,6 +39,10 @@ public static void CompareTo_ObjectNotDouble_ThrowsArgumentException(object valu
         [InlineData(double.NaN, double.NaN, 0)]
         [InlineData(double.NaN, 0.0, -1)]
         [InlineData(234.0, null, 1)]
+        [InlineData(double.MinValue, double.NegativeInfinity, 1)]
+        [InlineData(double.NegativeInfinity, double.MinValue, -1)]
+        [InlineData(-0d, double.NegativeInfinity, 1)]
+        [InlineData(double.NegativeInfinity, -0d, -1)]
         public static void CompareTo_Other_ReturnsExpected(double d1, object value, int expected)
         {
             if (value is double d2)

diff --git a/src/libraries/System.Runtime/tests/System/HalfTests.cs b/src/libraries/System.Runtime/tests/System/HalfTests.cs
@@ -281,6 +281,10 @@ public static IEnumerable<object[]> CompareTo_TestData()
             yield return new object[] { Half.NaN, Half.NaN, 0 };
             yield return new object[] { Half.NaN, UInt16BitsToHalf(0x0000), -1 };
             yield return new object[] { Half.MaxValue, null, 1 };
+            yield return new object[] { Half.MinValue, Half.NegativeInfinity, 1 };
+            yield return new object[] { Half.NegativeInfinity, Half.MinValue, -1 };
+            yield return new object[] { UInt16BitsToHalf(0x8000), Half.NegativeInfinity, 1 }; // Negative zero
+            yield return new object[] { Half.NegativeInfinity, UInt16BitsToHalf(0x8000), -1 }; // Negative zero
         }
 
         [Theory]

diff --git a/src/libraries/System.Runtime/tests/System/SingleTests.cs b/src/libraries/System.Runtime/tests/System/SingleTests.cs
@@ -40,6 +40,10 @@ public static void CompareTo_ObjectNotFloat_ThrowsArgumentException(object value
         [InlineData(float.NaN, float.NaN, 0)]
         [InlineData(float.NaN, 0.0f, -1)]
         [InlineData(234.0f, null, 1)]
+        [InlineData(float.MinValue, float.NegativeInfinity, 1)]
+        [InlineData(float.NegativeInfinity, float.MinValue, -1)]
+        [InlineData(-0f, float.NegativeInfinity, 1)]
+        [InlineData(float.NegativeInfinity, -0f, -1)]
         public static void CompareTo_Other_ReturnsExpected(float f1, object value, int expected)
         {
             if (value is float f2)