From 6f2048cd5d609c055742d91539457567a463cb4d Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sun, 4 Oct 2026 11:46:25 +0200 Subject: [PATCH] Accept lvalues in ordered_map::emplace's value parameter (#5685) * Accept lvalues in ordered_map::emplace's value parameter ordered_map::emplace(key, value) took the mapped value only by T&&, an rvalue reference rather than a forwarding reference, so ordered_json::emplace("a", value) failed to compile whenever value was an lvalue or a const lvalue, even though the same call compiles for json (whose object_t is std::map, with a variadic emplace). Turn the value parameter into a separately-deduced forwarding reference, constrained with std::is_constructible so the overloads still only accept something convertible to the mapped type. std::map-compatible semantics are unchanged: emplace still does nothing if the key already exists. Open PR #5609 also touches ordered_map.hpp (moving values on vector growth); this change only touches the two emplace() overloads and should not conflict. Fixes #5673. Signed-off-by: Niels Lohmann * Avoid astyle's padding in ordered_map::emplace's template headers Use detail::conjunction instead of && and drop the redundant V&& in detail::is_constructible, so astyle keeps the usual template formatting. Addresses review comment by @gregmarr. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- docs/mkdocs/docs/api/basic_json/emplace.md | 2 + include/nlohmann/ordered_map.hpp | 15 +++-- single_include/nlohmann/json.hpp | 15 +++-- tests/src/unit-ordered_json.cpp | 41 ++++++++++++ tests/src/unit-ordered_map.cpp | 73 ++++++++++++++++++++++ 5 files changed, 134 insertions(+), 12 deletions(-) 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")