From cc10db1d045addcdfbcc39c4539a9c42feb3522f Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Thu, 26 Sep 2024 11:53:31 +0200 Subject: [PATCH 1/6] Append `.0` when dumping doubles. --- src/Dumper.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Dumper.cpp b/src/Dumper.cpp index aee9aa0f..40fe3302 100644 --- a/src/Dumper.cpp +++ b/src/Dumper.cpp @@ -54,8 +54,7 @@ void Dumper::dump(Slice slice) { dumpValue(slice); } -/*static*/ void Dumper::dump(Slice slice, Sink* sink, - Options const* options) { +/*static*/ void Dumper::dump(Slice slice, Sink* sink, Options const* options) { Dumper dumper(sink, options); dumper.dump(slice); } @@ -65,8 +64,7 @@ void Dumper::dump(Slice slice) { dump(*slice, sink, options); } -/*static*/ std::string Dumper::toString(Slice slice, - Options const* options) { +/*static*/ std::string Dumper::toString(Slice slice, Options const* options) { std::string buffer; StringSink sink(&buffer); dump(slice, &sink, options); @@ -219,6 +217,7 @@ void Dumper::appendDouble(double v) { char temp[24]; int len = fpconv_dtoa(v, &temp[0]); _sink->append(&temp[0], static_cast(len)); + _sink->append(".0", 2); } void Dumper::dumpUnicodeCharacter(uint16_t value) { @@ -545,8 +544,7 @@ void Dumper::dumpValue(Slice slice, Slice const* base) { base = &slice; } - Slice external( - reinterpret_cast(slice.getExternal())); + Slice external(reinterpret_cast(slice.getExternal())); dumpValue(external, base); break; } @@ -624,8 +622,8 @@ void Dumper::handleUnsupportedType(Slice slice) { return; } else if (options->unsupportedTypeBehavior == Options::ConvertUnsupportedType) { - _sink->append(std::string("\"(non-representable type ") + - slice.typeName() + ")\""); + _sink->append(std::string("\"(non-representable type ") + slice.typeName() + + ")\""); return; } From 8e33c8d4d69d16cb4bed2dad6a870b24669a2957 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Thu, 26 Sep 2024 12:40:28 +0200 Subject: [PATCH 2/6] Fix test. --- src/Dumper.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Dumper.cpp b/src/Dumper.cpp index 40fe3302..15a0d6b6 100644 --- a/src/Dumper.cpp +++ b/src/Dumper.cpp @@ -217,6 +217,11 @@ void Dumper::appendDouble(double v) { char temp[24]; int len = fpconv_dtoa(v, &temp[0]); _sink->append(&temp[0], static_cast(len)); + for (size_t i = 0; i < len; ++i) { + if (temp[i] == '.') { + return; + } + } _sink->append(".0", 2); } From 28604f87446ab3140924aac68af2d30b9e58f05a Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Thu, 26 Sep 2024 15:06:05 +0200 Subject: [PATCH 3/6] Restrict to large numbers. --- src/Dumper.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Dumper.cpp b/src/Dumper.cpp index 15a0d6b6..935e6271 100644 --- a/src/Dumper.cpp +++ b/src/Dumper.cpp @@ -217,6 +217,9 @@ void Dumper::appendDouble(double v) { char temp[24]; int len = fpconv_dtoa(v, &temp[0]); _sink->append(&temp[0], static_cast(len)); + if (fabs(v) < ldexpl(1.0, 53)) { + return; + } for (size_t i = 0; i < len; ++i) { if (temp[i] == '.') { return; From d08a3ad4ffe565bafb057a35f60cb086bf3c9e2c Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Thu, 26 Sep 2024 15:13:12 +0200 Subject: [PATCH 4/6] Fix. --- src/Dumper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dumper.cpp b/src/Dumper.cpp index 935e6271..97124d18 100644 --- a/src/Dumper.cpp +++ b/src/Dumper.cpp @@ -221,7 +221,7 @@ void Dumper::appendDouble(double v) { return; } for (size_t i = 0; i < len; ++i) { - if (temp[i] == '.') { + if (temp[i] == '.' || temp[i] == 'e' || temp[i] == 'E') { return; } } From c9e3f69864ecaddc8b1dadf091f6bbb70ccd0a99 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Fri, 27 Sep 2024 11:27:35 +0200 Subject: [PATCH 5/6] Faithfully represent large doubles. --- src/Dumper.cpp | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/Dumper.cpp b/src/Dumper.cpp index 97124d18..1bdc7427 100644 --- a/src/Dumper.cpp +++ b/src/Dumper.cpp @@ -215,17 +215,34 @@ void Dumper::appendUInt(uint64_t v) { void Dumper::appendDouble(double v) { char temp[24]; - int len = fpconv_dtoa(v, &temp[0]); - _sink->append(&temp[0], static_cast(len)); - if (fabs(v) < ldexpl(1.0, 53)) { - return; - } - for (size_t i = 0; i < len; ++i) { - if (temp[i] == '.' || temp[i] == 'e' || temp[i] == 'E') { - return; + double a = fabs(v); + if (a >= ldexpl(1.0, 53) && a < ldexpl(1.0, 64)) { + // This is a special case which we want to handle separately, because + // of two reasons: + // (1) The function fpconv_dtoa below only guarantees to write a + // decimal representation which gives the same double value when + // parsed back into a double. It can write a wrong integer. + // Therefore we want to use the integer code in this case. + // (2) The function fpconv_dtoa will write a normal integer + // representation in this case without a decimal point. If we + // parse this back to vpack later, we end up in a different + // representation (uint64_t or int64_t), so we want to append + // ".0" to the string in this case. + // Note that this automatically excludes all infinities and NaNs, + // which will be handled in the function fpconv_dtoa below. + uint64_t u; + if (v < 0) { + u = static_cast(-v); + _sink->append("-", 1); + } else { + u = static_cast(v); } + appendUInt(u); + _sink->append(".0", 2); + return; } - _sink->append(".0", 2); + int len = fpconv_dtoa(v, &temp[0]); + _sink->append(&temp[0], static_cast(len)); } void Dumper::dumpUnicodeCharacter(uint16_t value) { From aa48d58ed2b33140d6738bd517eab059f59d683c Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Fri, 27 Sep 2024 12:38:09 +0200 Subject: [PATCH 6/6] Add tests. --- tests/testsDumper.cpp | 97 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/tests/testsDumper.cpp b/tests/testsDumper.cpp index 5850261a..409a43cb 100644 --- a/tests/testsDumper.cpp +++ b/tests/testsDumper.cpp @@ -490,8 +490,9 @@ TEST(StringDumperTest, SuppressControlChars) { TEST(StringDumperTest, EscapeControlChars) { Builder b; - b.add(Value( - "Before\nAfter\r\t\v\f\b\x01\x02/\u00B0\uf0f9\u9095\uf0f9\u90b6\v\n\\\"")); + b.add( + Value("Before\nAfter\r\t\v\f\b\x01\x02/" + "\u00B0\uf0f9\u9095\uf0f9\u90b6\v\n\\\"")); Options options; options.escapeControl = true; ASSERT_EQ(std::string("\"Before\\nAfter\\r\\t\\u000B\\f\\b\\u0001\\u0002/" @@ -1958,3 +1959,95 @@ int main(int argc, char* argv[]) { return RUN_ALL_TESTS(); } + +TEST(DumperLargeDoubleTest, TwoToThePowerOf60Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(ldexp(1.0, 60))); + dumper.dump(builder.slice()); + ASSERT_EQ("1152921504606846976.0", buffer); +} + +TEST(DumperLargeDoubleTest, TwoToThePowerOf60Plus1Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(ldexp(1.0, 60) + 1.0)); + dumper.dump(builder.slice()); + ASSERT_EQ("1152921504606846976.0", buffer); +} + +TEST(DumperLargeDoubleTest, MinusTwoToThePowerOf60Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(-ldexp(1.0, 60))); + dumper.dump(builder.slice()); + ASSERT_EQ("-1152921504606846976.0", buffer); +} + +TEST(DumperLargeDoubleTest, MinusTwoToThePowerOf60Plus1Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(-ldexp(1.0, 60) + 1.0)); + dumper.dump(builder.slice()); + ASSERT_EQ("-1152921504606846976.0", buffer); +} + +TEST(DumperLargeDoubleTest, TwoToThePowerOf52Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(ldexp(1.0, 52))); + dumper.dump(builder.slice()); + ASSERT_EQ("4503599627370496", buffer); +} + +TEST(DumperLargeDoubleTest, TwoToThePowerOf53Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(ldexp(1.0, 53))); + dumper.dump(builder.slice()); + ASSERT_EQ("9007199254740992.0", buffer); +} + +TEST(DumperLargeDoubleTest, TwoToThePowerOf63Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(ldexp(1.0, 63))); + dumper.dump(builder.slice()); + ASSERT_EQ("9223372036854775808.0", buffer); +} + +TEST(DumperLargeDoubleTest, TwoToThePowerOf64Double) { + Options options; + std::string buffer; + StringSink sink(&buffer); + Dumper dumper(&sink, &options); + Builder builder; + builder.add(Value(ldexp(1.0, 64))); + dumper.dump(builder.slice()); + // Note that this is, strictly speaking, not correct. But since this is + // at least 2^64, it will be parsed back to double anywa and then we + // get back the actual result. This is a sacrifice we make for performance. + ASSERT_EQ("18446744073709552000", buffer); +} +