Do not construct the base class when destroying a container (#6000)
* Do not construct the base class when destroying a container (#5995) json_value::destroy_container() and destroy_walk() used default- constructed basic_json values as stand-ins, which construct a CustomBaseClass that belongs to no value. A base class whose default constructor allocates made the noexcept destructor allocate again, and terminate if that allocation failed, which #5762 set out to prevent. Use data (type and value, no base class) as the stand-in instead, and pass the children's m_data to the walk helpers. Add a test with a counting base class: destroying a value constructs no base object and destroys exactly one per value, also for values nested deeper than the recursion depth limit. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Release the test value with = nullptr instead of reset() clang-tidy (readability-ambiguous-smartptr-reset-call) rejects j.reset() because the pointee inherits a reset() from counting_base. 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> * Use std::swap for the container stand-in in destroy_container The unqualified call after `using std::swap` would let a user-provided swap(json_value&, json_value&) intercept the destruction path. Call std::swap directly. Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me> Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
1 parent
03d408aafa
commit
66e2eeeec0
3 files changed
+224
-128
No files matched your search
@@ -7,6 +7,7 @@
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
#include <algorithm>
|
||||
#include <memory>
|
||||
#include <set>
|
||||
#include <sstream>
|
||||
#include <string>
|
||||
@@ -495,3 +496,92 @@ TEST_CASE("Regression test for issue #5674 - copy construction must not require
|
||||
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));
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user