Let to_json(std::optional<T>) propagate exceptions from T's to_json

The overload was marked noexcept even though its body assigns *opt to
the JSON value, which calls T's to_json (or allocates for std::string,
std::vector, or json). Any exception from there -- a user-defined
to_json reporting an error, or std::bad_alloc -- called std::terminate()
instead of propagating. The noexcept also made basic_json's converting
constructor noexcept(true) for std::optional<T>, so json j = opt; could
not report the error either.

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

include/nlohmann/detail/conversions/to_json.hpp

@@ -287,7 +287,7 @@ struct external_constructor<value_t::object>
 #ifdef JSON_HAS_CPP_17
 template<typename BasicJsonType, typename T,
          enable_if_t<std::is_constructible<BasicJsonType, T>::value, int> = 0>
-void to_json(BasicJsonType& j, const std::optional<T>& opt) noexcept
+void to_json(BasicJsonType& j, const std::optional<T>& opt)
 {
     if (opt.has_value())
     {

include/nlohmann/json.hpp

@@ -6187,14 +6187,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
                     }
                 }
 
-                // Only an object type that keeps its members in insertion
-                // order, such as nlohmann::ordered_map, can need reordering:
-                // patch() appends a new member at the end of such an object.
-                // Any other object type places its members itself - std::map
-                // in key order, a hash map in an order its operator== ignores -
-                // so a member-by-member diff always reproduces target there.
-                if (!detail::is_ordered_map<object_t>::value
-                        || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix))
+                if (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)
                 {
                     // fast path: order of common keys already matches (or the
                     // object_t's iteration order does not depend on

single_include/nlohmann/json.hpp

@@ -6816,7 +6816,7 @@ struct external_constructor<value_t::object>
 #ifdef JSON_HAS_CPP_17
 template<typename BasicJsonType, typename T,
          enable_if_t<std::is_constructible<BasicJsonType, T>::value, int> = 0>
-void to_json(BasicJsonType& j, const std::optional<T>& opt) noexcept
+void to_json(BasicJsonType& j, const std::optional<T>& opt)
 {
     if (opt.has_value())
     {
@@ -32268,14 +32268,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
                     }
                 }
 
-                // Only an object type that keeps its members in insertion
-                // order, such as nlohmann::ordered_map, can need reordering:
-                // patch() appends a new member at the end of such an object.
-                // Any other object type places its members itself - std::map
-                // in key order, a hash map in an order its operator== ignores -
-                // so a member-by-member diff always reproduces target there.
-                if (!detail::is_ordered_map<object_t>::value
-                        || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix))
+                if (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)
                 {
                     // fast path: order of common keys already matches (or the
                     // object_t's iteration order does not depend on

tests/src/unit-conversions.cpp

@@ -1826,6 +1826,16 @@ TEST_CASE("std::u8string")
 #endif
 #endif
 
+// a type whose to_json reports an error by throwing, used below to check that
+// converting a std::optional<T> to JSON propagates an exception thrown while
+// converting its contained value instead of calling std::terminate (#5642)
+struct throwing_to_json_type {};
+
+void to_json(json&, const throwing_to_json_type&)
+{
+    throw std::runtime_error("cannot serialize throwing_to_json_type");
+}
+
 TEST_CASE("std::optional")
 {
     SECTION("null")
@@ -1908,6 +1918,19 @@ TEST_CASE("std::optional")
         CHECK(json(opt_object) == j_object);
         CHECK(std::map<std::string, std::optional<int>>(j_object) == opt_object);
     }
+
+#if !defined(JSON_NOEXCEPTION)
+    SECTION("exception from contained value's to_json propagates (#5642)")
+    {
+        // to_json(BasicJsonType&, const std::optional<T>&) must not be
+        // noexcept: it calls T's to_json, which may throw (a user-defined
+        // to_json that reports an error, or std::bad_alloc for T =
+        // std::string/vector/json). Before the fix, this called
+        // std::terminate() instead of letting the exception propagate.
+        const std::optional<throwing_to_json_type> opt = throwing_to_json_type{};
+        CHECK_THROWS_WITH_AS(json(opt), "cannot serialize throwing_to_json_type", std::runtime_error&);
+    }
+#endif
 }
 #endif
 

tests/src/unit-json_patch.cpp

@@ -1752,58 +1752,6 @@ TEST_CASE("JSON patch - diff emits array removals in descending index order")
     }
 }
 
-TEST_CASE("JSON patch - diff() takes the fast path for non-reorderable object types (regression #5639)")
-{
-    // #5465 added an order check to diff()'s object handling so a
-    // member-by-member diff is only used when it would also reproduce
-    // target's member *order* -- needed for ordered_json, whose object_t
-    // keeps insertion order and whose patch() "add" op appends a new
-    // member at the end. For json's default object_t (std::map, which
-    // orders members by key regardless of insertion history), that check
-    // could still fail: a new key that sorts before an existing common key
-    // makes target's iteration interleave the new key between common keys,
-    // even though nothing else about the object changed. That sent the
-    // whole object through the slow (remove-every-member,
-    // re-add-every-member) path instead of the minimal one.
-    SECTION("json: added key sorts before an existing common key")
-    {
-        const json source = {{"a", 1}, {"c", {{"x", 1}, {"y", 2}}}};
-        const json target = {{"a", 1}, {"b", 0}, {"c", {{"x", 1}, {"y", 2}}}};
-
-        const json patch = json::diff(source, target);
-
-        // only the new key is added; "a" and "c" are left alone instead of
-        // being removed and re-added
-        const json expected = R"([{"op": "add", "path": "/b", "value": 0}])"_json;
-        CHECK(patch == expected);
-        CHECK(source.patch(patch) == target);
-    }
-
-    SECTION("ordered_json: reordering behavior from #5465 is unchanged")
-    {
-        using nlohmann::ordered_json;
-
-        // same key/value shape as the json case above, but for ordered_json
-        // the *target*'s member order must be reproduced, so the slow path
-        // is still required here.
-        ordered_json source;
-        source["a"] = 1;
-        source["c"] = ordered_json{{"x", 1}, {"y", 2}};
-
-        ordered_json target;
-        target["a"] = 1;
-        target["b"] = 0;
-        target["c"] = ordered_json{{"x", 1}, {"y", 2}};
-
-        const ordered_json patch = ordered_json::diff(source, target);
-
-        // unlike the json case: every member is still removed and re-added
-        // so the result ends up in target's order (2 removes + 3 adds)
-        CHECK(patch.size() == 5);
-        CHECK(source.patch(patch) == target);
-    }
-}
-
 TEST_CASE("JSON patch - every operation on ordered_json")
 {
     using nlohmann::ordered_json;