dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.38k stars 4.75k forks source link

Add trailing zero check to `NumberBuffer.CheckConsistency` #48418

Open pgovind opened 3 years ago

pgovind commented 3 years ago

For floating point numbers, we don't keep track of trailing zeros that occur after a decimal point. This check needs to be added to the NumberBuffer.CheckConsistency method. My first attempt to do this was

                uint totalDigits = (uint)(DigitsCount);
                uint positiveExponent = (uint)(Math.Max(0, Scale));
                uint integerDigitsPresent = Math.Min(positiveExponent, totalDigits);
                uint fractionalDigitsPresent = totalDigits - integerDigitsPresent;

                // For a number like 1.23000, verify that we don't store trailing zeros in Digits
                // However, if the number of digits exceeds maxDigCount and rounding is required, we store the trailing zeros in the buffer.
                if (fractionalDigitsPresent > 0 && !HasNonZeroTail)
                {
                    Debug.Assert(Digits[DigitsCount - 1] != '0', ToString());
                }

However while this works for double.Parse, it doesn't work for double.Format because of how the number buffer is populated. Example error below:

["21474836470000000", Length = 17, Scale = 10, IsNegative = True, HasNonZeroTail = False, Kind = FloatingPoint]
   at System.Number.FormatDouble(ValueStringBuilder& sb, Double value, ReadOnlySpan`1 format, NumberFormatInfo info) in /_/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs:line 554
   at System.Number.FormatDouble(Double value, String format, NumberFormatInfo info) in /_/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs:line 383
   at System.Double.ToString(String format) in /_/src/libraries/System.Private.CoreLib/src/System/Double.cs:line 266
   at System.Numerics.Tests.BigIntegerConstructorTest.VerifyCtorDouble(Double value) in /_/src/libraries/System.Runtime.Numerics/tests/BigInteger/ctor.cs:line 548
   at System.Numerics.Tests.BigIntegerConstructorTest.RunCtorDoubleTests() in /_/src/libraries/System.Runtime.Numerics/tests/BigInteger/ctor.cs:line 402

The likely fix here is to either turn on the trailing zero check only for the *Parse methods, or to improve the trailing zero check to better track a decimal point in the input/format.

ghost commented 3 years ago

Tagging subscribers to this area: @tannergooding, @pgovind See info in area-owners.md if you want to be subscribed.

Issue Details
For floating point numbers, we don't keep track of trailing zeros that occur after a decimal point. This check needs to be added to the `NumberBuffer.CheckConsistency` method. My first attempt to do this was ``` csharp uint totalDigits = (uint)(DigitsCount); uint positiveExponent = (uint)(Math.Max(0, Scale)); uint integerDigitsPresent = Math.Min(positiveExponent, totalDigits); uint fractionalDigitsPresent = totalDigits - integerDigitsPresent; // For a number like 1.23000, verify that we don't store trailing zeros in Digits // However, if the number of digits exceeds maxDigCount and rounding is required, we store the trailing zeros in the buffer. if (fractionalDigitsPresent > 0 && !HasNonZeroTail) { Debug.Assert(Digits[DigitsCount - 1] != '0', ToString()); } ``` However while this works for `double.Parse`, it doesn't work for `double.Format` because of how the number buffer is populated. Example error below: ``` csharp ["21474836470000000", Length = 17, Scale = 10, IsNegative = True, HasNonZeroTail = False, Kind = FloatingPoint] at System.Number.FormatDouble(ValueStringBuilder& sb, Double value, ReadOnlySpan`1 format, NumberFormatInfo info) in /_/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs:line 554 at System.Number.FormatDouble(Double value, String format, NumberFormatInfo info) in /_/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs:line 383 at System.Double.ToString(String format) in /_/src/libraries/System.Private.CoreLib/src/System/Double.cs:line 266 at System.Numerics.Tests.BigIntegerConstructorTest.VerifyCtorDouble(Double value) in /_/src/libraries/System.Runtime.Numerics/tests/BigInteger/ctor.cs:line 548 at System.Numerics.Tests.BigIntegerConstructorTest.RunCtorDoubleTests() in /_/src/libraries/System.Runtime.Numerics/tests/BigInteger/ctor.cs:line 402 ``` The likely fix here is to either turn on the trailing zero check only for the `*Parse` methods, or to improve the trailing zero check to better track a decimal point in the input/format.
Author: pgovind
Assignees: -
Labels: `area-System.Numerics`, `untriaged`
Milestone: -