From 8f5dbb56c5a8e73b178ecd7963ef873d5ccd35f2 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sat, 5 Sep 2026 21:32:43 +0200 Subject: [PATCH] Swap diagnostic positions in basic_json::swap() basic_json::swap() (and the friend swap() that forwards to it) only exchanged m_data.m_type/m_data.m_value, leaving start_position/end_position untouched under JSON_DIAGNOSTIC_POSITIONS. This is inconsistent with copy-assignment's operator=(basic_json), which swaps positions as part of its copy-and-swap implementation, so after swap(a, b) each value ended up with the other value's content but its own original position. Signed-off-by: Niels Lohmann --- docs/mkdocs/docs/api/basic_json/swap.md | 8 +- include/nlohmann/json.hpp | 5 ++ single_include/nlohmann/json.hpp | 5 ++ ...unit-class_parser_diagnostic_positions.cpp | 77 +++++++++++++++++++ 4 files changed, 93 insertions(+), 2 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/swap.md b/docs/mkdocs/docs/api/basic_json/swap.md index 3a3d288fb..aa5aa6c4c 100644 --- a/docs/mkdocs/docs/api/basic_json/swap.md +++ b/docs/mkdocs/docs/api/basic_json/swap.md @@ -34,10 +34,14 @@ 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. + 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. 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. + 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. 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 diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index be4ccb90f..0346f6ba5 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3547,6 +3547,11 @@ 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(); diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 31bfb869d..bdc92f1a9 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -24975,6 +24975,11 @@ 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(); diff --git a/tests/src/unit-class_parser_diagnostic_positions.cpp b/tests/src/unit-class_parser_diagnostic_positions.cpp index 2697ecf8a..db5259378 100644 --- a/tests/src/unit-class_parser_diagnostic_positions.cpp +++ b/tests/src/unit-class_parser_diagnostic_positions.cpp @@ -1955,3 +1955,80 @@ 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); + } +}