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 <mail@nlohmann.me>

* 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 <mail@nlohmann.me>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

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}");
+    }
+}

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<std::string, std::string> 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")