Compare commits

..
Author SHA1 Message Date
Niels Lohmann 3bb551f46f Fix swap(array_t&)/swap(object_t&) to update parent pointers under JSON_DIAGNOSTICS
Both overloads swapped the underlying container storage but never called
set_parents(), leaving elements moved into *this with stale m_parent
pointers (typically nullptr from the free-standing array_t/object_t).
This produced wrong JSON Pointer paths in diagnostic messages and could
trip assert_invariant() on subsequent copies. Mirrors the fix already
applied in swap(reference other).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 22:01:39 +02:00
5 changed files with 39 additions and 135 deletions
+2 -14
View File
@@ -71,7 +71,7 @@ class serializer
, thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep)))
, decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point)))
, indent_char(ichar)
, indent_string()
, indent_string(512, indent_char)
, error_handler(error_handler_)
{}
@@ -126,10 +126,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -203,10 +199,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -268,10 +260,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -1022,7 +1010,7 @@ class serializer
/// the indentation character
const char indent_char;
/// the indentation string (lazily allocated on first use by a pretty-print branch)
/// the indentation string
string_t indent_string;
/// error_handler how to react on decoding errors
+2
View File
@@ -3573,6 +3573,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{
using std::swap;
swap(*(m_data.m_value.array), other);
set_parents();
}
else
{
@@ -3589,6 +3590,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{
using std::swap;
swap(*(m_data.m_value.object), other);
set_parents();
}
else
{
+4 -14
View File
@@ -20150,7 +20150,7 @@ class serializer
, thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep)))
, decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point)))
, indent_char(ichar)
, indent_string()
, indent_string(512, indent_char)
, error_handler(error_handler_)
{}
@@ -20205,10 +20205,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -20282,10 +20278,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -20347,10 +20339,6 @@ class serializer
// variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{
indent_string.resize(indent_string.size() * 2, ' ');
@@ -21101,7 +21089,7 @@ class serializer
/// the indentation character
const char indent_char;
/// the indentation string (lazily allocated on first use by a pretty-print branch)
/// the indentation string
string_t indent_string;
/// error_handler how to react on decoding errors
@@ -25013,6 +25001,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{
using std::swap;
swap(*(m_data.m_value.array), other);
set_parents();
}
else
{
@@ -25029,6 +25018,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{
using std::swap;
swap(*(m_data.m_value.object), other);
set_parents();
}
else
{
+31
View File
@@ -273,5 +273,36 @@ TEST_CASE("Regression tests for extended diagnostics")
CHECK(j1["numbers"]["two"] == 2);
CHECK(j1["string"] == "t");
}
SECTION("Regression test - swap(array_t&)/swap(object_t&) must update JSON_DIAGNOSTICS parent pointers")
{
// swap(array_t&)
{
json j = json::array();
json::array_t arr = {json::array({1})};
j.swap(arr);
// parent pointers of the moved-in elements must point into j, not
// into the now-defunct free-standing array_t
CHECK_THROWS_WITH_AS(j[0][0].get<std::string>(), "[json.exception.type_error.302] (/0/0) type must be string, but is number", json::type_error);
// must not trigger assert_invariant() in a debug/assert-enabled build
json const k = j;
CHECK(k == j);
}
// swap(object_t&)
{
json o = json::object();
json::object_t obj = {{"a", json::array({1})}};
o.swap(obj);
CHECK_THROWS_WITH_AS(o["a"][0].get<std::string>(), "[json.exception.type_error.302] (/a/0) type must be string, but is number", json::type_error);
// must not trigger assert_invariant() in a debug/assert-enabled build
json const p = o;
CHECK(p == o);
}
}
}
-107
View File
@@ -14,40 +14,6 @@ using nlohmann::json;
#include <array>
#include <sstream>
#include <iomanip>
#include <cstdlib>
#include <new>
namespace
{
// heap allocation counter used by the regression test for issue #5413
// (https://github.com/nlohmann/json/issues/5413); disabled (and thus a
// no-op besides the counting) unless explicitly toggled on
bool count_heap_allocations = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
std::size_t heap_allocations = 0; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
} // namespace
void* operator new (std::size_t size) // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
if (count_heap_allocations)
{
++heap_allocations;
}
if (void* ptr = std::malloc(size)) // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
{
return ptr;
}
throw std::bad_alloc(); // NOLINT(hicpp-exception-baseclass)
}
void operator delete (void* ptr) noexcept // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
std::free(ptr); // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
}
void operator delete (void* ptr, std::size_t /*size*/) noexcept // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
std::free(ptr); // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
}
TEST_CASE("serialization")
{
@@ -416,76 +382,3 @@ TEST_CASE("dump for basic_json with long double number_float_t")
check_same(100.0L, 100.0);
}
}
TEST_CASE("regression test for issue #5413 - lazily allocated indent_string")
{
// the serializer used to unconditionally allocate a 512-byte
// indent_string in its constructor, even though it is only ever read
// inside the pretty_print branches of dump(). This wasted a heap
// allocation (and its matching deallocation) on every single compact
// (i.e. non-pretty, the default) dump() call. indent_string is now
// allocated lazily, the first time a pretty-print branch actually
// needs it -- so a compact dump() must perform strictly fewer heap
// allocations than a pretty dump() of the same value.
const json j = {{"level", "info"}, {"msg", "hello world"}, {"id", 12345}};
// warm up anything unrelated to indentation (e.g., one-time locale
// lookups) that might otherwise allocate on first use regardless of
// pretty-printing, so it does not skew the counts measured below
const auto warmup = j.dump();
const auto warmup_pretty = j.dump(4);
CHECK(!warmup.empty());
CHECK(!warmup_pretty.empty());
SECTION("compact dump() has a stable, minimal allocation count")
{
count_heap_allocations = true;
heap_allocations = 0;
const auto compact1 = j.dump();
const auto allocs_compact1 = heap_allocations;
heap_allocations = 0;
const auto compact2 = j.dump(-1);
const auto allocs_compact2 = heap_allocations;
count_heap_allocations = false;
CHECK(compact1 == compact2);
// dump() and dump(-1) both take the compact code path and must
// never touch indent_string, so they allocate identically often
CHECK(allocs_compact1 == allocs_compact2);
}
SECTION("first pretty dump() allocates more than a compact dump()")
{
// use a tiny value whose compact ({"a":1}, 7 bytes) and pretty
// ({"a": 1} with 1-space indent, 11 bytes) serializations both stay
// well inside every common std::string small-string-optimization
// buffer (>= 15 bytes on libstdc++/MSVC STL, >= 22 on libc++), so
// building the result string itself causes no heap allocation
// either way -- isolating indent_string as the only thing that can
// possibly account for a difference in allocation count
const json tiny = {{"a", 1}};
count_heap_allocations = true;
heap_allocations = 0;
const auto compact = tiny.dump();
const auto allocs_compact = heap_allocations;
heap_allocations = 0;
const auto pretty = tiny.dump(1);
const auto allocs_pretty = heap_allocations;
count_heap_allocations = false;
CHECK(compact == "{\"a\":1}");
CHECK(pretty == "{\n \"a\": 1\n}");
// a fresh serializer is created per dump() call; the pretty branch
// lazily allocates indent_string on its first use, so it must
// allocate at least once more than the compact branch, which never
// touches indent_string at all
CHECK(allocs_pretty > allocs_compact);
}
}