Revert "Integer Precision for JSON <-> Protobuf conversions."
This reverts commit 4807db6ceb7fafe59c28cbcfb31334ca7061dd59. Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f8703752 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f8703752 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f8703752 Branch: refs/heads/master Commit: f8703752995a1a538cc9cd13a55666587c492d96 Parents: fa80dcb Author: Joris Van Remoortere <[email protected]> Authored: Wed Sep 16 18:42:50 2015 -0400 Committer: Joris Van Remoortere <[email protected]> Committed: Wed Sep 16 18:42:50 2015 -0400 ---------------------------------------------------------------------- .../3rdparty/stout/include/stout/json.hpp | 157 ++----------------- .../3rdparty/stout/include/stout/protobuf.hpp | 54 +++++-- .../3rdparty/stout/tests/json_tests.cpp | 74 +++------ .../3rdparty/stout/tests/protobuf_tests.cpp | 82 +--------- 4 files changed, 78 insertions(+), 289 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/f8703752/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp index 6e667a1..f28138c 100644 --- a/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp +++ b/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp @@ -21,7 +21,6 @@ #include <limits> #include <map> #include <string> -#include <type_traits> #include <vector> #include <boost/type_traits/is_arithmetic.hpp> @@ -73,71 +72,11 @@ struct String }; -// NOTE: Due to how PicoJson parses unsigned integers, a roundtrip from Number -// to JSON and back to Number will result in: -// - a signed integer, if the value is less than or equal to INT64_MAX; -// - or a double, if the value is greater than INT64_MAX. -// See: https://github.com/kazuho/picojson/blob/rel/v1.3.0/picojson.h#L777-L781 struct Number { Number() : value(0) {} - - template <typename T> - Number( - T _value, - typename std::enable_if<std::is_floating_point<T>::value, int>::type = 0) - : type(FLOATING), value(_value) {} - - template <typename T> - Number( - T _value, - typename std::enable_if< - std::is_integral<T>::value && std::is_signed<T>::value, - int>::type = 0) - : type(SIGNED_INTEGER), signed_integer(_value) {} - - template <typename T> - Number( - T _value, - typename std::enable_if< - std::is_integral<T>::value && std::is_unsigned<T>::value, - int>::type = 0) - : type(UNSIGNED_INTEGER), unsigned_integer(_value) {} - - template <typename T> - T as() const - { - switch (type) { - case FLOATING: - return static_cast<T>(value); - case SIGNED_INTEGER: - return static_cast<T>(signed_integer); - case UNSIGNED_INTEGER: - return static_cast<T>(unsigned_integer); - - // NOTE: By not setting a default we leverage the compiler - // errors when the enumeration is augmented to find all - // the cases we need to provide. - } - } - - enum Type - { - FLOATING, - SIGNED_INTEGER, - UNSIGNED_INTEGER, - } type; - -private: - friend struct Value; - friend struct Comparator; - friend std::ostream& operator<<(std::ostream& stream, const Number& number); - - union { - double value; - int64_t signed_integer; - uint64_t unsigned_integer; - }; + Number(double _value) : value(_value) {} + double value; }; @@ -268,8 +207,6 @@ struct Value : internal::Variant bool contains(const Value& other) const; private: - friend struct Comparator; - // A class which follows the visitor pattern and implements the // containment rules described in the documentation of 'contains'. // See 'bool Value::contains(const Value& other) const'. @@ -320,6 +257,7 @@ inline const Value& Value::as<Value>() const } + template <typename T> Result<T> Object::find(const std::string& path) const { @@ -446,53 +384,7 @@ inline bool Value::ContainmentComparator::operator()(const Number& other) const if (!self.is<Number>()) { return false; } - - // NOTE: For the following switch statements, we do not set a default - // case in order to leverage the compiler errors when the enumeration - // is augmented to find all the cases we need to provide. - - // NOTE: Using '==' is unsafe for unsigned-signed integer comparisons. - // The compiler will automatically cast the signed integer to an - // unsigned integer. i.e. If the signed integer was negative, it - // might be converted to a large positive number. - // Using the '==' operator *is* safe for double-integer comparisons. - - const Number& number = self.as<Number>(); - switch (number.type) { - case Number::FLOATING: - switch (other.type) { - case Number::FLOATING: - return number.value == other.value; - case Number::SIGNED_INTEGER: - return number.value == other.signed_integer; - case Number::UNSIGNED_INTEGER: - return number.value == other.unsigned_integer; - } - - case Number::SIGNED_INTEGER: - switch (other.type) { - case Number::FLOATING: - return number.signed_integer == other.value; - case Number::SIGNED_INTEGER: - return number.signed_integer == other.signed_integer; - case Number::UNSIGNED_INTEGER: - // See note above for why this is not a simple '==' expression. - return number.signed_integer >= 0 && - number.as<uint64_t>() == other.unsigned_integer; - } - - case Number::UNSIGNED_INTEGER: - switch (other.type) { - case Number::FLOATING: - return number.unsigned_integer == other.value; - case Number::SIGNED_INTEGER: - // See note above for why this is not a simple '==' expression. - return other.signed_integer >= 0 && - number.unsigned_integer == other.as<uint64_t>(); - case Number::UNSIGNED_INTEGER: - return number.unsigned_integer == other.unsigned_integer; - } - } + return self.as<Number>().value == other.value; } @@ -554,11 +446,12 @@ struct Comparator : boost::static_visitor<bool> return false; } - bool operator()(const Number& other) const + bool operator()(const Number& number) const { - // Delegate to the containment comparator. - // See Value::ContainmentComparator::operator(Number). - return Value::ContainmentComparator(value)(other); + if (value.is<Number>()) { + return value.as<Number>().value == number.value; + } + return false; } bool operator()(const Array& array) const @@ -609,32 +502,10 @@ inline std::ostream& operator<<(std::ostream& out, const String& string) inline std::ostream& operator<<(std::ostream& out, const Number& number) { - switch (number.type) { - case Number::FLOATING: { - // Prints a floating point value, with the specified precision, see: - // http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2005.pdf - // Additionally ensures that a decimal point is in the output. - char buffer[50] {}; // More than long enough for the specified precision. - snprintf( - buffer, - sizeof(buffer), - "%#.*g", - std::numeric_limits<double>::digits10, - number.value); - - // Get rid of trailing zeroes before outputting. - // Otherwise, printing 1.0 would result in "1.00000000000000". - return out << strings::trim(buffer, strings::SUFFIX, "0"); - } - case Number::SIGNED_INTEGER: - return out << number.signed_integer; - case Number::UNSIGNED_INTEGER: - return out << number.unsigned_integer; - - // NOTE: By not setting a default we leverage the compiler - // errors when the enumeration is augmented to find all - // the cases we need to provide. - } + // Use the guaranteed accurate precision, see: + // http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2005.pdf + return out << std::setprecision(std::numeric_limits<double>::digits10) + << number.value; } @@ -704,8 +575,6 @@ inline Value convert(const picojson::value& value) array.values.push_back(convert(value)); } return array; - } else if (value.is<int64_t>()) { - return Number(value.get<int64_t>()); } else if (value.is<double>()) { return Number(value.get<double>()); } else if (value.is<std::string>()) { http://git-wip-us.apache.org/repos/asf/mesos/blob/f8703752/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp index 2285ce9..115cc0c 100644 --- a/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp +++ b/3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp @@ -391,50 +391,80 @@ struct Parser : boost::static_visitor<Try<Nothing> > switch (field->type()) { case google::protobuf::FieldDescriptor::TYPE_DOUBLE: if (field->is_repeated()) { - reflection->AddDouble(message, field, number.as<double>()); + reflection->AddDouble(message, field, number.value); } else { - reflection->SetDouble(message, field, number.as<double>()); + reflection->SetDouble(message, field, number.value); } break; case google::protobuf::FieldDescriptor::TYPE_FLOAT: if (field->is_repeated()) { - reflection->AddFloat(message, field, number.as<float>()); + reflection->AddFloat( + message, + field, + static_cast<float>(number.value)); } else { - reflection->SetFloat(message, field, number.as<float>()); + reflection->SetFloat( + message, + field, + static_cast<float>(number.value)); } break; case google::protobuf::FieldDescriptor::TYPE_INT64: case google::protobuf::FieldDescriptor::TYPE_SINT64: case google::protobuf::FieldDescriptor::TYPE_SFIXED64: if (field->is_repeated()) { - reflection->AddInt64(message, field, number.as<int64_t>()); + reflection->AddInt64( + message, + field, + static_cast<int64_t>(number.value)); } else { - reflection->SetInt64(message, field, number.as<int64_t>()); + reflection->SetInt64( + message, + field, + static_cast<int64_t>(number.value)); } break; case google::protobuf::FieldDescriptor::TYPE_UINT64: case google::protobuf::FieldDescriptor::TYPE_FIXED64: if (field->is_repeated()) { - reflection->AddUInt64(message, field, number.as<uint64_t>()); + reflection->AddUInt64( + message, + field, + static_cast<uint64_t>(number.value)); } else { - reflection->SetUInt64(message, field, number.as<uint64_t>()); + reflection->SetUInt64( + message, + field, + static_cast<uint64_t>(number.value)); } break; case google::protobuf::FieldDescriptor::TYPE_INT32: case google::protobuf::FieldDescriptor::TYPE_SINT32: case google::protobuf::FieldDescriptor::TYPE_SFIXED32: if (field->is_repeated()) { - reflection->AddInt32(message, field, number.as<int32_t>()); + reflection->AddInt32( + message, + field, + static_cast<int32_t>(number.value)); } else { - reflection->SetInt32(message, field, number.as<int32_t>()); + reflection->SetInt32( + message, + field, + static_cast<int32_t>(number.value)); } break; case google::protobuf::FieldDescriptor::TYPE_UINT32: case google::protobuf::FieldDescriptor::TYPE_FIXED32: if (field->is_repeated()) { - reflection->AddUInt32(message, field, number.as<uint32_t>()); + reflection->AddUInt32( + message, + field, + static_cast<uint32_t>(number.value)); } else { - reflection->SetUInt32(message, field, number.as<uint32_t>()); + reflection->SetUInt32( + message, + field, + static_cast<uint32_t>(number.value)); } break; default: http://git-wip-us.apache.org/repos/asf/mesos/blob/f8703752/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp index 88b6147..850650c 100644 --- a/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp +++ b/3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp @@ -50,50 +50,18 @@ TEST(JsonTest, BinaryData) TEST(JsonTest, NumberFormat) { - // Test whole numbers (as doubles). - EXPECT_EQ("0.", stringify(JSON::Number(0.0))); - EXPECT_EQ("1.", stringify(JSON::Number(1.0))); + // Test whole numbers. + EXPECT_EQ("0", stringify(JSON::Number(0.0))); + EXPECT_EQ("1", stringify(JSON::Number(1.0))); // Negative. - EXPECT_EQ("-1.", stringify(JSON::Number(-1.0))); - - // Test integers. - EXPECT_EQ("0", stringify(JSON::Number(0))); - EXPECT_EQ("2", stringify(JSON::Number(2))); - EXPECT_EQ("-2", stringify(JSON::Number(-2))); + EXPECT_EQ("-1", stringify(JSON::Number(-1.0))); // Expect at least 15 digits of precision. EXPECT_EQ("1234567890.12345", stringify(JSON::Number(1234567890.12345))); } -TEST(JsonTest, NumberComparisons) -{ - // Unsigned and signed comparisons. - EXPECT_EQ(JSON::Number(1U), JSON::Number((int64_t) 1)); - EXPECT_EQ(JSON::Number(0U), JSON::Number((int64_t) 0)); - - EXPECT_NE(JSON::Number(1U), JSON::Number(-1)); - EXPECT_NE(JSON::Number((uint64_t) -1), JSON::Number(-1)); - - // Signed and unsigned comparisons (opposite order of above). - EXPECT_EQ(JSON::Number((int64_t) 1), JSON::Number(1U)); - EXPECT_EQ(JSON::Number((int64_t) 0), JSON::Number(0U)); - - EXPECT_NE(JSON::Number(-1), JSON::Number(1U)); - EXPECT_NE(JSON::Number(-1), JSON::Number((uint64_t) -1)); - - // Make sure we aren't doing an implicit cast from int64_t to uint64_t. - // These have the same bit representation (64h'8000_0000_0000_0001). - EXPECT_NE( - JSON::Number(9223372036854775809U), - JSON::Number(-9223372036854775807)); - EXPECT_NE( - JSON::Number(-9223372036854775807), - JSON::Number(9223372036854775809U)); -} - - TEST(JsonTest, BooleanFormat) { EXPECT_EQ("false", stringify(JSON::False())); @@ -153,50 +121,44 @@ TEST(JsonTest, NumericAssignment) s.st_nlink = 1; JSON::Value v = s.st_nlink; JSON::Number d = s.st_nlink; - EXPECT_NE(get<JSON::Number>(v).type, JSON::Number::FLOATING); - EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 1); - EXPECT_EQ(d.as<int64_t>(), 1); + EXPECT_EQ(get<JSON::Number>(v).value, 1.0); + EXPECT_EQ(d.value, 1.0); s.st_size = 2; v = s.st_size; d = s.st_size; - EXPECT_NE(get<JSON::Number>(v).type, JSON::Number::FLOATING); - EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 2); - EXPECT_EQ(d.as<int64_t>(), 2); + EXPECT_EQ(get<JSON::Number>(v).value, 2.0); + EXPECT_EQ(d.value, 2.0); s.st_mtime = 3; v = s.st_mtime; d = s.st_mtime; - EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 3); - EXPECT_EQ(d.as<int64_t>(), 3); + EXPECT_EQ(get<JSON::Number>(v).value, 3.0); + EXPECT_EQ(d.value, 3.0); size_t st = 4; v = st; d = st; - EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::UNSIGNED_INTEGER); - EXPECT_EQ(get<JSON::Number>(v).as<uint64_t>(), 4); - EXPECT_EQ(d.as<uint64_t>(), 4); + EXPECT_EQ(get<JSON::Number>(v).value, 4.0); + EXPECT_EQ(d.value, 4.0); uint64_t ui64 = 5; v = ui64; d = ui64; - EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::UNSIGNED_INTEGER); - EXPECT_EQ(get<JSON::Number>(v).as<uint64_t>(), 5); - EXPECT_EQ(d.as<uint64_t>(), 5); + EXPECT_EQ(get<JSON::Number>(v).value, 5.0); + EXPECT_EQ(d.value, 5.0); const unsigned int ui = 6; v = ui; d = ui; - EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::UNSIGNED_INTEGER); - EXPECT_EQ(get<JSON::Number>(v).as<uint64_t>(), 6); - EXPECT_EQ(d.as<uint64_t>(), 6); + EXPECT_EQ(get<JSON::Number>(v).value, 6.0); + EXPECT_EQ(d.value, 6.0); int i = 7; v = i; d = i; - EXPECT_EQ(get<JSON::Number>(v).type, JSON::Number::SIGNED_INTEGER); - EXPECT_EQ(get<JSON::Number>(v).as<int64_t>(), 7); - EXPECT_EQ(d.as<int64_t>(), 7); + EXPECT_EQ(get<JSON::Number>(v).value, 7.0); + EXPECT_EQ(d.value, 7.0); } http://git-wip-us.apache.org/repos/asf/mesos/blob/f8703752/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp index 01d5ec7..87737dd 100644 --- a/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp +++ b/3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp @@ -108,18 +108,18 @@ TEST(ProtobufTest, JSON) "{" " \"b\": true," " \"bytes\": \"Ynl0ZXM=\"," - " \"d\": 1.," + " \"d\": 1," " \"e\": \"ONE\"," - " \"f\": 1.," + " \"f\": 1," " \"int32\": -1," " \"int64\": -1," " \"nested\": { \"str\": \"nested\"}," - " \"optional_default\": 42.," + " \"optional_default\": 42," " \"repeated_bool\": [true]," " \"repeated_bytes\": [\"cmVwZWF0ZWRfYnl0ZXM=\"]," - " \"repeated_double\": [1., 2.]," + " \"repeated_double\": [1, 2]," " \"repeated_enum\": [\"TWO\"]," - " \"repeated_float\": [1.]," + " \"repeated_float\": [1]," " \"repeated_int32\": [-2]," " \"repeated_int64\": [-2]," " \"repeated_nested\": [ { \"str\": \"repeated_nested\" } ]," @@ -233,75 +233,3 @@ TEST(ProtobufTest, ParseJSONArray) EXPECT_EQ(message, repeated.Get(0)); EXPECT_EQ(message, repeated.Get(1)); } - - -// Tests that integer precision is maintained between -// JSON <-> Protobuf conversions. -TEST(ProtobufTest, JsonLargeIntegers) -{ - // These numbers are equal or close to the integer limits. - tests::Message message; - message.set_int32(-2147483647); - message.set_int64(-9223372036854775807); - message.set_uint32(4294967295U); - message.set_uint64(9223372036854775807); - message.set_sint32(-1234567890); - message.set_sint64(-1234567890123456789); - message.add_repeated_int32(-2000000000); - message.add_repeated_int64(-9000000000000000000); - message.add_repeated_uint32(3000000000U); - message.add_repeated_uint64(7000000000000000000); - message.add_repeated_sint32(-1000000000); - message.add_repeated_sint64(-8000000000000000000); - - // Parts of the protobuf that are required. Copied from the above test. - message.set_b(true); - message.set_str("string"); - message.set_bytes("bytes"); - message.set_f(1.0); - message.set_d(1.0); - message.set_e(tests::ONE); - message.mutable_nested()->set_str("nested"); - - // The keys are in alphabetical order. - string expected = strings::remove( - "{" - " \"b\": true," - " \"bytes\": \"Ynl0ZXM=\"," - " \"d\": 1.," - " \"e\": \"ONE\"," - " \"f\": 1.," - " \"int32\": -2147483647," - " \"int64\": -9223372036854775807," - " \"nested\": {\"str\": \"nested\"}," - " \"optional_default\": 42.," - " \"repeated_int32\": [-2000000000]," - " \"repeated_int64\": [-9000000000000000000]," - " \"repeated_sint32\": [-1000000000]," - " \"repeated_sint64\": [-8000000000000000000]," - " \"repeated_uint32\": [3000000000]," - " \"repeated_uint64\": [7000000000000000000]," - " \"sint32\": -1234567890," - " \"sint64\": -1234567890123456789," - " \"str\": \"string\"," - " \"uint32\": 4294967295," - " \"uint64\": 9223372036854775807" - "}", - " "); - - // Check JSON -> String. - JSON::Object object = JSON::Protobuf(message); - EXPECT_EQ(expected, stringify(object)); - - // Check JSON -> Protobuf. - Try<tests::Message> parse = protobuf::parse<tests::Message>(object); - ASSERT_SOME(parse); - - // Check Protobuf -> JSON. - EXPECT_EQ(object, JSON::Protobuf(parse.get())); - - // Check String -> JSON. - Try<JSON::Object> json = JSON::parse<JSON::Object>(expected); - ASSERT_SOME(json); - EXPECT_EQ(object, json.get()); -}
