diff --git a/docs/mkdocs/docs/api/basic_json/emplace.md b/docs/mkdocs/docs/api/basic_json/emplace.md index 18286e83f..d6c1287df 100644 --- a/docs/mkdocs/docs/api/basic_json/emplace.md +++ b/docs/mkdocs/docs/api/basic_json/emplace.md @@ -69,3 +69,5 @@ Logarithmic in the size of the container, O(log(`size()`)). ## Version history - Since version 2.0.8. +- Fixed in version 3.13.0: for [`ordered_json`](../ordered_json.md), the value could previously only be passed as an + rvalue; it can now also be passed as an lvalue or a `#!cpp const` lvalue, matching the behavior of `json`. diff --git a/include/nlohmann/ordered_map.hpp b/include/nlohmann/ordered_map.hpp index 7b8cf70f4..e17b8333a 100644 --- a/include/nlohmann/ordered_map.hpp +++ b/include/nlohmann/ordered_map.hpp @@ -70,7 +70,9 @@ template , return *this; } - std::pair emplace(const key_type& key, T&& t) + template < class V, detail::enable_if_t < + detail::is_constructible < T, V&& >::value, int > = 0 > + std::pair emplace(const key_type& key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -79,13 +81,14 @@ template , return {it, false}; } } - Container::emplace_back(key, std::forward(t)); + Container::emplace_back(key, std::forward(t)); return {std::prev(this->end()), true}; } - template::value, int> = 0> - std::pair emplace(KeyType && key, T && t) + template < class KeyType, class V, detail::enable_if_t < + detail::is_usable_as_key_type::value&& + detail::is_constructible < T, V&& >::value, int > = 0 > + std::pair emplace(KeyType && key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -94,7 +97,7 @@ template , return {it, false}; } } - Container::emplace_back(std::forward(key), std::forward(t)); + Container::emplace_back(std::forward(key), std::forward(t)); return {std::prev(this->end()), true}; } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 576498738..a6b320e71 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -25832,7 +25832,9 @@ template , return *this; } - std::pair emplace(const key_type& key, T&& t) + template < class V, detail::enable_if_t < + detail::is_constructible < T, V&& >::value, int > = 0 > + std::pair emplace(const key_type& key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -25841,13 +25843,14 @@ template , return {it, false}; } } - Container::emplace_back(key, std::forward(t)); + Container::emplace_back(key, std::forward(t)); return {std::prev(this->end()), true}; } - template::value, int> = 0> - std::pair emplace(KeyType && key, T && t) + template < class KeyType, class V, detail::enable_if_t < + detail::is_usable_as_key_type::value&& + detail::is_constructible < T, V&& >::value, int > = 0 > + std::pair emplace(KeyType && key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -25856,7 +25859,7 @@ template , return {it, false}; } } - Container::emplace_back(std::forward(key), std::forward(t)); + Container::emplace_back(std::forward(key), std::forward(t)); return {std::prev(this->end()), true}; } diff --git a/tests/src/unit-ordered_json.cpp b/tests/src/unit-ordered_json.cpp index 45fbf5493..135dedade 100644 --- a/tests/src/unit-ordered_json.cpp +++ b/tests/src/unit-ordered_json.cpp @@ -196,3 +196,44 @@ TEST_CASE("regression test - diff() must account for ordered_json member order") CHECK(a.patch(p) == b); } } + +TEST_CASE("regression test for issue #5673 - ordered_json::emplace with a non-rvalue value") +{ + SECTION("lvalue value") + { + ordered_json oj = ordered_json::object(); + ordered_json value = 1; + auto res = oj.emplace("a", value); + CHECK(res.second == true); + CHECK(oj.dump() == "{\"a\":1}"); + } + + SECTION("const lvalue value") + { + ordered_json oj = ordered_json::object(); + const ordered_json value = 1; + auto res = oj.emplace("a", value); + CHECK(res.second == true); + CHECK(oj.dump() == "{\"a\":1}"); + } + + SECTION("rvalue value") + { + ordered_json oj = ordered_json::object(); + auto res = oj.emplace("a", ordered_json(1)); + CHECK(res.second == true); + CHECK(oj.dump() == "{\"a\":1}"); + } + + SECTION("existing key is not overwritten (std::map-compatible semantics)") + { + ordered_json oj = ordered_json::object(); + ordered_json value = 1; + oj.emplace("a", value); + + ordered_json other_value = 2; + auto res = oj.emplace("a", other_value); + CHECK(res.second == false); + CHECK(oj.dump() == "{\"a\":1}"); + } +} diff --git a/tests/src/unit-ordered_map.cpp b/tests/src/unit-ordered_map.cpp index f380a9869..7497d25de 100644 --- a/tests/src/unit-ordered_map.cpp +++ b/tests/src/unit-ordered_map.cpp @@ -312,4 +312,77 @@ TEST_CASE("ordered_map") CHECK(om.size() == 4); } } + + SECTION("emplace") + { + // regression test for issue #5673: the mapped-value parameter must + // accept lvalues and const lvalues, not just rvalues + ordered_map om; + om["eins"] = "one"; + om["zwei"] = "two"; + om["drei"] = "three"; + + SECTION("with T&& (rvalue)") + { + auto res1 = om.emplace("eins", std::string("1")); + CHECK(res1.first == om.begin()); + CHECK(res1.second == false); + CHECK(om.size() == 3); + CHECK(om.at("eins") == "one"); // existing key is not overwritten + + auto res4 = om.emplace("vier", std::string("four")); + CHECK(res4.first == om.begin() + 3); + CHECK(res4.second == true); + CHECK(om.size() == 4); + CHECK(om.at("vier") == "four"); + } + + SECTION("with T& (lvalue)") + { + std::string one = "1"; + std::string four = "four"; + + auto res1 = om.emplace("eins", one); + CHECK(res1.first == om.begin()); + CHECK(res1.second == false); + CHECK(om.size() == 3); + CHECK(om.at("eins") == "one"); // existing key is not overwritten + + auto res4 = om.emplace("vier", four); + CHECK(res4.first == om.begin() + 3); + CHECK(res4.second == true); + CHECK(om.size() == 4); + CHECK(om.at("vier") == "four"); + CHECK(four == "four"); // source was copied, not moved from + } + + SECTION("with const T&") + { + const std::string one = "1"; + const std::string four = "four"; + + auto res1 = om.emplace("eins", one); + CHECK(res1.first == om.begin()); + CHECK(res1.second == false); + CHECK(om.size() == 3); + + auto res4 = om.emplace("vier", four); + CHECK(res4.first == om.begin() + 3); + CHECK(res4.second == true); + CHECK(om.size() == 4); + CHECK(om.at("vier") == "four"); + } + + SECTION("with key of key_type (non-template overload)") + { + const std::string key_vier{"vier"}; + std::string four = "four"; + + auto res4 = om.emplace(key_vier, four); + CHECK(res4.first == om.begin() + 3); + CHECK(res4.second == true); + CHECK(om.size() == 4); + CHECK(om.at("vier") == "four"); + } + } }