Index: include/llvm/Support/JSON.h =================================================================== --- include/llvm/Support/JSON.h +++ include/llvm/Support/JSON.h @@ -134,7 +134,7 @@ /// Each Value is one of the JSON kinds: /// null (nullptr_t) /// boolean (bool) -/// number (double) +/// number (double or int64) /// string (StringRef) /// array (json::Array) /// object (json::Object) @@ -184,6 +184,8 @@ enum Kind { Null, Boolean, + /// Number values can store both int64s and doubles at full precision, + /// depending on what they were constructed/parsed from. Number, String, Array, @@ -211,25 +213,38 @@ Value(llvm::StringRef V) : Type(T_StringRef) { create(V); } Value(const char *V) : Type(T_StringRef) { create(V); } Value(std::nullptr_t) : Type(T_Null) {} - // Prevent implicit conversions to boolean. - template ::value>::type> + // Boolean (disallow implicit conversions). + // (The last template parameter is a dummy to keep templates distinct.) + template < + typename T, + typename = typename std::enable_if::value>::type, + bool = false> Value(T B) : Type(T_Boolean) { create(B); } - // Numbers: arithmetic types that are not boolean. + // Integers (except boolean). Must be non-narrowing convertible to int64_t. template < typename T, - typename = typename std::enable_if::value>::type, + typename = typename std::enable_if::value>::type, typename = typename std::enable_if::value>::value>::type> - Value(T D) : Type(T_Number) { - create(D); + bool, !std::is_same::value>::value>::type, + int64_t = 0> + Value(T I) : Type(T_Integer) { + create(int64_t{I}); + } + // Floating point. Must be non-narrowing convertible to double. + template ::value>::type, + double* = nullptr> + Value(T D) : Type(T_Double) { + create(double{D}); } // Serializable types: with a toJSON(const T&)->Value function, found by ADL. template ::value>> + Value, decltype(toJSON(*(const T *)nullptr))>::value>, + Value* = nullptr> Value(const T &V) : Value(toJSON(V)) {} Value &operator=(const Value &M) { @@ -250,7 +265,8 @@ return Null; case T_Boolean: return Boolean; - case T_Number: + case T_Double: + case T_Integer: return Number; case T_String: case T_StringRef: @@ -275,12 +291,17 @@ return llvm::None; } llvm::Optional asNumber() const { - if (LLVM_LIKELY(Type == T_Number)) + if (LLVM_LIKELY(Type == T_Double)) return as(); + if (LLVM_LIKELY(Type == T_Integer)) + return as(); return llvm::None; } + // Succeeds if the Value is a Number, and exactly representable as int64_t. llvm::Optional asInteger() const { - if (LLVM_LIKELY(Type == T_Number)) { + if (LLVM_LIKELY(Type == T_Integer)) + return as(); + if (LLVM_LIKELY(Type == T_Double)) { double D = as(); if (LLVM_LIKELY(std::modf(D, &D) == 0 && D >= std::numeric_limits::min() && @@ -338,9 +359,8 @@ enum ValueType : char { T_Null, T_Boolean, - // FIXME: splitting Number into Double and Integer would allow us to - // round-trip 64-bit integers. - T_Number, + T_Double, + T_Integer, T_StringRef, T_String, T_Object, @@ -348,7 +368,7 @@ }; // All members mutable, see moveFrom(). mutable ValueType Type; - mutable llvm::AlignedCharArrayUnion Union; }; @@ -428,6 +448,13 @@ } return false; } +inline bool fromJSON(const Value &E, int64_t &Out) { + if (auto S = E.asInteger()) { + Out = *S; + return true; + } + return false; +} inline bool fromJSON(const Value &E, double &Out) { if (auto S = E.asNumber()) { Out = *S; Index: lib/Support/JSON.cpp =================================================================== --- lib/Support/JSON.cpp +++ lib/Support/JSON.cpp @@ -113,7 +113,8 @@ switch (Type) { case T_Null: case T_Boolean: - case T_Number: + case T_Double: + case T_Integer: memcpy(Union.buffer, M.Union.buffer, sizeof(Union.buffer)); break; case T_StringRef: @@ -136,7 +137,8 @@ switch (Type) { case T_Null: case T_Boolean: - case T_Number: + case T_Double: + case T_Integer: memcpy(Union.buffer, M.Union.buffer, sizeof(Union.buffer)); break; case T_StringRef: @@ -161,7 +163,8 @@ switch (Type) { case T_Null: case T_Boolean: - case T_Number: + case T_Double: + case T_Integer: break; case T_StringRef: as().~StringRef(); @@ -226,7 +229,7 @@ } // On invalid syntax, parseX() functions return false and set Err. - bool parseNumber(char First, double &Out); + bool parseNumber(char First, Value &Out); bool parseString(std::string &Out); bool parseUnicode(std::string &Out); bool parseError(const char *Msg); // always returns false @@ -326,25 +329,28 @@ } } default: - if (isNumber(C)) { - double Num; - if (parseNumber(C, Num)) { - Out = Num; - return true; - } else { - return false; - } - } + if (isNumber(C)) + return parseNumber(C, Out); return parseError("Invalid JSON value"); } } - bool Parser::parseNumber(char First, double &Out) { + bool Parser::parseNumber(char First, Value &Out) { + // Read the number into a string. (Must be null-terminated for strto*). SmallString<24> S; S.push_back(First); while (isNumber(peek())) S.push_back(next()); char *End; + // Try first to parse as integer, and if so preserve full 64 bits. + // strtoll returns long long >= 64 bits, so check it's in range too. + auto I = strtoll(S.c_str(), &End, 10); + if (End == S.end() && I >= std::numeric_limits::min() && + I <= std::numeric_limits::max()) { + Out = int64_t(I); + return true; + } + // If it's not an integer Out = std::strtod(S.c_str(), &End); return End == S.end() || parseError("Invalid JSON value (number?)"); } @@ -556,8 +562,12 @@ case T_Boolean: OS << (as() ? "true" : "false"); break; - case T_Number: - OS << format("%g", as()); + case T_Double: + OS << format("%.*g", std::numeric_limits::max_digits10, + as()); + break; + case T_Integer: + OS << as(); break; case T_StringRef: quote(OS, as()); Index: unittests/Support/JSONTest.cpp =================================================================== --- unittests/Support/JSONTest.cpp +++ unittests/Support/JSONTest.cpp @@ -226,6 +226,58 @@ } } +// Verify special integer handling - we try to preserve exact int64 values. +TEST(JSONTest, Integers) { + struct { + const char* Desc; + Value Val; + const char* Str; + llvm::Optional AsInt; + llvm::Optional AsNumber; + } TestCases[] = { + { + "Non-integer. Stored as double, not convertible.", + double{1.5}, + "1.5", + llvm::None, + 1.5, + }, + + { + "Integer, not exact double. Stored as int64, convertible.", + int64_t{0x4000000000000001}, + "4611686018427387905", + int64_t{0x4000000000000001}, + double{0x4000000000000000}, + }, + + { + "Dynamically exact integer. Stored as double, convertible.", + double{0x6000000000000000}, + "6.9175290276410819e+18", + int64_t{0x6000000000000000}, + double{0x6000000000000000}, + }, + + { + "Dynamically integer, >64 bits. Stored as double, not convertible.", + 1.5 * double{0x8000000000000000}, + "1.3835058055282164e+19", + llvm::None, + 1.5 * double{0x8000000000000000}, + }, + }; + for (const auto& T : TestCases) { + EXPECT_EQ(T.Str, s(T.Val)) << T.Desc; + llvm::Expected Doc = parse(T.Str); + EXPECT_TRUE(!!Doc) << T.Desc; + EXPECT_EQ(Doc->asInteger(), T.AsInt) << T.Desc; + EXPECT_EQ(Doc->asNumber(), T.AsNumber) << T.Desc; + EXPECT_EQ(T.Val, *Doc) << T.Desc; + EXPECT_EQ(T.Str, s(*Doc)) << T.Desc; + } +} + // Sample struct with typical JSON-mapping rules. struct CustomStruct { CustomStruct() : B(false) {}