Skip to content

Move instead of deep-copy ordered_json values when an object grows - #5609

Merged
nlohmann merged 3 commits into
developfrom
claude/ordered-map-growth-moves-values
Sep 30, 2026
Merged

nlohmann merged 3 commits into
developfrom
claude/ordered-map-growth-moves-values

Conversation

@nlohmann

Copy link
Copy Markdown
Owner

What & why

nlohmann::ordered_map stores its elements in a std::vector<std::pair<const Key, T>>. With a std::string key, that pair is not nothrow move constructible, because the const key has to be copied. So whenever the vector reallocates, std::vector copy-constructs every element via move_if_noexcept.

For ordered_json, this means that growing an object deep-copies every member value it already holds, including whole nested subtrees. Parsing is affected, because the parser inserts members one by one. So is any code that fills an object through operator[], emplace, push_back, or update.

I found this while measuring allocations for #5298 (comment): ordered_json created exactly twice as many strings as json for the same document.

Changes

  • ordered_map now grows its storage itself: the keys are copied and the values are moved. Only three places append, and all of them go through the new private append():

    • both emplace overloads;
    • insert(const value_type&).

    Everything else already reaches one of them: operator[], insert(value_type&&), insert(first, last), and the basic_json and parser paths.

  • Strong exception guarantee without try/catch (the library also builds with -fno-exceptions):

    • The first phase may throw, but it only changes a temporary buffer: it copies the keys, value-initializes the values, and constructs the new element.
    • The second phase cannot throw: it move-assigns the values over and swaps the buffers.
  • Arguments that point into the map stay valid, as with std::vector. The new element is constructed before any value is moved, so a call like j.emplace("new", std::move(j["old"])) still works.

  • Which types take the new path: it is chosen at compile time, only when std::vector would copy (the element is not nothrow move constructible), the key is copyable, and T is default constructible and nothrow move assignable. basic_json meets all of these. Every other type keeps the current std::vector behavior. The trait is evaluated inside the member template, because T is still incomplete when basic_json instantiates its object_t.

  • Benchmark: ParseStringOrdered rows mirror ParseString. So far, the benchmark suite had no ordered_json rows at all.

  • Docs: ordered_map.md now describes the growth behavior, has an "Exception safety" section, and gets a version history entry.

Benchmark

ParseString / ParseStringOrdered, measured on an Apple M1 Max with Apple clang 21 -O3:

file json ordered_json before ordered_json after
twitter.json 1.64 ms 3.20 ms 1.70 ms (−47%)
citm_catalog.json 3.26 ms 7.73 ms 3.67 ms (−53%)
jeopardy.json 165 ms 219 ms 177 ms (−19%)
canada.json 10.4 ms 10.5 ms 10.8 ms (no objects)
  • Allocations while parsing twitter drop from 60,456 to 19,182, and for citm_catalog from 157,142 to 53,004. Both are now below json, whose std::map allocates one node per member.
  • Building 2,000 objects of 64 members each through operator[] goes from 97 to 60 ms.

Tests

  • unit-ordered_map.cpp: new test case "ordered_map growth".
    • No value copies: growing through emplace, operator[] and insert(value_type&&) makes no copies of the values. insert(const value_type&) and insert(first, last) make exactly one copy per inserted element.
    • Order and contents are preserved over many growth steps.
    • Strong guarantee: the container is unchanged if copying any of the keys throws while it grows. This is tested for every position of the throwing key, for emplace and for insert(const value_type&).
    • Aliasing: arguments that refer to elements of the full container work, for ordered_map and for ordered_json::emplace.
    • Types that keep the std::vector path: nothrow-movable elements, and a mapped type without a default constructor.
    • Guard against silent regressions: static_asserts check that ordered_json::object_t meets all the conditions, so a future noexcept change can't silently disable the optimization.
  • unit-disabled_exceptions.cpp: grows an ordered_json object with exceptions switched off.
  • Against develop's ordered_map.hpp, the copy-count checks fail (for example 127 value copies instead of 0 for 100 emplaces), and so do the argument checks in the strong-guarantee test.
  • Local runs:
    • the full suite with ASan/UBSan;
    • unit-ordered_map with C++11/14/20, clang -Weverything, the CI's GCC warning flags (GCC 14), clang-tidy, -DJSON_NOEXCEPTION, and -fno-exceptions;
    • make check-amalgamation;
    • the documentation structure check.

Public API impact

No breaking changes.

  • API: value_type stays std::pair<const Key, T>. The public members keep their signatures, and only private member templates are added.
  • Requirements on Key and T: unchanged. Types that don't qualify for the new path keep the std::vector behavior, so everything that compiles today still compiles.
  • Exception safety and iterators: the strong exception guarantee is kept. As before, growing invalidates all iterators and references.
  • Layout: no new data members.
  • Observable differences:
    • fewer copies of T, which only matters if its copy constructor has side effects;
    • on the new path, capacity() grows by a factor of 2. That is what libstdc++ and libc++ already do; MSVC's std::vector uses 1.5. Capacity growth is not a documented guarantee.

This PR was written by Claude Code on behalf of @nlohmann.

🤖 Generated with Claude Code

ordered_map keeps its elements in a std::vector<std::pair<const Key, T>>.
With a std::string key, that pair is not nothrow move constructible (the
const key has to be copied), so std::vector copies every element when it
reallocates. For ordered_json, this deep-copies every member value an
object already holds, including whole nested subtrees, on each growth
step.

Grow the storage in ordered_map instead, copying the keys and moving the
values. This happens in two phases, so the strong exception guarantee is
kept without try/catch. The first phase may throw, but only touches a
temporary buffer: it copies the keys, value-initializes the values, and
constructs the new element. The second phase moves the values (noexcept)
and swaps the buffers. Because the new element is constructed before any
value is moved, arguments that refer to elements of the container stay
valid, as with std::vector. Types that cannot take this path keep the
std::vector behavior.

Parsing into ordered_json (ParseStringOrdered, Apple M1 Max, clang -O3):
twitter 3.20 -> 1.70 ms, citm_catalog 7.73 -> 3.67 ms, jeopardy 219 ->
177 ms, canada unchanged. The number of allocations for twitter and
citm_catalog drops by two thirds.

Also add ParseStringOrdered rows to the benchmarks, and document the
growth behavior and the exception safety of ordered_map.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann marked this pull request as draft September 28, 2026 16:49
@nlohmann
nlohmann marked this pull request as ready for review September 28, 2026 17:23
@nlohmann nlohmann added this to the Release 3.13.0 milestone Sep 28, 2026
@nlohmann nlohmann added the 🚀 ready to merge Ready to merge - just waiting for CI to complete. label Sep 28, 2026
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
ci_icpc and ci_nvhpc failed to compile unit-ordered_map.cpp: the static
assertion that std::pair<const std::string, ordered_json> is not nothrow
move-constructible fails there. The EDG front end (Intel icpc 2021.10,
NVIDIA nvc++ 25.5) considers the defaulted move constructor of
std::pair<const Key, T> noexcept even if copying Key can throw. With these
compilers, std::vector already moves such elements itself when it grows,
and ordered_map correctly leaves growing to it.

The same misjudgement makes std::vector call std::terminate when a key copy
throws during growth, so the exception-safety test with throwing_key would
abort on these compilers as well.

Skip the static assertion and the exception-safety section when __EDG__ is
defined. Verified with icpc 2021.10 (-std=gnu++11) and nvc++ 25.5 (C++11 and
C++17) on Compiler Explorer: unit-ordered_map and unit-disabled_exceptions
build and pass.

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

Copy link
Copy Markdown
Owner Author

CI fix: ci_icpc and ci_nvhpc

Both jobs failed while compiling tests/src/unit-ordered_map.cpp:

unit-ordered_map.cpp(89): error: static assertion failed with "std::vector would move the elements itself"

Cause (PR-caused, test only): icpc 2021.10 and nvc++ 25.5 share the EDG front end, which treats the defaulted move constructor of std::pair<const Key, T> as noexcept even when copying Key can throw. A small probe on Compiler Explorer, with libstdc++ 12, shows std::is_nothrow_move_constructible<std::pair<const std::string, V>> evaluates to 1 on both, but to 0 on GCC. On these compilers std::vector already "moves" the elements itself when it grows: it copies the key and moves the value. ordered_map::append then correctly falls back to emplace_back, so the library code is fine. Only the test's assumption fails.

The same misjudgement means std::vector calls std::terminate there if a key copy throws during growth (the probe aborts). So once the compile error was fixed, the throwing_key exception-safety section would also abort on these compilers.

Fix: c5d40f0 skips the first static_assert and the exception-safety section (together with throwing_key) when __EDG__ is defined. Both compilers define it. The comment in the test explains why. Library code is unchanged.

Verification:

  • Compiler Explorer, icpc 2021.10 (-std=gnu++11) and nvc++ 25.5 (--c++11 --gnu_extensions, --c++17): unit-ordered_map.cpp builds and passes (3350/3352 assertions). unit-disabled_exceptions.cpp with JSON_NOEXCEPTION builds and passes.
  • Local clang, C++11 and C++17: all assertions pass, including with -D__EDG__ to exercise the guarded path.
  • astyle 3.4.13: clean.

— posted by Claude Code on behalf of @nlohmann

nlohmann added a commit that referenced this pull request Sep 30, 2026
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>
@nlohmann
nlohmann merged commit 65af260 into develop Sep 30, 2026
162 of 163 checks passed
@nlohmann
nlohmann deleted the claude/ordered-map-growth-moves-values branch September 30, 2026 18:06
nlohmann added a commit that referenced this pull request Sep 30, 2026
Conflicts:
- include/nlohmann/ordered_map.hpp: both emplace() overloads call
  develop's new append() helper (#5609) with the PR's std::forward<V>(t)
- single_include/nlohmann/json.hpp: regenerated with make amalgamate

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann added a commit that referenced this pull request Sep 30, 2026
Conflicts: none textual. Follow-up fixes for develop's changes:
- docs/mkdocs/docs/api/ordered_map/index.md: develop's new paragraph
  (#5609) was written for api/ordered_map.md; fixed its relative
  ordered_json.md link for the page's new location, plus two
  pre-existing links from an earlier merge (ordered_json.md,
  features/object_order.md)
- tools/api_checker/api_surface.json: regenerated with libclang 18.1.1
  on Linux (swap() noexcept now includes json_base_class_t; new
  integral-key contains/count/find/value overloads)

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann added a commit that referenced this pull request Oct 4, 2026
* Accept lvalues in ordered_map::emplace's value parameter

ordered_map::emplace(key, value) took the mapped value only by T&&, an
rvalue reference rather than a forwarding reference, so
ordered_json::emplace("a", value) failed to compile whenever value was
an lvalue or a const lvalue, even though the same call compiles for
json (whose object_t is std::map, with a variadic emplace). Turn the
value parameter into a separately-deduced forwarding reference,
constrained with std::is_constructible so the overloads still only
accept something convertible to the mapped type. std::map-compatible
semantics are unchanged: emplace still does nothing if the key already
exists.

Open PR #5609 also touches ordered_map.hpp (moving values on vector
growth); this change only touches the two emplace() overloads and
should not conflict.

Fixes #5673.

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

* Avoid astyle's padding in ordered_map::emplace's template headers

Use detail::conjunction instead of && and drop the redundant V&& in detail::is_constructible, so astyle keeps the usual template formatting. Addresses review comment by @gregmarr.

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

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann added a commit that referenced this pull request Oct 4, 2026
* 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 <mail@nlohmann.me>

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

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

* Remove four cppcheck accessForwarded suppressions in the move constructor

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>

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

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

* 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<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>

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

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

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

* 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<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>

* 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<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>

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

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

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

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation L 🚀 ready to merge Ready to merge - just waiting for CI to complete. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants