diff --git a/src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs b/src/System.Private.CoreLib/shared/System/Globalization/TimeSpanParse.cs index 1bf81742a075..a9172e280207 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 @@ public TimeSpanToken(TTT type, int number, int leadingZeroes, ReadOnlySpan _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 @@ internal TimeSpanToken GetNextToken() } 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 @@ private static bool TryTimeToTicks(bool positive, TimeSpanToken days, TimeSpanTo hours._num > MaxHours || minutes._num > MaxMinutes || seconds._num > MaxSeconds || - fraction.IsInvalidFraction()) + !fraction.NormalizeAndValidateFraction()) { result = 0; return false; @@ -570,31 +601,7 @@ private static bool TryTimeToTicks(bool positive, TimeSpanToken days, TimeSpanTo 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 @@ private static bool TryParseByFormat(ReadOnlySpan input, ReadOnlySpan /// 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. /// private static bool TryParseTimeSpanConstant(ReadOnlySpan input, ref TimeSpanResult result) => new StringParser().TryParse(input, ref result); @@ -1628,7 +1635,7 @@ internal bool ParseTime(out long time, ref TimeSpanResult result) 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 3699ee5aeea0..25970b3f237a 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" } ] }