From a0b71e272006c400703daeb2f4dc3248e47ddb01 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sun, 4 Oct 2026 11:57:41 +0200 Subject: [PATCH] Deduplicate basic_json internals; make insert(pos, json&&) move (#5727) * Remove unused private aliases from basic_json The private aliases primitive_iterator_t, internal_iterator and output_adapter_t are not used anywhere: iter_impl, binary_writer and the tests refer to the detail:: names directly. As the aliases are private, no user or derived class can depend on them. The internal_iterator.hpp include stays because iter_impl needs it. Part of #5724 Signed-off-by: Niels Lohmann * Fix meta()'s dead, syntactically invalid HP aCC branch The HP aCC branch of basic_json::meta() was missing a semicolon and has therefore never compiled; adding only a semicolon would also make it throw type_error.305, since it assigned a plain string to result["compiler"] and then indexed into it like the other branches do into an object. Make the branch consistent with the others by assigning an object with "family" and "version" keys, narrow the condition to __HP_aCC (a C compiler cannot build this header-only library), and fix meta.md, which documented the old (impossible) plain-string behavior. Part of #5724 Signed-off-by: Niels Lohmann * Stop the noexcept null constructor delegating to a throwing one basic_json(std::nullptr_t) delegated to basic_json(value_t), whose underlying json_value(value_t) constructor allocates for other types and can therefore throw, which is why the noexcept had a NOLINT(bugprone-exception-escape). The delegated-to constructor also called assert_invariant() a second time. The default member initializers of data already produce the same null state (a value-initialized, i.e. zeroed, union with object == nullptr), so the delegation and its NOLINT can simply be dropped. Part of #5724 Signed-off-by: Niels Lohmann * Remove four cppcheck accessForwarded suppressions in the move constructor basic_json(basic_json&&) built its base subobject with std::forward(other), so cppcheck saw the whole of other as forwarded and flagged every subsequent access to it as accessForwarded, three of them still marked "TODO check". Only the base subobject is actually moved from; cast explicitly to the base type instead, the way ordered_map already does, so cppcheck can tell the two are unrelated. Behavior is unchanged: for a non-reference T, std::forward(x) is defined as static_cast(x). Part of #5724 Signed-off-by: Niels Lohmann * Remove stale cppcheck suppressions and name the local parser in parse() Running the pinned cppcheck (ci_cppcheck's invocation) without --inline-suppr across all configurations reports no syntaxError, no ignoredReturnValue and no assertWithSideEffect, so the corresponding suppressions in json_fwd.hpp, string_concat.hpp and assert_invariant() no longer match anything (json_fwd.hpp's is kept, since downstream users who run an older cppcheck against it could still hit the warning it once silenced). The three basic_json::parse() overloads still trigger a false-positive accessMoved/accessForwarded because they build a temporary parser and call .parse() on it in the same expression; giving that parser a name makes the warning go away without changing behavior, and removes the last of the inline suppressions on these functions. Part of #5724 Signed-off-by: Niels Lohmann * Deduplicate the string/binary cleanup in the two erase() overloads erase(pos) and erase(first, last) each carried a byte-identical 14-line block that destroys and deallocates a string or binary value before resetting the type to null. That reimplements the string/binary cases of json_value::destroy(), so any future change to how those values are freed would have to be made in three places instead of one. Both overloads now just call destroy() and reset the union; for the other primitive types (boolean, numbers) destroy() is a no-op, so behavior is unchanged. Also fix erase(first, last)'s error-path branch hint, which used JSON_HEDLEY_LIKELY where erase(pos), the iterator-range constructor, and every other error path in the class use JSON_HEDLEY_UNLIKELY. This only affects code layout, not semantics. Part of #5724 Signed-off-by: Niels Lohmann * Fix stale and copy-pasted comments in basic_json Several comments no longer match the code: the class invariant and assert_invariant()'s doc still named the members m_value/m_type (now m_data.m_value/m_data.m_type) and did not mention the binary invariant that assert_invariant() already checks; the json_value note and the get() @tparam list omitted binary_t even though binary is a variable-length, pointer-stored type like the others; the key-based value() overload's brief said "via JSON Pointer", which is the other overload; and swap(binary_t&)/swap(binary_t::container_type&) both carried "swap only works for strings", copied from swap(string_t&). Comment-only change; behavior, the public API and the ABI are unchanged. The private get_impl() doxygen and the emplace() comments that border #5585's hunk are intentionally left alone. Part of #5724 Signed-off-by: Niels Lohmann * Deduplicate the 16 copied from_cbor/msgpack/ubjson/bjdata/bon8/bson bodies Each of the 16 binary deserialization overloads (from_cbor, from_msgpack, from_ubjson, from_bjdata, from_bon8, from_bson, each in an InputType&& and an iterator/sentinel version, plus the deprecated span overloads of from_cbor/from_msgpack/from_ubjson/from_bson) had the same body, differing only in the input_format_t value. Every copy built a temporary binary_reader from std::move(ia) and called sax_parse on it in the same expression, which also produced a false-positive cppcheck accessMoved on all 16 lines and needed a NOLINTNEXTLINE(hicpp-move-const-arg,performance-move-const-arg) on the four span overloads. Add a private from_binary_impl() helper that builds the reader as a named local instead, and make each of the 16 overloads a one-line forward to it. All public signatures, default arguments, JSON_HEDLEY_WARN_UNUSED_RESULT and JSON_HEDLEY_DEPRECATED_FOR attributes are unchanged, tag_handler keeps defaulting to cbor_tag_handler_t::error for the non-CBOR formats (matching binary_reader::sax_parse's own default), and the helper is placed in the existing private section before the binary section banner rather than between the from_* overloads, so from_binary_impl() itself does not collide with #5688's insertion point. Collapsing the from_bjdata/from_bon8 bodies into one-line forwards does rewrite the "return result; }" context lines that #5688 inserts its two deprecated overloads after, so that PR will need a small manual rebase (reinserting its overloads after the new one-line bodies) rather than applying cleanly. Part of #5724 Signed-off-by: Niels Lohmann * Make insert(pos, basic_json&&) move its argument instead of copying it insert(const_iterator pos, basic_json&& val) delegated to insert(pos, val), but val is a named rvalue reference, so inside the function it is an lvalue: the call always resolved to insert(const_iterator, const basic_json&) and deep-copied the value. This has been the case since the overload was introduced, in every release. push_back(basic_json&&), by contrast, already moves. Give the rvalue overload its own body with the same two checks (type_error.309, invalid_iterator.202), then move the argument into a local before inserting it. Moving into a local first, rather than inserting std::move(val) directly, keeps this safe even when val aliases an element of the same array (e.g. arr.insert(arr.begin(), std::move(arr[1]))), since std::vector::insert(pos, T&&) is not guaranteed to handle an argument that aliases one of its own elements. This is a deliberate, small behavior change: the moved-from argument now ends up null afterwards, the same as after push_back(&&), instead of keeping its old value unchanged. No signature changes, so the public API and ABI are unaffected. Add unit-modifiers coverage for the moved-from state and for self-aliasing insertion, both with and without reallocation of the underlying array. Part of #5724 Signed-off-by: Niels Lohmann * Deduplicate object key lookup and checked at() access at()/find()/count()/contains()/const operator[]/erase_internal() each repeated the raw object lookup (m_value.object->find(key)), and the six at() overloads additionally repeated the type_error.304 check and out_of_range.401/403 throw. Route them all through two new private helpers, object_lookup()/object_at() (plus array_at() for the index overloads of at()), templated on the constness of the receiver so one body serves both the const and non-const overload. count() is left untouched, since it already goes through object_t::count() rather than a second find(). The at(KeyType&&) overloads used to forward the same key twice: once into object->find() and again, on the not-found path, into the string_t() conversion for the exception message. object_at() now forwards it only into the lookup and reuses the (unmoved) key for the message. clang-tidy 22 (Docker silkeh/clang:22) still flags that reuse under bugprone-use-after-move/hicpp-invalid-access-moved even with the single forward, since it cannot see that object_t::find() (a plain std::map or ordered_map) never actually moves from its argument; add a NOLINTNEXTLINE with that reasoning rather than avoid the pattern. No signature, exception id/message, or set_parent() behavior changes. Overlaps #5689, #5705, #5606, #5687 and #5585, which touch the same hunks; whichever of this commit and those PRs lands second will need a small rebase. Signed-off-by: Niels Lohmann * Deduplicate the lookup/default/throw body of value() The six non-deprecated value() overloads each held a full copy of the same body: the four key-based overloads looked up the key and either returned the found element converted to the requested type or the default value (throwing type_error.306 if this is not an object), and the two json_pointer overloads did the same via ptr.get_checked_or_null(), throwing type_error.306 unless is_structured(). Replace the duplicated bodies with two private helpers, value_member() and value_pointee(), that return a const basic_json* (null when not found) and do the type check/throw once each. Every value() overload now just picks between the found pointer's get() and the default. Same signatures, template parameters, SFINAE conditions, exception id, message and this context on every overload. Overlaps #5689 (routes find() through lookup_key()) and #5705 (adds a deleted integral-key value() next to these overloads); whichever of this commit and those PRs lands second will need a small rebase. #5724 item 4 Signed-off-by: Niels Lohmann * Deduplicate the null-to-container conversion into convert_null_to() Nine sites wrote out the same "turn a null value into an empty array or object" logic with two different idioms: operator[](size_type), operator[](key_type), operator[](KeyType&&) and update() set m_type then assigned m_value.array/object directly via create(), while the three push_back() overloads, emplace_back() and emplace() set m_type then assigned m_value = value_t::array/object (going through json_value's converting constructor and a temporary). Both idioms end up calling create() and produce the same state, just via a different path; both also share a latent exception-safety bug, since m_type is written before the (possibly throwing) allocation, so a throwing allocator leaves m_type == array/object with a null pointer behind it, violating the class invariant and crashing on the next access to, or destruction of, the value. Add a private convert_null_to(value_t) helper and call it from all nine sites. Unlike the idioms it replaces, it allocates the container first and only then writes m_type, so a throwing allocation leaves the value as a valid null instead of a mistyped, half-constructed one; verified with a throwing allocator (see unit-allocator.cpp's bad_allocator) that j["x"] = ... on a null j now stays null, and no longer trips assert_invariant()/crashes, when create() throws. Same allocator usage and assert_invariant() call as before, otherwise. Overlaps #5585, which reorders these same nine blocks for exception safety; whichever of this commit and that PR lands second will need a small rebase. Signed-off-by: Niels Lohmann * Deduplicate the linear key search in ordered_map emplace, at, erase(key), count and find each repeated the same "for (auto it = begin(); it != end(); ++it) if (m_compare(it->first, key)) ..." loop (15 copies across their key_type and transparent KeyType&& overloads), and both erase(key) overloads additionally repeated the exception-sensitive in-place reconstruction (destroy, placement-new, pop_back) used to remove an element while keeping the const Key non-movable. Add two private helpers: find_impl(Self&, KeyType&&), a static member template that runs the search once for either constness of the receiver, and erase_at(iterator), which keeps the existing pop_back-based reconstruction instead of switching to erase()/resize() (which would add a DefaultInsertable requirement). Route find, at, count, emplace, insert(const value_type&) and both erase(key) overloads through them. Same signatures, is_usable_as_key_type constraints and exception messages/types. Overlaps #5609 and #5685, which both rewrite emplace (#5609 also touches insert and adds private members at the end of the class); whichever of this commit and those PRs lands second will need a rebase. #5724 item 6 Signed-off-by: Niels Lohmann * Re-enable bugprone-use-after-move/hicpp-invalid-access-moved These two checks (and portability-template-virtual-member-function) were disabled in #4489 (November 2024) "only removed to get the CI going". portability-template-virtual-member-function is a separate, still-open cleanup (#5725 item 3 on its own branch) and stays disabled here; this commit only re-enables the move/forward checks and cleans up what they flag on this branch. The move constructor (json.hpp) already casts to the base type instead of forwarding the whole object (#5724 item 9), so it no longer trips either check. The at(KeyType&&) double-forward this check used to flag was reduced to a single forward with the now-unforwarded reuse annotated by a NOLINTNEXTLINE in #5724 item 3's object_at() helper (clang-tidy 22 still flags that reuse even after a single forward; see that commit's message). What is left here: - from_json_inplace_array_impl(), from_json_tuple_impl_base() and the std::pair overload of from_json_tuple_impl() forwarded j into every j.at(...) call in a pack expansion or a pair of calls. at() has no ref-qualified overloads, so the forward was a no-op; call j.at(...) directly. - container_input_adapter_factory::create() forwards container twice on purpose, into begin() and end(), so both see the same value category and produce matching iterator types. Annotate it with NOLINTNEXTLINE and a comment instead of changing it. - unit-class_parser.cpp's "move constructor resets the moved-from value to npos" test still pointed at the pre-static_cast move constructor by line number and mentioned the cppcheck-suppress annotation that #5724 item 9 already removed; update the comment. No behavior change anywhere in include/. Verified with clang-tidy 22.1.8 (Docker silkeh/clang:22, --platform linux/amd64) against a TU including json.hpp with the repo's .clang-tidy: bugprone-use-after-move and hicpp-invalid-access-moved report nothing unsuppressed. Overlaps #5737 (open PR for the rest of #5725 item 3: the from_json.hpp/input_adapters.hpp cleanup above, and portability-template-virtual-member-function), which currently keeps both checks disabled pending this move-constructor change; whichever of this commit and that PR lands second will need a small rebase of .clang-tidy. Signed-off-by: Niels Lohmann * Make convert_null_to() take the container type as a template argument Passing array_t or object_t instead of a value_t makes an invalid target a compile error instead of a runtime assertion, and removes the branch. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- .clang-tidy | 9 - docs/mkdocs/docs/api/basic_json/meta.md | 2 +- .../nlohmann/detail/input/input_adapters.hpp | 2 +- include/nlohmann/detail/string_concat.hpp | 1 - include/nlohmann/json.hpp | 644 +++++++----------- include/nlohmann/ordered_map.hpp | 183 ++--- tests/src/unit-class_parser.cpp | 8 +- tests/src/unit-modifiers.cpp | 43 ++ 8 files changed, 366 insertions(+), 526 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index fa3e03ae3..a16136785 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,18 +1,9 @@ -# bugprone-use-after-move (hicpp-invalid-access-moved is its alias) still flags -# the basic_json move constructor, which forwards the whole object to its base -# class (#5724), and two forwards in the error-message construction of -# at(KeyType&&) (json.hpp, both overloads: find(std::forward(key)) -# followed by string_t(std::forward(key)) in the throw), which #5689 -# rewrites. Re-enable both checks once those changes have landed. # portability-avoid-pragma-once: kept disabled on purpose. #pragma once is accepted # by every supported compiler, and tools/amalgamate/amalgamate.py strips it from # single_include, so there is nothing left to fix here. Checks: '*, - -bugprone-use-after-move, - -hicpp-invalid-access-moved, - -altera-id-dependent-backward-branch, -altera-struct-pack-align, -altera-unroll-loops, diff --git a/docs/mkdocs/docs/api/basic_json/meta.md b/docs/mkdocs/docs/api/basic_json/meta.md index 55476c5f3..9b1ac2a4e 100644 --- a/docs/mkdocs/docs/api/basic_json/meta.md +++ b/docs/mkdocs/docs/api/basic_json/meta.md @@ -13,7 +13,7 @@ JSON object holding version information | key | description | |-------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `compiler` | Information on the used compiler. It is an object with the following keys: `c++` (the used C++ standard), `family` (the compiler family; possible values are `clang`, `icc`, `gcc`, `ilecpp`, `msvc`, `pgcpp`, `sunpro`, and `unknown`), and `version` (the compiler version). On HP aCC compilers, `compiler` is instead the plain string `hp`. | +| `compiler` | Information on the used compiler. It is an object with the following keys: `c++` (the used C++ standard), `family` (the compiler family; possible values are `clang`, `icc`, `gcc`, `hp`, `ilecpp`, `msvc`, `pgcpp`, `sunpro`, and `unknown`), and `version` (the compiler version). | | `copyright` | The copyright line for the library as string. | | `name` | The name of the library as string. | | `platform` | The used platform as string. Possible values are `win32`, `linux`, `apple`, `unix`, and `unknown`. | diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index 09f0ded14..7f2f79cbf 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -746,7 +746,7 @@ struct container_input_adapter_factory< ContainerType, { // container is forwarded twice on purpose: the resulting begin/end // iterator types must match adapter_type, computed the same way - // NOLINTNEXTLINE(bugprone-use-after-move) + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) return input_adapter(begin(std::forward(container)), end(std::forward(container))); } }; diff --git a/include/nlohmann/detail/string_concat.hpp b/include/nlohmann/detail/string_concat.hpp index a39dd5e63..74002c878 100644 --- a/include/nlohmann/detail/string_concat.hpp +++ b/include/nlohmann/detail/string_concat.hpp @@ -39,7 +39,6 @@ inline std::size_t concat_length(const char /*c*/, const Args& ... rest) template inline std::size_t concat_length(const char* cstr, const Args& ... rest) { - // cppcheck-suppress ignoredReturnValue return ::strlen(cstr) + concat_length(rest...); } diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 87e1dbc04..89ba13233 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -115,11 +115,12 @@ struct is_std_optional> : std::true_type {}; @brief a class to store JSON values @internal -@invariant The member variables @a m_value and @a m_type have the following -relationship: -- If `m_type == value_t::object`, then `m_value.object != nullptr`. -- If `m_type == value_t::array`, then `m_value.array != nullptr`. -- If `m_type == value_t::string`, then `m_value.string != nullptr`. +@invariant The member variables @a m_data.m_value and @a m_data.m_type have +the following relationship: +- If `m_data.m_type == value_t::object`, then `m_data.m_value.object != nullptr`. +- If `m_data.m_type == value_t::array`, then `m_data.m_value.array != nullptr`. +- If `m_data.m_type == value_t::string`, then `m_data.m_value.string != nullptr`. +- If `m_data.m_type == value_t::binary`, then `m_data.m_value.binary != nullptr`. The invariants are checked by member function assert_invariant(). @note ObjectType trick from https://stackoverflow.com/a/9860911 @@ -182,18 +183,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } private: - using primitive_iterator_t = ::nlohmann::detail::primitive_iterator_t; - template - using internal_iterator = ::nlohmann::detail::internal_iterator; template using iter_impl = ::nlohmann::detail::iter_impl; template using iteration_proxy = ::nlohmann::detail::iteration_proxy; template using json_reverse_iterator = ::nlohmann::detail::json_reverse_iterator; - template - using output_adapter_t = ::nlohmann::detail::output_adapter_t; - template using binary_reader = ::nlohmann::detail::binary_reader; template using binary_writer = ::nlohmann::detail::binary_writer; @@ -334,8 +329,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::to_string(__GNUC_PATCHLEVEL__)) } }; -#elif defined(__HP_cc) || defined(__HP_aCC) - result["compiler"] = "hp" +#elif defined(__HP_aCC) + result["compiler"] = {{"family", "hp"}, {"version", __HP_aCC}}; #elif defined(__IBMCPP__) result["compiler"] = {{"family", "ilecpp"}, {"version", __IBMCPP__}}; #elif defined(_MSC_VER) @@ -477,9 +472,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec binary | binary | pointer to @ref binary_t null | null | *no value is stored* - @note Variable-length types (objects, arrays, and strings) are stored as - pointers. The size of the union should not exceed 64 bits if the default - value types are used. + @note Variable-length types (objects, arrays, strings, and binary + values) are stored as pointers. The size of the union should not exceed + 64 bits if the default value types are used. @since version 1.0.0 */ @@ -731,7 +726,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec end of every constructor to make sure that created objects respect the invariant. Furthermore, it has to be called each time the type of a JSON value is changed, because the invariant expresses a relationship between - @a m_type and @a m_value. + @a m_data.m_type and @a m_data.m_value. Furthermore, the parent relation is checked for arrays and objects: If @a check_parents true and the value is an array or object, then the @@ -752,7 +747,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec #if JSON_DIAGNOSTICS JSON_TRY { - // cppcheck-suppress assertWithSideEffect JSON_ASSERT(!check_parents || !is_structured() || std::all_of(begin(), end(), [this](const basic_json & j) { return j.m_parent == this; @@ -1924,8 +1918,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @brief create a null object /// @sa https://json.nlohmann.me/api/basic_json/basic_json/ - basic_json(std::nullptr_t = nullptr) noexcept // NOLINT(bugprone-exception-escape) - : basic_json(value_t::null) + basic_json(std::nullptr_t = nullptr) noexcept { assert_invariant(); } @@ -2289,15 +2282,15 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @brief move constructor /// @sa https://json.nlohmann.me/api/basic_json/basic_json/ basic_json(basic_json&& other) noexcept - : json_base_class_t(std::forward(other)), - m_data(std::move(other.m_data)) // cppcheck-suppress[accessForwarded] TODO check + : json_base_class_t(std::move(static_cast(other))), + m_data(std::move(other.m_data)) #if JSON_DIAGNOSTIC_POSITIONS - , start_position(other.start_position) // cppcheck-suppress[accessForwarded] TODO check - , end_position(other.end_position) // cppcheck-suppress[accessForwarded] TODO check + , start_position(other.start_position) + , end_position(other.end_position) #endif { // check that the passed value is valid - other.assert_invariant(false); // cppcheck-suppress[accessForwarded] + other.assert_invariant(false); // invalidate payload other.m_data.m_type = value_t::null; @@ -2864,7 +2857,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec @tparam PointerType pointer type; must be a pointer to @ref array_t, @ref object_t, @ref string_t, @ref boolean_t, @ref number_integer_t, - @ref number_unsigned_t, or @ref number_float_t. + @ref number_unsigned_t, @ref number_float_t, or @ref binary_t. @return pointer to the internally stored JSON value if the requested pointer type @a PointerType fits to the JSON value; `nullptr` otherwise @@ -3032,6 +3025,90 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @} + private: + /// @brief look up @a key in the object held by @a j, for either constness of @a j + /// @note the single place that performs a (possibly transparent) object key lookup + template + static auto object_lookup(Self& j, KeyType&& key) + -> decltype(j.m_data.m_value.object->find(lookup_key(std::forward(key)))) + { + return j.m_data.m_value.object->find(lookup_key(std::forward(key))); + } + + /// @brief checked object element access used by the at() overloads taking a key + /// @throw type_error.304 if @a j is not an object + /// @throw out_of_range.403 if @a key is not found + template + static auto object_at(Self& j, KeyType&& key) + -> decltype((object_lookup(j, std::forward(key))->second)) + { + // at only works for objects + if (JSON_HEDLEY_UNLIKELY(!j.is_object())) + { + JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", j.type_name()), &j)); + } + + auto it = object_lookup(j, std::forward(key)); + if (it == j.m_data.m_value.object->end()) + { + // key is only forwarded into the lookup above: object_t::find() (a plain + // std::map or ordered_map) never moves from its argument, so key is still + // valid here regardless of whether KeyType was deduced as an rvalue reference + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) + JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(key), "' not found"), &j)); + } + return it->second; + } + + /// @brief checked array element access used by the at() overloads taking an index + /// @throw type_error.304 if @a j is not an array + /// @throw out_of_range.401 if @a idx is out of range + template + static auto array_at(Self& j, size_type idx) + -> decltype((*j.m_data.m_value.array)[idx]) + { + // at only works for arrays + if (JSON_HEDLEY_UNLIKELY(!j.is_array())) + { + JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", j.type_name()), &j)); + } + + if (JSON_HEDLEY_UNLIKELY(idx >= j.m_data.m_value.array->size())) + { + JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), &j)); + } + + return (*j.m_data.m_value.array)[idx]; + } + + /// @brief convert a null value to an empty container of type @a Container + /// @tparam Container array_t or object_t; any other type does not compile + template + void convert_null_to() + { + JSON_ASSERT(is_null()); + // create the container before touching the type, so a throwing + // allocation leaves this value as a valid null rather than a type + // tag with a dangling/null pointer behind it + set_container(create()); + assert_invariant(); + } + + /// @brief store a freshly created array and set the matching type + void set_container(array_t* array) noexcept + { + m_data.m_value.array = array; + m_data.m_type = value_t::array; + } + + /// @brief store a freshly created object and set the matching type + void set_container(object_t* object) noexcept + { + m_data.m_value.object = object; + m_data.m_type = value_t::object; + } + + public: //////////////////// // element access // //////////////////// @@ -3044,54 +3121,21 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/at/ reference at(size_type idx) { - // at only works for arrays - if (JSON_HEDLEY_UNLIKELY(!is_array())) - { - JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); - } - - if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) - { - JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); - } - - return set_parent((*m_data.m_value.array)[idx]); + return set_parent(array_at(*this, idx)); } /// @brief access specified array element with bounds checking /// @sa https://json.nlohmann.me/api/basic_json/at/ const_reference at(size_type idx) const { - // at only works for arrays - if (JSON_HEDLEY_UNLIKELY(!is_array())) - { - JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); - } - - if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) - { - JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); - } - - return (*m_data.m_value.array)[idx]; + return array_at(*this, idx); } /// @brief access specified object element with bounds checking /// @sa https://json.nlohmann.me/api/basic_json/at/ reference at(const typename object_t::key_type& key) { - // at only works for objects - if (JSON_HEDLEY_UNLIKELY(!is_object())) - { - JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); - } - - auto it = m_data.m_value.object->find(key); - if (it == m_data.m_value.object->end()) - { - JSON_THROW(out_of_range::create(403, detail::concat("key '", key, "' not found"), this)); - } - return set_parent(it->second); + return set_parent(object_at(*this, key)); } /// @brief access specified object element with bounds checking @@ -3100,36 +3144,14 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec detail::is_usable_as_basic_json_key_type::value, int> = 0> reference at(KeyType && key) { - // at only works for objects - if (JSON_HEDLEY_UNLIKELY(!is_object())) - { - JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); - } - - auto it = m_data.m_value.object->find(lookup_key(std::forward(key))); - if (it == m_data.m_value.object->end()) - { - JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(std::forward(key)), "' not found"), this)); - } - return set_parent(it->second); + return set_parent(object_at(*this, std::forward(key))); } /// @brief access specified object element with bounds checking /// @sa https://json.nlohmann.me/api/basic_json/at/ const_reference at(const typename object_t::key_type& key) const { - // at only works for objects - if (JSON_HEDLEY_UNLIKELY(!is_object())) - { - JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); - } - - auto it = m_data.m_value.object->find(key); - if (it == m_data.m_value.object->end()) - { - JSON_THROW(out_of_range::create(403, detail::concat("key '", key, "' not found"), this)); - } - return it->second; + return object_at(*this, key); } /// @brief access specified object element with bounds checking @@ -3138,18 +3160,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec detail::is_usable_as_basic_json_key_type::value, int> = 0> const_reference at(KeyType && key) const { - // at only works for objects - if (JSON_HEDLEY_UNLIKELY(!is_object())) - { - JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); - } - - auto it = m_data.m_value.object->find(lookup_key(std::forward(key))); - if (it == m_data.m_value.object->end()) - { - JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(std::forward(key)), "' not found"), this)); - } - return it->second; + return object_at(*this, std::forward(key)); } /// @brief access specified array element @@ -3159,9 +3170,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // implicitly convert a null value to an empty array if (is_null()) { - m_data.m_type = value_t::array; - m_data.m_value.array = create(); - assert_invariant(); + convert_null_to(); } // operator[] only works for arrays @@ -3227,9 +3236,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // implicitly convert a null value to an empty object if (is_null()) { - m_data.m_type = value_t::object; - m_data.m_value.object = create(); - assert_invariant(); + convert_null_to(); } // operator[] only works for objects @@ -3249,7 +3256,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // const operator[] only works for objects if (JSON_HEDLEY_LIKELY(is_object())) { - auto it = m_data.m_value.object->find(key); + auto it = object_lookup(*this, key); JSON_ASSERT(it != m_data.m_value.object->end()); return it->second; } @@ -3280,9 +3287,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // implicitly convert a null value to an empty object if (is_null()) { - m_data.m_type = value_t::object; - m_data.m_value.object = create(); - assert_invariant(); + convert_null_to(); } // operator[] only works for objects @@ -3304,7 +3309,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // const operator[] only works for objects if (JSON_HEDLEY_LIKELY(is_object())) { - auto it = m_data.m_value.object->find(lookup_key(std::forward(key))); + auto it = object_lookup(*this, std::forward(key)); JSON_ASSERT(it != m_data.m_value.object->end()); return it->second; } @@ -3324,6 +3329,36 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec detail::is_c_string_uncvref::value, string_t, typename std::decay::type >; + /// @brief look up @a key for value(), the single place shared by all key-based overloads + /// @throw type_error.306 if this is not an object + /// @return a pointer to the found value, or `nullptr` if @a key was not found + template + const basic_json* value_member(KeyType&& key) const + { + // value only works for objects + if (JSON_HEDLEY_UNLIKELY(!is_object())) + { + JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + } + + const auto it = find(std::forward(key)); + return it != end() ? &*it : nullptr; + } + + /// @brief resolve @a ptr for value(), the single place shared by both json_pointer overloads + /// @throw type_error.306 if this is not an array or object + /// @return a pointer to the resolved value, or `nullptr` if @a ptr does not resolve + const basic_json* value_pointee(const json_pointer& ptr) const + { + // value only works for arrays and objects + if (JSON_HEDLEY_UNLIKELY(!is_structured())) + { + JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + } + + return ptr.get_checked_or_null(this); + } + public: // an integer literal 0 would otherwise convert to a null const char* and from there to key_type template::value, int> = 0> @@ -3337,20 +3372,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec && !std::is_same>::value, int > = 0 > ValueType value(const typename object_t::key_type& key, const ValueType& default_value) const { - // value only works for objects - if (JSON_HEDLEY_LIKELY(is_object())) - { - // If 'key' is found, return its value. Otherwise, return `default_value'. - const auto it = find(key); - if (it != end()) - { - return it->template get(); - } - - return default_value; - } - - JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + // If 'key' is found, return its value. Otherwise, return `default_value'. + const auto* found = value_member(key); + return found != nullptr ? found->template get() : default_value; } /// @brief access specified object element with default value @@ -3362,20 +3386,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec && !std::is_same>::value, int > = 0 > ReturnType value(const typename object_t::key_type& key, ValueType && default_value) const { - // value only works for objects - if (JSON_HEDLEY_LIKELY(is_object())) - { - // If 'key' is found, return its value. Otherwise, return `default_value'. - const auto it = find(key); - if (it != end()) - { - return it->template get(); - } - - return std::forward(default_value); - } - - JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + // If 'key' is found, return its value. Otherwise, return `default_value'. + const auto* found = value_member(key); + return found != nullptr ? found->template get() : std::forward(default_value); } /// @brief access specified object element with default value @@ -3388,23 +3401,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec && !std::is_same>::value, int > = 0 > ValueType value(KeyType && key, const ValueType& default_value) const { - // value only works for objects - if (JSON_HEDLEY_LIKELY(is_object())) - { - // If 'key' is found, return its value. Otherwise, return `default_value'. - const auto it = find(std::forward(key)); - if (it != end()) - { - return it->template get(); - } - - return default_value; - } - - JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + // If 'key' is found, return its value. Otherwise, return `default_value'. + const auto* found = value_member(std::forward(key)); + return found != nullptr ? found->template get() : default_value; } - /// @brief access specified object element via JSON Pointer with default value + /// @brief access specified object element with default value /// @sa https://json.nlohmann.me/api/basic_json/value/ template < class ValueType, class KeyType, class ReturnType = typename value_return_type::type, detail::enable_if_t < @@ -3415,20 +3417,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec && !std::is_same>::value, int > = 0 > ReturnType value(KeyType && key, ValueType && default_value) const { - // value only works for objects - if (JSON_HEDLEY_LIKELY(is_object())) - { - // If 'key' is found, return its value. Otherwise, return `default_value'. - const auto it = find(std::forward(key)); - if (it != end()) - { - return it->template get(); - } - - return std::forward(default_value); - } - - JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + // If 'key' is found, return its value. Otherwise, return `default_value'. + const auto* found = value_member(std::forward(key)); + return found != nullptr ? found->template get() : std::forward(default_value); } /// @brief access specified object element via JSON Pointer with default value @@ -3438,21 +3429,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec && !std::is_same>::value, int > = 0 > ValueType value(const json_pointer& ptr, const ValueType& default_value) const { - // value only works for arrays and objects - if (JSON_HEDLEY_LIKELY(is_structured())) - { - // If the pointer resolves to a value, return it. Otherwise, return - // 'default_value'. - const auto* res = ptr.get_checked_or_null(this); - if (JSON_HEDLEY_LIKELY(res != nullptr)) - { - return res->template get(); - } - - return default_value; - } - - JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + // If the pointer resolves to a value, return it. Otherwise, return + // 'default_value'. + const auto* found = value_pointee(ptr); + return found != nullptr ? found->template get() : default_value; } /// @brief access specified object element via JSON Pointer with default value @@ -3463,21 +3443,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec && !std::is_same>::value, int > = 0 > ReturnType value(const json_pointer& ptr, ValueType && default_value) const { - // value only works for arrays and objects - if (JSON_HEDLEY_LIKELY(is_structured())) - { - // If the pointer resolves to a value, return it. Otherwise, return - // 'default_value'. - const auto* res = ptr.get_checked_or_null(this); - if (JSON_HEDLEY_LIKELY(res != nullptr)) - { - return res->template get(); - } - - return std::forward(default_value); - } - - JSON_THROW(type_error::create(306, detail::concat("cannot use value() with ", type_name()), this)); + // If the pointer resolves to a value, return it. Otherwise, return + // 'default_value'. + const auto* found = value_pointee(ptr); + return found != nullptr ? found->template get() : std::forward(default_value); } template < class ValueType, class BasicJsonType, detail::enable_if_t < @@ -3562,21 +3531,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_THROW(invalid_iterator::create(205, "iterator out of range", this)); } - if (is_string()) - { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, m_data.m_value.string); - std::allocator_traits::deallocate(alloc, m_data.m_value.string, 1); - m_data.m_value.string = nullptr; - } - else if (is_binary()) - { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, m_data.m_value.binary); - std::allocator_traits::deallocate(alloc, m_data.m_value.binary, 1); - m_data.m_value.binary = nullptr; - } - + m_data.m_value.destroy(m_data.m_type); + m_data.m_value = {}; m_data.m_type = value_t::null; assert_invariant(); break; @@ -3628,27 +3584,14 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec case value_t::string: case value_t::binary: { - if (JSON_HEDLEY_LIKELY(!first.m_it.primitive_iterator.is_begin() - || !last.m_it.primitive_iterator.is_end())) + if (JSON_HEDLEY_UNLIKELY(!first.m_it.primitive_iterator.is_begin() + || !last.m_it.primitive_iterator.is_end())) { JSON_THROW(invalid_iterator::create(204, "iterators out of range", this)); } - if (is_string()) - { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, m_data.m_value.string); - std::allocator_traits::deallocate(alloc, m_data.m_value.string, 1); - m_data.m_value.string = nullptr; - } - else if (is_binary()) - { - AllocatorType alloc; - std::allocator_traits::destroy(alloc, m_data.m_value.binary); - std::allocator_traits::deallocate(alloc, m_data.m_value.binary, 1); - m_data.m_value.binary = nullptr; - } - + m_data.m_value.destroy(m_data.m_type); + m_data.m_value = {}; m_data.m_type = value_t::null; assert_invariant(); break; @@ -3704,7 +3647,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_THROW(type_error::create(307, detail::concat("cannot use erase() with ", type_name()), this)); } - const auto it = m_data.m_value.object->find(lookup_key(std::forward(key))); + const auto it = object_lookup(*this, std::forward(key)); if (it != m_data.m_value.object->end()) { m_data.m_value.object->erase(it); @@ -3781,7 +3724,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec if (is_object()) { - result.m_it.object_iterator = m_data.m_value.object->find(key); + result.m_it.object_iterator = object_lookup(*this, key); } return result; @@ -3795,7 +3738,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec if (is_object()) { - result.m_it.object_iterator = m_data.m_value.object->find(key); + result.m_it.object_iterator = object_lookup(*this, key); } return result; @@ -3811,7 +3754,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec if (is_object()) { - result.m_it.object_iterator = m_data.m_value.object->find(lookup_key(std::forward(key))); + result.m_it.object_iterator = object_lookup(*this, std::forward(key)); } return result; @@ -3827,7 +3770,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec if (is_object()) { - result.m_it.object_iterator = m_data.m_value.object->find(lookup_key(std::forward(key))); + result.m_it.object_iterator = object_lookup(*this, std::forward(key)); } return result; @@ -3858,7 +3801,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_HEDLEY_WARN_UNUSED_RESULT bool contains(const typename object_t::key_type& key) const { - return is_object() && m_data.m_value.object->find(key) != m_data.m_value.object->end(); + return is_object() && object_lookup(*this, key) != m_data.m_value.object->end(); } /// @brief check the existence of an element in a JSON object @@ -3868,7 +3811,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_HEDLEY_WARN_UNUSED_RESULT bool contains(KeyType && key) const { - return is_object() && m_data.m_value.object->find(lookup_key(std::forward(key))) != m_data.m_value.object->end(); + return is_object() && object_lookup(*this, std::forward(key)) != m_data.m_value.object->end(); } /// @brief check the existence of an element in a JSON object given a JSON pointer @@ -4233,9 +4176,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // transform a null object into an array if (is_null()) { - m_data.m_type = value_t::array; - m_data.m_value = value_t::array; - assert_invariant(); + convert_null_to(); } // add the element to the array (move semantics) @@ -4266,9 +4207,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // transform a null object into an array if (is_null()) { - m_data.m_type = value_t::array; - m_data.m_value = value_t::array; - assert_invariant(); + convert_null_to(); } // add the element to the array @@ -4298,9 +4237,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // transform a null object into an object if (is_null()) { - m_data.m_type = value_t::object; - m_data.m_value = value_t::object; - assert_invariant(); + convert_null_to(); } // add the element to the object @@ -4354,9 +4291,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // transform a null object into an array if (is_null()) { - m_data.m_type = value_t::array; - m_data.m_value = value_t::array; - assert_invariant(); + convert_null_to(); } // add the element to the array (perfect forwarding) @@ -4379,9 +4314,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // transform a null object into an object if (is_null()) { - m_data.m_type = value_t::object; - m_data.m_value = value_t::object; - assert_invariant(); + convert_null_to(); } // add the element to the array (perfect forwarding) @@ -4441,7 +4374,22 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/insert/ iterator insert(const_iterator pos, basic_json&& val) // NOLINT(performance-unnecessary-value-param) { - return insert(std::move(pos), val); + // insert only works for arrays + if (JSON_HEDLEY_LIKELY(is_array())) + { + // check if iterator pos fits to this JSON value + if (JSON_HEDLEY_UNLIKELY(pos.m_object != this)) + { + JSON_THROW(invalid_iterator::create(202, "iterator does not fit current value", this)); + } + + // moving into a local first keeps this safe even if val aliases + // an element of this array + basic_json tmp(std::move(val)); + return insert_iterator(pos, std::move(tmp)); + } + + JSON_THROW(type_error::create(309, detail::concat("cannot use insert() with ", type_name()), this)); } /// @brief inserts copies of element into array @@ -4617,14 +4565,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// value is an object; called first by both @ref update overloads void prepare_update() { - // implicitly convert a null value to an empty object; create the - // object before setting the type, so a throwing allocation leaves - // this value null + // implicitly convert a null value to an empty object if (is_null()) { - m_data.m_value.object = create(); - m_data.m_type = value_t::object; - assert_invariant(); + convert_null_to(); } if (JSON_HEDLEY_UNLIKELY(!is_object())) @@ -4833,7 +4777,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/swap/ void swap(binary_t& other) // NOLINT(bugprone-exception-escape,cppcoreguidelines-noexcept-swap,performance-noexcept-swap) { - // swap only works for strings + // swap only works for binary values if (JSON_HEDLEY_LIKELY(is_binary())) { using std::swap; @@ -4849,7 +4793,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/swap/ void swap(typename binary_t::container_type& other) // NOLINT(bugprone-exception-escape) { - // swap only works for strings + // swap only works for binary values if (JSON_HEDLEY_LIKELY(is_binary())) { using std::swap; @@ -5350,7 +5294,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool ignore_trailing_commas = false) { basic_json result; - parser(detail::input_adapter(std::forward(i)), std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas).parse(true, result); // cppcheck-suppress[accessMoved,accessForwarded] + auto p = parser(detail::input_adapter(std::forward(i)), std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas); + p.parse(true, result); return result; } @@ -5367,7 +5312,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool ignore_trailing_commas = false) { basic_json result; - parser(detail::input_adapter(std::move(first), std::move(last)), std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas).parse(true, result); // cppcheck-suppress[accessMoved] + auto p = parser(detail::input_adapter(std::move(first), std::move(last)), std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas); + p.parse(true, result); return result; } @@ -5380,7 +5326,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool ignore_trailing_commas = false) { basic_json result; - parser(i.get(), std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas).parse(true, result); // cppcheck-suppress[accessMoved] + auto p = parser(i.get(), std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas); + p.parse(true, result); return result; } @@ -5607,6 +5554,31 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } #endif + private: + /*! + @brief shared implementation of the binary from_cbor/from_msgpack/ + from_ubjson/from_bjdata/from_bon8/from_bson overloads + + Building the @ref binary_reader as a named local, rather than as a + temporary that @ref detail::json_sax_dom_parser::parse is called on in + the same expression, avoids a false-positive cppcheck accessMoved + warning in each of the 16 callers. + */ + template + static basic_json from_binary_impl(InputAdapterType ia, const input_format_t format, + const bool strict, const bool allow_exceptions, + const cbor_tag_handler_t tag_handler = cbor_tag_handler_t::error) + { + basic_json result; + detail::json_sax_dom_parser sdp(result, allow_exceptions); + binary_reader reader(std::move(ia), format); + if (!reader.sax_parse(&sdp, strict, tag_handler)) + { + result = value_t::discarded; + } + return result; + } + ////////////////////////////////////////// // binary serialization/deserialization // ////////////////////////////////////////// @@ -5779,14 +5751,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool allow_exceptions = true, const cbor_tag_handler_t tag_handler = cbor_tag_handler_t::error) { - basic_json result; - auto ia = detail::input_adapter(std::forward(i)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::cbor).sax_parse(&sdp, strict, tag_handler)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::forward(i)), input_format_t::cbor, strict, allow_exceptions, tag_handler); } /// @brief create a JSON value from an input in CBOR format (iterator pair, or iterator+sentinel pair for C++20 ranges support) @@ -5799,14 +5764,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool allow_exceptions = true, const cbor_tag_handler_t tag_handler = cbor_tag_handler_t::error) { - basic_json result; - auto ia = detail::input_adapter(std::move(first), std::move(last)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::cbor).sax_parse(&sdp, strict, tag_handler)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::move(first), std::move(last)), input_format_t::cbor, strict, allow_exceptions, tag_handler); } template @@ -5827,15 +5785,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool allow_exceptions = true, const cbor_tag_handler_t tag_handler = cbor_tag_handler_t::error) { - basic_json result; - auto ia = i.get(); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - // NOLINTNEXTLINE(hicpp-move-const-arg,performance-move-const-arg) - if (!binary_reader(std::move(ia), input_format_t::cbor).sax_parse(&sdp, strict, tag_handler)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(i.get(), input_format_t::cbor, strict, allow_exceptions, tag_handler); } /// @brief create a JSON value from an input in MessagePack format @@ -5846,14 +5796,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::forward(i)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::msgpack).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::forward(i)), input_format_t::msgpack, strict, allow_exceptions); } /// @brief create a JSON value from an input in MessagePack format (iterator pair, or iterator+sentinel pair for C++20 ranges support) @@ -5865,14 +5808,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::move(first), std::move(last)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::msgpack).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::move(first), std::move(last)), input_format_t::msgpack, strict, allow_exceptions); } template @@ -5891,15 +5827,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = i.get(); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - // NOLINTNEXTLINE(hicpp-move-const-arg,performance-move-const-arg) - if (!binary_reader(std::move(ia), input_format_t::msgpack).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(i.get(), input_format_t::msgpack, strict, allow_exceptions); } /// @brief create a JSON value from an input in UBJSON format @@ -5910,14 +5838,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::forward(i)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::ubjson).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::forward(i)), input_format_t::ubjson, strict, allow_exceptions); } /// @brief create a JSON value from an input in UBJSON format (iterator pair, or iterator+sentinel pair for C++20 ranges support) @@ -5929,14 +5850,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::move(first), std::move(last)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::ubjson).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::move(first), std::move(last)), input_format_t::ubjson, strict, allow_exceptions); } template @@ -5955,15 +5869,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = i.get(); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - // NOLINTNEXTLINE(hicpp-move-const-arg,performance-move-const-arg) - if (!binary_reader(std::move(ia), input_format_t::ubjson).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(i.get(), input_format_t::ubjson, strict, allow_exceptions); } /// @brief create a JSON value from an input in BJData format @@ -5974,14 +5880,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::forward(i)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::bjdata).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::forward(i)), input_format_t::bjdata, strict, allow_exceptions); } /// @brief create a JSON value from an input in BJData format (iterator pair, or iterator+sentinel pair for C++20 ranges support) @@ -5993,14 +5892,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::move(first), std::move(last)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::bjdata).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::move(first), std::move(last)), input_format_t::bjdata, strict, allow_exceptions); } /// @brief create a JSON value from an input in BON8 format @@ -6011,14 +5903,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::forward(i)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::bon8).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::forward(i)), input_format_t::bon8, strict, allow_exceptions); } /// @brief create a JSON value from an input in BON8 format (iterator pair, or iterator+sentinel pair for C++20 ranges support) @@ -6030,14 +5915,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::move(first), std::move(last)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::bon8).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::move(first), std::move(last)), input_format_t::bon8, strict, allow_exceptions); } /// @brief create a JSON value from an input in BSON format @@ -6048,14 +5926,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::forward(i)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::bson).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::forward(i)), input_format_t::bson, strict, allow_exceptions); } /// @brief create a JSON value from an input in BSON format (iterator pair, or iterator+sentinel pair for C++20 ranges support) @@ -6067,14 +5938,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = detail::input_adapter(std::move(first), std::move(last)); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - if (!binary_reader(std::move(ia), input_format_t::bson).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(detail::input_adapter(std::move(first), std::move(last)), input_format_t::bson, strict, allow_exceptions); } template @@ -6093,15 +5957,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const bool strict = true, const bool allow_exceptions = true) { - basic_json result; - auto ia = i.get(); - detail::json_sax_dom_parser sdp(result, allow_exceptions); - // NOLINTNEXTLINE(hicpp-move-const-arg,performance-move-const-arg) - if (!binary_reader(std::move(ia), input_format_t::bson).sax_parse(&sdp, strict)) // cppcheck-suppress[accessMoved] - { - result = value_t::discarded; - } - return result; + return from_binary_impl(i.get(), input_format_t::bson, strict, allow_exceptions); } /// @} diff --git a/include/nlohmann/ordered_map.hpp b/include/nlohmann/ordered_map.hpp index d1c247483..eb521114e 100644 --- a/include/nlohmann/ordered_map.hpp +++ b/include/nlohmann/ordered_map.hpp @@ -74,16 +74,43 @@ template , return *this; } +private: + /// @brief find the entry for @a key, for either constness of @a self + /// @note the single place that performs the linear key search + template + static auto find_impl(Self& self, KeyType&& key) -> decltype(self.begin()) + { + for (auto it = self.begin(); it != self.end(); ++it) + { + if (self.m_compare(it->first, key)) + { + return it; + } + } + return self.end(); + } + + /// @brief remove the entry @a it points to, preserving order + /// @note keys are not movable, so the tail is destroyed and re-constructed in place + void erase_at(iterator it) + { + for (auto next = it; ++next != this->end(); ++it) + { + it->~value_type(); // Destroy but keep allocation + new (&*it) value_type{std::move(*next)}; + } + Container::pop_back(); + } + +public: template::value, int> = 0> std::pair emplace(const key_type& key, V && t) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it != this->end()) { - if (m_compare(it->first, key)) - { - return {it, false}; - } + return {it, false}; } append(key, std::forward(t)); return {std::prev(this->end()), true}; @@ -94,12 +121,10 @@ template , detail::is_constructible>::value, int> = 0> std::pair emplace(KeyType && key, V && t) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it != this->end()) { - if (m_compare(it->first, key)) - { - return {it, false}; - } + return {it, false}; } append(std::forward(key), std::forward(t)); return {std::prev(this->end()), true}; @@ -131,75 +156,55 @@ template , T& at(const key_type& key) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it == this->end()) { - if (m_compare(it->first, key)) - { - return it->second; - } + JSON_THROW(std::out_of_range("key not found")); } - - JSON_THROW(std::out_of_range("key not found")); + return it->second; } template::value, int> = 0> T & at(KeyType && key) // NOLINT(cppcoreguidelines-missing-std-forward) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it == this->end()) { - if (m_compare(it->first, key)) - { - return it->second; - } + JSON_THROW(std::out_of_range("key not found")); } - - JSON_THROW(std::out_of_range("key not found")); + return it->second; } const T& at(const key_type& key) const { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it == this->end()) { - if (m_compare(it->first, key)) - { - return it->second; - } + JSON_THROW(std::out_of_range("key not found")); } - - JSON_THROW(std::out_of_range("key not found")); + return it->second; } template::value, int> = 0> const T & at(KeyType && key) const // NOLINT(cppcoreguidelines-missing-std-forward) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it == this->end()) { - if (m_compare(it->first, key)) - { - return it->second; - } + JSON_THROW(std::out_of_range("key not found")); } - - JSON_THROW(std::out_of_range("key not found")); + return it->second; } size_type erase(const key_type& key) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it != this->end()) { - if (m_compare(it->first, key)) - { - // Since we cannot move const Keys, re-construct them in place - for (auto next = it; ++next != this->end(); ++it) - { - it->~value_type(); // Destroy but keep allocation - new (&*it) value_type{std::move(*next)}; - } - Container::pop_back(); - return 1; - } + erase_at(it); + return 1; } return 0; } @@ -208,19 +213,11 @@ template , detail::is_usable_as_key_type::value, int> = 0> size_type erase(KeyType && key) // NOLINT(cppcoreguidelines-missing-std-forward) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, key); + if (it != this->end()) { - if (m_compare(it->first, key)) - { - // Since we cannot move const Keys, re-construct them in place - for (auto next = it; ++next != this->end(); ++it) - { - it->~value_type(); // Destroy but keep allocation - new (&*it) value_type{std::move(*next)}; - } - Container::pop_back(); - return 1; - } + erase_at(it); + return 1; } return 0; } @@ -285,80 +282,38 @@ template , size_type count(const key_type& key) const { - for (auto it = this->begin(); it != this->end(); ++it) - { - if (m_compare(it->first, key)) - { - return 1; - } - } - return 0; + return find_impl(*this, key) != this->end() ? 1 : 0; } template::value, int> = 0> size_type count(KeyType && key) const // NOLINT(cppcoreguidelines-missing-std-forward) { - for (auto it = this->begin(); it != this->end(); ++it) - { - if (m_compare(it->first, key)) - { - return 1; - } - } - return 0; + return find_impl(*this, key) != this->end() ? 1 : 0; } iterator find(const key_type& key) { - for (auto it = this->begin(); it != this->end(); ++it) - { - if (m_compare(it->first, key)) - { - return it; - } - } - return Container::end(); + return find_impl(*this, key); } template::value, int> = 0> iterator find(KeyType && key) // NOLINT(cppcoreguidelines-missing-std-forward) { - for (auto it = this->begin(); it != this->end(); ++it) - { - if (m_compare(it->first, key)) - { - return it; - } - } - return Container::end(); + return find_impl(*this, key); } const_iterator find(const key_type& key) const { - for (auto it = this->begin(); it != this->end(); ++it) - { - if (m_compare(it->first, key)) - { - return it; - } - } - return Container::end(); + return find_impl(*this, key); } template::value, int> = 0> const_iterator find(KeyType && key) const // NOLINT(cppcoreguidelines-missing-std-forward) { - for (auto it = this->begin(); it != this->end(); ++it) - { - if (m_compare(it->first, key)) - { - return it; - } - } - return Container::end(); + return find_impl(*this, key); } std::pair insert( value_type&& value ) @@ -368,12 +323,10 @@ template , std::pair insert( const value_type& value ) { - for (auto it = this->begin(); it != this->end(); ++it) + const auto it = find_impl(*this, value.first); + if (it != this->end()) { - if (m_compare(it->first, value.first)) - { - return {it, false}; - } + return {it, false}; } append(value); return {--this->end(), true}; diff --git a/tests/src/unit-class_parser.cpp b/tests/src/unit-class_parser.cpp index 75f3757e8..0ae5e382b 100644 --- a/tests/src/unit-class_parser.cpp +++ b/tests/src/unit-class_parser.cpp @@ -2684,12 +2684,10 @@ TEST_CASE("diagnostic positions: value lifetime, input adapters, and SAX") SECTION("move constructor resets the moved-from value to npos") { - // basic_json(basic_json&&) (json.hpp, around line 1951) copies + // basic_json(basic_json&&) copies // other's start_position/end_position into *this and then resets - // other's to npos (see the cppcheck-suppress[accessForwarded] - // annotation there, which flags this reset as worth a second - // look). Only the top-level moved-from value is affected; its - // (moved-away) children are gone along with it. + // other's to npos. Only the top-level moved-from value is + // affected; its (moved-away) children are gone along with it. const std::string s = R"({"a":1,"b":[1,2,3]})"; json a = json::parse(s); const auto a_start = a.start_pos(); diff --git a/tests/src/unit-modifiers.cpp b/tests/src/unit-modifiers.cpp index ee81120cf..532283f95 100644 --- a/tests/src/unit-modifiers.cpp +++ b/tests/src/unit-modifiers.cpp @@ -630,6 +630,49 @@ TEST_CASE("modifiers") } } + SECTION("rvalue at position moves rather than copies") + { + // regression test: insert(pos, basic_json&&) used to forward to + // insert(pos, const basic_json&) because the named rvalue + // reference parameter is itself an lvalue, so it always + // deep-copied its argument instead of moving it + json j_big = std::string(1000, 'x'); + const auto* const original_buffer = j_big.get_ref().data(); + + auto it = j_array.insert(j_array.begin(), std::move(j_big)); + CHECK(j_array.size() == 5); + CHECK(*it == json(std::string(1000, 'x'))); + CHECK((*it).get_ref().data() == original_buffer); + + // the moved-from value is null, the same as after push_back(&&) + CHECK(j_big.is_null()); // NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved) + } + + SECTION("self-aliasing insertion") + { + SECTION("without reallocation") + { + json j_self = {1, 2, 3, 4}; + j_self.get_ref().reserve(j_self.size() + 1); + + auto it = j_self.insert(j_self.begin(), std::move(j_self[1])); + CHECK(j_self.size() == 5); + CHECK(*it == json(2)); + CHECK(j_self == json({2, 1, nullptr, 3, 4})); + } + + SECTION("with reallocation") + { + json j_self = {1, 2, 3, 4}; + j_self.get_ref().shrink_to_fit(); + + auto it = j_self.insert(j_self.begin(), std::move(j_self[1])); + CHECK(j_self.size() == 5); + CHECK(*it == json(2)); + CHECK(j_self == json({2, 1, nullptr, 3, 4})); + } + } + SECTION("copies at position") { SECTION("insert before begin()")