From 7dacf7774523a454fce7216c13eb03c6badf8d7b Mon Sep 17 00:00:00 2001 From: Jonathan Marler Date: Sat, 5 Aug 2023 21:56:00 -0600 Subject: [PATCH 1/3] std.json: fix roundtrip stringify for large integers std.json follows interoperability recommendations from RFC8259 to limit JSON number values to those that fit inside an f64. However, since Zig supports arbitrarily large JSON numbers, this breaks roundtrip data congruence. To appease both use cases, I've added an option `emit_big_numbers_quoted` to StringifyOptions. It's disabled by default which preserves roundtrip but can be enabled to favor interoperability. --- lib/std/json/stringify.zig | 27 +++++++++++++-------------- lib/std/json/stringify_test.zig | 6 ++++++ 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/std/json/stringify.zig b/lib/std/json/stringify.zig index 3e00531eaa..aa719e620b 100644 --- a/lib/std/json/stringify.zig +++ b/lib/std/json/stringify.zig @@ -33,6 +33,9 @@ pub const StringifyOptions = struct { /// Should unicode characters be escaped in strings? escape_unicode: bool = false, + + /// When true, renders numbers outside the range `±1<<53` (the precise integer range of f64) as JSON strings in base 10. + emit_big_numbers_quoted: bool = false, }; /// Writes the given value to the `std.io.Writer` stream. @@ -161,7 +164,7 @@ pub fn writeStreamArbitraryDepth( /// * Zig `bool` -> JSON `true` or `false`. /// * Zig `?T` -> `null` or the rendering of `T`. /// * Zig `i32`, `u64`, etc. -> JSON number or string. -/// * If the value is outside the range `±1<<53` (the precise integer rage of f64), it is rendered as a JSON string in base 10. Otherwise, it is rendered as JSON number. +/// * If the value is outside the range `±1<<53` (the precise integer range of f64), it is rendered as a JSON string in base 10. Otherwise, it is rendered as JSON number. /// * Zig floats -> JSON number or string. /// * If the value cannot be precisely represented by an f64, it is rendered as a JSON string. Otherwise, it is rendered as JSON number. /// * TODO: Float rendering will likely change in the future, e.g. to remove the unnecessary "e+00". @@ -400,20 +403,16 @@ pub fn WriteStream( const T = @TypeOf(value); switch (@typeInfo(T)) { .Int => |info| { - if (info.bits < 53) { - try self.valueStart(); - try self.stream.print("{}", .{value}); - self.valueDone(); - return; - } - if (value < 4503599627370496 and (info.signedness == .unsigned or value > -4503599627370496)) { - try self.valueStart(); - try self.stream.print("{}", .{value}); - self.valueDone(); - return; - } + const emit_unquoted = + if (!self.options.emit_big_numbers_quoted) true + else if (info.bits < 53) true + else (value < 4503599627370496 and (info.signedness == .unsigned or value > -4503599627370496)); try self.valueStart(); - try self.stream.print("\"{}\"", .{value}); + if (emit_unquoted) { + try self.stream.print("{}", .{value}); + } else { + try self.stream.print("\"{}\"", .{value}); + } self.valueDone(); return; }, diff --git a/lib/std/json/stringify_test.zig b/lib/std/json/stringify_test.zig index 4eec97e667..c2c6b6f5e7 100644 --- a/lib/std/json/stringify_test.zig +++ b/lib/std/json/stringify_test.zig @@ -126,6 +126,7 @@ test "stringify basic types" { try testStringify("4.2e+01", 42.0, .{}); try testStringify("42", @as(u8, 42), .{}); try testStringify("42", @as(u128, 42), .{}); + try testStringify("9999999999999999", 9999999999999999, .{}); try testStringify("4.2e+01", @as(f32, 42), .{}); try testStringify("4.2e+01", @as(f64, 42), .{}); try testStringify("\"ItBroke\"", @as(anyerror, error.ItBroke), .{}); @@ -432,3 +433,8 @@ test "print" { ; try std.testing.expectEqualStrings(expected, result); } + +test "big integers" { + try testStringify("9999999999999999", 9999999999999999, .{}); + try testStringify("\"9999999999999999\"", 9999999999999999, .{ .emit_big_numbers_quoted = true }); +} From 1cce539ddcba1e21615e11dcf75f011074942414 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Sun, 6 Aug 2023 00:17:37 -0400 Subject: [PATCH 2/3] json.stringify: properly implement RFC8259 recommendation The previous magic numbers used `1 << 52`, which did not account for the implicit leading one in the floating point format. The RFC is correct when it uses an exponent of 53. Technically these exclusive endpoints are also representable, but everyone including the RFC seems to use them exclusively. Also, delete special case optimizations related to the type which have already been implemented in the zig compiler to produce comptime values for tautological runtime comparisons. --- lib/std/json/stringify.zig | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/std/json/stringify.zig b/lib/std/json/stringify.zig index aa719e620b..81a09d26ba 100644 --- a/lib/std/json/stringify.zig +++ b/lib/std/json/stringify.zig @@ -34,7 +34,7 @@ pub const StringifyOptions = struct { /// Should unicode characters be escaped in strings? escape_unicode: bool = false, - /// When true, renders numbers outside the range `±1<<53` (the precise integer range of f64) as JSON strings in base 10. + /// When true, renders numbers outside the range `+-1<<53` (the precise integer range of f64) as JSON strings in base 10. emit_big_numbers_quoted: bool = false, }; @@ -164,7 +164,7 @@ pub fn writeStreamArbitraryDepth( /// * Zig `bool` -> JSON `true` or `false`. /// * Zig `?T` -> `null` or the rendering of `T`. /// * Zig `i32`, `u64`, etc. -> JSON number or string. -/// * If the value is outside the range `±1<<53` (the precise integer range of f64), it is rendered as a JSON string in base 10. Otherwise, it is rendered as JSON number. +/// * If the value is outside the range `+-1<<53` (the precise integer range of f64), it is rendered as a JSON string in base 10. Otherwise, it is rendered as JSON number. /// * Zig floats -> JSON number or string. /// * If the value cannot be precisely represented by an f64, it is rendered as a JSON string. Otherwise, it is rendered as JSON number. /// * TODO: Float rendering will likely change in the future, e.g. to remove the unnecessary "e+00". @@ -402,13 +402,11 @@ pub fn WriteStream( pub fn write(self: *Self, value: anytype) Error!void { const T = @TypeOf(value); switch (@typeInfo(T)) { - .Int => |info| { - const emit_unquoted = - if (!self.options.emit_big_numbers_quoted) true - else if (info.bits < 53) true - else (value < 4503599627370496 and (info.signedness == .unsigned or value > -4503599627370496)); + .Int => { try self.valueStart(); - if (emit_unquoted) { + if (!self.options.emit_big_numbers_quoted or + (value > -(1 << 53) and value < (1 << 53))) + { try self.stream.print("{}", .{value}); } else { try self.stream.print("\"{}\"", .{value}); From 2046880de868c50766d3cbfb6dd78b45c0aa39aa Mon Sep 17 00:00:00 2001 From: Jonathan Marler Date: Sun, 6 Aug 2023 09:25:21 -0600 Subject: [PATCH 3/3] std.json: josh review fixes * renamed enum_big_numbers_quoted option to enum_nonportable_numbers_as_strings * updated stringify doc to mention the option I also reversed the logic to determine whether an integer is nonportable, it seemed easier to reason about. I also took a stab at applying the new option to floats, but, I got stuck at trying to print large floats, not sure if Zig supports that yet. --- lib/std/json/stringify.zig | 12 ++++++------ lib/std/json/stringify_test.zig | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/std/json/stringify.zig b/lib/std/json/stringify.zig index 81a09d26ba..1654da421c 100644 --- a/lib/std/json/stringify.zig +++ b/lib/std/json/stringify.zig @@ -35,7 +35,7 @@ pub const StringifyOptions = struct { escape_unicode: bool = false, /// When true, renders numbers outside the range `+-1<<53` (the precise integer range of f64) as JSON strings in base 10. - emit_big_numbers_quoted: bool = false, + emit_nonportable_numbers_as_strings: bool = false, }; /// Writes the given value to the `std.io.Writer` stream. @@ -164,7 +164,7 @@ pub fn writeStreamArbitraryDepth( /// * Zig `bool` -> JSON `true` or `false`. /// * Zig `?T` -> `null` or the rendering of `T`. /// * Zig `i32`, `u64`, etc. -> JSON number or string. -/// * If the value is outside the range `+-1<<53` (the precise integer range of f64), it is rendered as a JSON string in base 10. Otherwise, it is rendered as JSON number. +/// * When option `emit_nonportable_numbers_as_strings` is true, if the value is outside the range `+-1<<53` (the precise integer range of f64), it is rendered as a JSON string in base 10. Otherwise, it is rendered as JSON number. /// * Zig floats -> JSON number or string. /// * If the value cannot be precisely represented by an f64, it is rendered as a JSON string. Otherwise, it is rendered as JSON number. /// * TODO: Float rendering will likely change in the future, e.g. to remove the unnecessary "e+00". @@ -404,12 +404,12 @@ pub fn WriteStream( switch (@typeInfo(T)) { .Int => { try self.valueStart(); - if (!self.options.emit_big_numbers_quoted or - (value > -(1 << 53) and value < (1 << 53))) + if (self.options.emit_nonportable_numbers_as_strings and + (value <= -(1 << 53) or value >= (1 << 53))) { - try self.stream.print("{}", .{value}); - } else { try self.stream.print("\"{}\"", .{value}); + } else { + try self.stream.print("{}", .{value}); } self.valueDone(); return; diff --git a/lib/std/json/stringify_test.zig b/lib/std/json/stringify_test.zig index c2c6b6f5e7..cc825ff64a 100644 --- a/lib/std/json/stringify_test.zig +++ b/lib/std/json/stringify_test.zig @@ -434,7 +434,7 @@ test "print" { try std.testing.expectEqualStrings(expected, result); } -test "big integers" { +test "nonportable numbers" { try testStringify("9999999999999999", 9999999999999999, .{}); - try testStringify("\"9999999999999999\"", 9999999999999999, .{ .emit_big_numbers_quoted = true }); + try testStringify("\"9999999999999999\"", 9999999999999999, .{ .emit_nonportable_numbers_as_strings = true }); }