Compare commits

..
Author SHA1 Message Date
Claude 3c38455c7d BSON writer: drop the SFINAE guard that Visual Studio 2017 cannot deduce
write_bson_document_single_pass() was a member template enabled by
is_detected<sink_written_t, SinkType>. MSVC 19.16 (Visual Studio 2017)
fails to deduce it through the class-scope alias template (C2672, C2783)
when it compiles to_bson() and to_bon8(), so every Visual Studio 2017
job on AppVeyor fails since #5979.

The guard is not needed: as a non-template member of the class template,
the function is only instantiated when called, and its only caller is
the write_bson_document() overload for sinks that have written(), which
already calls oa.written() and oa.rewind() itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N5NasAgbDGG28MqXjQvLqp
Signed-off-by: Claude <noreply@anthropic.com>
2026-11-02 10:35:13 +00:00
Claude 4571446da7 CI: give the Valgrind tests a timeout of one hour
Under Valgrind, test-json_view_builder_cpp11 and test-ordered_json_cpp11
took 1435 s and 1390 s on develop (ddc34f5), close to ctest's default
timeout of 1500 s. On the next run (b2674a4) the whole job was about 10 %
slower and both timed out, failing ci_test_valgrind on develop and on
every PR based on it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N5NasAgbDGG28MqXjQvLqp
Signed-off-by: Claude <noreply@anthropic.com>
2026-11-02 10:29:38 +00:00
5 changed files with 129 additions and 227 deletions

No files matched your search

+3 -1
View File
@@ -178,8 +178,10 @@ function(_json_test_add_test test_name file main cxx_standard)
COMMAND ${memcheck_command} $<TARGET_FILE:${test_target}> ${DOCTEST_TEST_FILTER} COMMAND ${memcheck_command} $<TARGET_FILE:${test_target}> ${DOCTEST_TEST_FILTER}
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
) )
# under Valgrind the slowest tests take close to 25 minutes, so ctest's
# default timeout of 1500 seconds leaves no headroom
set_tests_properties(${test_target}_valgrind PROPERTIES set_tests_properties(${test_target}_valgrind PROPERTIES
LABELS "valgrind" FIXTURES_REQUIRED TEST_DATA LABELS "valgrind" FIXTURES_REQUIRED TEST_DATA TIMEOUT 3600
) )
endif() endif()
endfunction() endfunction()
@@ -2422,7 +2422,6 @@ class binary_writer
nothing large has been written then, and the caller writes the nothing large has been written then, and the caller writes the
document in two passes, which reports that error document in two passes, which reports that error
*/ */
template<typename SinkType = OutputSinkType, typename std::enable_if<is_detected<sink_written_t, SinkType>::value, int>::type = 0>
bool write_bson_document_single_pass(const BasicJsonType& document) bool write_bson_document_single_pass(const BasicJsonType& document)
{ {
const std::size_t start = oa.written(); const std::size_t start = oa.written();
+63 -67
View File
@@ -543,10 +543,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
//////////////////////// ////////////////////////
JSON_PRIVATE_UNLESS_TESTED: JSON_PRIVATE_UNLESS_TESTED:
// the type and value of a basic_json without its base class (defined
// below); the destruction helpers of json_value use it as a stand-in
struct data;
/*! /*!
@brief a JSON value @brief a JSON value
@@ -723,28 +719,27 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
explicit json_value(array_t* value) noexcept : array(value) {} explicit json_value(array_t* value) noexcept : array(value) {}
private: private:
// raw, allocation-free transfer of the type and value from src to // raw, allocation-free transfer of m_data from src to dst: no
// dst: no set_parents()/assert_invariant() (the former is // set_parents()/assert_invariant() (the former is O(#children) per
// O(#children) per call under JSON_DIAGNOSTICS, which would make the // call under JSON_DIAGNOSTICS, which would make the walk below
// walk below quadratic); dst takes ownership, src is left as // quadratic); dst takes ownership, src is left as value_t::null.
// value_t::null. static void take(basic_json& dst, basic_json& src) noexcept
static void take(data& dst, data& src) noexcept
{ {
dst.m_type = src.m_type; dst.m_data.m_type = src.m_data.m_type;
dst.m_value = src.m_value; dst.m_data.m_value = src.m_data.m_value;
src.m_type = value_t::null; src.m_data.m_type = value_t::null;
} }
// true if v is not an array/object, or is an already-empty one // true if v is not an array/object, or is an already-empty one
static bool has_no_children(const data& v) noexcept static bool has_no_children(const basic_json& v) noexcept
{ {
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
return v.m_value.array->empty(); return v.m_data.m_value.array->empty();
} }
if (v.m_type == value_t::object) if (v.m_data.m_type == value_t::object)
{ {
return v.m_value.object->empty(); return v.m_data.m_value.object->empty();
} }
return true; return true;
} }
@@ -753,21 +748,21 @@ private:
// its own, i.e. v can be freed in one go (its destructor then // its own, i.e. v can be freed in one go (its destructor then
// destroys scalars and empty containers only, so it recurses at most // destroys scalars and empty containers only, so it recurses at most
// one level deep); stops at the first structured child it meets // one level deep); stops at the first structured child it meets
static bool all_children_leaves(const data& v) noexcept static bool all_children_leaves(const basic_json& v) noexcept
{ {
// array_t only needs forward iterators (see custom_array_type) // array_t only needs forward iterators (see custom_array_type)
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
const auto& a = *v.m_value.array; const auto& a = *v.m_data.m_value.array;
return std::all_of(a.begin(), a.end(), [](const basic_json & el) return std::all_of(a.begin(), a.end(), [](const basic_json & el)
{ {
return has_no_children(el.m_data); return has_no_children(el);
}); });
} }
const auto& o = *v.m_value.object; const auto& o = *v.m_data.m_value.object;
return std::all_of(o.begin(), o.end(), [](const typename object_t::value_type & el) return std::all_of(o.begin(), o.end(), [](const typename object_t::value_type & el)
{ {
return has_no_children(el.second.m_data); return has_no_children(el.second);
}); });
} }
@@ -799,30 +794,30 @@ private:
// the child of a non-empty array/object v that the walk in // the child of a non-empty array/object v that the walk in
// destroy_container() continues with (see walk_child_it() above) // destroy_container() continues with (see walk_child_it() above)
static data& walk_child(data& v) static basic_json& walk_child(basic_json& v)
{ {
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
return v.m_value.array->back().m_data; return v.m_data.m_value.array->back();
} }
JSON_ASSERT(v.m_type == value_t::object); JSON_ASSERT(v.m_data.m_type == value_t::object);
return walk_child_it(*v.m_value.object)->second.m_data; return walk_child_it(*v.m_data.m_value.object)->second;
} }
// removes walk_child(v) from a non-empty array/object v; this never // removes walk_child(v) from a non-empty array/object v; this never
// allocates, and since it is only ever called when that child is a // allocates, and since it is only ever called when that child is a
// scalar or an already-empty array/object, destroying it never // scalar or an already-empty array/object, destroying it never
// recurses more than one level deep (see destroy() below) // recurses more than one level deep (see destroy() below)
static void pop_walk_child(data& v) static void pop_walk_child(basic_json& v)
{ {
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
v.m_value.array->pop_back(); v.m_data.m_value.array->pop_back();
} }
else else
{ {
JSON_ASSERT(v.m_type == value_t::object); JSON_ASSERT(v.m_data.m_type == value_t::object);
v.m_value.object->erase(walk_child_it(*v.m_value.object)); v.m_data.m_value.object->erase(walk_child_it(*v.m_data.m_value.object));
} }
} }
@@ -831,23 +826,23 @@ private:
// recurses at most one level deep); this is the same allocator-based // recurses at most one level deep); this is the same allocator-based
// free the old recursive implementation used, just factored out so // free the old recursive implementation used, just factored out so
// every level of the walk in destroy() can share it // every level of the walk in destroy() can share it
static void free_container(data& v) noexcept static void free_container(basic_json& v) noexcept
{ {
JSON_ASSERT(has_no_children(v) || all_children_leaves(v)); JSON_ASSERT(has_no_children(v) || all_children_leaves(v));
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
AllocatorType<array_t> alloc; AllocatorType<array_t> alloc;
std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_value.array); std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_data.m_value.array);
std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_value.array, 1); std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_data.m_value.array, 1);
} }
else else
{ {
JSON_ASSERT(v.m_type == value_t::object); JSON_ASSERT(v.m_data.m_type == value_t::object);
AllocatorType<object_t> alloc; AllocatorType<object_t> alloc;
std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_value.object); std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_data.m_value.object);
std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_value.object, 1); std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_data.m_value.object, 1);
} }
v.m_type = value_t::null; // avoid a double free if v is later destructed v.m_data.m_type = value_t::null; // avoid a double free if v is later destructed
} }
public: public:
@@ -878,16 +873,14 @@ public:
// This json_value is not itself a basic_json, so the top-level // This json_value is not itself a basic_json, so the top-level
// container is first moved into a local stand-in ("cur"); a // container is first moved into a local stand-in ("cur"); a
// default-constructed data has a null pointer in its m_value // default-constructed basic_json has a null pointer in its
// (see data::m_value's initializer), so swapping it with *this // m_value (see data::m_value's initializer), so swapping it
// leaves this union's own pointer null, and it is never looked // with *this leaves this union's own pointer null, and it is
// at or freed a second time. The stand-ins are data, not // never looked at or freed a second time.
// basic_json, so that destroying a value never constructs a basic_json cur;
// CustomBaseClass, whose constructor may allocate (#5995). cur.m_data.m_type = t;
data cur;
cur.m_type = t;
using std::swap; using std::swap;
swap(cur.m_value, *this); swap(cur.m_data.m_value, *this);
destroy_bounded(cur, 0); destroy_bounded(cur, 0);
} }
@@ -896,7 +889,7 @@ public:
// The first recursion_depth_limit() levels are destroyed by plain // The first recursion_depth_limit() levels are destroyed by plain
// recursion, which is the fastest way and cannot throw; anything // recursion, which is the fastest way and cannot throw; anything
// nested deeper is handed to destroy_walk(), which uses no call stack. // nested deeper is handed to destroy_walk(), which uses no call stack.
static void destroy_bounded(data& v, std::size_t depth) noexcept static void destroy_bounded(basic_json& v, std::size_t depth) noexcept
{ {
if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit()))
{ {
@@ -904,23 +897,23 @@ public:
return; return;
} }
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
for (auto& child : *v.m_value.array) for (auto& child : *v.m_data.m_value.array)
{ {
if (child.m_data.m_type == value_t::array || child.m_data.m_type == value_t::object) if (child.m_data.m_type == value_t::array || child.m_data.m_type == value_t::object)
{ {
destroy_bounded(child.m_data, depth + 1); destroy_bounded(child, depth + 1);
} }
} }
} }
else else
{ {
for (auto& child : *v.m_value.object) for (auto& child : *v.m_data.m_value.object)
{ {
if (child.second.m_data.m_type == value_t::array || child.second.m_data.m_type == value_t::object) if (child.second.m_data.m_type == value_t::array || child.second.m_data.m_type == value_t::object)
{ {
destroy_bounded(child.second.m_data, depth + 1); destroy_bounded(child.second, depth + 1);
} }
} }
} }
@@ -947,15 +940,18 @@ public:
// array/object, which neither allocates nor recurses more // array/object, which neither allocates nor recurses more
// than one level deep. // than one level deep.
// //
// Like destroy_container(), the walk keeps the containers it // This json_value is not itself a basic_json, so the
// holds in data stand-ins (cur, prev, gp, tmp), which never // top-level container is first moved into a local stand-in
// construct a CustomBaseClass. // ("cur"); a default-constructed basic_json has a null
static void destroy_walk(data& top) noexcept // pointer in its m_value (see data::m_value's initializer),
// so swapping it with *this leaves this union's own pointer
// null, and it is never looked at or freed a second time.
static void destroy_walk(basic_json& top) noexcept
{ {
data cur; basic_json cur;
take(cur, top); take(cur, top);
data prev; // value_t::null: no parent basic_json prev; // value_t::null: no parent
// true right after cur was entered: its children have not been // true right after cur was entered: its children have not been
// looked at yet. If none of them has children of its own, cur is // looked at yet. If none of them has children of its own, cur is
@@ -969,7 +965,7 @@ public:
fresh = false; fresh = false;
if (has_no_children(cur) || (fresh_now && all_children_leaves(cur))) if (has_no_children(cur) || (fresh_now && all_children_leaves(cur)))
{ {
if (prev.m_type == value_t::null) if (prev.m_data.m_type == value_t::null)
{ {
free_container(cur); free_container(cur);
return; // back at the top with nothing left to do return; // back at the top with nothing left to do
@@ -978,7 +974,7 @@ public:
// ascend: detach the grandparent link from prev's // ascend: detach the grandparent link from prev's
// walk_child() slot, drop that (now null) slot, free cur // walk_child() slot, drop that (now null) slot, free cur
// (it is empty), then move up one level // (it is empty), then move up one level
data gp; basic_json gp;
take(gp, walk_child(prev)); take(gp, walk_child(prev));
pop_walk_child(prev); pop_walk_child(prev);
@@ -989,7 +985,7 @@ public:
continue; continue;
} }
data& cur_child_ref = walk_child(cur); basic_json& cur_child_ref = walk_child(cur);
if (has_no_children(cur_child_ref)) if (has_no_children(cur_child_ref))
{ {
@@ -1001,7 +997,7 @@ public:
// descend into the non-empty child, reversing the // descend into the non-empty child, reversing the
// link: its slot takes over prev, and the child becomes // link: its slot takes over prev, and the child becomes
// the new cur // the new cur
data tmp; basic_json tmp;
take(tmp, cur_child_ref); take(tmp, cur_child_ref);
take(cur_child_ref, prev); take(cur_child_ref, prev);
take(prev, cur); take(prev, cur);
+63 -68
View File
@@ -30394,7 +30394,6 @@ class binary_writer
nothing large has been written then, and the caller writes the nothing large has been written then, and the caller writes the
document in two passes, which reports that error document in two passes, which reports that error
*/ */
template<typename SinkType = OutputSinkType, typename std::enable_if<is_detected<sink_written_t, SinkType>::value, int>::type = 0>
bool write_bson_document_single_pass(const BasicJsonType& document) bool write_bson_document_single_pass(const BasicJsonType& document)
{ {
const std::size_t start = oa.written(); const std::size_t start = oa.written();
@@ -38373,10 +38372,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
//////////////////////// ////////////////////////
JSON_PRIVATE_UNLESS_TESTED: JSON_PRIVATE_UNLESS_TESTED:
// the type and value of a basic_json without its base class (defined
// below); the destruction helpers of json_value use it as a stand-in
struct data;
/*! /*!
@brief a JSON value @brief a JSON value
@@ -38553,28 +38548,27 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
explicit json_value(array_t* value) noexcept : array(value) {} explicit json_value(array_t* value) noexcept : array(value) {}
private: private:
// raw, allocation-free transfer of the type and value from src to // raw, allocation-free transfer of m_data from src to dst: no
// dst: no set_parents()/assert_invariant() (the former is // set_parents()/assert_invariant() (the former is O(#children) per
// O(#children) per call under JSON_DIAGNOSTICS, which would make the // call under JSON_DIAGNOSTICS, which would make the walk below
// walk below quadratic); dst takes ownership, src is left as // quadratic); dst takes ownership, src is left as value_t::null.
// value_t::null. static void take(basic_json& dst, basic_json& src) noexcept
static void take(data& dst, data& src) noexcept
{ {
dst.m_type = src.m_type; dst.m_data.m_type = src.m_data.m_type;
dst.m_value = src.m_value; dst.m_data.m_value = src.m_data.m_value;
src.m_type = value_t::null; src.m_data.m_type = value_t::null;
} }
// true if v is not an array/object, or is an already-empty one // true if v is not an array/object, or is an already-empty one
static bool has_no_children(const data& v) noexcept static bool has_no_children(const basic_json& v) noexcept
{ {
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
return v.m_value.array->empty(); return v.m_data.m_value.array->empty();
} }
if (v.m_type == value_t::object) if (v.m_data.m_type == value_t::object)
{ {
return v.m_value.object->empty(); return v.m_data.m_value.object->empty();
} }
return true; return true;
} }
@@ -38583,21 +38577,21 @@ private:
// its own, i.e. v can be freed in one go (its destructor then // its own, i.e. v can be freed in one go (its destructor then
// destroys scalars and empty containers only, so it recurses at most // destroys scalars and empty containers only, so it recurses at most
// one level deep); stops at the first structured child it meets // one level deep); stops at the first structured child it meets
static bool all_children_leaves(const data& v) noexcept static bool all_children_leaves(const basic_json& v) noexcept
{ {
// array_t only needs forward iterators (see custom_array_type) // array_t only needs forward iterators (see custom_array_type)
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
const auto& a = *v.m_value.array; const auto& a = *v.m_data.m_value.array;
return std::all_of(a.begin(), a.end(), [](const basic_json & el) return std::all_of(a.begin(), a.end(), [](const basic_json & el)
{ {
return has_no_children(el.m_data); return has_no_children(el);
}); });
} }
const auto& o = *v.m_value.object; const auto& o = *v.m_data.m_value.object;
return std::all_of(o.begin(), o.end(), [](const typename object_t::value_type & el) return std::all_of(o.begin(), o.end(), [](const typename object_t::value_type & el)
{ {
return has_no_children(el.second.m_data); return has_no_children(el.second);
}); });
} }
@@ -38629,30 +38623,30 @@ private:
// the child of a non-empty array/object v that the walk in // the child of a non-empty array/object v that the walk in
// destroy_container() continues with (see walk_child_it() above) // destroy_container() continues with (see walk_child_it() above)
static data& walk_child(data& v) static basic_json& walk_child(basic_json& v)
{ {
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
return v.m_value.array->back().m_data; return v.m_data.m_value.array->back();
} }
JSON_ASSERT(v.m_type == value_t::object); JSON_ASSERT(v.m_data.m_type == value_t::object);
return walk_child_it(*v.m_value.object)->second.m_data; return walk_child_it(*v.m_data.m_value.object)->second;
} }
// removes walk_child(v) from a non-empty array/object v; this never // removes walk_child(v) from a non-empty array/object v; this never
// allocates, and since it is only ever called when that child is a // allocates, and since it is only ever called when that child is a
// scalar or an already-empty array/object, destroying it never // scalar or an already-empty array/object, destroying it never
// recurses more than one level deep (see destroy() below) // recurses more than one level deep (see destroy() below)
static void pop_walk_child(data& v) static void pop_walk_child(basic_json& v)
{ {
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
v.m_value.array->pop_back(); v.m_data.m_value.array->pop_back();
} }
else else
{ {
JSON_ASSERT(v.m_type == value_t::object); JSON_ASSERT(v.m_data.m_type == value_t::object);
v.m_value.object->erase(walk_child_it(*v.m_value.object)); v.m_data.m_value.object->erase(walk_child_it(*v.m_data.m_value.object));
} }
} }
@@ -38661,23 +38655,23 @@ private:
// recurses at most one level deep); this is the same allocator-based // recurses at most one level deep); this is the same allocator-based
// free the old recursive implementation used, just factored out so // free the old recursive implementation used, just factored out so
// every level of the walk in destroy() can share it // every level of the walk in destroy() can share it
static void free_container(data& v) noexcept static void free_container(basic_json& v) noexcept
{ {
JSON_ASSERT(has_no_children(v) || all_children_leaves(v)); JSON_ASSERT(has_no_children(v) || all_children_leaves(v));
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
AllocatorType<array_t> alloc; AllocatorType<array_t> alloc;
std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_value.array); std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_data.m_value.array);
std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_value.array, 1); std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_data.m_value.array, 1);
} }
else else
{ {
JSON_ASSERT(v.m_type == value_t::object); JSON_ASSERT(v.m_data.m_type == value_t::object);
AllocatorType<object_t> alloc; AllocatorType<object_t> alloc;
std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_value.object); std::allocator_traits<decltype(alloc)>::destroy(alloc, v.m_data.m_value.object);
std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_value.object, 1); std::allocator_traits<decltype(alloc)>::deallocate(alloc, v.m_data.m_value.object, 1);
} }
v.m_type = value_t::null; // avoid a double free if v is later destructed v.m_data.m_type = value_t::null; // avoid a double free if v is later destructed
} }
public: public:
@@ -38708,16 +38702,14 @@ public:
// This json_value is not itself a basic_json, so the top-level // This json_value is not itself a basic_json, so the top-level
// container is first moved into a local stand-in ("cur"); a // container is first moved into a local stand-in ("cur"); a
// default-constructed data has a null pointer in its m_value // default-constructed basic_json has a null pointer in its
// (see data::m_value's initializer), so swapping it with *this // m_value (see data::m_value's initializer), so swapping it
// leaves this union's own pointer null, and it is never looked // with *this leaves this union's own pointer null, and it is
// at or freed a second time. The stand-ins are data, not // never looked at or freed a second time.
// basic_json, so that destroying a value never constructs a basic_json cur;
// CustomBaseClass, whose constructor may allocate (#5995). cur.m_data.m_type = t;
data cur;
cur.m_type = t;
using std::swap; using std::swap;
swap(cur.m_value, *this); swap(cur.m_data.m_value, *this);
destroy_bounded(cur, 0); destroy_bounded(cur, 0);
} }
@@ -38726,7 +38718,7 @@ public:
// The first recursion_depth_limit() levels are destroyed by plain // The first recursion_depth_limit() levels are destroyed by plain
// recursion, which is the fastest way and cannot throw; anything // recursion, which is the fastest way and cannot throw; anything
// nested deeper is handed to destroy_walk(), which uses no call stack. // nested deeper is handed to destroy_walk(), which uses no call stack.
static void destroy_bounded(data& v, std::size_t depth) noexcept static void destroy_bounded(basic_json& v, std::size_t depth) noexcept
{ {
if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit()))
{ {
@@ -38734,23 +38726,23 @@ public:
return; return;
} }
if (v.m_type == value_t::array) if (v.m_data.m_type == value_t::array)
{ {
for (auto& child : *v.m_value.array) for (auto& child : *v.m_data.m_value.array)
{ {
if (child.m_data.m_type == value_t::array || child.m_data.m_type == value_t::object) if (child.m_data.m_type == value_t::array || child.m_data.m_type == value_t::object)
{ {
destroy_bounded(child.m_data, depth + 1); destroy_bounded(child, depth + 1);
} }
} }
} }
else else
{ {
for (auto& child : *v.m_value.object) for (auto& child : *v.m_data.m_value.object)
{ {
if (child.second.m_data.m_type == value_t::array || child.second.m_data.m_type == value_t::object) if (child.second.m_data.m_type == value_t::array || child.second.m_data.m_type == value_t::object)
{ {
destroy_bounded(child.second.m_data, depth + 1); destroy_bounded(child.second, depth + 1);
} }
} }
} }
@@ -38777,15 +38769,18 @@ public:
// array/object, which neither allocates nor recurses more // array/object, which neither allocates nor recurses more
// than one level deep. // than one level deep.
// //
// Like destroy_container(), the walk keeps the containers it // This json_value is not itself a basic_json, so the
// holds in data stand-ins (cur, prev, gp, tmp), which never // top-level container is first moved into a local stand-in
// construct a CustomBaseClass. // ("cur"); a default-constructed basic_json has a null
static void destroy_walk(data& top) noexcept // pointer in its m_value (see data::m_value's initializer),
// so swapping it with *this leaves this union's own pointer
// null, and it is never looked at or freed a second time.
static void destroy_walk(basic_json& top) noexcept
{ {
data cur; basic_json cur;
take(cur, top); take(cur, top);
data prev; // value_t::null: no parent basic_json prev; // value_t::null: no parent
// true right after cur was entered: its children have not been // true right after cur was entered: its children have not been
// looked at yet. If none of them has children of its own, cur is // looked at yet. If none of them has children of its own, cur is
@@ -38799,7 +38794,7 @@ public:
fresh = false; fresh = false;
if (has_no_children(cur) || (fresh_now && all_children_leaves(cur))) if (has_no_children(cur) || (fresh_now && all_children_leaves(cur)))
{ {
if (prev.m_type == value_t::null) if (prev.m_data.m_type == value_t::null)
{ {
free_container(cur); free_container(cur);
return; // back at the top with nothing left to do return; // back at the top with nothing left to do
@@ -38808,7 +38803,7 @@ public:
// ascend: detach the grandparent link from prev's // ascend: detach the grandparent link from prev's
// walk_child() slot, drop that (now null) slot, free cur // walk_child() slot, drop that (now null) slot, free cur
// (it is empty), then move up one level // (it is empty), then move up one level
data gp; basic_json gp;
take(gp, walk_child(prev)); take(gp, walk_child(prev));
pop_walk_child(prev); pop_walk_child(prev);
@@ -38819,7 +38814,7 @@ public:
continue; continue;
} }
data& cur_child_ref = walk_child(cur); basic_json& cur_child_ref = walk_child(cur);
if (has_no_children(cur_child_ref)) if (has_no_children(cur_child_ref))
{ {
@@ -38831,7 +38826,7 @@ public:
// descend into the non-empty child, reversing the // descend into the non-empty child, reversing the
// link: its slot takes over prev, and the child becomes // link: its slot takes over prev, and the child becomes
// the new cur // the new cur
data tmp; basic_json tmp;
take(tmp, cur_child_ref); take(tmp, cur_child_ref);
take(cur_child_ref, prev); take(cur_child_ref, prev);
take(prev, cur); take(prev, cur);
-90
View File
@@ -7,7 +7,6 @@
// SPDX-License-Identifier: MIT // SPDX-License-Identifier: MIT
#include <algorithm> #include <algorithm>
#include <memory>
#include <set> #include <set>
#include <sstream> #include <sstream>
#include <string> #include <string>
@@ -496,92 +495,3 @@ TEST_CASE("Regression test for issue #5674 - copy construction must not require
CHECK(*c == 1); CHECK(*c == 1);
} }
} }
// counts the constructions and destructions of the base class (#5995)
struct counting_base
{
static std::size_t constructions;
static std::size_t destructions;
counting_base() noexcept
{
++constructions;
}
counting_base(const counting_base& /*unused*/) noexcept
{
++constructions;
}
counting_base(counting_base&& /*unused*/) noexcept
{
++constructions;
}
counting_base& operator=(const counting_base& /*unused*/) noexcept = default;
counting_base& operator=(counting_base&& /*unused*/) noexcept = default;
~counting_base()
{
++destructions;
}
static void reset() noexcept
{
constructions = 0;
destructions = 0;
}
};
std::size_t counting_base::constructions = 0;
std::size_t counting_base::destructions = 0;
using json_with_counting_base = nlohmann::json::with_base_class_t<counting_base>;
TEST_CASE("destroying a value does not construct the custom base class")
{
// regression test for #5995: destroying an array or object used a
// default-constructed basic_json as a stand-in, which constructed a base
// class that belonged to no value (and could allocate in the noexcept
// destructor); now every destroyed value destroys exactly its own base
// object, and none is constructed
const auto check_destroy = [](std::unique_ptr<json_with_counting_base> j, std::size_t values)
{
counting_base::reset();
j = nullptr;
CHECK(counting_base::constructions == 0);
CHECK(counting_base::destructions == values);
};
SECTION("scalar")
{
check_destroy(std::unique_ptr<json_with_counting_base>(new json_with_counting_base(42)), 1);
}
SECTION("array and object")
{
check_destroy(std::unique_ptr<json_with_counting_base>(new json_with_counting_base(json_with_counting_base::parse(R"({"a":[1,2,3],"b":{"c":true}})"))), 7);
check_destroy(std::unique_ptr<json_with_counting_base>(new json_with_counting_base(json_with_counting_base::array())), 1);
}
SECTION("nested deeper than the recursion depth limit")
{
// alternating arrays and objects, so both kinds of container are
// walked by destroy_walk()
constexpr std::size_t depth = 1000;
std::unique_ptr<json_with_counting_base> j(new json_with_counting_base(json_with_counting_base::array({1, 2})));
for (std::size_t i = 0; i < depth; ++i)
{
json_with_counting_base outer = (i % 2 == 0) ? json_with_counting_base::object() : json_with_counting_base::array();
if (outer.is_object())
{
outer["x"] = std::move(*j);
outer["y"] = nullptr;
}
else
{
outer.push_back(std::move(*j));
outer.push_back(true);
}
*j = std::move(outer);
}
// the innermost array with its two elements, and two values per level
check_destroy(std::move(j), 3 + (2 * depth));
}
}