Add JsonUtf8Reader (for ReadOnlySpan) along with unit tests#33216
Conversation
JsonReader behaviour
|
@dotnet-bot test NETFX x86 Release Build |
|
Any other PR feedback? |
Maybe we need to increase the timeout? |
|
OK, So I am not sure why my recent changes would cause CI to fail (it was passing earlier): I am using the same seed for the random number generator. Is this an issue with doubles not round-tripping or am I missing something? The actual and expected match (at least from the output). @dotnet-bot test Windows x86 Release Build |
|
Maybe it picked up @tannergooding floating point changes in the latest run? |
Yep, that is what I am looking into now. I am going to merge master into my branch and try to repro locally. The repro tool isn't working atm. This dotnet/coreclr#20707 came in as part of #33324 |
|
Presumably the assert message is doing rounding otherwise it doesn't makes sense |
|
Yep, inspecting double values without strings is difficult, but looks like the actual double values are different. Original double value: 29607.7425729519 // doubles[count] = 29607.7425729519
var str = string.Format(CultureInfo.InvariantCulture, "{0}", doubles[count]);
double expected = double.Parse(str, CultureInfo.InvariantCulture); // expected != doubles[count]
json.TryGetDoubleValue(out double numberDouble); // returns true
// numberDouble != doubles[count] && numberDouble != expected
Assert.Equal(expected, numberDouble); // fails |
|
Hmm, looks like Another one: Tried to use "R" and "G17" to get some round-tripped values: string roundTripActual = numberDouble.ToString("G17", CultureInfo.InvariantCulture);
double actual = double.Parse(roundTripActual, CultureInfo.InvariantCulture);
string roundTripExpected = doubles[count].ToString("G17", CultureInfo.InvariantCulture);
double expected = double.Parse(roundTripExpected, CultureInfo.InvariantCulture); |
Yep. The Utf8Parser and Utf16Parser currently live apart and cannot easily share code. I am currently working on moving the Utf8Parser/Formatter into S.P.Corelib so that it can share all the necessary code, as needed.
|
Sounds good. Thanks for the update. I added a workaround in the tests temporarily to unblock my PR. |
…orefx#33216) * Initial check-in for System.Text.Json library and solution * Auto-generate the reference assembly * Provide an overview README and roadmap * Change the solution GUID to be unique * Fix package description nit * Change to MIT license instead of GPL * Fix file extension of images to png * Address PR feedback and only support netcoreapp * Fix the configuration props and project files * Add comments to the JsonTokenType enum * Add an internal class for commonly used constants when reading JSON. * Add a comment handling enum for lenient JSON reading * Add a JsonReaderOptions struct to provide extensible customization of JsonReader behaviour * Add a JsonReaderException class which is thrown when we see an invalid JSON text * Add the skeleton for the JsonUtf8Reader ref struct and add files to csproj * Add JsonReaderState, ThrowHelper, and Strings resource file * Address PR feedback, remove OS-specific configurations * Complete the implementation of core JsonReader APIs for reading spans * Update the reference assembly by auto-generating it * Add unit tests and TryGet APIs * Add XML comments to describe the JsonUtf8Reader at a high level * Fix config and csproj to reference S.R.CS.U from Corelib * Rename type from JsonUtf8Reader to Utf8JsonReader * Remove remnants of ReadOnlySequeunce related fields/properties * Add the remaining TryGetValueAsX APIs * Change JsonReaderState ctor to accept JsonReaderOptions instead * Seal the JsonReaderException class * Rename LineBytePosition to BytePositionInLine * Fix typo in ArrayEndWithinObject exception message * Capitalize the acronym JSON within exception messages. * Override GetObjectData and add a private deserialization ctor on JsonReaderException * Be explicit with lessThan in the IndexOfAny call site * Only use ulong when Options isn't AllowComments to align stack usage * Rename IndexOfAny helper to IndexOfOrLessThan * Remove stray comment and fix xml comment for BytesConsumed * Braces everywhere * Rename localCopy to localBuffer to tie it to _buffer * Add IsDigit and IsHexDigit helper methods * Rename Solidus to Slash, ReverseSolidus to BackSlash, and Use Asterisk constant * Use static encoder and avoid unnecessary allocation. * Combine multiple TryParse calls that only differ by standardFormat * Use SR.Format instead of string.Format and add debug.fail for default case * Make the exception as serializable and add test to BinaryFormatterTestData * Fix exception messages, avoid printing control characters, and remove unnecessary else. * Make sure all exception messages are consistent and that they make sense in context. * Fix and add new comments, rename internal enum values, and remove JsonLab * Throw InvalidCastException from TryGetAsX if token type doesn't match * Resolve TODOs, add issues, fix search for end of string, and remove recursive calls + add more tests. * Use Unicode escape sequences for non-ASCII characters in tests. * Add more tests related to skipping and consuming comments. * Allocate the stack lazily, only when depth actually surpasses 64. * Add a netcoreapp config to System.Runtime.Serialization.Formatters.Tests.csproj * Remove unnecessary DefineConstants property in test csproj * Use Invariant Culture where necessary to avoid tests failing on es-ES machines * Fix up nits in the exception messages * Add an IsTokenTypePrimitive helper method and fix comment spelling * Use helper, use while(true) instead of goto, and change TryGet methods to throw InvalidOperationException * Shorten CommentHandling enum values and rename Default to Disallow * Fix "is the stack empty" check in EndObject/EndArray * Add previous token type to state and update it when token type is not a comment * Include comment delimiters as part of comment token, fix Debug.Assert and tests. * Add helper to avoid code duplication * Add temporary workaround for Utf8 double parsing issue Commit migrated from dotnet/corefx@4375952
13 participants
Builds on top of Initial check-in for the System.Text.Json library and solution #33203, so ignore the already reviewed changes (project skeleton/roadmap/etc.).Moved on top of this change.TODO: (in future PRs, will file issues for some and replace the TODO comments in code)
Implement other TryGetValueAsX APIsOptimize ConsumeStringWithNestedQuotes and avoid redundant workOptimize string validationCache the result of TryGetNumber to help avoid unnecessary workAvoid unconstrained recursive callsOut-dated code coverage numbers
cc @joshfree, @KrzysztofCwalina, @steveharter, @GrabYourPitchforks, @davidfowl, @stephentoub