Updating Number.Formatting to properly print -0
authorTanner Gooding <tagoo@outlook.com>
Thu, 30 Aug 2018 17:09:59 +0000 (10:09 -0700)
committerTanner Gooding <tagoo@outlook.com>
Fri, 7 Sep 2018 00:07:08 +0000 (17:07 -0700)
Commit migrated from https://github.com/dotnet/coreclr/commit/ad3c8cc8934ec89631c221713b86238f6c86e1b6

src/coreclr/src/classlibnative/bcltype/grisu3.cpp
src/coreclr/src/classlibnative/bcltype/grisu3.h
src/coreclr/src/classlibnative/bcltype/number.cpp
src/coreclr/src/classlibnative/bcltype/number.h
src/coreclr/src/pal/inc/pal.h
src/coreclr/src/pal/src/cruntime/math.cpp
src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs
src/libraries/System.Private.CoreLib/src/System/Number.NumberBuffer.cs

index 0fc9e88..e0138f6 100644 (file)
@@ -44,7 +44,7 @@ bool Grisu3::Run(double value, int count, int* dec, int* sign, wchar_t* digits)
     // kappa: A factor used for generating digits. See step 5 of the Grisu3 procedure in the paper.
 
     // Handle sign bit.
-    if (value < 0)
+    if (_signbit(value) != 0)
     {
         value = -value;
         *sign = 1;
@@ -378,4 +378,4 @@ void Grisu3::BiggestPowerTenLessThanOrEqualTo(UINT32 number,
         *exponent = 0;
         UNREACHABLE();
     }
-}
\ No newline at end of file
+}
index b3c5a40..7f8ec82 100644 (file)
 
 #include "diyfp.h"
 
+#ifdef _MSC_VER
+#define _signbit signbit
+#define _signbitf signbit
+#endif
+
 struct PowerOfTen
 {
     UINT64 significand;
index b20044e..ea63122 100644 (file)
@@ -288,7 +288,7 @@ void DoubleToNumberWorker( double value, int count, int* dec, int* sign, wchar_t
     if (value == 0.0)
     {
         *dec = 0;
-        *sign = 0;
+        *sign = _signbit(value);
 
         // Instead of zeroing digits, we just make it as an empty string due to performance reason.
         *digits = 0;
index 22c74ca..480e6ad 100644 (file)
@@ -19,13 +19,21 @@ static const double LOG10V2 = 0.30102999566398119521373889472449;
 // DRIFT_FACTOR = 1 - LOG10V2 - epsilon (a small number account for drift of floating point multiplication)
 static const double DRIFT_FACTOR = 0.69;
 
+enum NUMBER_KIND : int {
+    NUMBER_KIND_Unknown = 0,
+    NUMBER_KIND_Integer = 1,
+    NUMBER_KIND_Decimal = 2,
+    NUMBER_KIND_Double = 3
+};
+
 struct NUMBER {
-    int precision;
-    int scale;
-    int sign;
-    wchar_t digits[NUMBER_MAXDIGITS + 1];
-    wchar_t* allDigits;
-    NUMBER() : precision(0), scale(0), sign(0), allDigits(NULL) {}
+    int precision;                          //  0
+    int scale;                              //  4
+    int sign;                               //  8
+    NUMBER_KIND kind;                       // 12
+    wchar_t* allDigits;                     // 16
+    wchar_t digits[NUMBER_MAXDIGITS + 1];   // 20 or 24
+    NUMBER() : precision(0), scale(0), sign(0), kind(NUMBER_KIND_Unknown), allDigits(NULL) {}
 };
 
 class COMNumber
index e7ec886..2d0c471 100644 (file)
@@ -4459,6 +4459,7 @@ PALIMPORT LONG __cdecl labs(LONG);
 // clang complains if this is declared with __int64
 PALIMPORT long long __cdecl llabs(long long);
 
+PALIMPORT int __cdecl _signbit(double);
 PALIMPORT int __cdecl _finite(double);
 PALIMPORT int __cdecl _isnan(double);
 PALIMPORT double __cdecl _copysign(double, double);
@@ -4487,6 +4488,7 @@ PALIMPORT double __cdecl sqrt(double);
 PALIMPORT double __cdecl tan(double);
 PALIMPORT double __cdecl tanh(double);
 
+PALIMPORT int __cdecl _signbitf(float);
 PALIMPORT int __cdecl _finitef(float);
 PALIMPORT int __cdecl _isnanf(float);
 PALIMPORT float __cdecl _copysignf(float, float);
index a36ac9a..af2f994 100644 (file)
@@ -45,6 +45,34 @@ SET_DEFAULT_DEBUG_CHANNEL(CRT);
 
 /*++
 Function:
+  _signbit
+
+Determines whether given double-precision floating point value has a negative sign.
+
+Return Value
+
+_signbit returns a nonzero value (TRUE) if the sign of its argument x is negative.
+
+Parameter
+
+x  Double-precision floating-point value
+
+--*/
+int __cdecl _signbit(double x)
+{
+    int ret;
+    PERF_ENTRY(_signbit);
+    ENTRY("_signbit (x=%f)\n", x);
+
+    ret = signbit(x);
+
+    LOGEXIT("_signbit returns int %d\n", ret);
+    PERF_EXIT(_signbit);
+    return ret;
+}
+
+/*++
+Function:
   _finite
 
 Determines whether given double-precision floating point value is finite.
@@ -455,6 +483,34 @@ PALIMPORT double __cdecl PAL_pow(double x, double y)
 
 /*++
 Function:
+  _signbitf
+
+Determines whether given single-precision floating point value has a negative sign.
+
+Return Value
+
+_signbitf returns a nonzero value (TRUE) if the sign of its argument x is negative.
+
+Parameter
+
+x  Single-precision floating-point value
+
+--*/
+int __cdecl _signbitf(float x)
+{
+    int ret;
+    PERF_ENTRY(_signbitf);
+    ENTRY("_signbitf (x=%f)\n", x);
+
+    ret = signbit(x);
+
+    LOGEXIT("_signbitf returns int %d\n", ret);
+    PERF_EXIT(_signbitf);
+    return ret;
+}
+
+/*++
+Function:
   _finitef
 
 Determines whether given single-precision floating point value is finite.
index 106fd15..3d3c15b 100644 (file)
@@ -301,7 +301,7 @@ namespace System
 
             if (fmt != 0)
             {
-                NumberToString(ref sb, ref number, fmt, digits, info, isDecimal:true);
+                NumberToString(ref sb, ref number, fmt, digits, info);
             }
             else
             {
@@ -327,7 +327,7 @@ namespace System
 
             if (fmt != 0)
             {
-                NumberToString(ref sb, ref number, fmt, digits, info, isDecimal: true);
+                NumberToString(ref sb, ref number, fmt, digits, info);
             }
             else
             {
@@ -342,6 +342,7 @@ namespace System
             char* buffer = number.digits;
             number.precision = DecimalPrecision;
             number.sign = d.IsNegative;
+            number.kind = NumberBufferKind.Decimal;
 
             char* p = buffer + DecimalPrecision;
             while ((d.Mid | d.High) != 0)
@@ -388,6 +389,7 @@ namespace System
             char fmt = ParseFormatSpecifier(format, out int digits);
             int precision = DoublePrecision;
             NumberBuffer number = default;
+            number.kind = NumberBufferKind.Double;
 
             switch (fmt)
             {
@@ -409,12 +411,12 @@ namespace System
 
                         if (NumberToDouble(ref number) == value)
                         {
-                            NumberToString(ref sb, ref number, 'G', DoublePrecision, info, isDecimal: false);
+                            NumberToString(ref sb, ref number, 'G', DoublePrecision, info);
                         }
                         else
                         {
                             DoubleToNumber(value, 17, ref number);
-                            NumberToString(ref sb, ref number, 'G', 17, info, isDecimal: false);
+                            NumberToString(ref sb, ref number, 'G', 17, info);
                         }
 
                         return null;
@@ -451,7 +453,7 @@ namespace System
 
             if (fmt != 0)
             {
-                NumberToString(ref sb, ref number, fmt, digits, info, isDecimal: false);
+                NumberToString(ref sb, ref number, fmt, digits, info);
             }
             else
             {
@@ -488,6 +490,7 @@ namespace System
             char fmt = ParseFormatSpecifier(format, out int digits);
             int precision = FloatPrecision;
             NumberBuffer number = default;
+            number.kind = NumberBufferKind.Double;
 
             switch (fmt)
             {
@@ -509,12 +512,12 @@ namespace System
 
                         if ((float)NumberToDouble(ref number) == value)
                         {
-                            NumberToString(ref sb, ref number, 'G', FloatPrecision, info, isDecimal: false);
+                            NumberToString(ref sb, ref number, 'G', FloatPrecision, info);
                         }
                         else
                         {
                             DoubleToNumber(value, 9, ref number);
-                            NumberToString(ref sb, ref number, 'G', 9, info, isDecimal: false);
+                            NumberToString(ref sb, ref number, 'G', 9, info);
                         }
                         return null;
                     }
@@ -550,7 +553,7 @@ namespace System
 
             if (fmt != 0)
             {
-                NumberToString(ref sb, ref number, fmt, digits, info, false);
+                NumberToString(ref sb, ref number, fmt, digits, info);
             }
             else
             {
@@ -608,7 +611,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -653,7 +656,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -696,7 +699,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -739,7 +742,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -785,7 +788,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -831,7 +834,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -875,7 +878,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -919,7 +922,7 @@ namespace System
                 }
                 if (fmt != 0)
                 {
-                    NumberToString(ref sb, ref number, fmt, digits, info, false);
+                    NumberToString(ref sb, ref number, fmt, digits, info);
                 }
                 else
                 {
@@ -949,6 +952,7 @@ namespace System
             int i = (int)(buffer + Int32Precision - p);
 
             number.scale = i;
+            number.kind = NumberBufferKind.Integer;
 
             char* dst = number.digits;
             while (--i >= 0)
@@ -1065,6 +1069,7 @@ namespace System
             char* p = UInt32ToDecChars(buffer + UInt32Precision, value, 0);
             int i = (int)(buffer + UInt32Precision - p);
             number.scale = i;
+            number.kind = NumberBufferKind.Integer;
 
             char* dst = number.digits;
             while (--i >= 0)
@@ -1184,6 +1189,7 @@ namespace System
             int i = (int)(buffer + Int64Precision - p);
 
             number.scale = i;
+            number.kind = NumberBufferKind.Integer;
 
             char* dst = number.digits;
             while (--i >= 0)
@@ -1325,6 +1331,7 @@ namespace System
             int i = (int)(buffer + UInt64Precision - p);
 
             number.scale = i;
+            number.kind = NumberBufferKind.Integer;
 
             char* dst = number.digits;
             while (--i >= 0)
@@ -1453,8 +1460,10 @@ namespace System
                 '\0';
         }
 
-        internal static unsafe void NumberToString(ref ValueStringBuilder sb, ref NumberBuffer number, char format, int nMaxDigits, NumberFormatInfo info, bool isDecimal)
+        internal static unsafe void NumberToString(ref ValueStringBuilder sb, ref NumberBuffer number, char format, int nMaxDigits, NumberFormatInfo info)
         {
+            Debug.Assert(number.kind != NumberBufferKind.Unknown);
+
             switch (format)
             {
                 case 'C':
@@ -1522,14 +1531,9 @@ namespace System
                         bool noRounding = false;
                         if (nMaxDigits < 1)
                         {
-                            if (isDecimal && (nMaxDigits == -1))
+                            if ((number.kind == NumberBufferKind.Decimal) && (nMaxDigits == -1))
                             {
                                 noRounding = true;  // Turn off rounding for ECMA compliance to output trailing 0's after decimal as significant
-                                if (number.digits[0] == 0)
-                                {
-                                    // Minus zero should be formatted as 0
-                                    goto SkipSign;
-                                }
                                 goto SkipRounding;
                             }
                             else
@@ -1539,13 +1543,12 @@ namespace System
                             }
                         }
 
-                        RoundNumber(ref number, nMaxDigits); // This also fixes up the minus zero case
+                        RoundNumber(ref number, nMaxDigits);
 
 SkipRounding:
                         if (number.sign)
                             sb.Append(info.NegativeSign);
 
-SkipSign:
                         FormatGeneral(ref sb, ref number, nMaxDigits, info, (char)(format - ('G' - 'E')), noRounding);
 
                         break;
@@ -1572,6 +1575,8 @@ SkipSign:
 
         internal static unsafe void NumberToStringFormat(ref ValueStringBuilder sb, ref NumberBuffer number, ReadOnlySpan<char> format, NumberFormatInfo info)
         {
+            Debug.Assert(number.kind != NumberBufferKind.Unknown);
+
             int digitCount;
             int decimalPos;
             int firstDigit;
@@ -1694,7 +1699,6 @@ SkipSign:
                 }
                 else
                 {
-                    number.sign = false;   // We need to format -0 without the sign set.
                     number.scale = 0;      // Decimals with scale ('0.00') should be rounded.
                 }
 
@@ -2220,7 +2224,11 @@ SkipSign:
             if (i == 0)
             {
                 number.scale = 0;
-                number.sign = false;
+
+                if (number.kind == NumberBufferKind.Integer)
+                {
+                    number.sign = false;
+                }
             }
             dig[i] = '\0';
         }
index 2316f99..1730cf1 100644 (file)
@@ -15,17 +15,27 @@ namespace System
         [StructLayout(LayoutKind.Sequential, Pack = 1)]
         internal unsafe ref struct NumberBuffer // needs to match layout of NUMBER in coreclr's src/classlibnative/bcltype/number.h
         {
-            public int precision;
-            public int scale;
-            private int _sign;
-            private DigitsAndNullTerminator _digits;
-            private char* _allDigits;
+            public int precision;                       //  0
+            public int scale;                           //  4
+            private int _sign;                          //  8
+            private NumberBufferKind _kind;             // 12
+            private char* _allDigits;                   // 16
+            private DigitsAndNullTerminator _digits;    // 20 or 24
 
             public bool sign { get => _sign != 0; set => _sign = value ? 1 : 0; }
             public char* digits => (char*)Unsafe.AsPointer(ref _digits);
+            public NumberBufferKind kind { get => _kind; set => _kind = value; }
 
             [StructLayout(LayoutKind.Sequential, Size = (NumberMaxDigits + 1) * sizeof(char))]
             private struct DigitsAndNullTerminator { }
         }
+
+        internal enum NumberBufferKind // needs to match NUMBER_KIND in coreclr's src/classlibnative/bcltype/number.h
+        {
+            Unknown = 0,
+            Integer = 1,
+            Decimal = 2,
+            Double = 3
+        }
     }
 }