From e3e9ccfc027c26b75234ce1b681bb93929035563 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 8 Jul 2020 14:02:28 +0200 Subject: [PATCH 1/4] :ambulance: fix regression from #2181 --- include/nlohmann/json.hpp | 12 ++++++------ single_include/nlohmann/json.hpp | 12 ++++++------ test/src/unit-regression.cpp | 9 +++++++++ test/src/unit-udt_macro.cpp | 12 ++++++------ 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 1dedb809b5..8a3957a600 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3785,7 +3785,7 @@ class basic_json template::value and not std::is_same::value, int>::type = 0> - ValueType value(const typename object_t::key_type& key, ValueType && default_value) const + ValueType value(const typename object_t::key_type& key, const ValueType& default_value) const { // at only works for objects if (JSON_HEDLEY_LIKELY(is_object())) @@ -3797,7 +3797,7 @@ class basic_json return *it; } - return std::move(default_value); + return default_value; } JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); @@ -3809,7 +3809,7 @@ class basic_json */ string_t value(const typename object_t::key_type& key, const char* default_value) const { - return value(key, std::move(string_t(default_value))); + return value(key, string_t(default_value)); } /*! @@ -3857,7 +3857,7 @@ class basic_json */ template::value, int>::type = 0> - ValueType value(const json_pointer& ptr, ValueType && default_value) const + ValueType value(const json_pointer& ptr, const ValueType& default_value) const { // at only works for objects if (JSON_HEDLEY_LIKELY(is_object())) @@ -3869,7 +3869,7 @@ class basic_json } JSON_INTERNAL_CATCH (out_of_range&) { - return std::move(default_value); + return default_value; } } @@ -3883,7 +3883,7 @@ class basic_json JSON_HEDLEY_NON_NULL(3) string_t value(const json_pointer& ptr, const char* default_value) const { - return value(ptr, std::move(string_t(default_value))); + return value(ptr, string_t(default_value)); } /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 87222900bb..05c3c925e0 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -19715,7 +19715,7 @@ class basic_json template::value and not std::is_same::value, int>::type = 0> - ValueType value(const typename object_t::key_type& key, ValueType && default_value) const + ValueType value(const typename object_t::key_type& key, const ValueType& default_value) const { // at only works for objects if (JSON_HEDLEY_LIKELY(is_object())) @@ -19727,7 +19727,7 @@ class basic_json return *it; } - return std::move(default_value); + return default_value; } JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); @@ -19739,7 +19739,7 @@ class basic_json */ string_t value(const typename object_t::key_type& key, const char* default_value) const { - return value(key, std::move(string_t(default_value))); + return value(key, string_t(default_value)); } /*! @@ -19787,7 +19787,7 @@ class basic_json */ template::value, int>::type = 0> - ValueType value(const json_pointer& ptr, ValueType && default_value) const + ValueType value(const json_pointer& ptr, const ValueType& default_value) const { // at only works for objects if (JSON_HEDLEY_LIKELY(is_object())) @@ -19799,7 +19799,7 @@ class basic_json } JSON_INTERNAL_CATCH (out_of_range&) { - return std::move(default_value); + return default_value; } } @@ -19813,7 +19813,7 @@ class basic_json JSON_HEDLEY_NON_NULL(3) string_t value(const json_pointer& ptr, const char* default_value) const { - return value(ptr, std::move(string_t(default_value))); + return value(ptr, string_t(default_value)); } /*! diff --git a/test/src/unit-regression.cpp b/test/src/unit-regression.cpp index e5cd0337b3..3f9d9c26da 100644 --- a/test/src/unit-regression.cpp +++ b/test/src/unit-regression.cpp @@ -1945,6 +1945,15 @@ TEST_CASE("regression tests") ) ); } + + SECTION("PR #2181 - regression bug with lvalue") + { + // see https://github.com/nlohmann/json/pull/2181#issuecomment-653326060 + json j{{"x", "test"}}; + std::string defval = "default value"; + auto val = j.value("x", defval); + auto val2 = j.value("y", defval); + } } #if not defined(JSON_NOEXCEPTION) diff --git a/test/src/unit-udt_macro.cpp b/test/src/unit-udt_macro.cpp index a6b66e9550..90d5e1cc4c 100644 --- a/test/src/unit-udt_macro.cpp +++ b/test/src/unit-udt_macro.cpp @@ -60,9 +60,9 @@ class person_with_private_data class person_without_private_data_1 { public: - std::string name = ""; - int age = 0; - json metadata = nullptr; + std::string name = ""; + int age = 0; + json metadata = nullptr; bool operator==(const person_without_private_data_1& rhs) const { @@ -82,9 +82,9 @@ class person_without_private_data_1 class person_without_private_data_2 { public: - std::string name = ""; - int age = 0; - json metadata = nullptr; + std::string name = ""; + int age = 0; + json metadata = nullptr; bool operator==(const person_without_private_data_2& rhs) const { From d13873482e734fa8a2b3a42dfc3a342844ee2ae3 Mon Sep 17 00:00:00 2001 From: chenguoping Date: Wed, 8 Jul 2020 22:02:49 +0800 Subject: [PATCH 2/4] fix the bug --- include/nlohmann/json.hpp | 46 ++++++++++++++++++++++++++++++-- single_include/nlohmann/json.hpp | 46 ++++++++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 8a3957a600..8c6622c43e 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3803,13 +3803,34 @@ class basic_json JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); } + template::value + and not std::is_same::value, int>::type = 0> + ValueType value(const typename object_t::key_type& key, ValueType && default_value) const + { + // at only works for objects + if (JSON_HEDLEY_LIKELY(is_object())) + { + // if key is found, return value and given default value otherwise + const auto it = find(key); + if (it != end()) + { + return *it; + } + + return std::move(default_value); + } + + JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); + } + /*! @brief overload for a default value of type const char* @copydoc basic_json::value(const typename object_t::key_type&, const ValueType&) const */ string_t value(const typename object_t::key_type& key, const char* default_value) const { - return value(key, string_t(default_value)); + return value(key, std::move(string_t(default_value))); } /*! @@ -3876,6 +3897,27 @@ class basic_json JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); } + template::value, int>::type = 0> + ValueType value(const json_pointer& ptr, ValueType && default_value) const + { + // at only works for objects + if (JSON_HEDLEY_LIKELY(is_object())) + { + // if pointer resolves a value, return it or use default value + JSON_TRY + { + return ptr.get_checked(this); + } + JSON_INTERNAL_CATCH (out_of_range&) + { + return std::move(default_value); + } + } + + JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); + } + /*! @brief overload for a default value of type const char* @copydoc basic_json::value(const json_pointer&, ValueType) const @@ -3883,7 +3925,7 @@ class basic_json JSON_HEDLEY_NON_NULL(3) string_t value(const json_pointer& ptr, const char* default_value) const { - return value(ptr, string_t(default_value)); + return value(ptr, std::move(string_t(default_value))); } /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 05c3c925e0..b7cf9f7042 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -19733,13 +19733,34 @@ class basic_json JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); } + template::value + and not std::is_same::value, int>::type = 0> + ValueType value(const typename object_t::key_type& key, ValueType && default_value) const + { + // at only works for objects + if (JSON_HEDLEY_LIKELY(is_object())) + { + // if key is found, return value and given default value otherwise + const auto it = find(key); + if (it != end()) + { + return *it; + } + + return std::move(default_value); + } + + JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); + } + /*! @brief overload for a default value of type const char* @copydoc basic_json::value(const typename object_t::key_type&, const ValueType&) const */ string_t value(const typename object_t::key_type& key, const char* default_value) const { - return value(key, string_t(default_value)); + return value(key, std::move(string_t(default_value))); } /*! @@ -19806,6 +19827,27 @@ class basic_json JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); } + template::value, int>::type = 0> + ValueType value(const json_pointer& ptr, ValueType && default_value) const + { + // at only works for objects + if (JSON_HEDLEY_LIKELY(is_object())) + { + // if pointer resolves a value, return it or use default value + JSON_TRY + { + return ptr.get_checked(this); + } + JSON_INTERNAL_CATCH (out_of_range&) + { + return std::move(default_value); + } + } + + JSON_THROW(type_error::create(306, "cannot use value() with " + std::string(type_name()))); + } + /*! @brief overload for a default value of type const char* @copydoc basic_json::value(const json_pointer&, ValueType) const @@ -19813,7 +19855,7 @@ class basic_json JSON_HEDLEY_NON_NULL(3) string_t value(const json_pointer& ptr, const char* default_value) const { - return value(ptr, string_t(default_value)); + return value(ptr, std::move(string_t(default_value))); } /*! From 524f11765cb7527bfc8d3bc376766116f9f52f6c Mon Sep 17 00:00:00 2001 From: chenguoping Date: Mon, 13 Jul 2020 20:44:25 +0800 Subject: [PATCH 3/4] remove std::move() --- include/nlohmann/json.hpp | 4 ++-- single_include/nlohmann/json.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 8c6622c43e..f32d110812 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3830,7 +3830,7 @@ class basic_json */ string_t value(const typename object_t::key_type& key, const char* default_value) const { - return value(key, std::move(string_t(default_value))); + return value(key, string_t(default_value)); } /*! @@ -3925,7 +3925,7 @@ class basic_json JSON_HEDLEY_NON_NULL(3) string_t value(const json_pointer& ptr, const char* default_value) const { - return value(ptr, std::move(string_t(default_value))); + return value(ptr, string_t(default_value)); } /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index b7cf9f7042..467d242f57 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -19760,7 +19760,7 @@ class basic_json */ string_t value(const typename object_t::key_type& key, const char* default_value) const { - return value(key, std::move(string_t(default_value))); + return value(key, string_t(default_value)); } /*! @@ -19855,7 +19855,7 @@ class basic_json JSON_HEDLEY_NON_NULL(3) string_t value(const json_pointer& ptr, const char* default_value) const { - return value(ptr, std::move(string_t(default_value))); + return value(ptr, string_t(default_value)); } /*! From 30dd9bdb1aa68d9aee65338ef29aea0c6401adad Mon Sep 17 00:00:00 2001 From: chenguoping Date: Mon, 13 Jul 2020 20:44:47 +0800 Subject: [PATCH 4/4] boost code coverage --- test/src/unit-element_access2.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/src/unit-element_access2.cpp b/test/src/unit-element_access2.cpp index 0a73964a26..59a97c38d3 100644 --- a/test/src/unit-element_access2.cpp +++ b/test/src/unit-element_access2.cpp @@ -282,6 +282,18 @@ TEST_CASE("element access 2") CHECK_THROWS_WITH(j_nonobject_const.value("foo", 1), "[json.exception.type_error.306] cannot use value() with number"); } + SECTION("default value is lvalue") + { + json j_nonobject(json::value_t::number_float); + const json j_nonobject_const(j_nonobject); + std::string defval = "default value"; + CHECK_THROWS_AS(j_nonobject.value("foo", defval), json::type_error&); + CHECK_THROWS_AS(j_nonobject_const.value("foo", defval), json::type_error&); + CHECK_THROWS_WITH(j_nonobject.value("foo", defval), + "[json.exception.type_error.306] cannot use value() with number"); + CHECK_THROWS_WITH(j_nonobject_const.value("foo", defval), + "[json.exception.type_error.306] cannot use value() with number"); + } } }