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 37 additions and 93 deletions
+2 -6
View File
@@ -34,14 +34,10 @@ void swap(typename binary_t::container_type& other);
```
1. Exchanges the contents of the JSON value with those of `other`. Does not invoke any move, copy, or swap operations on
individual elements. All iterators and references remain valid. The past-the-end iterator is invalidated. If macro
[`JSON_DIAGNOSTIC_POSITIONS`](../macros/json_diagnostic_positions.md) is defined to `#!cpp 1`, the
[`start_pos()`](start_pos.md)/[`end_pos()`](end_pos.md) diagnostic positions are exchanged along with the value.
individual elements. All iterators and references remain valid. The past-the-end iterator is invalidated.
2. Exchanges the contents of the JSON value from `left` with those of `right`. Does not invoke any move, copy, or swap
operations on individual elements. All iterators and references remain valid. The past-the-end iterator is
invalidated. Implemented as a friend function callable via ADL. If macro
[`JSON_DIAGNOSTIC_POSITIONS`](../macros/json_diagnostic_positions.md) is defined to `#!cpp 1`, the
[`start_pos()`](start_pos.md)/[`end_pos()`](end_pos.md) diagnostic positions are exchanged along with the value.
invalidated. Implemented as a friend function callable via ADL.
3. Exchanges the contents of a JSON array with those of `other`. Does not invoke any move, copy, or swap operations on
individual elements. All iterators and references remain valid. The past-the-end iterator is invalidated.
4. Exchanges the contents of a JSON object with those of `other`. Does not invoke any move, copy, or swap operations on
+2 -5
View File
@@ -3547,11 +3547,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
std::swap(m_data.m_type, other.m_data.m_type);
std::swap(m_data.m_value, other.m_data.m_value);
#if JSON_DIAGNOSTIC_POSITIONS
std::swap(start_position, other.start_position);
std::swap(end_position, other.end_position);
#endif
set_parents();
other.set_parents();
assert_invariant();
@@ -3578,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
{
@@ -3594,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
{
+2 -5
View File
@@ -24975,11 +24975,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
std::swap(m_data.m_type, other.m_data.m_type);
std::swap(m_data.m_value, other.m_data.m_value);
#if JSON_DIAGNOSTIC_POSITIONS
std::swap(start_position, other.start_position);
std::swap(end_position, other.end_position);
#endif
set_parents();
other.set_parents();
assert_invariant();
@@ -25006,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
{
@@ -25022,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
{
@@ -1955,80 +1955,3 @@ TEST_CASE("parser class")
}
}
}
TEST_CASE("diagnostic positions: value lifetime")
{
SECTION("copy constructor copies positions, recursively")
{
const std::string s = R"({"a":1,"b":[1,2,3]})";
const json a = json::parse(s);
const json b = a; // NOLINT(performance-unnecessary-copy-initialization)
CHECK(b.start_pos() == a.start_pos());
CHECK(b.end_pos() == a.end_pos());
CHECK(b["b"].start_pos() == a["b"].start_pos());
CHECK(b["b"].end_pos() == a["b"].end_pos());
}
SECTION("move constructor resets the moved-from value to npos")
{
const std::string s = R"({"a":1,"b":[1,2,3]})";
json a = json::parse(s);
const auto a_start = a.start_pos();
const auto a_end = a.end_pos();
const json b(std::move(a));
CHECK(b.start_pos() == a_start);
CHECK(b.end_pos() == a_end);
CHECK(a.start_pos() == std::string::npos); // NOLINT(bugprone-use-after-move,clang-analyzer-cplusplus.Move)
CHECK(a.end_pos() == std::string::npos); // NOLINT(bugprone-use-after-move,clang-analyzer-cplusplus.Move)
}
SECTION("swap() exchanges positions along with the values")
{
// basic_json::swap() (and the friend swap() that forwards to it) used
// to swap only m_data.m_type/m_data.m_value, leaving
// start_position/end_position untouched -- unlike copy-assignment's
// operator=(basic_json), which swaps positions as part of its
// copy-and-swap implementation. After swap(a, b), each value ended up
// with the *other* value's content but its *own* original position.
// This is now fixed so that swap() is consistent with copy-assignment.
json a = json::parse(R"({"a":1})");
json b = json::parse(R"([1,2,3,4,5])");
const auto a_start = a.start_pos();
const auto a_end = a.end_pos();
const auto b_start = b.start_pos();
const auto b_end = b.end_pos();
// lengths (and thus end positions) differ, which is enough to tell
// after the swap whether positions actually moved with the values
CHECK(a_end != b_end);
using std::swap;
swap(a, b);
CHECK(a == json::parse(R"([1,2,3,4,5])"));
CHECK(b == json::parse(R"({"a":1})"));
CHECK(a.start_pos() == b_start);
CHECK(a.end_pos() == b_end);
CHECK(b.start_pos() == a_start);
CHECK(b.end_pos() == a_end);
// member swap() behaves the same as the free function
json c = json::parse(R"({"a":1})");
json d = json::parse(R"([1,2,3,4,5])");
const auto c_start = c.start_pos();
const auto c_end = c.end_pos();
const auto d_start = d.start_pos();
const auto d_end = d.end_pos();
c.swap(d);
CHECK(c.start_pos() == d_start);
CHECK(c.end_pos() == d_end);
CHECK(d.start_pos() == c_start);
CHECK(d.end_pos() == c_end);
}
}
+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);
}
}
}