Deduplicate basic_json internals; make insert(pos, json&&) move - #5727
Merged
Merged
Conversation
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 <mail@nlohmann.me>
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 <mail@nlohmann.me>
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 <mail@nlohmann.me>
…ctor basic_json(basic_json&&) built its base subobject with std::forward<json_base_class_t>(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<T>(x) is defined as static_cast<T&&>(x). Part of #5724 Signed-off-by: Niels Lohmann <mail@nlohmann.me>
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 <mail@nlohmann.me>
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 <mail@nlohmann.me>
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<PointerType>() @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 <mail@nlohmann.me>
…odies 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 <mail@nlohmann.me>
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 <mail@nlohmann.me>
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 <mail@nlohmann.me>
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<T>() 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 <mail@nlohmann.me>
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<T>(), 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<T>() 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<object_t>() 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 <mail@nlohmann.me>
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 <mail@nlohmann.me>
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 <mail@nlohmann.me>
Contributor
There are 14 commits. What are the other 5? |
Owner
Author
gregmarr
reviewed
Sep 30, 2026
| } | ||
|
|
||
| /// @brief convert a null value to an empty container of type @a t (array or object) | ||
| void convert_null_to(value_t t) |
Contributor
There was a problem hiding this comment.
I'm wondering if this should be a template function that takes either array_t or object_t so you aren't passing value_t objects and it's not possible to pass an invalid t.
Owner
Author
There was a problem hiding this comment.
Good point, thanks. Done in 3a04f78: convert_null_to() is now template<typename Container> void convert_null_to(), called as convert_null_to<array_t>() / convert_null_to<object_t>(). The created container is stored by two set_container(array_t*) / set_container(object_t*) overloads, which also set the matching type tag, so passing any other type fails to compile. That removes the runtime JSON_ASSERT on t and the if/else. The container is still allocated before the type is set, so a throwing allocator still leaves a valid null.
This reply was written by Claude Code on behalf of @nlohmann.
nlohmann
added
the
review needed
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 <mail@nlohmann.me>
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated and/or formatted correctly, or |
gregmarr
approved these changes
Oct 4, 2026
nlohmann
added
🚀 ready to merge
nlohmann
added a commit
that referenced
this pull request
Oct 4, 2026
- binary_reader: rename the error_handler constructor parameter, which shadowed the member (-Wshadow, -Wshadow-field-in-constructor; #5746) - basic_json(copy_construct_tag, ...): declare it noexcept when copying the base class is (GCC 16 -Wnoexcept; #5690) - the scalar-on-left legacy comparison operators: noexcept only when converting the scalar is, like their member counterparts (#5682, #5751) - compare_leaves: use std::is_eq/is_lt/is_gt instead of comparing a std::partial_ordering with 0 (-Wzero-as-null-pointer-constant; #5686) - serializer: silence MSVC C4127 for the EnsureAscii template parameter (#5741, #5746) - clang-tidy: return the sanitized reference in binary_writer, take the key of ordered_map::find_impl by const reference (#5727), and mark the switches over parse_array_index (#5728) - ordered_map: keep <memory> for std::allocator (IWYU) Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
added a commit
that referenced
this pull request
Oct 4, 2026
* Keep the serializer conversion for objects whose keys cannot be converted #5591 added a test converting nlohmann::json into a basic_json whose string type cannot be constructed from std::string. That instantiates convert_iteratively(), whose members.emplace_back(next.key(), ...) needs exactly that key conversion, and broke the build of unit-alt-string. Dispatch on the key's constructibility and leave such conversions to the serializers, as the levels above the nesting bound already do (#3425). Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix the remaining CI failures on develop - unit-wstring: with a 16-bit wchar_t (Windows), a lone surrogate is reported as the ill-formed byte 0xFF since #5704; the std::wstring expectations still had the previous <U+0000>. - ci_single_binaries: json_literals.hpp (#5610) and json.hpp include each other on purpose, and IWYU, not following the cycle, asks to replace json.hpp with json_fwd.hpp. Report its findings without failing the build, as already done for json.hpp. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix the library warnings and noexcept specifications from the merged PRs - binary_reader: rename the error_handler constructor parameter, which shadowed the member (-Wshadow, -Wshadow-field-in-constructor; #5746) - basic_json(copy_construct_tag, ...): declare it noexcept when copying the base class is (GCC 16 -Wnoexcept; #5690) - the scalar-on-left legacy comparison operators: noexcept only when converting the scalar is, like their member counterparts (#5682, #5751) - compare_leaves: use std::is_eq/is_lt/is_gt instead of comparing a std::partial_ordering with 0 (-Wzero-as-null-pointer-constant; #5686) - serializer: silence MSVC C4127 for the EnsureAscii template parameter (#5741, #5746) - clang-tidy: return the sanitized reference in binary_writer, take the key of ordered_map::find_impl by const reference (#5727), and mark the switches over parse_array_index (#5728) - ordered_map: keep <memory> for std::allocator (IWYU) Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Split unit-conversions.cpp so MinGW can link it clang 18 with the MinGW linker failed to link test-conversions_cpp17 ("relocation truncated to fit: IMAGE_REL_AMD64_REL32"). As windows.yml recommends, keep the objects small by splitting the test file. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix the tests added by the merged PRs for all CI configurations - discard the results of dump() and from_*() in CHECK_THROWS with utils::ignore_return_value (GCC -Werror=unused-result) - give unit-bson's huge_string_t a default constructor (MSVC C2512, GCC 5, clang 3.5) - unit-disabled_exceptions: use the literals namespace when the global UDLs are off (ci_test_noglobaludls; #5700) - unit-binary_utf8_strict: expect the JSON pointer prefix with JSON_DIAGNOSTICS (#5741) - skip the tests that rely on exceptions under JSON_NOEXCEPTION (#5678, #5732) - clang-tidy and clang -Werror: static test data, CAPTURE(...);, const-correctness, use-after-move alias, unused conversion operator, a missing <iterator> include Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Title the macro examples and add JSON_STRICT_BINARY_UTF8 to the docset The documentation style check requires "Example: ..." titles on pages with several examples (#5741, #5591) and a docset entry for every macro page. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Regenerate BUILD.bazel and nlohmann_json.natvis Signed-off-by: Niels Lohmann <mail@nlohmann.me> #5746 added detail/output/error_handler.hpp and #5741 the json_abi_sbu8 ABI tag. * Install libidn11 for the CMake 3.5.0 binary in ci_cmake_flags Signed-off-by: Niels Lohmann <mail@nlohmann.me> #5733 moved ci_cmake_options from ubuntu:focal to ubuntu:24.04, which no longer ships libidn.so.11; the CMake 3.5.0 release binary links against it, so every ci_cmake_flags run has failed since. Install focal's libidn11 package for that matrix entry only. * Suppress Infer's false STACK_VARIABLE_ADDRESS_ESCAPE in get_impl get_impl() returns its local by value. A test added by the merged PRs instantiates it with a type Infer misreads, so ci_infer reported the 2021 code for the first time. Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements the "do-now" items of the technical-debt umbrella issue #5724 for
basic_json(include/nlohmann/json.hpp): duplicated code, avoidable lint suppressions, stale comments, and a copy that should be a move. Each of the 14 commits is exactly one checklist item and can be reviewed and merged on its own: the first 9 are the original "do-now" items of #5724, and the 5 follow-up commits listed under "Remaining checklist items" implement #5724 items 3–6 and the move-constructor part of #5725 item 3. All items preserve behavior, the public API and the ABI, except item 1, which is a deliberate, disclosed behavior change. The follow-up commits listed under "Remaining checklist items" implement every item that was left open, so this PR now closes #5724; see "Public API" for their effects.Changes
primitive_iterator_t,internal_iterator,output_adapter_tfrombasic_json(basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 13)meta()'s dead, syntactically invalid HP aCC branch (missing semicolon), narrow the condition to__HP_aCC, and correct meta.md, which documented the resulting (impossible) plain-string behavior (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 8)basic_json(std::nullptr_t)constructor from delegating to the throwingbasic_json(value_t)constructor, dropping theNOLINT(bugprone-exception-escape); the default member initializers already produce the same null state (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 11)accessForwardedsuppressions in the move constructor by castingotherto the base type explicitly instead of forwarding the whole object (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 9)parse()overloads a named local parser to drop theiraccessMoved/accessForwardedsuppressions (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 10)erase(pos)anderase(first, last)by routing both throughjson_value::destroy(), and fixerase(first, last)'s invertedJSON_HEDLEY_LIKELYbranch hint to match the rest of the class (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 7)assert_invariant(),json_valuenote,get<PointerType>()'s@tparamlist,value()'s brief,swap(binary_t&)/swap(binary_t::container_type&)); comment-only, behavior/API/ABI unchanged (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 12)from_cbor/from_msgpack/from_ubjson/from_bjdata/from_bon8/from_bsonbodies behind a new privatefrom_binary_impl()helper, removing the 16accessMovedsuppressions and 4NOLINTNEXTLINEcomments. The helper sits in the private section before the binary block, not between thefrom_*overloads, but the refactor still rewrites thefrom_bjdata/from_bon8"return result; }" lines that open PR Add deprecated from_bon8/from_bjdata(ptr, len) overloads #5688's two insertion hunks use as unchanged context, so those two patches touch the same source lines: whichever of this PR and Add deprecated from_bon8/from_bjdata(ptr, len) overloads #5688 lands second will need a small manual rebase (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 2)insert(const_iterator pos, basic_json&& val)move its argument through a local instead of resolving to the copyingconst basic_json&overload; addunit-modifierscoverage for the moved-from state and for self-aliasing insertion, with and without reallocation (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 1)Tests
Each commit was built and run locally under
clang++ -fsanitize=address,undefined:make check-amalgamation__HP_aCC, so the branch cannot be exercised hereunit-constructor1(323/323),unit-constructor2(21/21),unit-noexcept(6/6)unit-custom-base-class(99/99)unit-deserialization(466/466)unit-element_access1(217/217),unit-element_access2(1120/1120),unit-modifiers(1123/1123),unit-iterators2(1344/1344),unit-allocator(24/24)make check-amalgamation; plain compile of json.hpp (no code tokens changed)unit-cbor,unit-msgpack,unit-ubjson,unit-bjdata,unit-bson,unit-bon8,unit-regression2against the json_test_data corpus, all passing (242390/237368/211279/667293/959/900639/65 assertions)unit-modifiers(1133/1133, both plain and with-DJSON_DIAGNOSTICS=1),unit-diagnostics(51/51),unit-diagnostic-positions(2786/2786), to check for parent-pointer regressionsmake check-amalgamationpasses clean after every commit and at the branch tipCI must confirm:
ci_cppcheckfor items 9, 10 and 2 (cppcheck 2.17.1 could not be installed offline in this environment), confirming the removed suppressions no longer match anythingci_clang_tidyfor items 11, 9, 10 and 2ci_test_valgrindandci_test_diagnosticsfor item 7ci_test_diagnostic_positionsfor item 9ci_cpplint/astyle formatting for item 12Public API
No breaking changes, with one deliberate, disclosed exception:
insert(pos, basic_json&&)now actually moves its argument instead of deep-copying it. The moved-frombasic_jsonisnullafterwards, the same as afterpush_back(basic_json&&), instead of keeping its old value unchanged. This copy existed in every release. No signature changed, so the public API and ABI are otherwise unaffected.__HP_aCC, which cannot compile the library today (the branch has never compiled). No observable change on any compiler in the supported matrix.Follow-up commits:
nullvalue throws, the value now staysnull(previously the type tag was set first, which violated the class invariant). Correct callers see no change.Remaining checklist items (follow-up commits)
The items that were left open because they overlap open PRs are now implemented here as well, so this PR closes #5724. It also takes over the move-constructor part of #5725 item 3.
a0e01e3b9Deduplicate object key lookup and checkedat()access: privateobject_lookup()/object_at()/array_at()helpers used byat()(6 overloads), constoperator[],find(),contains()anderase_internal(); the key is forwarded only once, and the exception message keeps using the key (NOLINT with a reason, sinceobject_t::find()never moves from its argument) (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 3)03d53596fDeduplicate the lookup/default/throw body of the sixvalue()overloads throughvalue_member()/value_pointee()(basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 4)51233dc17Deduplicate the null-to-container conversion at all 9 sites intoconvert_null_to(), which allocates before it sets the type, so a throwing allocator leaves a validnullinstead of an object with a null pointer (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 5)3e771c2adDeduplicate the linear key search inordered_map(find_impl()) and the pop_back-based erase (erase_at()) (basic_json and ordered_map: insert(pos, json&&) copies, duplicated from_*/lookup/value()/null-to-container bodies, avoidable lint suppressions, stale comments #5724 item 6)09d41a894Re-enablebugprone-use-after-move/hicpp-invalid-access-movedin.clang-tidy; drop the no-opstd::forwardcalls infrom_json.hppand annotate the one intentional double forward ininput_adapters.hpp(Lint and static-analysis config: clang-tidy checks disabled "to get the CI going", no-op scan-build job, stale NOLINTs, library-wide warning suppressions in tests, stale GCC flags and frozen clang-tidy image #5725 item 3, move-constructor part)Tests for these commits:
unit-element_access1/2,unit-modifiers,unit-json_pointer,unit-ordered_map,unit-ordered_json,unit-allocator,unit-diagnostics(also withJSON_DIAGNOSTICS=1),unit-regression2,unit-custom-base-class,unit-conversions,unit-class_parser, andunit-disabled_exceptionswithJSON_NOEXCEPTION, with ASan/UBSan, unchanged assertion countsconvert_null_to(), and a check that theat(KeyType&&)message shows an rvalue key correctlysilkeh/clang:22): no unsuppressedbugprone-use-after-move/hicpp-invalid-access-movedfindingsmake check-amalgamationcleanThese commits touch the same hunks as #5689, #5705, #5606, #5687, #5585 (items 3–5), #5609 and #5685 (item 6), and #5737 (
.clang-tidy,from_json.hpp,input_adapters.hpp). Whichever PR lands second needs a rebase.Closes #5724
This PR was written by Claude Code on behalf of @nlohmann.
🤖 Generated with Claude Code