Fix Int128/UInt128 serialization and JValue comparison logic - #3106
Fix Int128/UInt128 serialization and JValue comparison logic#3106KitKeen wants to merge 6 commits into
Conversation
| public override Task WriteValueAsync(Int128 value, CancellationToken cancellationToken = default) | ||
| { | ||
| return _safeAsync ? DoWriteValueAsync(value, cancellationToken) : base.WriteValueAsync(value, cancellationToken); | ||
| } | ||
|
|
||
| internal Task DoWriteValueAsync(Int128 value, CancellationToken cancellationToken) | ||
| { | ||
| return WriteValueInternalAsync(JsonToken.Integer, value.ToString(CultureInfo.InvariantCulture), cancellationToken); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Asynchronously writes a <see cref="UInt128"/> value. | ||
| /// </summary> | ||
| /// <param name="value">The <see cref="UInt128"/> value to write.</param> | ||
| /// <param name="cancellationToken">The token to monitor for cancellation requests. The default value is <see cref="CancellationToken.None"/>.</param> | ||
| /// <returns>A <see cref="Task"/> that represents the asynchronous operation.</returns> | ||
| public override Task WriteValueAsync(UInt128 value, CancellationToken cancellationToken = default) | ||
| { | ||
| return _safeAsync ? DoWriteValueAsync(value, cancellationToken) : base.WriteValueAsync(value, cancellationToken); | ||
| } | ||
|
|
||
| internal Task DoWriteValueAsync(UInt128 value, CancellationToken cancellationToken) | ||
| { | ||
| return WriteValueInternalAsync(JsonToken.Integer, value.ToString(CultureInfo.InvariantCulture), cancellationToken); | ||
| } |
There was a problem hiding this comment.
Is it possible to support this without adding new public methods? Is calling WriteValue(object) enough? Performance will suffer because the value is boxed, but it means existing implementations don't need to worry about implementing the new overloads.
There was a problem hiding this comment.
will fix it without adding new public methods, I'm working on it, I'll let you know when I've finished, thank you.
There was a problem hiding this comment.
Yes, I've refactored it exactly as you suggested. The public WriteValue(Int128) / WriteValue(UInt128) overloads are removed. Instead, Int128 and UInt128 are handled in the existing WriteValue(object) path via pattern matching in JsonTextWriter. Existing JsonWriter implementations don't need any changes. The boxing overhead is acceptable since these types are rare in practice.
There was a problem hiding this comment.
Fixed the CS0234 build error: moved usings inside the #if HAVE_INT128 guard for net20/net35 compatibility. James, could you possiblly rerun the workflow?
…tead of public API
53c465c to
39b10fb
Compare
…, and cross-type comparison coverage; enable HAVE_INT128 in test csproj
3 participants
This PR improves the handling of Int128 and UInt128 types to ensure they are treated as native numeric values rather than strings during serialization. It also implements correct equality and comparison logic within JValue.
Key changes:
Numeric Serialization: Fixed an issue where Int128/UInt128 were serialized as strings. They are now correctly serialized as raw JSON numbers.
Deserialization: Ensured that large 128-bit integers can be correctly deserialized from JSON numbers without precision loss or format errors.
JValue Logic: Updated JValue.Equals and JValue.CompareTo to support 128-bit integer types, enabling proper sorting and comparison in LINQ to JSON.