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.

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

Add binary parsing support to integer types #84998

Merged
merged 3 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@
<data name="Arg_HandleNotSync" xml:space="preserve">
<value>Handle does not support synchronous operations. The parameters to the FileStream constructor may need to be changed to indicate that the handle was opened asynchronously (that is, it was opened explicitly for overlapped I/O).</value>
</data>
<data name="Arg_HexStyleNotSupported" xml:space="preserve">
<value>The number style AllowHexSpecifier is not supported on floating point data types.</value>
<data name="Arg_HexBinaryStylesNotSupported" xml:space="preserve">
<value>The number styles AllowHexSpecifier and AllowBinarySpecifier are not supported on floating point data types.</value>
</data>
<data name="Arg_HTCapacityOverflow" xml:space="preserve">
<value>Hashtable's capacity overflowed and went negative. Check load factor, capacity and the current size of the table.</value>
Expand Down Expand Up @@ -415,8 +415,8 @@
<data name="Arg_InvalidHandle" xml:space="preserve">
<value>Invalid handle.</value>
</data>
<data name="Arg_InvalidHexStyle" xml:space="preserve">
<value>With the AllowHexSpecifier bit set in the enum bit field, the only other valid bits that can be combined into the enum value must be a subset of those in HexNumber.</value>
<data name="Arg_InvalidHexBinaryStyle" xml:space="preserve">
<value>With the AllowHexSpecifier or AllowBinarySpecifier bit set in the enum bit field, the only other valid bits that can be combined into the enum value must be AllowLeadingWhite and AllowTrailingWhite.</value>
</data>
<data name="Arg_InvalidNeutralResourcesLanguage_Asm_Culture" xml:space="preserve">
<value>The NeutralResourcesLanguageAttribute on the assembly "{0}" specifies an invalid culture name: "{1}".</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -828,45 +828,37 @@ public static NumberFormatInfo ReadOnly(NumberFormatInfo nfi)
| NumberStyles.AllowLeadingSign | NumberStyles.AllowTrailingSign
| NumberStyles.AllowParentheses | NumberStyles.AllowDecimalPoint
| NumberStyles.AllowThousands | NumberStyles.AllowExponent
| NumberStyles.AllowCurrencySymbol | NumberStyles.AllowHexSpecifier);
| NumberStyles.AllowCurrencySymbol | NumberStyles.AllowHexSpecifier
| NumberStyles.AllowBinarySpecifier);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static void ValidateParseStyleInteger(NumberStyles style)
{
// Check for undefined flags or invalid hex number flags
if ((style & (InvalidNumberStyles | NumberStyles.AllowHexSpecifier)) != 0
&& (style & ~NumberStyles.HexNumber) != 0)
// Check for undefined flags or using AllowHexSpecifier/AllowBinarySpecifier each with anything other than AllowLeadingWhite/AllowTrailingWhite.
if ((style & (InvalidNumberStyles | NumberStyles.AllowHexSpecifier | NumberStyles.AllowBinarySpecifier)) != 0 &&
(style & ~NumberStyles.HexNumber) != 0 &&
(style & ~NumberStyles.BinaryNumber) != 0)
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
ThrowInvalid(style);

static void ThrowInvalid(NumberStyles value)
{
if ((value & InvalidNumberStyles) != 0)
{
throw new ArgumentException(SR.Argument_InvalidNumberStyles, nameof(style));
}

throw new ArgumentException(SR.Arg_InvalidHexStyle);
throw new ArgumentException(
(value & InvalidNumberStyles) != 0 ? SR.Argument_InvalidNumberStyles : SR.Arg_InvalidHexBinaryStyle,
nameof(style));
}
}
}

internal static void ValidateParseStyleFloatingPoint(NumberStyles style)
{
// Check for undefined flags or hex number
if ((style & (InvalidNumberStyles | NumberStyles.AllowHexSpecifier)) != 0)
if ((style & (InvalidNumberStyles | NumberStyles.AllowHexSpecifier | NumberStyles.AllowBinarySpecifier)) != 0)
{
ThrowInvalid(style);

static void ThrowInvalid(NumberStyles value)
{
if ((value & InvalidNumberStyles) != 0)
{
throw new ArgumentException(SR.Argument_InvalidNumberStyles, nameof(style));
}

throw new ArgumentException(SR.Arg_HexStyleNotSupported);
}
static void ThrowInvalid(NumberStyles value) =>
throw new ArgumentException((value & InvalidNumberStyles) != 0 ? SR.Argument_InvalidNumberStyles : SR.Arg_HexBinaryStylesNotSupported, nameof(style));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,23 @@ public enum NumberStyles

AllowHexSpecifier = 0x00000200,

/// <summary>
/// Indicates that the numeric string represents a binary value. Valid binary values include the numeric digits 0 and 1.
/// Strings that are parsed using this style do not employ a prefix; "0b" cannot be used. A string that is parsed with
/// the <see cref="AllowBinarySpecifier"/> style will always be interpreted as a binary value. The only flags that can
/// be combined with <see cref="AllowBinarySpecifier"/> are <see cref="AllowLeadingWhite"/> and <see cref="AllowTrailingWhite"/>.
/// The <see cref="NumberStyles"/> enumeration includes a composite style, <see cref="BinaryNumber"/>, that consists of
/// these three flags.
/// </summary>
AllowBinarySpecifier = 0x00000400,

Integer = AllowLeadingWhite | AllowTrailingWhite | AllowLeadingSign,

HexNumber = AllowLeadingWhite | AllowTrailingWhite | AllowHexSpecifier,

/// <summary>Indicates that the <see cref="AllowLeadingWhite"/>, <see cref="AllowTrailingWhite"/>, and <see cref="AllowBinarySpecifier"/> styles are used. This is a composite number style.</summary>
BinaryNumber = AllowLeadingWhite | AllowTrailingWhite | AllowBinarySpecifier,

Number = AllowLeadingWhite | AllowTrailingWhite | AllowLeadingSign | AllowTrailingSign |
AllowDecimalPoint | AllowThousands,

Expand Down
66 changes: 53 additions & 13 deletions src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private static unsafe bool TryParseNumber(scoped ref char* str, char* strEnd, Nu
Debug.Assert(str != null);
Debug.Assert(strEnd != null);
Debug.Assert(str <= strEnd);
Debug.Assert((styles & NumberStyles.AllowHexSpecifier) == 0);
Debug.Assert((styles & (NumberStyles.AllowHexSpecifier | NumberStyles.AllowBinarySpecifier)) == 0);

const int StateSign = 0x0001;
const int StateParens = 0x0002;
Expand Down Expand Up @@ -412,6 +412,11 @@ internal static ParsingStatus TryParseBinaryInteger<TInteger>(ReadOnlySpan<char>
return TryParseBinaryIntegerHexNumberStyle(value, styles, out result);
}

if ((styles & NumberStyles.AllowBinarySpecifier) != 0)
{
return TryParseBinaryIntegerHexOrBinaryNumberStyle<TInteger, BinaryParser<TInteger>>(value, styles, out result);
}

return TryParseBinaryIntegerNumber(value, styles, info, out result);
}

Expand Down Expand Up @@ -650,11 +655,46 @@ internal static ParsingStatus TryParseBinaryIntegerStyle<TInteger>(ReadOnlySpan<
goto DoneAtEndButPotentialOverflow;
}

/// <summary>Parses uint limited to styles that make up NumberStyles.HexNumber.</summary>
/// <summary>Parses <typeparamref name="TInteger"/> limited to styles that make up NumberStyles.HexNumber.</summary>
internal static ParsingStatus TryParseBinaryIntegerHexNumberStyle<TInteger>(ReadOnlySpan<char> value, NumberStyles styles, out TInteger result)
where TInteger : unmanaged, IBinaryIntegerParseAndFormatInfo<TInteger> =>
TryParseBinaryIntegerHexOrBinaryNumberStyle<TInteger, HexParser<TInteger>>(value, styles, out result);

private interface IHexOrBinaryParser<TInteger> where TInteger : unmanaged, IBinaryIntegerParseAndFormatInfo<TInteger>
{
static abstract NumberStyles AllowedStyles { get; }
static abstract bool IsValidChar(int ch);
static abstract uint FromChar(int ch);
static abstract uint MaxDigitValue { get; }
static abstract int MaxDigitCount { get; }
static abstract TInteger ShiftLeftForNextDigit(TInteger value);
}

private readonly struct HexParser<TInteger> : IHexOrBinaryParser<TInteger> where TInteger : unmanaged, IBinaryIntegerParseAndFormatInfo<TInteger>
{
public static NumberStyles AllowedStyles => NumberStyles.HexNumber;
public static bool IsValidChar(int ch) => HexConverter.IsHexChar(ch);
public static uint FromChar(int ch) => (uint)HexConverter.FromChar(ch);
public static uint MaxDigitValue => 0xF;
public static int MaxDigitCount => TInteger.MaxHexDigitCount;
public static TInteger ShiftLeftForNextDigit(TInteger value) => TInteger.MultiplyBy16(value);
}

private readonly struct BinaryParser<TInteger> : IHexOrBinaryParser<TInteger> where TInteger : unmanaged, IBinaryIntegerParseAndFormatInfo<TInteger>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if its worth optimizing for processing multiple bits at a time. Doing up to 64-iterations for -1L is a lot for example.

-- Not something that needs to be in this PR, but might be worth an up-for-grabs issue if anyone is interested in optimizing it further.

{
public static NumberStyles AllowedStyles => NumberStyles.BinaryNumber;
public static bool IsValidChar(int ch) => (uint)(ch - '0') <= 1;
public static uint FromChar(int ch) => (uint)(ch - '0');
public static uint MaxDigitValue => 1;
public static unsafe int MaxDigitCount => sizeof(TInteger) * 8;
public static TInteger ShiftLeftForNextDigit(TInteger value) => value << 1;
}

private static ParsingStatus TryParseBinaryIntegerHexOrBinaryNumberStyle<TInteger, TParser>(ReadOnlySpan<char> value, NumberStyles styles, out TInteger result)
where TInteger : unmanaged, IBinaryIntegerParseAndFormatInfo<TInteger>
where TParser : struct, IHexOrBinaryParser<TInteger>
{
Debug.Assert((styles & ~NumberStyles.HexNumber) == 0, "Only handles subsets of HexNumber format");
Debug.Assert((styles & ~TParser.AllowedStyles) == 0, $"Only handles subsets of {TParser.AllowedStyles} format");

if (value.IsEmpty)
goto FalseExit;
Expand All @@ -678,7 +718,7 @@ internal static ParsingStatus TryParseBinaryIntegerHexNumberStyle<TInteger>(Read
bool overflow = false;
TInteger answer = TInteger.Zero;

if (HexConverter.IsHexChar(num))
if (TParser.IsValidChar(num))
{
// Skip past leading zeros.
if (num == '0')
Expand All @@ -690,32 +730,32 @@ internal static ParsingStatus TryParseBinaryIntegerHexNumberStyle<TInteger>(Read
goto DoneAtEnd;
num = value[index];
} while (num == '0');
if (!HexConverter.IsHexChar(num))
if (!TParser.IsValidChar(num))
goto HasTrailingChars;
}

// Parse up through MaxHexDigitCount digits, as no overflow is possible
answer = TInteger.CreateTruncating((uint)HexConverter.FromChar(num)); // first digit
// Parse up through MaxDigitCount digits, as no overflow is possible
answer = TInteger.CreateTruncating(TParser.FromChar(num)); // first digit
index++;
for (int i = 0; i < TInteger.MaxHexDigitCount - 1; i++) // next MaxHexDigitCount - 1 digits can't overflow
for (int i = 0; i < TParser.MaxDigitCount - 1; i++) // next MaxDigitCount - 1 digits can't overflow
{
if ((uint)index >= (uint)value.Length)
goto DoneAtEnd;
num = value[index];

uint numValue = (uint)HexConverter.FromChar(num);
if (numValue == 0xFF)
uint numValue = TParser.FromChar(num);
if (numValue > TParser.MaxDigitValue)
goto HasTrailingChars;
index++;
answer = TInteger.MultiplyBy16(answer);
answer = TParser.ShiftLeftForNextDigit(answer);
answer += TInteger.CreateTruncating(numValue);
}

// If there's another digit, it's an overflow.
if ((uint)index >= (uint)value.Length)
goto DoneAtEnd;
num = value[index];
if (!HexConverter.IsHexChar(num))
if (!TParser.IsValidChar(num))
goto HasTrailingChars;

// At this point, we're either overflowing or hitting a formatting error.
Expand All @@ -726,7 +766,7 @@ internal static ParsingStatus TryParseBinaryIntegerHexNumberStyle<TInteger>(Read
if ((uint)index >= (uint)value.Length)
goto OverflowExit;
num = value[index];
} while (HexConverter.IsHexChar(num));
} while (TParser.IsValidChar(num));
overflow = true;
goto HasTrailingChars;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ public static NFloat Parse(string s)
/// <exception cref="ArgumentException">
/// <para><paramref name="style" /> is not a <see cref="NumberStyles" /> value.</para>
/// <para>-or-</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> value.</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> or <see cref="NumberStyles.AllowBinarySpecifier" /> value.</para>
/// </exception>
/// <exception cref="ArgumentNullException"><paramref name="s" /> is <c>null</c>.</exception>
/// <exception cref="FormatException"><paramref name="s" /> does not represent a number in a valid format.</exception>
Expand Down Expand Up @@ -661,7 +661,7 @@ public static NFloat Parse(string s, IFormatProvider? provider)
/// <exception cref="ArgumentException">
/// <para><paramref name="style" /> is not a <see cref="NumberStyles" /> value.</para>
/// <para>-or-</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> value.</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> or <see cref="NumberStyles.AllowBinarySpecifier" /> value.</para>
/// </exception>
/// <exception cref="ArgumentNullException"><paramref name="s" /> is <c>null</c>.</exception>
/// <exception cref="FormatException"><paramref name="s" /> does not represent a number in a valid format.</exception>
Expand All @@ -679,7 +679,7 @@ public static NFloat Parse(string s, NumberStyles style, IFormatProvider? provid
/// <exception cref="ArgumentException">
/// <para><paramref name="style" /> is not a <see cref="NumberStyles" /> value.</para>
/// <para>-or-</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> value.</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> or <see cref="NumberStyles.AllowBinarySpecifier" /> value.</para>
/// </exception>
/// <exception cref="FormatException"><paramref name="s" /> does not represent a number in a valid format.</exception>
public static NFloat Parse(ReadOnlySpan<char> s, NumberStyles style = DefaultNumberStyles, IFormatProvider? provider = null)
Expand Down Expand Up @@ -717,7 +717,7 @@ public static bool TryParse(ReadOnlySpan<char> s, out NFloat result)
/// <exception cref="ArgumentException">
/// <para><paramref name="style" /> is not a <see cref="NumberStyles" /> value.</para>
/// <para>-or-</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> value.</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> or <see cref="NumberStyles.AllowBinarySpecifier" /> value.</para>
/// </exception>
public static bool TryParse([NotNullWhen(true)] string? s, NumberStyles style, IFormatProvider? provider, out NFloat result)
{
Expand All @@ -734,7 +734,7 @@ public static bool TryParse([NotNullWhen(true)] string? s, NumberStyles style, I
/// <exception cref="ArgumentException">
/// <para><paramref name="style" /> is not a <see cref="NumberStyles" /> value.</para>
/// <para>-or-</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> value.</para>
/// <para><paramref name="style" /> includes the <see cref="NumberStyles.AllowHexSpecifier" /> or <see cref="NumberStyles.AllowBinarySpecifier" /> value.</para>
/// </exception>
public static bool TryParse(ReadOnlySpan<char> s, NumberStyles style, IFormatProvider? provider, out NFloat result)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3150,8 +3150,8 @@ private void AssertValid()
Debug.Assert(_bits.Length > 0);
// Wasted space: _bits[0] could have been packed into _sign
Debug.Assert(_bits.Length > 1 || _bits[0] >= kuMaskHighBit);
// Wasted space: leading zeros could have been truncated
Debug.Assert(_bits[_bits.Length - 1] != 0);
//// Wasted space: leading zeros could have been truncated // TODO: https://github.com/dotnet/runtime/issues/84991
//Debug.Assert(_bits[_bits.Length - 1] != 0);
// Arrays larger than this can't fit into a Span<byte>
Debug.Assert(_bits.Length <= MaxLength);
}
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Runtime/ref/System.Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9026,6 +9026,8 @@ public enum NumberStyles
Any = 511,
AllowHexSpecifier = 512,
HexNumber = 515,
AllowBinarySpecifier = 1024,
BinaryNumber = 1027,
}
public partial class PersianCalendar : System.Globalization.Calendar
{
Expand Down