From 81609393f1925b22b4b812d1f60cb1d826052c65 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Sun, 21 Feb 2021 16:33:05 -0500 Subject: [PATCH] Fixing issue 1449. (#1451) --- benchmark/bench_dom_api.cpp | 2 +- fuzz/fuzz_dump.cpp | 4 +- include/simdjson/dom/element.h | 37 +++++------ include/simdjson/dom/object.h | 12 ++-- include/simdjson/error-inl.h | 3 - .../implementation_simdjson_result_base-inl.h | 3 - include/simdjson/generic/ondemand/document.h | 12 ++-- tests/basictests.cpp | 62 +++++++++---------- tests/integer_tests.cpp | 4 +- 9 files changed, 65 insertions(+), 74 deletions(-) diff --git a/benchmark/bench_dom_api.cpp b/benchmark/bench_dom_api.cpp index 778d1439..b8f5937b 100644 --- a/benchmark/bench_dom_api.cpp +++ b/benchmark/bench_dom_api.cpp @@ -522,7 +522,7 @@ static void twitter_image_sizes(State& state) { dom::array media; if (not (error = tweet["entities"]["media"].get(media))) { for (dom::object image : media) { - for (auto size : image["sizes"].get()) { + for (auto size : image["sizes"].get_object()) { image_sizes.emplace(size.value["w"], size.value["h"]); } } diff --git a/fuzz/fuzz_dump.cpp b/fuzz/fuzz_dump.cpp index 30270d36..2c5b1517 100644 --- a/fuzz/fuzz_dump.cpp +++ b/fuzz/fuzz_dump.cpp @@ -17,7 +17,7 @@ static void print_json(std::ostream& os, simdjson::dom::element element) noexcep case simdjson::dom::element_type::ARRAY: os << "["; { - simdjson::dom::array array = element.get().value_unsafe(); + simdjson::dom::array array = element.get_array().value_unsafe(); for (simdjson::dom::element child : array) { print_json(os, child); os << ","; @@ -28,7 +28,7 @@ static void print_json(std::ostream& os, simdjson::dom::element element) noexcep case simdjson::dom::element_type::OBJECT: os << "{"; { - simdjson::dom::object object = element.get().value_unsafe(); + simdjson::dom::object object = element.get_object().value_unsafe(); for (simdjson::dom::key_value_pair field : object) { os << "\"" << field.key << "\": "; print_json(os, field.value); diff --git a/include/simdjson/dom/element.h b/include/simdjson/dom/element.h index 93b7adf0..eb33befe 100644 --- a/include/simdjson/dom/element.h +++ b/include/simdjson/dom/element.h @@ -48,8 +48,6 @@ public: /** * Cast this element to an array. * - * Equivalent to get(). - * * @returns An object that can be used to iterate the array, or: * INCORRECT_TYPE if the JSON element is not an array. */ @@ -57,8 +55,6 @@ public: /** * Cast this element to an object. * - * Equivalent to get(). - * * @returns An object that can be used to look up or iterate the object's fields, or: * INCORRECT_TYPE if the JSON element is not an object. */ @@ -68,8 +64,6 @@ public: * * The string is guaranteed to be valid UTF-8. * - * The get_c_str() function is equivalent to get(). - * * The length of the string is given by get_string_length(). Because JSON strings * may contain null characters, it may be incorrect to use strlen to determine the * string length. @@ -97,8 +91,6 @@ public: * * The string is guaranteed to be valid UTF-8. * - * Equivalent to get(). - * * @returns An UTF-8 string. The string is stored in the parser and will be invalidated the next time it * parses a document or when it is destroyed. * Returns INCORRECT_TYPE if the JSON element is not a string. @@ -107,8 +99,6 @@ public: /** * Cast this element to a signed integer. * - * Equivalent to get(). - * * @returns A signed 64-bit integer. * Returns INCORRECT_TYPE if the JSON element is not an integer, or NUMBER_OUT_OF_RANGE * if it is negative. @@ -117,8 +107,6 @@ public: /** * Cast this element to an unsigned integer. * - * Equivalent to get(). - * * @returns An unsigned 64-bit integer. * Returns INCORRECT_TYPE if the JSON element is not an integer, or NUMBER_OUT_OF_RANGE * if it is too large. @@ -127,8 +115,6 @@ public: /** * Cast this element to a double floating-point. * - * Equivalent to get(). - * * @returns A double value. * Returns INCORRECT_TYPE if the JSON element is not a number. */ @@ -136,8 +122,6 @@ public: /** * Cast this element to a bool. * - * Equivalent to get(). - * * @returns A bool value. * Returns INCORRECT_TYPE if the JSON element is not a boolean. */ @@ -214,6 +198,14 @@ public: simdjson_really_inline bool is() const noexcept; /** + * @private + * Deprecated as a public interface. These methods will be made private in a future + * release. Use get_double(), get_bool(), get_uint64(), get_int64(), + * get_object(), get_array() or get_string() instead. We found in practice that + * the template would mislead users into writing get() for types X that + * are not among the supported types (e.g., get(), get(), + * get()), and the resulting C++ compiler error is difficult to parse. + * * Get the value as the provided type (T). * * Supported types: @@ -228,6 +220,7 @@ public: * @returns The value cast to the given type, or: * INCORRECT_TYPE if the value cannot be cast to the given type. */ + template inline simdjson_result get() const noexcept; @@ -363,8 +356,8 @@ public: * The key will be matched against **unescaped** JSON: * * dom::parser parser; - * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get().first == 1 - * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get().error() == NO_SUCH_FIELD + * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get_uint64().first == 1 + * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get_uint64().error() == NO_SUCH_FIELD * * @return The value associated with this field, or: * - NO_SUCH_FIELD if the field does not exist in the object @@ -378,8 +371,8 @@ public: * The key will be matched against **unescaped** JSON: * * dom::parser parser; - * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get().first == 1 - * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get().error() == NO_SUCH_FIELD + * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get_uint64().first == 1 + * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get_uint64().error() == NO_SUCH_FIELD * * @return The value associated with this field, or: * - NO_SUCH_FIELD if the field does not exist in the object @@ -450,8 +443,8 @@ public: * The key will be matched against **unescaped** JSON: * * dom::parser parser; - * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get().first == 1 - * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get().error() == NO_SUCH_FIELD + * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get_uint64().first == 1 + * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get_uint64().error() == NO_SUCH_FIELD * * @return The value associated with this field, or: * - NO_SUCH_FIELD if the field does not exist in the object diff --git a/include/simdjson/dom/object.h b/include/simdjson/dom/object.h index d16dd8c8..93fdade5 100644 --- a/include/simdjson/dom/object.h +++ b/include/simdjson/dom/object.h @@ -123,8 +123,8 @@ public: * The key will be matched against **unescaped** JSON: * * dom::parser parser; - * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get().first == 1 - * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get().error() == NO_SUCH_FIELD + * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get_uint64().first == 1 + * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get_uint64().error() == NO_SUCH_FIELD * * This function has linear-time complexity: the keys are checked one by one. * @@ -140,8 +140,8 @@ public: * The key will be matched against **unescaped** JSON: * * dom::parser parser; - * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get().first == 1 - * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get().error() == NO_SUCH_FIELD + * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get_uint64().first == 1 + * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get_uint64().error() == NO_SUCH_FIELD * * This function has linear-time complexity: the keys are checked one by one. * @@ -182,8 +182,8 @@ public: * The key will be matched against **unescaped** JSON: * * dom::parser parser; - * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get().first == 1 - * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get().error() == NO_SUCH_FIELD + * parser.parse(R"({ "a\n": 1 })"_padded)["a\n"].get_uint64().first == 1 + * parser.parse(R"({ "a\n": 1 })"_padded)["a\\n"].get_uint64().error() == NO_SUCH_FIELD * * This function has linear-time complexity: the keys are checked one by one. * diff --git a/include/simdjson/error-inl.h b/include/simdjson/error-inl.h index 681df206..2dc94a8a 100644 --- a/include/simdjson/error-inl.h +++ b/include/simdjson/error-inl.h @@ -45,9 +45,6 @@ namespace internal { template simdjson_really_inline void simdjson_result_base::tie(T &value, error_code &error) && noexcept { - // on the clang compiler that comes with current macOS (Apple clang version 11.0.0), - // tie(width, error) = size["w"].get(); - // fails with "error: no viable overloaded '='"" error = this->second; if (!error) { value = std::forward>(*this).first; diff --git a/include/simdjson/generic/implementation_simdjson_result_base-inl.h b/include/simdjson/generic/implementation_simdjson_result_base-inl.h index a053abc9..71a8bb21 100644 --- a/include/simdjson/generic/implementation_simdjson_result_base-inl.h +++ b/include/simdjson/generic/implementation_simdjson_result_base-inl.h @@ -7,9 +7,6 @@ namespace SIMDJSON_IMPLEMENTATION { template simdjson_really_inline void implementation_simdjson_result_base::tie(T &value, error_code &error) && noexcept { - // on the clang compiler that comes with current macOS (Apple clang version 11.0.0), - // tie(width, error) = size["w"].get(); - // fails with "error: no viable overloaded '='"" error = this->second; if (!error) { value = std::forward>(*this).first; diff --git a/include/simdjson/generic/ondemand/document.h b/include/simdjson/generic/ondemand/document.h index 7de29101..c86ee734 100644 --- a/include/simdjson/generic/ondemand/document.h +++ b/include/simdjson/generic/ondemand/document.h @@ -71,8 +71,6 @@ public: * * The string is guaranteed to be valid UTF-8. * - * Equivalent to get(). - * * @returns An UTF-8 string. The string is stored in the parser and will be invalidated the next * time it parses a document or when it is destroyed. * @returns INCORRECT_TYPE if the JSON value is not a string. @@ -102,6 +100,14 @@ public: simdjson_really_inline bool is_null() noexcept; /** + * @private + * Deprecated as a public interface. These methods will be made private in a future + * release. Use get_double(), get_bool(), get_uint64(), get_int64(), + * get_object(), get_array() or get_string() instead. We found in practice that + * the template would mislead users into writing get() for types X that + * are not among the supported types (e.g., get(), get(), + * get()), and the resulting C++ compiler error is difficult to parse. + * * Get this value as the given type. * * Supported types: object, array, raw_json_string, string_view, uint64_t, int64_t, double, bool @@ -167,8 +173,6 @@ public: * * The string is guaranteed to be valid UTF-8. * - * Equivalent to get(). - * * @returns An UTF-8 string. The string is stored in the parser and will be invalidated the next * time it parses a document or when it is destroyed. * @exception simdjson_error(INCORRECT_TYPE) if the JSON value is not a string. diff --git a/tests/basictests.cpp b/tests/basictests.cpp index a73c535c..ca12f37f 100644 --- a/tests/basictests.cpp +++ b/tests/basictests.cpp @@ -737,7 +737,7 @@ namespace dom_api_tests { int i = 0; for (auto [key, value] : object) { ASSERT_EQUAL( key, expected_key[i] ); - ASSERT_EQUAL( value.get().value_unsafe(), expected_value[i] ); + ASSERT_EQUAL( value.get_uint64().value_unsafe(), expected_value[i] ); i++; } ASSERT_EQUAL( i*sizeof(uint64_t), sizeof(expected_value) ); @@ -822,18 +822,18 @@ namespace dom_api_tests { ASSERT_SUCCESS( parser.parse(json).get(array) ); auto iter = array.begin(); - ASSERT_EQUAL( (*iter).get().value_unsafe(), 0 ); - ASSERT_EQUAL( (*iter).get().value_unsafe(), 0 ); - ASSERT_EQUAL( (*iter).get().value_unsafe(), 0 ); + ASSERT_EQUAL( (*iter).get_uint64().value_unsafe(), 0 ); + ASSERT_EQUAL( (*iter).get_int64().value_unsafe(), 0 ); + ASSERT_EQUAL( (*iter).get_double().value_unsafe(), 0 ); ++iter; - ASSERT_EQUAL( (*iter).get().value_unsafe(), 1 ); - ASSERT_EQUAL( (*iter).get().value_unsafe(), 1 ); - ASSERT_EQUAL( (*iter).get().value_unsafe(), 1 ); + ASSERT_EQUAL( (*iter).get_uint64().value_unsafe(), 1 ); + ASSERT_EQUAL( (*iter).get_int64().value_unsafe(), 1 ); + ASSERT_EQUAL( (*iter).get_double().value_unsafe(), 1 ); ++iter; - ASSERT_EQUAL( (*iter).get().value_unsafe(), -1 ); - ASSERT_EQUAL( (*iter).get().value_unsafe(), -1 ); + ASSERT_EQUAL( (*iter).get_int64().value_unsafe(), -1 ); + ASSERT_EQUAL( (*iter).get_double().value_unsafe(), -1 ); ++iter; - ASSERT_EQUAL( (*iter).get().value_unsafe(), 1.1 ); + ASSERT_EQUAL( (*iter).get_double().value_unsafe(), 1.1 ); return true; } @@ -875,13 +875,13 @@ namespace dom_api_tests { auto mylambda = [](dom::element e) { return int64_t(e); }; ASSERT_EQUAL( mylambda(node), 1 ); #endif - ASSERT_EQUAL( object["a"].get().value_unsafe(), 1 ); - ASSERT_EQUAL( object["b"].get().value_unsafe(), 2 ); - ASSERT_EQUAL( object["c/d"].get().value_unsafe(), 3 ); + ASSERT_EQUAL( object["a"].get_uint64().value_unsafe(), 1 ); + ASSERT_EQUAL( object["b"].get_uint64().value_unsafe(), 2 ); + ASSERT_EQUAL( object["c/d"].get_uint64().value_unsafe(), 3 ); // Check all three again in backwards order, to ensure we can go backwards - ASSERT_EQUAL( object["c/d"].get().value_unsafe(), 3 ); - ASSERT_EQUAL( object["b"].get().value_unsafe(), 2 ); - ASSERT_EQUAL( object["a"].get().value_unsafe(), 1 ); + ASSERT_EQUAL( object["c/d"].get_uint64().value_unsafe(), 3 ); + ASSERT_EQUAL( object["b"].get_uint64().value_unsafe(), 2 ); + ASSERT_EQUAL( object["a"].get_uint64().value_unsafe(), 1 ); simdjson::error_code error; simdjson_unused element val; @@ -906,20 +906,20 @@ namespace dom_api_tests { dom::parser parser; dom::element doc; ASSERT_SUCCESS( parser.parse(json).get(doc) ); - ASSERT_EQUAL( doc["obj"]["a"].get().value_unsafe(), 1); + ASSERT_EQUAL( doc["obj"]["a"].get_uint64().value_unsafe(), 1); object obj; ASSERT_SUCCESS( doc.get(obj) ); - ASSERT_EQUAL( obj["obj"]["a"].get().value_unsafe(), 1); + ASSERT_EQUAL( obj["obj"]["a"].get_uint64().value_unsafe(), 1); ASSERT_SUCCESS( obj["obj"].get(obj) ); - ASSERT_EQUAL( obj["a"].get().value_unsafe(), 1 ); - ASSERT_EQUAL( obj["b"].get().value_unsafe(), 2 ); - ASSERT_EQUAL( obj["c/d"].get().value_unsafe(), 3 ); + ASSERT_EQUAL( obj["a"].get_uint64().value_unsafe(), 1 ); + ASSERT_EQUAL( obj["b"].get_uint64().value_unsafe(), 2 ); + ASSERT_EQUAL( obj["c/d"].get_uint64().value_unsafe(), 3 ); // Check all three again in backwards order, to ensure we can go backwards - ASSERT_EQUAL( obj["c/d"].get().value_unsafe(), 3 ); - ASSERT_EQUAL( obj["b"].get().value_unsafe(), 2 ); - ASSERT_EQUAL( obj["a"].get().value_unsafe(), 1 ); + ASSERT_EQUAL( obj["c/d"].get_uint64().value_unsafe(), 3 ); + ASSERT_EQUAL( obj["b"].get_uint64().value_unsafe(), 2 ); + ASSERT_EQUAL( obj["a"].get_uint64().value_unsafe(), 1 ); simdjson_unused element val; ASSERT_ERROR( doc["d"].get(val), NO_SUCH_FIELD); @@ -1107,7 +1107,7 @@ namespace dom_api_tests { set default_users; dom::parser parser; element doc = parser.load(TWITTER_JSON); - for (object tweet : doc["statuses"].get()) { + for (object tweet : doc["statuses"].get_array()) { object user = tweet["user"]; if (user["default_profile"]) { default_users.insert(user["screen_name"]); @@ -1858,14 +1858,14 @@ namespace format_tests { std::cout << "Running " << __func__ << std::endl; dom::parser parser; ostringstream s; - s << parser.parse(DOCUMENT)["bar"].get(); + s << parser.parse(DOCUMENT)["bar"].get_array(); return assert_minified(s, "[1,2,0.11111111111111113]"); } bool print_minify_array_result_exception() { std::cout << "Running " << __func__ << std::endl; dom::parser parser; ostringstream s; - s << minify(parser.parse(DOCUMENT)["bar"].get()); + s << minify(parser.parse(DOCUMENT)["bar"].get_array()); return assert_minified(s, "[1,2,0.11111111111111113]"); } @@ -1873,14 +1873,14 @@ namespace format_tests { std::cout << "Running " << __func__ << std::endl; dom::parser parser; ostringstream s; - s << parser.parse(DOCUMENT)["baz"].get(); + s << parser.parse(DOCUMENT)["baz"].get_object(); return assert_minified(s, R"({"a":3.1415926535897936,"b":2,"c":3.141592653589794})"); } bool print_minify_object_result_exception() { std::cout << "Running " << __func__ << std::endl; dom::parser parser; ostringstream s; - s << minify(parser.parse(DOCUMENT)["baz"].get()); + s << minify(parser.parse(DOCUMENT)["baz"].get_object()); return assert_minified(s, R"({"a":3.1415926535897936,"b":2,"c":3.141592653589794})"); } @@ -2041,7 +2041,7 @@ namespace to_string_tests { std::cout << "Running " << __func__ << std::endl; dom::parser parser; ostringstream s; - s << to_string(parser.parse(DOCUMENT)["bar"].get()); + s << to_string(parser.parse(DOCUMENT)["bar"].get_array()); return assert_minified(s, "[1,2,0.11111111111111113]"); } @@ -2050,7 +2050,7 @@ namespace to_string_tests { std::cout << "Running " << __func__ << std::endl; dom::parser parser; ostringstream s; - s << to_string(parser.parse(DOCUMENT)["baz"].get()); + s << to_string(parser.parse(DOCUMENT)["baz"].get_object()); return assert_minified(s, R"({"a":3.1415926535897936,"b":2,"c":3.141592653589794})"); } diff --git a/tests/integer_tests.cpp b/tests/integer_tests.cpp index c35d717c..66178b3d 100644 --- a/tests/integer_tests.cpp +++ b/tests/integer_tests.cpp @@ -54,7 +54,7 @@ static bool parse_and_check_signed(const std::string src) { const padded_string pstr{src}; simdjson::dom::parser parser; simdjson::dom::element value; - ASSERT_SUCCESS( parser.parse(pstr).get()["key"].get(value) ); + ASSERT_SUCCESS( parser.parse(pstr).get_object()["key"].get(value) ); ASSERT_EQUAL( value.is(), true ); return true; } @@ -64,7 +64,7 @@ static bool parse_and_check_unsigned(const std::string src) { const padded_string pstr{src}; simdjson::dom::parser parser; simdjson::dom::element value; - ASSERT_SUCCESS( parser.parse(pstr).get()["key"].get(value) ); + ASSERT_SUCCESS( parser.parse(pstr).get_object()["key"].get(value) ); ASSERT_EQUAL( value.is(), true ); return true; }