[llvm] r336541 - [Support] Make JSON handle doubles and int64s losslessly
Sam McCall via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 9 05:16:41 PDT 2018
Author: sammccall
Date: Mon Jul 9 05:16:40 2018
New Revision: 336541
URL: http://llvm.org/viewvc/llvm-project?rev=336541&view=rev
Log:
[Support] Make JSON handle doubles and int64s losslessly
Summary:
This patch adds a new "integer" ValueType, and renames Number -> Double.
This allows us to preserve the full precision of int64_t when parsing integers
from the wire, or constructing from an integer.
The API is unchanged, other than giving asInteger() a clearer contract.
In addition, always output doubles with enough precision that parsing will
reconstruct the same double.
Reviewers: simon_tatham
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D46209
Modified:
llvm/trunk/include/llvm/Support/JSON.h
llvm/trunk/lib/Support/JSON.cpp
llvm/trunk/unittests/Support/JSONTest.cpp
Modified: llvm/trunk/include/llvm/Support/JSON.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/JSON.h?rev=336541&r1=336540&r2=336541&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/JSON.h (original)
+++ llvm/trunk/include/llvm/Support/JSON.h Mon Jul 9 05:16:40 2018
@@ -204,7 +204,7 @@ inline bool operator!=(const Array &L, c
/// 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)
@@ -226,7 +226,7 @@ inline bool operator!=(const Array &L, c
/// fromJSON(const json::Value&, T&)->bool
/// Deserializers are provided for:
/// - bool
-/// - int
+/// - int and int64_t
/// - double
/// - std::string
/// - vector<T>, where T is deserializable
@@ -254,6 +254,8 @@ public:
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,
@@ -281,24 +283,36 @@ public:
Value(llvm::StringRef V) : Type(T_StringRef) { create<llvm::StringRef>(V); }
Value(const char *V) : Type(T_StringRef) { create<llvm::StringRef>(V); }
Value(std::nullptr_t) : Type(T_Null) {}
- // Prevent implicit conversions to boolean.
- template <typename T, typename = typename std::enable_if<
- std::is_same<T, bool>::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<std::is_same<T, bool>::value>::type,
+ bool = false>
Value(T B) : Type(T_Boolean) {
create<bool>(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<std::is_arithmetic<T>::value>::type,
+ typename = typename std::enable_if<std::is_integral<T>::value>::type,
typename = typename std::enable_if<!std::is_same<T, bool>::value>::type>
- Value(T D) : Type(T_Number) {
- create<double>(D);
+ Value(T I) : Type(T_Integer) {
+ create<int64_t>(int64_t{I});
+ }
+ // Floating point. Must be non-narrowing convertible to double.
+ template <typename T,
+ typename =
+ typename std::enable_if<std::is_floating_point<T>::value>::type,
+ double * = nullptr>
+ Value(T D) : Type(T_Double) {
+ create<double>(double{D});
}
// Serializable types: with a toJSON(const T&)->Value function, found by ADL.
template <typename T,
typename = typename std::enable_if<std::is_same<
- Value, decltype(toJSON(*(const T *)nullptr))>::value>>
+ Value, decltype(toJSON(*(const T *)nullptr))>::value>,
+ Value * = nullptr>
Value(const T &V) : Value(toJSON(V)) {}
Value &operator=(const Value &M) {
@@ -319,7 +333,8 @@ public:
return Null;
case T_Boolean:
return Boolean;
- case T_Number:
+ case T_Double:
+ case T_Integer:
return Number;
case T_String:
case T_StringRef:
@@ -344,12 +359,17 @@ public:
return llvm::None;
}
llvm::Optional<double> getAsNumber() const {
- if (LLVM_LIKELY(Type == T_Number))
+ if (LLVM_LIKELY(Type == T_Double))
return as<double>();
+ if (LLVM_LIKELY(Type == T_Integer))
+ return as<int64_t>();
return llvm::None;
}
+ // Succeeds if the Value is a Number, and exactly representable as int64_t.
llvm::Optional<int64_t> getAsInteger() const {
- if (LLVM_LIKELY(Type == T_Number)) {
+ if (LLVM_LIKELY(Type == T_Integer))
+ return as<int64_t>();
+ if (LLVM_LIKELY(Type == T_Double)) {
double D = as<double>();
if (LLVM_LIKELY(std::modf(D, &D) == 0.0 &&
D >= double(std::numeric_limits<int64_t>::min()) &&
@@ -407,9 +427,8 @@ private:
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,
@@ -417,7 +436,7 @@ private:
};
// All members mutable, see moveFrom().
mutable ValueType Type;
- mutable llvm::AlignedCharArrayUnion<bool, double, llvm::StringRef,
+ mutable llvm::AlignedCharArrayUnion<bool, double, int64_t, llvm::StringRef,
std::string, json::Array, json::Object>
Union;
};
@@ -502,6 +521,13 @@ inline bool fromJSON(const Value &E, int
if (auto S = E.getAsInteger()) {
Out = *S;
return true;
+ }
+ return false;
+}
+inline bool fromJSON(const Value &E, int64_t &Out) {
+ if (auto S = E.getAsInteger()) {
+ Out = *S;
+ return true;
}
return false;
}
Modified: llvm/trunk/lib/Support/JSON.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/JSON.cpp?rev=336541&r1=336540&r2=336541&view=diff
==============================================================================
--- llvm/trunk/lib/Support/JSON.cpp (original)
+++ llvm/trunk/lib/Support/JSON.cpp Mon Jul 9 05:16:40 2018
@@ -104,7 +104,8 @@ void Value::copyFrom(const Value &M) {
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:
@@ -127,7 +128,8 @@ void Value::moveFrom(const Value &&M) {
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:
@@ -152,7 +154,8 @@ void Value::destroy() {
switch (Type) {
case T_Null:
case T_Boolean:
- case T_Number:
+ case T_Double:
+ case T_Integer:
break;
case T_StringRef:
as<StringRef>().~StringRef();
@@ -217,7 +220,7 @@ private:
}
// 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
@@ -317,25 +320,28 @@ bool Parser::parseValue(Value &Out) {
}
}
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 = std::strtoll(S.c_str(), &End, 10);
+ if (End == S.end() && I >= std::numeric_limits<int64_t>::min() &&
+ I <= std::numeric_limits<int64_t>::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?)");
}
@@ -558,8 +564,12 @@ void llvm::json::Value::print(raw_ostrea
case T_Boolean:
OS << (as<bool>() ? "true" : "false");
break;
- case T_Number:
- OS << format("%g", as<double>());
+ case T_Double:
+ OS << format("%.*g", std::numeric_limits<double>::max_digits10,
+ as<double>());
+ break;
+ case T_Integer:
+ OS << as<int64_t>();
break;
case T_StringRef:
quote(OS, as<StringRef>());
Modified: llvm/trunk/unittests/Support/JSONTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/JSONTest.cpp?rev=336541&r1=336540&r2=336541&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/JSONTest.cpp (original)
+++ llvm/trunk/unittests/Support/JSONTest.cpp Mon Jul 9 05:16:40 2018
@@ -227,6 +227,66 @@ TEST(JSONTest, Inspection) {
}
}
+// 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<int64_t> AsInt;
+ llvm::Optional<double> 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},
+ },
+
+ {
+ "Negative 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<Value> Doc = parse(T.Str);
+ EXPECT_TRUE(!!Doc) << T.Desc;
+ EXPECT_EQ(Doc->getAsInteger(), T.AsInt) << T.Desc;
+ EXPECT_EQ(Doc->getAsNumber(), 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) {}
More information about the llvm-commits
mailing list