diff options
author | Tarek Mahmoud Sayed <tarekms@microsoft.com> | 2019-01-14 14:49:00 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-14 14:49:00 -0800 |
commit | e62ee275e4c1b35d217e2f760627751cbd4b2a11 (patch) | |
tree | 12f357d778fa4522280492c17b2d0f18f115d8e5 | |
parent | 004ada13365e22ee60b68cb4a21234c154964ed9 (diff) | |
download | coreclr-e62ee275e4c1b35d217e2f760627751cbd4b2a11.tar.gz coreclr-e62ee275e4c1b35d217e2f760627751cbd4b2a11.tar.bz2 coreclr-e62ee275e4c1b35d217e2f760627751cbd4b2a11.zip |
Fix TimeSpan parsing (#21968)
* Fix TimeSpan parsing
* Temporary disabling the failed CI tests
-rw-r--r-- | src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs | 85 | ||||
-rw-r--r-- | tests/CoreFX/CoreFX.issues.json | 16 |
2 files changed, 62 insertions, 39 deletions
diff --git a/src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs b/src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs index 1bf81742a0..a9172e2802 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs @@ -8,10 +8,10 @@ // // Standard Format: // -=-=-=-=-=-=-=- -// "c": Constant format. [-][d'.']hh':'mm':'ss['.'fffffff] +// "c": Constant format. [-][d'.']hh':'mm':'ss['.'fffffff] // Not culture sensitive. Default format (and null/empty format string) map to this format. // -// "g": General format, short: [-][d':']h':'mm':'ss'.'FFFFFFF +// "g": General format, short: [-][d':']h':'mm':'ss'.'FFFFFFF // Only print what's needed. Localized (if you want Invariant, pass in Invariant). // The fractional seconds separator is localized, equal to the culture's DecimalSeparator. // @@ -43,8 +43,8 @@ // - For multi-letter formats "TryParseByFormat" is called // - TryParseByFormat uses helper methods (ParseExactLiteral, ParseExactDigits, etc) // which drive the underlying TimeSpanTokenizer. However, unlike standard formatting which -// operates on whole-tokens, ParseExact operates at the character-level. As such, -// TimeSpanTokenizer.NextChar and TimeSpanTokenizer.BackOne() are called directly. +// operates on whole-tokens, ParseExact operates at the character-level. As such, +// TimeSpanTokenizer.NextChar and TimeSpanTokenizer.BackOne() are called directly. // //////////////////////////////////////////////////////////////////////////// @@ -104,19 +104,50 @@ namespace System.Globalization _sep = separator; } - public bool IsInvalidFraction() + public bool NormalizeAndValidateFraction() { Debug.Assert(_ttt == TTT.Num); Debug.Assert(_num > -1); - if (_num > MaxFraction || _zeroes > MaxFractionDigits) + if (_num == 0) return true; - if (_num == 0 || _zeroes == 0) + if (_zeroes == 0 && _num > MaxFraction) return false; - // num > 0 && zeroes > 0 && num <= maxValue && zeroes <= maxPrecision - return _num >= MaxFraction / Pow10(_zeroes - 1); + int totalDigitsCount = ((int) Math.Floor(Math.Log10(_num))) + 1 + _zeroes; + + if (totalDigitsCount == MaxFractionDigits) + { + // Already normalized. no more action needed + // .9999999 normalize to 9,999,999 ticks + // .0000001 normalize to 1 ticks + return true; + } + + if (totalDigitsCount < MaxFractionDigits) + { + // normalize the fraction to the 7-digits + // .999999 normalize to 9,999,990 ticks + // .99999 normalize to 9,999,900 ticks + // .000001 normalize to 10 ticks + // .1 normalize to 1,000,000 ticks + + _num *= (int) Pow10(MaxFractionDigits - totalDigitsCount); + return true; + } + + // totalDigitsCount is greater then MaxFractionDigits, we'll need to do the rounding to 7-digits length + // .00000001 normalized to 0 ticks + // .00000005 normalized to 1 ticks + // .09999999 normalize to 1,000,000 ticks + // .099999999 normalize to 1,000,000 ticks + + Debug.Assert(_zeroes > 0); // Already validated that in the condition _zeroes == 0 && _num > MaxFraction + _num = (int) Math.Round((double)_num / Pow10(totalDigitsCount - MaxFractionDigits), MidpointRounding.AwayFromZero); + Debug.Assert(_num < MaxFraction); + + return true; } } @@ -184,7 +215,7 @@ namespace System.Globalization } num = num * 10 + digit; - if ((num & 0xF0000000) != 0) + if ((num & 0xF0000000) != 0) // Max limit we can support 268435455 which is FFFFFFF { return new TimeSpanToken(TTT.NumOverflow); } @@ -557,7 +588,7 @@ namespace System.Globalization hours._num > MaxHours || minutes._num > MaxMinutes || seconds._num > MaxSeconds || - fraction.IsInvalidFraction()) + !fraction.NormalizeAndValidateFraction()) { result = 0; return false; @@ -570,31 +601,7 @@ namespace System.Globalization return false; } - // Normalize the fraction component - // - // string representation => (zeroes,num) => resultant fraction ticks - // --------------------- ------------ ------------------------ - // ".9999999" => (0,9999999) => 9,999,999 ticks (same as constant maxFraction) - // ".1" => (0,1) => 1,000,000 ticks - // ".01" => (1,1) => 100,000 ticks - // ".001" => (2,1) => 10,000 ticks - long f = fraction._num; - if (f != 0) - { - long lowerLimit = InternalGlobalizationHelper.TicksPerTenthSecond; - if (fraction._zeroes > 0) - { - long divisor = Pow10(fraction._zeroes); - lowerLimit = lowerLimit / divisor; - } - - while (f < lowerLimit) - { - f *= 10; - } - } - - result = ticks * TimeSpan.TicksPerMillisecond + f; + result = ticks * TimeSpan.TicksPerMillisecond + fraction._num; if (positive && result < 0) { result = 0; @@ -1338,7 +1345,7 @@ namespace System.Globalization case '%': // Optional format character. - // For example, format string "%d" will print day + // For example, format string "%d" will print day // Most of the cases, "%" can be ignored. nextFormatChar = DateTimeFormat.ParseNextChar(format, i); @@ -1455,7 +1462,7 @@ namespace System.Globalization /// <summary> /// Parses the "c" (constant) format. This code is 100% identical to the non-globalized v1.0-v3.5 TimeSpan.Parse() routine - /// and exists for performance/appcompat with legacy callers who cannot move onto the globalized Parse overloads. + /// and exists for performance/appcompat with legacy callers who cannot move onto the globalized Parse overloads. /// </summary> private static bool TryParseTimeSpanConstant(ReadOnlySpan<char> input, ref TimeSpanResult result) => new StringParser().TryParse(input, ref result); @@ -1628,7 +1635,7 @@ namespace System.Globalization if (_ch == ':') { NextChar(); - + // allow seconds with the leading zero if (_ch != '.') { diff --git a/tests/CoreFX/CoreFX.issues.json b/tests/CoreFX/CoreFX.issues.json index 3699ee5aee..25970b3f23 100644 --- a/tests/CoreFX/CoreFX.issues.json +++ b/tests/CoreFX/CoreFX.issues.json @@ -1001,6 +1001,22 @@ { "name": "System.Tests.ArrayTests.Copy", "reason": "Needs updates for XUnit 2.4" + }, + { + "name": "System.Tests.TimeSpanTests.Parse_Invalid", + "reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561" + }, + { + "name": "System.Tests.TimeSpanTests.Parse_Span", + "reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561" + }, + { + "name": "System.Tests.TimeSpanTests.Parse_Span_Invalid", + "reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561" + }, + { + "name": "System.Tests.TimeSpanTests.Parse", + "reason": "Temporary disabling till merging the PR https://github.com/dotnet/corefx/pull/34561" } ] } |