diff --git a/docs/mkdocs/docs/api/basic_json/emplace.md b/docs/mkdocs/docs/api/basic_json/emplace.md index 26044a597..09953d347 100644 --- a/docs/mkdocs/docs/api/basic_json/emplace.md +++ b/docs/mkdocs/docs/api/basic_json/emplace.md @@ -70,3 +70,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 656f24264..d1c247483 100644 --- a/include/nlohmann/ordered_map.hpp +++ b/include/nlohmann/ordered_map.hpp @@ -74,7 +74,9 @@ template , return *this; } - std::pair emplace(const key_type& key, T&& t) + template::value, int> = 0> + std::pair emplace(const key_type& key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -83,13 +85,14 @@ template , return {it, false}; } } - append(key, std::forward(t)); + append(key, std::forward(t)); return {std::prev(this->end()), true}; } - template::value, int> = 0> - std::pair emplace(KeyType && key, T && t) + template, + detail::is_constructible>::value, int> = 0> + std::pair emplace(KeyType && key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -98,7 +101,7 @@ template , return {it, false}; } } - append(std::forward(key), std::forward(t)); + append(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 2adc9978f..fe3547ab2 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -26401,7 +26401,9 @@ template , return *this; } - std::pair emplace(const key_type& key, T&& t) + template::value, int> = 0> + std::pair emplace(const key_type& key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -26410,13 +26412,14 @@ template , return {it, false}; } } - append(key, std::forward(t)); + append(key, std::forward(t)); return {std::prev(this->end()), true}; } - template::value, int> = 0> - std::pair emplace(KeyType && key, T && t) + template, + detail::is_constructible>::value, int> = 0> + std::pair emplace(KeyType && key, V && t) { for (auto it = this->begin(); it != this->end(); ++it) { @@ -26425,7 +26428,7 @@ template , return {it, false}; } } - append(std::forward(key), std::forward(t)); + append(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 98b6fa0d1..dce3f61a5 100644 --- a/tests/src/unit-ordered_map.cpp +++ b/tests/src/unit-ordered_map.cpp @@ -403,6 +403,79 @@ 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"); + } + } } TEST_CASE("ordered_map growth")