Skip to content

Remove unnecessary logic to determine E format when parsing floating... #42298

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

Merged
jozkee merged 1 commit into from
Sep 17, 2020
Merged

Remove unnecessary logic to determine E format when parsing floating... #42298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.DbRow.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ internal readonly struct DbRow
internal bool IsUnknownSize => _sizeOrLengthUnion == UnknownSize;

/// <summary>
/// Number: Use scientific format.
/// String/PropertyName: Unescaping is required.
/// Array: At least one element is an object/array.
/// Otherwise; false
Expand Down
27 changes: 3 additions & 24 deletions src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,7 @@ internal bool TryGetValue(int index, out double value)
ReadOnlySpan<byte> data = _utf8Json.Span;
ReadOnlySpan<byte> segment = data.Slice(row.Location, row.SizeOrLength);

char standardFormat = row.HasComplexChildren ? JsonConstants.ScientificNotationFormat : default;

if (Utf8Parser.TryParse(segment, out double tmp, out int bytesConsumed, standardFormat) &&
if (Utf8Parser.TryParse(segment, out double tmp, out int bytesConsumed) &&
segment.Length == bytesConsumed)
{
value = tmp;
Expand All @@ -610,9 +608,7 @@ internal bool TryGetValue(int index, out float value)
ReadOnlySpan<byte> data = _utf8Json.Span;
ReadOnlySpan<byte> segment = data.Slice(row.Location, row.SizeOrLength);

char standardFormat = row.HasComplexChildren ? JsonConstants.ScientificNotationFormat : default;

if (Utf8Parser.TryParse(segment, out float tmp, out int bytesConsumed, standardFormat) &&
if (Utf8Parser.TryParse(segment, out float tmp, out int bytesConsumed) &&
segment.Length == bytesConsumed)
{
value = tmp;
Expand All @@ -634,9 +630,7 @@ internal bool TryGetValue(int index, out decimal value)
ReadOnlySpan<byte> data = _utf8Json.Span;
ReadOnlySpan<byte> segment = data.Slice(row.Location, row.SizeOrLength);

char standardFormat = row.HasComplexChildren ? JsonConstants.ScientificNotationFormat : default;

if (Utf8Parser.TryParse(segment, out decimal tmp, out int bytesConsumed, standardFormat) &&
if (Utf8Parser.TryParse(segment, out decimal tmp, out int bytesConsumed) &&
segment.Length == bytesConsumed)
{
value = tmp;
Expand Down Expand Up @@ -1067,21 +1061,6 @@ private static void Parse(
else
{
database.Append(tokenType, tokenStart, reader.ValueSpan.Length);

if (tokenType == JsonTokenType.Number)
{
switch (reader._numberFormat)
{
case JsonConstants.ScientificNotationFormat:
database.SetHasComplexChildren(database.Length - DbRow.Size);
break;
default:
Debug.Assert(
reader._numberFormat == default,
$"Unhandled numeric format {reader._numberFormat}");
break;
}
}
}
}

Expand Down
2 changes: 0 additions & 2 deletions src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ internal static class JsonConstants
public const int MinimumDateTimeParseLength = 10; // YYYY-MM-DD
public const int MaximumEscapedDateTimeOffsetParseLength = MaxExpansionFactorWhileEscaping * MaximumDateTimeOffsetParseLength;

internal const char ScientificNotationFormat = 'e';

// Encoding Helpers
public const char HighSurrogateStart = '\ud800';
public const char HighSurrogateEnd = '\udbff';
Expand Down
15 changes: 0 additions & 15 deletions src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,21 +322,6 @@ public static bool TryGetEscapedGuid(ReadOnlySpan<byte> source, out Guid value)
return false;
}

public static char GetFloatingPointStandardParseFormat(ReadOnlySpan<byte> span)
{
// Assume that 'e/E' is closer to the end.
int startIndex = span.Length - 1;
for (int i = startIndex; i >= 0; i--)
{
byte token = span[i];
if (token == 'E' || token == 'e')
{
return JsonConstants.ScientificNotationFormat;
}
}
return default;
}

public static bool TryGetFloatingPointConstant(ReadOnlySpan<byte> span, out float value)
{
if (span.Length == 3)
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public struct JsonReaderState
internal long _bytePositionInLine;
internal bool _inObject;
internal bool _isNotPrimitive;
internal char _numberFormat;
internal bool _stringHasEscaping;
internal bool _trailingCommaBeforeComment;
internal JsonTokenType _tokenType;
Expand All @@ -43,7 +42,6 @@ public JsonReaderState(JsonReaderOptions options = default)
_bytePositionInLine = default;
_inObject = default;
_isNotPrimitive = default;
_numberFormat = default;
_stringHasEscaping = default;
_trailingCommaBeforeComment = default;
_tokenType = default;
Expand Down
4 changes: 0 additions & 4 deletions src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public Utf8JsonReader(ReadOnlySequence<byte> jsonData, bool isFinalBlock, JsonRe
_bytePositionInLine = state._bytePositionInLine;
_inObject = state._inObject;
_isNotPrimitive = state._isNotPrimitive;
_numberFormat = state._numberFormat;
_stringHasEscaping = state._stringHasEscaping;
_trailingCommaBeforeComment = state._trailingCommaBeforeComment;
_tokenType = state._tokenType;
Expand Down Expand Up @@ -1122,8 +1121,6 @@ private bool TryGetNumberMultiSegment(ReadOnlySpan<byte> data, out int consumed)
// TODO: https://github.com/dotnet/runtime/issues/27837
Debug.Assert(data.Length > 0);

_numberFormat = default;

PartialStateForRollback rollBackState = CaptureState();

consumed = 0;
Expand Down Expand Up @@ -1207,7 +1204,6 @@ private bool TryGetNumberMultiSegment(ReadOnlySpan<byte> data, out int consumed)

Debug.Assert(nextByte == 'E' || nextByte == 'e');
i++;
_numberFormat = JsonConstants.ScientificNotationFormat;
_bytePositionInLine++;

signResult = ConsumeSignMultiSegment(ref data, ref i, rollBackState);
Expand Down
29 changes: 14 additions & 15 deletions src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,8 +422,7 @@ internal float GetSingleWithQuotes()
return value;
}

char numberFormat = JsonReaderHelper.GetFloatingPointStandardParseFormat(span);
if (Utf8Parser.TryParse(span, out value, out int bytesConsumed, numberFormat)
if (Utf8Parser.TryParse(span, out value, out int bytesConsumed)
&& span.Length == bytesConsumed)
{
// NETCOREAPP implementation of the TryParse method above permits case-insenstive variants of the
Expand Down Expand Up @@ -482,11 +481,10 @@ internal double GetDoubleWithQuotes()
return value;
}

char numberFormat = JsonReaderHelper.GetFloatingPointStandardParseFormat(span);
if (Utf8Parser.TryParse(span, out value, out int bytesConsumed, numberFormat)
if (Utf8Parser.TryParse(span, out value, out int bytesConsumed)
&& span.Length == bytesConsumed)
{
// NETCOREAPP implmentation of the TryParse method above permits case-insenstive variants of the
// NETCOREAPP implementation of the TryParse method above permits case-insenstive variants of the
// float constants "NaN", "Infinity", "-Infinity". This differs from the NETFRAMEWORK implementation.
// The following logic reconciles the two implementations to enforce consistent behavior.
if (!double.IsNaN(value) && !double.IsPositiveInfinity(value) && !double.IsNegativeInfinity(value))
Expand Down Expand Up @@ -536,15 +534,11 @@ public decimal GetDecimal()
internal decimal GetDecimalWithQuotes()
{
ReadOnlySpan<byte> span = GetUnescapedSpan();

char numberFormat = JsonReaderHelper.GetFloatingPointStandardParseFormat(span);
if (Utf8Parser.TryParse(span, out decimal value, out int bytesConsumed, numberFormat)
&& span.Length == bytesConsumed)
if (!TryGetDecimalCore(out decimal value, span))
{
return value;
throw ThrowHelper.GetFormatException(NumericType.Decimal);
}

throw ThrowHelper.GetFormatException(NumericType.Decimal);
return value;
}

/// <summary>
Expand Down Expand Up @@ -979,7 +973,7 @@ public bool TryGetSingle(out float value)

ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;

if (Utf8Parser.TryParse(span, out float tmp, out int bytesConsumed, _numberFormat)
if (Utf8Parser.TryParse(span, out float tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
{
value = tmp;
Expand Down Expand Up @@ -1009,7 +1003,7 @@ public bool TryGetDouble(out double value)

ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;

if (Utf8Parser.TryParse(span, out double tmp, out int bytesConsumed, _numberFormat)
if (Utf8Parser.TryParse(span, out double tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
{
value = tmp;
Expand Down Expand Up @@ -1038,8 +1032,13 @@ public bool TryGetDecimal(out decimal value)
}

ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
return TryGetDecimalCore(out value, span);
}

if (Utf8Parser.TryParse(span, out decimal tmp, out int bytesConsumed, _numberFormat)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool TryGetDecimalCore(out decimal value, ReadOnlySpan<byte> span)
Copy link
Contributor

@steveharter steveharter Sep 17, 2020

Choose a reason for hiding this comment

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

nit: the out parameter is normally last or towards the end.

Copy link
Member Author

@jozkee jozkee Sep 17, 2020

Choose a reason for hiding this comment

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

All other TryGet*Core methods have this same parameter ordering and since this is internal I think is no much value on changing the ordering here, perhaps that can be made in a clean-up PR later.

{
if (Utf8Parser.TryParse(span, out decimal tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
{
value = tmp;
Expand Down
5 changes: 0 additions & 5 deletions src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ public ref partial struct Utf8JsonReader
private int _consumed;
private bool _inObject;
private bool _isNotPrimitive;
internal char _numberFormat;
private JsonTokenType _tokenType;
private JsonTokenType _previousTokenType;
private JsonReaderOptions _readerOptions;
Expand Down Expand Up @@ -183,7 +182,6 @@ public SequencePosition Position
_bytePositionInLine = _bytePositionInLine,
_inObject = _inObject,
_isNotPrimitive = _isNotPrimitive,
_numberFormat = _numberFormat,
_stringHasEscaping = _stringHasEscaping,
_trailingCommaBeforeComment = _trailingCommaBeforeComment,
_tokenType = _tokenType,
Expand Down Expand Up @@ -215,7 +213,6 @@ public Utf8JsonReader(ReadOnlySpan<byte> jsonData, bool isFinalBlock, JsonReader
_bytePositionInLine = state._bytePositionInLine;
_inObject = state._inObject;
_isNotPrimitive = state._isNotPrimitive;
_numberFormat = state._numberFormat;
_stringHasEscaping = state._stringHasEscaping;
_trailingCommaBeforeComment = state._trailingCommaBeforeComment;
_tokenType = state._tokenType;
Expand Down Expand Up @@ -1415,7 +1412,6 @@ private bool TryGetNumber(ReadOnlySpan<byte> data, out int consumed)
// TODO: https://github.com/dotnet/runtime/issues/27837
Debug.Assert(data.Length > 0);

_numberFormat = default;
consumed = 0;
int i = 0;

Expand Down Expand Up @@ -1493,7 +1489,6 @@ private bool TryGetNumber(ReadOnlySpan<byte> data, out int consumed)

Debug.Assert(nextByte == 'E' || nextByte == 'e');
i++;
_numberFormat = JsonConstants.ScientificNotationFormat;

signResult = ConsumeSign(ref data, ref i);
if (signResult == ConsumeNumberResult.NeedMoreData)
Expand Down