Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix number parsing with negative sign #47613

Merged
merged 3 commits into from
Jan 29, 2021
Merged

Fix number parsing with negative sign #47613

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77c8fa4
Fix number parsing with negative sign
tarekgh Jan 29, 2021
980fb5a
Address the feedback
tarekgh Jan 29, 2021
669e470
Submit missed changes
tarekgh Jan 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions src/libraries/Common/src/System/Globalization/FormatProvider.Number.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;

Expand Down Expand Up @@ -328,6 +329,35 @@ private static bool IsWhite(char ch)
return null;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static unsafe bool AllowHyphenDuringParsing(NumberFormatInfo info)
{
string negativeSign = info.NegativeSign;
return negativeSign.Length == 1 &&
negativeSign[0] switch {
'\u2012' or // Figure Dash
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
'\u207B' or // Superscript Minus
'\u208B' or // Subscript Minus
'\u2212' or // Minus Sign
'\u2796' or // Heavy Minus Sign
'\uFE63' or // Small Hyphen-Minus
'\uFF0D' => true, // Fullwidth Hyphen-Minus
_ => false
};
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static unsafe char* MatchNegativeSignChars(char* p, char* pEnd, string negativeSign, bool allowHyphenDuringParsing)
{
char *ret = MatchChars(p, pEnd, negativeSign);
if (ret == null && allowHyphenDuringParsing && p <= pEnd && *p == '-')
{
ret = p + 1;
}

return ret;
}

private static unsafe bool ParseNumber(ref char* str, char* strEnd, NumberStyles options, ref NumberBuffer number, StringBuilder? sb, NumberFormatInfo numfmt, bool parseDecimal)
{
Debug.Assert(str != null);
Expand All @@ -345,7 +375,9 @@ private static unsafe bool ParseNumber(ref char* str, char* strEnd, NumberStyles
number.sign = false;
string decSep; // Decimal separator from NumberFormatInfo.
string groupSep; // Group separator from NumberFormatInfo.
string? currSymbol = null; // Currency symbol from NumberFormatInfo.
string? currSymbol = null; // Currency symbol from NumberFormatInfo.

bool allowHyphenDuringParsing = AllowHyphenDuringParsing(numfmt);

bool parsingCurrency = false;
if ((options & NumberStyles.AllowCurrencySymbol) != 0)
Expand Down Expand Up @@ -379,7 +411,7 @@ private static unsafe bool ParseNumber(ref char* str, char* strEnd, NumberStyles
// "-Kr 1231.47" is legal but "- 1231.47" is not.
if (!IsWhite(ch) || (options & NumberStyles.AllowLeadingWhite) == 0 || ((state & StateSign) != 0 && ((state & StateCurrency) == 0 && numfmt.NumberNegativePattern != 2)))
{
if ((((options & NumberStyles.AllowLeadingSign) != 0) && (state & StateSign) == 0) && ((next = MatchChars(p, strEnd, numfmt.PositiveSign)) != null || ((next = MatchChars(p, strEnd, numfmt.NegativeSign)) != null && (number.sign = true))))
if ((((options & NumberStyles.AllowLeadingSign) != 0) && (state & StateSign) == 0) && ((next = MatchChars(p, strEnd, numfmt.PositiveSign)) != null || ((next = MatchNegativeSignChars(p, strEnd, numfmt.NegativeSign, allowHyphenDuringParsing)) != null && (number.sign = true))))
{
state |= StateSign;
p = next - 1;
Expand Down Expand Up @@ -475,7 +507,7 @@ private static unsafe bool ParseNumber(ref char* str, char* strEnd, NumberStyles
{
ch = (p = next) < strEnd ? *p : '\0';
}
else if ((next = MatchChars(p, strEnd, numfmt.NegativeSign)) != null)
else if ((next = MatchNegativeSignChars(p, strEnd, numfmt.NegativeSign, allowHyphenDuringParsing)) != null)
{
ch = (p = next) < strEnd ? *p : '\0';
negExp = true;
Expand Down Expand Up @@ -512,7 +544,7 @@ private static unsafe bool ParseNumber(ref char* str, char* strEnd, NumberStyles
{
if (!IsWhite(ch) || (options & NumberStyles.AllowTrailingWhite) == 0)
{
if (((options & NumberStyles.AllowTrailingSign) != 0 && ((state & StateSign) == 0)) && ((next = MatchChars(p, strEnd, numfmt.PositiveSign)) != null || (((next = MatchChars(p, strEnd, numfmt.NegativeSign)) != null) && (number.sign = true))))
if (((options & NumberStyles.AllowTrailingSign) != 0 && ((state & StateSign) == 0)) && ((next = MatchChars(p, strEnd, numfmt.PositiveSign)) != null || (((next = MatchNegativeSignChars(p, strEnd, numfmt.NegativeSign, allowHyphenDuringParsing)) != null) && (number.sign = true))))
{
state |= StateSign;
p = next - 1;
Expand Down
27 changes: 23 additions & 4 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/NumberFormatInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;

namespace System.Globalization
{
/// <remarks>
Expand Down Expand Up @@ -71,6 +73,10 @@ public sealed class NumberFormatInfo : IFormatProvider, ICloneable

private bool _hasInvariantNumberSigns = true;

// When _allowHyphenDuringParsing is set to true, will allow the number parser to accept the hyphen `-` U+002D as a negative sign even if the culture
// negative sign is not the hyphen. Good example of that is Swedish culture (e.g. "sv-SE") which has U+2212 as negative sign.
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
private bool _allowHyphenDuringParsing;

public NumberFormatInfo()
{
}
Expand Down Expand Up @@ -155,10 +161,23 @@ private static void VerifyDigitSubstitution(DigitShapes digitSub, string propert
}

internal bool HasInvariantNumberSigns => _hasInvariantNumberSigns;
internal bool AllowHyphenDuringParsing => _allowHyphenDuringParsing;

private void UpdateHasInvariantNumberSigns()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
private void InitializeInvariantAndNegativeSignFlags()
{
_hasInvariantNumberSigns = _positiveSign == "+" && _negativeSign == "-";
_allowHyphenDuringParsing = _negativeSign.Length == 1 &&
_negativeSign[0] switch {
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
'\u2012' or // Figure Dash
'\u207B' or // Superscript Minus
'\u208B' or // Subscript Minus
'\u2212' or // Minus Sign
'\u2796' or // Heavy Minus Sign
'\uFE63' or // Small Hyphen-Minus
'\uFF0D' => true, // Fullwidth Hyphen-Minus
_ => false
};
}

internal NumberFormatInfo(CultureData? cultureData)
Expand All @@ -169,7 +188,7 @@ internal NumberFormatInfo(CultureData? cultureData)
// don't need to verify their values (except for invalid parsing situations).
cultureData.GetNFIValues(this);

UpdateHasInvariantNumberSigns();
InitializeInvariantAndNegativeSignFlags();
}
}

Expand Down Expand Up @@ -493,7 +512,7 @@ public string NegativeSign

VerifyWritable();
_negativeSign = value;
UpdateHasInvariantNumberSigns();
InitializeInvariantAndNegativeSignFlags();
}
}

Expand Down Expand Up @@ -582,7 +601,7 @@ public string PositiveSign

VerifyWritable();
_positiveSign = value;
UpdateHasInvariantNumberSigns();
InitializeInvariantAndNegativeSignFlags();
}
}

Expand Down
66 changes: 61 additions & 5 deletions src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ private static unsafe bool TryParseNumber(ref char* str, char* strEnd, NumberSty
// "-Kr 1231.47" is legal but "- 1231.47" is not.
if (!IsWhite(ch) || (styles & NumberStyles.AllowLeadingWhite) == 0 || ((state & StateSign) != 0 && ((state & StateCurrency) == 0 && info.NumberNegativePattern != 2)))
{
if ((((styles & NumberStyles.AllowLeadingSign) != 0) && (state & StateSign) == 0) && ((next = MatchChars(p, strEnd, info.PositiveSign)) != null || ((next = MatchChars(p, strEnd, info.NegativeSign)) != null && (number.IsNegative = true))))
if ((((styles & NumberStyles.AllowLeadingSign) != 0) && (state & StateSign) == 0) && ((next = MatchChars(p, strEnd, info.PositiveSign)) != null || ((next = MatchNegativeSignChars(p, strEnd, info)) != null && (number.IsNegative = true))))
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
{
state |= StateSign;
p = next - 1;
Expand Down Expand Up @@ -393,7 +393,7 @@ private static unsafe bool TryParseNumber(ref char* str, char* strEnd, NumberSty
{
ch = (p = next) < strEnd ? *p : '\0';
}
else if ((next = MatchChars(p, strEnd, info._negativeSign)) != null)
else if ((next = MatchNegativeSignChars(p, strEnd, info)) != null)
{
ch = (p = next) < strEnd ? *p : '\0';
negExp = true;
Expand Down Expand Up @@ -430,7 +430,7 @@ private static unsafe bool TryParseNumber(ref char* str, char* strEnd, NumberSty
{
if (!IsWhite(ch) || (styles & NumberStyles.AllowTrailingWhite) == 0)
{
if ((styles & NumberStyles.AllowTrailingSign) != 0 && ((state & StateSign) == 0) && ((next = MatchChars(p, strEnd, info.PositiveSign)) != null || (((next = MatchChars(p, strEnd, info.NegativeSign)) != null) && (number.IsNegative = true))))
if ((styles & NumberStyles.AllowTrailingSign) != 0 && ((state & StateSign) == 0) && ((next = MatchChars(p, strEnd, info.PositiveSign)) != null || (((next = MatchNegativeSignChars(p, strEnd, info)) != null) && (number.IsNegative = true))))
{
state |= StateSign;
p = next - 1;
Expand Down Expand Up @@ -555,6 +555,14 @@ internal static ParsingStatus TryParseInt32IntegerStyle(ReadOnlySpan<char> value
num = value[index];
}
}
else if (info.AllowHyphenDuringParsing && num == '-')
{
sign = -1;
index++;
if ((uint)index >= (uint)value.Length)
goto FalseExit;
num = value[index];
}
else
{
value = value.Slice(index);
Expand Down Expand Up @@ -726,6 +734,14 @@ internal static ParsingStatus TryParseInt64IntegerStyle(ReadOnlySpan<char> value
num = value[index];
}
}
else if (info.AllowHyphenDuringParsing && num == '-')
{
sign = -1;
index++;
if ((uint)index >= (uint)value.Length)
goto FalseExit;
num = value[index];
}
else
{
value = value.Slice(index);
Expand Down Expand Up @@ -970,6 +986,14 @@ internal static ParsingStatus TryParseUInt32IntegerStyle(ReadOnlySpan<char> valu
num = value[index];
}
}
else if (info.AllowHyphenDuringParsing && num == '-')
{
overflow = true;
index++;
if ((uint)index >= (uint)value.Length)
goto FalseExit;
num = value[index];
}
else
{
value = value.Slice(index);
Expand Down Expand Up @@ -1298,6 +1322,14 @@ internal static ParsingStatus TryParseUInt64IntegerStyle(ReadOnlySpan<char> valu
num = value[index];
}
}
else if (info.AllowHyphenDuringParsing && num == '-')
{
overflow = true;
index++;
if ((uint)index >= (uint)value.Length)
goto FalseExit;
num = value[index];
}
else
{
value = value.Slice(index);
Expand Down Expand Up @@ -1725,6 +1757,8 @@ internal static unsafe ParsingStatus TryParseDecimal(ReadOnlySpan<char> value, N
return ParsingStatus.OK;
}

internal static bool SpanStartsWith(ReadOnlySpan<char> span, char c) => span.Length > 0 && span[0] == c;
tarekgh marked this conversation as resolved.
Show resolved Hide resolved

internal static unsafe bool TryParseDouble(ReadOnlySpan<char> value, NumberStyles styles, NumberFormatInfo info, out double result)
{
byte* pDigits = stackalloc byte[DoubleNumberBufferLength];
Expand Down Expand Up @@ -1768,8 +1802,8 @@ internal static unsafe bool TryParseDouble(ReadOnlySpan<char> value, NumberStyle
return false;
}
}
else if (valueTrim.StartsWith(info.NegativeSign, StringComparison.OrdinalIgnoreCase) &&
valueTrim.Slice(info.NegativeSign.Length).EqualsOrdinalIgnoreCase(info.NaNSymbol))
else if ((valueTrim.StartsWith(info.NegativeSign, StringComparison.OrdinalIgnoreCase) && valueTrim.Slice(info.NegativeSign.Length).EqualsOrdinalIgnoreCase(info.NaNSymbol)) ||
(info.AllowHyphenDuringParsing && SpanStartsWith(valueTrim, '-') && valueTrim.Slice(1).EqualsOrdinalIgnoreCase(info.NaNSymbol)))
{
result = double.NaN;
}
Expand Down Expand Up @@ -1840,6 +1874,11 @@ internal static unsafe bool TryParseHalf(ReadOnlySpan<char> value, NumberStyles
{
result = Half.NaN;
}
else if (info.AllowHyphenDuringParsing && SpanStartsWith(valueTrim, '-') && !info.NaNSymbol.StartsWith(info.NegativeSign, StringComparison.OrdinalIgnoreCase) &&
!info.NaNSymbol.StartsWith('-') && valueTrim.Slice(1).EqualsOrdinalIgnoreCase(info.NaNSymbol))
{
result = Half.NaN;
}
else
{
result = (Half)0;
Expand Down Expand Up @@ -1907,6 +1946,11 @@ internal static unsafe bool TryParseSingle(ReadOnlySpan<char> value, NumberStyle
{
result = float.NaN;
}
else if (info.AllowHyphenDuringParsing && SpanStartsWith(valueTrim, '-') && !info.NaNSymbol.StartsWith(info.NegativeSign, StringComparison.OrdinalIgnoreCase) &&
!info.NaNSymbol.StartsWith('-') && valueTrim.Slice(1).EqualsOrdinalIgnoreCase(info.NaNSymbol))
{
result = float.NaN;
}
else
{
result = 0;
Expand Down Expand Up @@ -1955,6 +1999,18 @@ private static bool TrailingZeros(ReadOnlySpan<char> value, int index)

private static bool IsSpaceReplacingChar(char c) => c == '\u00a0' || c == '\u202f';

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static unsafe char* MatchNegativeSignChars(char* p, char* pEnd, NumberFormatInfo info)
{
char *ret = MatchChars(p, pEnd, info.NegativeSign);
if (ret == null && info.AllowHyphenDuringParsing && p <= pEnd && *p == '-')
{
ret = p + 1;
}

return ret;
}

private static unsafe char* MatchChars(char* p, char* pEnd, string value)
{
Debug.Assert(p != null && pEnd != null && p <= pEnd && value != null);
Expand Down
9 changes: 9 additions & 0 deletions src/libraries/System.Runtime/tests/System/DoubleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,5 +713,14 @@ public static void ToStringRoundtrip_R(double value)
double result = double.Parse(value.ToString("R"));
Assert.Equal(BitConverter.DoubleToInt64Bits(value), BitConverter.DoubleToInt64Bits(result));
}

[Fact]
public static void TestNegativeNumberParsingWithHyphen()
{
// CLDR data for Swedish culture has negative sign U+2212. This test ensure parsing with the hyphen with such cultures will succeed.
CultureInfo ci = CultureInfo.GetCultureInfo("sv-SE");
string s = string.Format(ci, "{0}", 158.68);
Assert.Equal(-158.68, double.Parse("-" + s, NumberStyles.Number, ci));
}
}
}
8 changes: 8 additions & 0 deletions src/libraries/System.Runtime/tests/System/Int32Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -834,5 +834,13 @@ public static void TryFormat(int i, string format, IFormatProvider provider, str
Assert.Equal(expected.ToLowerInvariant(), new string(actual));
}
}

[Fact]
public static void TestNegativeNumberParsingWithHyphen()
{
// CLDR data for Swedish culture has negative sign U+2212. This test ensure parsing with the hyphen with such cultures will succeed.
CultureInfo ci = CultureInfo.GetCultureInfo("sv-SE");
Assert.Equal(-158, int.Parse("-158", NumberStyles.Integer, ci));
}
}
}
8 changes: 8 additions & 0 deletions src/libraries/System.Runtime/tests/System/Int64Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,5 +439,13 @@ public static void TryFormat(long i, string format, IFormatProvider provider, st
Assert.Equal(expected.ToLowerInvariant(), new string(actual));
}
}

[Fact]
public static void TestNegativeNumberParsingWithHyphen()
{
// CLDR data for Swedish culture has negative sign U+2212. This test ensure parsing with the hyphen with such cultures will succeed.
CultureInfo ci = CultureInfo.GetCultureInfo("sv-SE");
Assert.Equal(-15868, long.Parse("-15868", NumberStyles.Number, ci));
}
}
}