* Compare values without recursing, and without comparing them twice
Comparing two values compared their containers, which compare their elements,
which brought the comparison back once per nesting level. Two values nested
deeply enough exhausted the call stack and terminated the process with a
segmentation fault - the same bug as #5387, in the last operation that still
had it.
Worse, an ordered comparison took exponentially long in the nesting depth
before C++20. std::vector's operator< is a lexicographical comparison, which
asks whether an element is less than its counterpart and then whether the
counterpart is less than it - two full comparisons of everything below that
element, at every level. Comparing two equal values nested 30 levels deep,
which is nothing unusual, took 3.8 seconds; 40 levels would have taken an
hour, and nothing about the value has to be pathological to get there. C++20
is unaffected: std::lexicographical_compare_three_way asks once.
Compare a value that is nested too deeply to descend into on an explicit
stack instead, in a single pass that yields less, equal, greater or unordered
at once. Equality and the three-way comparison descend as they always did for
the first 128 levels, which nothing measurable costs them; an ordered
comparison no longer descends at all, which is what takes the exponent out of
it. Objects and arrays that are not nested deeply are otherwise compared
exactly as before.
The results are unchanged for every pair of values: 68121 comparisons of a
corpus that covers NaN, discarded values, mixed number types, binary values,
empty containers and both object types are identical to develop, in C++11,
C++17 and C++20, with and without thread_local storage and legacy discarded
comparison. Reproducing that meant reproducing two subtleties: a lexicographic
comparison steps over a pair it cannot order, where a three-way comparison
stops at it, and an object compares its keys with < where its entries are
ordered but with == where they are only checked for equality - not with the
object's own comparator, which for nlohmann::ordered_map tells equality.
Equality needs no ordering, so it no longer asks for any: a key or string type
that can only be compared for equality still works.
Measured (medians of 7 interleaved runs, clang -O3, C++11): comparing two
equal values nested 30 levels deep 3778 ms -> 0.002 ms; ordering flat objects
-33.6%; ordering flat arrays of numbers +27.3%, the one shape that pays for
the single pass; equality unchanged throughout.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Describe comparison in the no-thread-local docs and CI target
Comparing two values now bounds its descent with a thread_local counter
just as copying does, so the JSON_NO_THREAD_LOCAL page, the macro
overview and the ci_test_no_thread_local target cover both rather than
copying alone.
Also record what switching the macro on costs a comparison: on the
benchmark documents, comparing two equal values takes 10% to 90% longer.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Take the descent flag as an argument rather than testing it
MSVC reports the test of a constant as C4127 ("conditional expression is
constant"), which the Windows builds treat as an error: may_descend is
false for operator<, so the operand short-circuits the whole condition.
Passing it to compare_descent_exhausted() puts the test where the value
is an ordinary parameter, and leaves the call sites with no condition of
their own.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Note the comparison fallback in the no-thread-local documentation
The macro page describes what the library defines JSON_NO_THREAD_LOCAL for
by itself in terms of copying alone; comparing falls back the same way.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Parenthesise the reserve() computation in the comparison test
clang-tidy reports the mixed * and + as readability-math-missing-
parentheses, as it does for the identical line in the copy test.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Use the shared descent bookkeeping rather than a second set
Comparing kept a thread_local count, a limit and a guard of its own beside
the ones copying already had, all three the same thing under a different
name. They are gone; the shared count, limit and guard do the work.
The guard grows a second constructor here, because the comparison
operators are written as a macro and a macro cannot use the preprocessor:
it cannot look the count up behind an #ifdef the way copy_structured does,
so the guard looks it up for it. nesting_depth_exhausted() arrives for the
same reason - whether an operator descends at all is a constant at every
call site, and testing it there is what MSVC reports as C4127.
Also say in compare_leaves what happens to a pair that is an array on one
side and an object on the other, since the answer is not obvious from the
code: an operator only descends into two values of the same type, so such
a pair is told apart by its types alone - unequal, and ordered the way the
types are - exactly as it is above the bound.
And record what the explicit stack costs: the comparison operators are
noexcept and the container comparison this replaces allocated nothing, so
running out of memory here ends the process instead of throwing. It takes
a value nested past the bound and an exhausted heap to reach, and the same
comparison used to exhaust the call stack, but it is a new way to fail.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Amalgamate
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
---------
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Bound the descent of the copy constructor
basic_json's copy constructor copied objects and arrays by handing the
container to its own copy constructor, which copy-constructs every element
and so reaches this constructor again, once per nesting level. A value
nested deeply enough exhausted the call stack and terminated the process
with a segmentation fault - no exception, nothing the caller could catch.
Parsing such a value works, as the parser is iterative, and so does
destroying one, as #1436 made destruction iterative.
Bound how far the copy descends rather than take the call stack away from
it. The first levels are copied exactly as they were - the containers copy
their own elements, which is by far the fastest way to fill them - and only
once the copy has descended 128 levels is the value below it finished
without the call stack, through an explicit worklist. Copying can therefore
no longer exhaust the stack, however deeply a value is nested, while a value
nested less deeply than the bound - all but a vanishing minority - is copied
by the very same code as before and pays only for one counter.
That counter lives in thread_local storage, as one shared between threads
would be raced. JSON_NO_THREAD_LOCAL switches it off for toolchains without
thread_local; copying then goes through the worklist right away, which
yields the same values but is measurably slower.
The deferred values are completed before the copy they belong to returns, so
a value copied while another copy is going on - by a custom base class, say -
is unaffected by the copy it is nested in.
operator= takes its argument by value, so copy assignment is fixed as well.
Copying is as fast as it was, within measurement noise (medians of 9
interleaved runs, clang -O3): -1.3% for an array of strings, +0.0% for a
flat object, +0.1% for a flat array of numbers, +0.3% for nested arrays,
+0.6% for nested objects and +1.2% for a twitter-like document. Copying a
three-key object costs about ten nanoseconds more, the counter. Deferring
every level instead, rather than only those below the bound, measured
between 3% and 9% slower depending on the shape of the value.
This fixes#5387 for the copy constructor. dump() is still recursive.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Test the copy constructor's iterative path in CI
The copy constructor descends into 128 levels before it finishes a value
without the call stack, so the iterative path is otherwise only reached
by the few tests that nest deeper than that.
JSON_NO_THREAD_LOCAL switches the descent off, which sends every value
down that path. Running the whole test suite that way covers it with
every object type, string type, allocator, and base class the suite
already exercises. The new ci_test_no_thread_local target does that; the
macro had no build coverage at all before.
Copying a nested value also has to carry over what the element-wise copy
constructor would have copied: the parents that JSON_DIAGNOSTICS relies
on, and the positions that JSON_DIAGNOSTIC_POSITIONS reports. Both are
now checked on either side of the descent bound, for objects and arrays.
Neither was tested before, and dropping either one makes the new tests
fail.
Also quantify what JSON_NO_THREAD_LOCAL costs a copy instead of calling
it "measurably slower".
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Split the regression tests so that they keep linking
Linking test-regression2 fails with "relocation truncated to fit:
IMAGE_REL_AMD64_REL32 against `.rdata'" once its object grows past what
the MinGW linker copes with, and the copy constructor's helpers push it
over: the object grows by 6.3%, from 4,654,128 to 4,944,920 bytes at -O0,
and develop links at the smaller of the two.
Building the tests optimized shrinks the object enough to link, but the
binaries clang 11.0.1 and clang 18.1.8 then produce crash before doctest
prints its first line - 39 of 102 tests on clang 18 - so the objects have
to become smaller rather than denser.
Moving the test cases that follow "regression tests 2" into a file of
their own brings that object to 4,687,888 bytes, which is 0.7% above the
size that links today rather than 6.3%. Both files still build for C++11,
C++17 and C++20, and run the same 9 test cases and 135 assertions as
before, now spread over two binaries.
New regression tests belong in unit-regression3.cpp from here on, which
is what CONTRIBUTING.md now says.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Do not use thread_local storage with Clang targeting MinGW
Every test that copies a value segfaults there - 42 of 105 on clang
11.0.1, 39 of 102 on clang 18.1.8 - while the same tests pass with GCC
targeting MinGW, with Clang targeting MSVC, and with every other
toolchain the library is tested on. The counter that bounds the copy
constructor's descent is the library's first use of thread_local, so
that job had never exercised it before.
JSON_NO_THREAD_LOCAL already covers toolchains without thread_local
storage, and copying yields the same values with it, only more slowly.
Define it for this one automatically.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Balance the warning suppression the split separated
unit-regression2.cpp opens a DOCTEST_CLANG_SUPPRESS_WARNING_PUSH block at
the top and closed it at the very bottom, which the split moved into
unit-regression3.cpp: one file was left with a push and no pop, the other
with a pop and no push, which clang reports as an error.
Give each file the pair it needs.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Check both shapes without a C-style array
clang-tidy rejects the array the two shapes were iterated over
(cppcoreguidelines-avoid-c-arrays). The array only existed because astyle
reformats a range-for over a braced initializer list into something
unreadable; naming the two cases avoids both.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Split the regression tests far enough to leave room
The first split left unit-regression2.cpp 0.7% below the size develop
links at, which the comparison change in the follow-up immediately used
up: the MinGW linker fails on test-regression2_cpp20 again, naming
copy_shallow and to_partial_ordering among the relocations it cannot fit.
Move the sections from "issue #2067" on, and the helper types they use,
so that the file stops being the one that decides whether the tests can
be linked at all. At -O0 and C++20, unit-regression2.cpp is now 2,964,944
bytes against develop's 4,708,248, and 3,070,568 bytes with the follow-up
applied - roughly a third smaller either way, rather than a fraction of a
percent larger.
The 135 assertions are the same ones as before, now spread over three
test cases in two files.
Also silence the clang-tidy findings the deep-nesting tests draw: the
copies they make are what is being tested, and the reserve() computation
gets its parentheses.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Move the #4804 alias to the file that uses it
The split left the json_4804 alias behind in unit-regression2.cpp while
the test case that uses it went to unit-regression3.cpp, which does not
build for C++17 and C++20 as a result.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Include <span> where the split moved its only use
The #2546 test case guards itself with __has_include(<span>), but the
include itself sat in unit-regression2.cpp's preamble and stayed behind,
so the section compiled without a declaration wherever the guard passed -
which nvhpc reported and libc++ builds do not, as they skip the section
altogether.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Keep the descent bookkeeping in one place
Copying carried a depth count, a depth limit and a guard of its own, and
the comparison in the follow-up added a second set beside them. Neither
operation needs its own: they are never nested inside one another by the
library - copying a value does not compare one, and comparing two values
does not copy them - and where user code nests them anyway, sharing the
count only ends a descent sooner than it had to.
So there is now one nesting_depth(), one nesting_depth_limit() and one
nesting_depth_guard, which the follow-up uses instead of adding its own.
Inverting the test in copy_structured leaves the too-deep case and the
no-thread-local case as the same code.
The guard takes the count rather than looking it up, because the caller
has looked it up already to test it against the limit, and reaching
thread-local storage twice on the path that is taken almost every time is
worth avoiding.
The switch that copies the value of anything that is not an object or an
array was written twice - once in the copy constructor, once in
copy_shallow - so that adding a value_t meant editing both, and missing
one would have been silent. It is copy_leaf_value now, and inlined: both
callers have already sorted the containers out, and folding that test into
the switch is what keeps a value made mostly of numbers copying as fast as
it did.
Copying canada.json, citm_catalog.json and twitter.json is within 0.6% of
what it was before, measured as a paired ratio over 18 interleaved rounds
against a run-to-run spread of 0.3%.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Check that an abandoned copy can still be destroyed
Copying a value without the call stack builds the copy from the top down,
and every value whose own copy has not been made yet stays a null value
until it is. That is what lets a copy be abandoned half-built: the
destructor finds nothing but complete values and null ones.
Nothing tested it. Failing an allocation part-way through a copy of a
deeply nested value does, with the allocator the file already has for
exactly this kind of test.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Name the test's locals so Flawfinder stops matching them
The code scanning job reports CWE-362 - "check when opening files" - for
a test that opens no files: Flawfinder matched a local variable called
open. Rename it and its partner.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Keep the descent guard's bookkeeping self-contained
nesting_depth_limit() and nesting_depth_guard were only used inside
the JSON_NO_THREAD_LOCAL-guarded branch of copy_structured(), but were
defined unconditionally. Move them inside the #ifndef, and have the
guard look up the depth and test it against the limit itself (via
okay()) instead of making the caller do it - the caller no longer
needs to touch nesting_depth() at all. Also shrink the thread-local
counter to std::uint8_t, matching what its own doc comment already
argued.
Addresses gregmarr's review comments on #5389.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Make nesting_depth_guard usable regardless of JSON_NO_THREAD_LOCAL
nesting_depth_limit() and nesting_depth() stay behind #ifndef
JSON_NO_THREAD_LOCAL, since a descent cannot be bounded without a
per-thread count. But the guard itself now always exists, becoming a
no-op that is never okay() under that macro - the same way the bound
is already reached on every call without one. copy_structured() no
longer needs to know which case it is in.
This is what lets #5390 reuse the guard for comparison, which cannot
test JSON_NO_THREAD_LOCAL where the macro-based operators use it: the
guard now carries that distinction itself instead of requiring every
caller to.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Silence VS2015's C4503 for the custom-base-class test
The deep-copy support added for #5387 lengthened the mangled name of
std::allocator_traits<...>::construct for the test's map type past
VS2015's limit, which /WX turns into a build failure even though the
name is only used for (now-truncated) debug info.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Remove dead unused-parameter casts from copy_metadata()
@gregmarr asked whether the static_cast<void> pair in the
JSON_DIAGNOSTIC_POSITIONS-off branch was needed for an empty
json_base_class_t. It isn't: src and dst are already referenced
unconditionally by the base-class copy above, so no -Wunused-parameter
warning fires either way (checked with -Wall -Wextra
-Wunused-parameter, JSON_DIAGNOSTIC_POSITIONS 0 and 1).
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Fix CI: build custom array types without a fill constructor, re-amalgamate
copy_array_level() built the destination array with the fill
constructor array_t(count, value), which is not part of the array
container interface the library otherwise assumes (e.g. custom
ArrayTypes that only provide a default and an iterator-pair
constructor, as covered by unit-custom-array-type.cpp). Default-
construct the array and resize() it instead, matching how the rest
of the codebase already grows array_t.
Also re-run the amalgamation, which had fallen out of sync with
include/nlohmann/json.hpp.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Niels Lohmann
