From 07fee4fd55e4f11d2b12e8a52929df2bd7bade1a Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Tue, 29 Sep 2026 23:38:56 +0200 Subject: [PATCH] Exchange the CustomBaseClass subobject in basic_json::swap() basic_json::swap() (and the friend swap() and the pre-C++20 std::swap overload that forward to it) only exchanged m_data.m_type/m_data.m_value, leaving each value's json_base_class_t subobject in place. This is inconsistent with the copy and move constructors and copy assignment, which all carry the base class along with the value, so after a.swap(b) any metadata stored in a CustomBaseClass ended up attached to the wrong value. Algorithms that mix swap() with moves, such as std::sort, scrambled the metadata across the whole container. Fix the member swap() to also exchange the json_base_class_t subobject and extend the noexcept specifications of swap() and the friend swap() accordingly. Fixes #5653. Signed-off-by: Niels Lohmann --- docs/mkdocs/docs/api/basic_json/swap.md | 16 ++++-- include/nlohmann/json.hpp | 15 +++++- single_include/nlohmann/json.hpp | 15 +++++- tests/src/unit-custom-base-class.cpp | 72 +++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 8 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/swap.md b/docs/mkdocs/docs/api/basic_json/swap.md index aa5aa6c4c..3ac74288d 100644 --- a/docs/mkdocs/docs/api/basic_json/swap.md +++ b/docs/mkdocs/docs/api/basic_json/swap.md @@ -6,7 +6,9 @@ void swap(reference other) noexcept ( std::is_nothrow_move_constructible::value && std::is_nothrow_move_assignable::value && std::is_nothrow_move_constructible::value && - std::is_nothrow_move_assignable::value + std::is_nothrow_move_assignable::value && + std::is_nothrow_move_constructible::value && + std::is_nothrow_move_assignable::value ); // (2) @@ -14,7 +16,9 @@ friend void swap(reference left, reference right) noexcept ( std::is_nothrow_move_constructible::value && std::is_nothrow_move_assignable::value && std::is_nothrow_move_constructible::value && - std::is_nothrow_move_assignable::value + std::is_nothrow_move_assignable::value && + std::is_nothrow_move_constructible::value && + std::is_nothrow_move_assignable::value ); // (3) @@ -37,11 +41,15 @@ void swap(typename binary_t::container_type& other); 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. + The [`json_base_class_t`](json_base_class_t.md) subobject is exchanged along with the value as well, the same way it + is copied or moved by the copy/move constructors and assignment operators. 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. + The [`json_base_class_t`](json_base_class_t.md) subobject is exchanged along with the value as well, the same way it + is copied or moved by the copy/move constructors and assignment operators. 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 @@ -164,8 +172,8 @@ Constant. ## Version history -1. Since version 1.0.0. -2. Since version 1.0.0. +1. Since version 1.0.0. Exchanges the `json_base_class_t` subobject along with the value since version 3.13.0. +2. Since version 1.0.0. Exchanges the `json_base_class_t` subobject along with the value since version 3.13.0. 3. Since version 1.0.0. 4. Since version 1.0.0. 5. Since version 1.0.0. diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 500fcddf2..35bfd1cf8 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -4335,12 +4335,21 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::is_nothrow_move_constructible::value&& std::is_nothrow_move_assignable::value&& std::is_nothrow_move_constructible::value&& // NOLINT(cppcoreguidelines-noexcept-swap,performance-noexcept-swap) - std::is_nothrow_move_assignable::value + std::is_nothrow_move_assignable::value&& + std::is_nothrow_move_constructible::value&& + std::is_nothrow_move_assignable::value ) { std::swap(m_data.m_type, other.m_data.m_type); std::swap(m_data.m_value, other.m_data.m_value); + // the custom base class travels with the value when it is copied or + // moved, so it is exchanged along with it + { + using std::swap; + swap(static_cast(*this), static_cast(other)); + } + #if JSON_DIAGNOSTIC_POSITIONS std::swap(start_position, other.start_position); std::swap(end_position, other.end_position); @@ -4357,7 +4366,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::is_nothrow_move_constructible::value&& std::is_nothrow_move_assignable::value&& std::is_nothrow_move_constructible::value&& // NOLINT(cppcoreguidelines-noexcept-swap,performance-noexcept-swap) - std::is_nothrow_move_assignable::value + std::is_nothrow_move_assignable::value&& + std::is_nothrow_move_constructible::value&& + std::is_nothrow_move_assignable::value ) { left.swap(right); diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 576498738..0080e280f 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -30416,12 +30416,21 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::is_nothrow_move_constructible::value&& std::is_nothrow_move_assignable::value&& std::is_nothrow_move_constructible::value&& // NOLINT(cppcoreguidelines-noexcept-swap,performance-noexcept-swap) - std::is_nothrow_move_assignable::value + std::is_nothrow_move_assignable::value&& + std::is_nothrow_move_constructible::value&& + std::is_nothrow_move_assignable::value ) { std::swap(m_data.m_type, other.m_data.m_type); std::swap(m_data.m_value, other.m_data.m_value); + // the custom base class travels with the value when it is copied or + // moved, so it is exchanged along with it + { + using std::swap; + swap(static_cast(*this), static_cast(other)); + } + #if JSON_DIAGNOSTIC_POSITIONS std::swap(start_position, other.start_position); std::swap(end_position, other.end_position); @@ -30438,7 +30447,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::is_nothrow_move_constructible::value&& std::is_nothrow_move_assignable::value&& std::is_nothrow_move_constructible::value&& // NOLINT(cppcoreguidelines-noexcept-swap,performance-noexcept-swap) - std::is_nothrow_move_assignable::value + std::is_nothrow_move_assignable::value&& + std::is_nothrow_move_constructible::value&& + std::is_nothrow_move_assignable::value ) { left.swap(right); diff --git a/tests/src/unit-custom-base-class.cpp b/tests/src/unit-custom-base-class.cpp index 7dab5c576..a7f466e36 100644 --- a/tests/src/unit-custom-base-class.cpp +++ b/tests/src/unit-custom-base-class.cpp @@ -6,9 +6,11 @@ // SPDX-FileCopyrightText: 2013-2026 Niels Lohmann // SPDX-License-Identifier: MIT +#include #include #include #include +#include #include "doctest_compatibility.h" @@ -180,6 +182,76 @@ TEST_CASE("JSON Node Metadata") CHECK(val.metadata().at(1) == 2); } } + SECTION("member swap") + { + using json = json_with_metadata; + json a = 1; + a.metadata() = 100; + json b = 2; + b.metadata() = 200; + + a.swap(b); + + CHECK(a.get() == 2); + CHECK(b.get() == 1); + CHECK(a.metadata() == 200); + CHECK(b.metadata() == 100); + } + SECTION("nonmember swap") + { + using json = json_with_metadata; + json a = 1; + a.metadata() = 100; + json b = 2; + b.metadata() = 200; + + using std::swap; + swap(a, b); + + CHECK(a.get() == 2); + CHECK(b.get() == 1); + CHECK(a.metadata() == 200); + CHECK(b.metadata() == 100); + } + SECTION("std::swap") + { + using json = json_with_metadata; + json a = 1; + a.metadata() = 100; + json b = 2; + b.metadata() = 200; + + std::swap(a, b); + + CHECK(a.get() == 2); + CHECK(b.get() == 1); + CHECK(a.metadata() == 200); + CHECK(b.metadata() == 100); + } + SECTION("std::sort keeps metadata attached to its value") + { + // std::sort mixes swap() with moves; each value's metadata must + // travel with it, just as it does for copy, move, and assignment + using json = json_with_metadata; + std::vector values; + for (int v : + { + 5, 3, 9, 1, 7, 2, 8, 4, 6, 0, 15, 13, 19, 11, 17, 12, 18, 14, 16, 10, + 25, 23, 29, 21, 27, 22, 28, 24, 26, 20, 35, 33 + }) + { + json value = v; + value.metadata() = v; + values.push_back(value); + } + + std::sort(values.begin(), values.end()); + + for (const auto& value : values) + { + CHECK(value.metadata() == value.get()); + } + } } // Test extending nlohmann::json by using a custom base class.