From 7bb0e4166b1fee0fb12d7a2290807f7542e9dce6 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 15 Oct 2026 08:30:18 +0200 Subject: [PATCH] Construct the JSON Patch exceptions out of line patch_inplace() built its error messages inline. In particular get_value() concatenated the context-sensitive "operation 'x'" string on every call, even when nothing was wrong. The patch exceptions now come from NEVER_INLINE helpers: the index and key errors reuse basic_json's existing throw_array_index_out_of_range/throw_key_not_found, and the seven patch-specific messages get one helper each in detail/exceptions.hpp. Ids and messages are unchanged, and unit-throw_messages pins them (with and without JSON_DIAGNOSTICS). citm_catalog patch: -6.6 % (Apple clang), -9.5 % (GCC 16). The other remaining throw sites (parser::syntax_error, binary_writer, serializer, input_adapters, enum_key_duplicate, format_error) were prototyped and measured: no gain beyond noise, so they stay as they are. Signed-off-by: Niels Lohmann --- include/nlohmann/detail/exceptions.hpp | 59 ++++++++++++++++++ include/nlohmann/json.hpp | 27 ++++---- single_include/nlohmann/json.hpp | 86 +++++++++++++++++++++----- tests/src/unit-throw_messages.cpp | 41 ++++++++++++ 4 files changed, 181 insertions(+), 32 deletions(-) diff --git a/include/nlohmann/detail/exceptions.hpp b/include/nlohmann/detail/exceptions.hpp index 1d5ff0db7..b2b5f36dd 100644 --- a/include/nlohmann/detail/exceptions.hpp +++ b/include/nlohmann/detail/exceptions.hpp @@ -463,6 +463,65 @@ JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_literal(const exceptio JSON_THROW(ExceptionType::create(id_, what_arg, context)); } +// JSON Patch errors; constructed out of line so that patch_inplace() does not carry them + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_add_parent(const BasicJsonType& parent) +{ + static_cast(parent); + JSON_THROW(out_of_range::create(exception_id::patch_add_parent_not_container, concat("cannot add value: the JSON Patch 'add' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_remove_parent(const BasicJsonType& parent) +{ + static_cast(parent); + JSON_THROW(out_of_range::create(exception_id::patch_remove_parent_not_container, concat("cannot remove value: the JSON Patch 'remove' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_not_an_array(const BasicJsonType& j) +{ + static_cast(j); + JSON_THROW(parse_error::create(exception_id::patch_not_an_array, 0, "JSON patch must be an array of objects", &j)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_missing_member(const StringType& op, const StringType& member, const bool string_type, const BasicJsonType& val) +{ + static_cast(op); + static_cast(member); + static_cast(string_type); + static_cast(val); + // context-sensitive error message + const std::string error_msg = (op == "op") ? std::string("operation") : concat("operation '", op, '\''); + JSON_THROW(parse_error::create(exception_id::patch_invalid_operation, 0, concat(error_msg, string_type ? " must have string member '" : " must have member '", member, "'"), &val)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_move_into_child(const StringType& from_path, const StringType& path, const BasicJsonType& result) +{ + static_cast(from_path); + static_cast(path); + static_cast(result); + JSON_THROW(out_of_range::create(exception_id::patch_move_into_child, concat("cannot move value: 'from' path '", from_path, "' is a proper prefix of 'path' '", path, "'"), &result)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_test_failed(const BasicJsonType& val) +{ + static_cast(val); + JSON_THROW(other_error::create(exception_id::patch_test_failed, concat("unsuccessful: ", val.dump()), &val)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_invalid_operation(const StringType& op, const BasicJsonType& val) +{ + static_cast(op); + static_cast(val); + JSON_THROW(parse_error::create(exception_id::patch_invalid_operation, 0, concat("operation value '", op, "' is invalid"), &val)); +} + } // namespace detail NLOHMANN_JSON_NAMESPACE_END diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 2e9553f94..9e05349d2 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -6954,7 +6954,7 @@ public: if (JSON_HEDLEY_UNLIKELY(idx > parent.size())) { // avoid undefined behavior - JSON_THROW(out_of_range::create(detail::exception_id::array_index_out_of_range, detail::concat("array index ", std::to_string(idx), " is out of range"), &parent)); + throw_array_index_out_of_range(idx, parent); } // default case: insert add offset @@ -6973,7 +6973,7 @@ public: case value_t::binary: case value_t::discarded: default: - JSON_THROW(out_of_range::create(detail::exception_id::patch_add_parent_not_container, detail::concat("cannot add value: the JSON Patch 'add' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); + detail::throw_patch_add_parent(parent); } }; @@ -6997,7 +6997,7 @@ public: } else { - JSON_THROW(out_of_range::create(detail::exception_id::key_not_found, detail::concat("key '", last_path, "' not found"), this)); + throw_key_not_found(last_path, *this); } } else if (parent.is_array()) @@ -7009,7 +7009,7 @@ public: { // the parent of a "remove" target must be an object or array // (see #5396) - JSON_THROW(out_of_range::create(detail::exception_id::patch_remove_parent_not_container, detail::concat("cannot remove value: the JSON Patch 'remove' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); + detail::throw_patch_remove_parent(parent); } }; @@ -7040,7 +7040,7 @@ public: // type check: top level value must be an array if (JSON_HEDLEY_UNLIKELY(!json_patch.is_array())) { - JSON_THROW(parse_error::create(detail::exception_id::patch_not_an_array, 0, "JSON patch must be an array of objects", &json_patch)); + detail::throw_patch_not_an_array(json_patch); } // iterate and apply the operations @@ -7054,21 +7054,16 @@ public: // find value auto it = val.m_data.m_value.object->find(member); - // context-sensitive error message - const auto error_msg = (op == "op") ? "operation" : detail::concat("operation '", op, '\''); // NOLINT(bugprone-unused-local-non-trivial-variable) - // check if the desired value is present if (JSON_HEDLEY_UNLIKELY(it == val.m_data.m_value.object->end())) { - // NOLINTNEXTLINE(performance-inefficient-string-concatenation) - JSON_THROW(parse_error::create(detail::exception_id::patch_invalid_operation, 0, detail::concat(error_msg, " must have member '", member, "'"), &val)); + detail::throw_patch_missing_member(op, member, false, val); } // check if the result is of type string if (JSON_HEDLEY_UNLIKELY(string_type && !it->second.is_string())) { - // NOLINTNEXTLINE(performance-inefficient-string-concatenation) - JSON_THROW(parse_error::create(detail::exception_id::patch_invalid_operation, 0, detail::concat(error_msg, " must have string member '", member, "'"), &val)); + detail::throw_patch_missing_member(op, member, true, val); } // no error: return value @@ -7078,7 +7073,7 @@ public: // type check: every element of the array must be an object if (JSON_HEDLEY_UNLIKELY(!val.is_object())) { - JSON_THROW(parse_error::create(detail::exception_id::patch_not_an_array, 0, "JSON patch must be an array of objects", &val)); + detail::throw_patch_not_an_array(val); } // collect mandatory members @@ -7114,7 +7109,7 @@ public: if (JSON_HEDLEY_UNLIKELY(is_proper_prefix(from_ptr, ptr))) { - JSON_THROW(out_of_range::create(detail::exception_id::patch_move_into_child, detail::concat("cannot move value: 'from' path '", from_path, "' is a proper prefix of 'path' '", path, "'"), &result)); + detail::throw_patch_move_into_child(from_path, path, result); } // the "from" location must exist - use at() @@ -7160,7 +7155,7 @@ public: // throw an exception if the test fails if (JSON_HEDLEY_UNLIKELY(!success)) { - JSON_THROW(other_error::create(detail::exception_id::patch_test_failed, detail::concat("unsuccessful: ", val.dump()), &val)); + detail::throw_patch_test_failed(val); } break; @@ -7171,7 +7166,7 @@ public: { // op must be "add", "remove", "replace", "move", "copy", or // "test" - JSON_THROW(parse_error::create(detail::exception_id::patch_invalid_operation, 0, detail::concat("operation value '", op, "' is invalid"), &val)); + detail::throw_patch_invalid_operation(op, val); } } } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index e56d665c5..4fb66c846 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5672,6 +5672,65 @@ JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_literal(const exceptio JSON_THROW(ExceptionType::create(id_, what_arg, context)); } +// JSON Patch errors; constructed out of line so that patch_inplace() does not carry them + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_add_parent(const BasicJsonType& parent) +{ + static_cast(parent); + JSON_THROW(out_of_range::create(exception_id::patch_add_parent_not_container, concat("cannot add value: the JSON Patch 'add' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_remove_parent(const BasicJsonType& parent) +{ + static_cast(parent); + JSON_THROW(out_of_range::create(exception_id::patch_remove_parent_not_container, concat("cannot remove value: the JSON Patch 'remove' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_not_an_array(const BasicJsonType& j) +{ + static_cast(j); + JSON_THROW(parse_error::create(exception_id::patch_not_an_array, 0, "JSON patch must be an array of objects", &j)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_missing_member(const StringType& op, const StringType& member, const bool string_type, const BasicJsonType& val) +{ + static_cast(op); + static_cast(member); + static_cast(string_type); + static_cast(val); + // context-sensitive error message + const std::string error_msg = (op == "op") ? std::string("operation") : concat("operation '", op, '\''); + JSON_THROW(parse_error::create(exception_id::patch_invalid_operation, 0, concat(error_msg, string_type ? " must have string member '" : " must have member '", member, "'"), &val)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_move_into_child(const StringType& from_path, const StringType& path, const BasicJsonType& result) +{ + static_cast(from_path); + static_cast(path); + static_cast(result); + JSON_THROW(out_of_range::create(exception_id::patch_move_into_child, concat("cannot move value: 'from' path '", from_path, "' is a proper prefix of 'path' '", path, "'"), &result)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_test_failed(const BasicJsonType& val) +{ + static_cast(val); + JSON_THROW(other_error::create(exception_id::patch_test_failed, concat("unsuccessful: ", val.dump()), &val)); +} + +template +JSON_HEDLEY_NO_RETURN JSON_HEDLEY_NEVER_INLINE void throw_patch_invalid_operation(const StringType& op, const BasicJsonType& val) +{ + static_cast(op); + static_cast(val); + JSON_THROW(parse_error::create(exception_id::patch_invalid_operation, 0, concat("operation value '", op, "' is invalid"), &val)); +} + } // namespace detail NLOHMANN_JSON_NAMESPACE_END @@ -36710,7 +36769,7 @@ public: if (JSON_HEDLEY_UNLIKELY(idx > parent.size())) { // avoid undefined behavior - JSON_THROW(out_of_range::create(detail::exception_id::array_index_out_of_range, detail::concat("array index ", std::to_string(idx), " is out of range"), &parent)); + throw_array_index_out_of_range(idx, parent); } // default case: insert add offset @@ -36729,7 +36788,7 @@ public: case value_t::binary: case value_t::discarded: default: - JSON_THROW(out_of_range::create(detail::exception_id::patch_add_parent_not_container, detail::concat("cannot add value: the JSON Patch 'add' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); + detail::throw_patch_add_parent(parent); } }; @@ -36753,7 +36812,7 @@ public: } else { - JSON_THROW(out_of_range::create(detail::exception_id::key_not_found, detail::concat("key '", last_path, "' not found"), this)); + throw_key_not_found(last_path, *this); } } else if (parent.is_array()) @@ -36765,7 +36824,7 @@ public: { // the parent of a "remove" target must be an object or array // (see #5396) - JSON_THROW(out_of_range::create(detail::exception_id::patch_remove_parent_not_container, detail::concat("cannot remove value: the JSON Patch 'remove' target's parent is of type ", parent.type_name(), ", but must be an object or array"), &parent)); + detail::throw_patch_remove_parent(parent); } }; @@ -36796,7 +36855,7 @@ public: // type check: top level value must be an array if (JSON_HEDLEY_UNLIKELY(!json_patch.is_array())) { - JSON_THROW(parse_error::create(detail::exception_id::patch_not_an_array, 0, "JSON patch must be an array of objects", &json_patch)); + detail::throw_patch_not_an_array(json_patch); } // iterate and apply the operations @@ -36810,21 +36869,16 @@ public: // find value auto it = val.m_data.m_value.object->find(member); - // context-sensitive error message - const auto error_msg = (op == "op") ? "operation" : detail::concat("operation '", op, '\''); // NOLINT(bugprone-unused-local-non-trivial-variable) - // check if the desired value is present if (JSON_HEDLEY_UNLIKELY(it == val.m_data.m_value.object->end())) { - // NOLINTNEXTLINE(performance-inefficient-string-concatenation) - JSON_THROW(parse_error::create(detail::exception_id::patch_invalid_operation, 0, detail::concat(error_msg, " must have member '", member, "'"), &val)); + detail::throw_patch_missing_member(op, member, false, val); } // check if the result is of type string if (JSON_HEDLEY_UNLIKELY(string_type && !it->second.is_string())) { - // NOLINTNEXTLINE(performance-inefficient-string-concatenation) - JSON_THROW(parse_error::create(detail::exception_id::patch_invalid_operation, 0, detail::concat(error_msg, " must have string member '", member, "'"), &val)); + detail::throw_patch_missing_member(op, member, true, val); } // no error: return value @@ -36834,7 +36888,7 @@ public: // type check: every element of the array must be an object if (JSON_HEDLEY_UNLIKELY(!val.is_object())) { - JSON_THROW(parse_error::create(detail::exception_id::patch_not_an_array, 0, "JSON patch must be an array of objects", &val)); + detail::throw_patch_not_an_array(val); } // collect mandatory members @@ -36870,7 +36924,7 @@ public: if (JSON_HEDLEY_UNLIKELY(is_proper_prefix(from_ptr, ptr))) { - JSON_THROW(out_of_range::create(detail::exception_id::patch_move_into_child, detail::concat("cannot move value: 'from' path '", from_path, "' is a proper prefix of 'path' '", path, "'"), &result)); + detail::throw_patch_move_into_child(from_path, path, result); } // the "from" location must exist - use at() @@ -36916,7 +36970,7 @@ public: // throw an exception if the test fails if (JSON_HEDLEY_UNLIKELY(!success)) { - JSON_THROW(other_error::create(detail::exception_id::patch_test_failed, detail::concat("unsuccessful: ", val.dump()), &val)); + detail::throw_patch_test_failed(val); } break; @@ -36927,7 +36981,7 @@ public: { // op must be "add", "remove", "replace", "move", "copy", or // "test" - JSON_THROW(parse_error::create(detail::exception_id::patch_invalid_operation, 0, detail::concat("operation value '", op, "' is invalid"), &val)); + detail::throw_patch_invalid_operation(op, val); } } } diff --git a/tests/src/unit-throw_messages.cpp b/tests/src/unit-throw_messages.cpp index bf19d45f2..1c2289626 100644 --- a/tests/src/unit-throw_messages.cpp +++ b/tests/src/unit-throw_messages.cpp @@ -11,6 +11,8 @@ #include using nlohmann::json; +#include + // The exceptions in hot core functions are constructed out of line. These tests pin the // exact what() texts of the sites that moved, so the refactoring cannot change them. @@ -91,4 +93,43 @@ TEST_CASE("exception messages of out-of-line throw sites") CHECK_MSG(obj.insert(arr.begin(), arr.end()), "[json.exception.invalid_iterator.202] iterators first and last must point to objects"); CHECK_MSG(obj.update(arr.begin(), arr2.end()), "[json.exception.invalid_iterator.210] iterators do not fit"); } + + SECTION("patch") + { + const auto patch_msg = [](const char* doc, const char* patch) + { + try + { + json d = json::parse(doc); + d.patch(json::parse(patch)); + } + catch (const json::exception& e) + { + // with JSON_DIAGNOSTICS, what() names the offending value like "(/0) "; drop it + std::string msg = e.what(); + const auto pos = msg.find(" (/"); + if (pos != std::string::npos) + { + msg.erase(pos + 1, msg.find(") ", pos) - pos + 1); + } + return msg; + } + return std::string(); + }; + + CHECK(patch_msg("{}", "{}") == "[json.exception.parse_error.104] parse error: JSON patch must be an array of objects"); + CHECK(patch_msg("{}", "[1]") == "[json.exception.parse_error.104] parse error: JSON patch must be an array of objects"); + CHECK(patch_msg("{}", "[{}]") == "[json.exception.parse_error.105] parse error: operation must have member 'op'"); + CHECK(patch_msg("{}", "[{\"op\":1}]") == "[json.exception.parse_error.105] parse error: operation must have string member 'op'"); + CHECK(patch_msg("{}", "[{\"op\":\"add\"}]") == "[json.exception.parse_error.105] parse error: operation 'add' must have member 'path'"); + CHECK(patch_msg("{}", "[{\"op\":\"add\",\"path\":1}]") == "[json.exception.parse_error.105] parse error: operation 'add' must have string member 'path'"); + CHECK(patch_msg("{}", "[{\"op\":\"foo\",\"path\":\"\"}]") == "[json.exception.parse_error.105] parse error: operation value 'foo' is invalid"); + CHECK(patch_msg("{\"a\":1}", "[{\"op\":\"remove\",\"path\":\"/b\"}]") == "[json.exception.out_of_range.403] key 'b' not found"); + CHECK(patch_msg("[1]", "[{\"op\":\"add\",\"path\":\"/5\",\"value\":0}]") == "[json.exception.out_of_range.401] array index 5 is out of range"); + CHECK(patch_msg("{\"a\":1}", "[{\"op\":\"add\",\"path\":\"/a/b\",\"value\":0}]") == "[json.exception.out_of_range.411] cannot add value: the JSON Patch 'add' target's parent is of type number, but must be an object or array"); + CHECK(patch_msg("{\"a\":1}", "[{\"op\":\"remove\",\"path\":\"/a/b\"}]") == "[json.exception.out_of_range.413] cannot remove value: the JSON Patch 'remove' target's parent is of type number, but must be an object or array"); + CHECK(patch_msg("{\"a\":{}}", "[{\"op\":\"move\",\"from\":\"/a\",\"path\":\"/a/b\"}]") == "[json.exception.out_of_range.414] cannot move value: 'from' path '/a' is a proper prefix of 'path' '/a/b'"); + CHECK(patch_msg("{\"a\":1}", "[{\"op\":\"test\",\"path\":\"/a\",\"value\":2}]") == "[json.exception.other_error.501] unsuccessful: {\"op\":\"test\",\"path\":\"/a\",\"value\":2}"); + CHECK(patch_msg("{\"a\":1}", "[{\"op\":\"move\",\"path\":\"/a\"}]") == "[json.exception.parse_error.105] parse error: operation 'move' must have member 'from'"); + } }