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 <mail@nlohmann.me>

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

include/nlohmann/detail/output/binary_writer.hpp

@@ -1647,20 +1647,6 @@ class binary_writer
         return 'D';  // float 64
     }
 
-    /*!
-    @brief checks whether a JSON number fits into @a TargetType
-    @param[in] el a JSON number of either the signed or unsigned integer kind
-    @return whether @a el's value can be represented by @a TargetType without
-            wrapping, regardless of which of the two kinds it is stored as
-    */
-    template<typename TargetType>
-    static bool bjdata_ndarray_value_in_range(const BasicJsonType& el)
-    {
-        return el.is_number_unsigned()
-               ? value_in_range_of<TargetType>(el.template get<std::uint64_t>())
-               : value_in_range_of<TargetType>(el.template get<std::int64_t>());
-    }
-
     /*!
     @return false if the object is successfully converted to a bjdata ndarray, true if the type or size is invalid
     */
@@ -1681,16 +1667,6 @@ class binary_writer
         }
         CharType dtype = it->second;
 
-        // the 'B' (byte) marker is only defined by BJData Draft 3; emitting it
-        // under the default Draft 2 mode would produce a stream that Draft 2
-        // readers reject, so such an object falls back to a plain object
-        // encoding instead (see the "Binary values" section of the BJData
-        // documentation)
-        if (dtype == 'B' && bjdata_version != bjdata_version_t::draft3)
-        {
-            return true;
-        }
-
         key = "_ArraySize_";
         // the dimensions are written verbatim as the header length below, so a
         // value that is not an array cannot produce a valid one: null emits 'Z'
@@ -1755,60 +1731,6 @@ class binary_writer
             }
         }
 
-        // every element is cast to the (possibly narrower) C++ type matching
-        // dtype below; a value that does not fit that type would silently
-        // wrap (integers) or overflow to infinity (the "single" precision
-        // float) instead of being reported, so such an object falls back to
-        // a plain object encoding as well
-        for (const auto& el : value.at(key))
-        {
-            bool in_range = true;
-            switch (dtype)
-            {
-                case 'U':
-                case 'C':
-                case 'B':
-                    in_range = bjdata_ndarray_value_in_range<std::uint8_t>(el);
-                    break;
-                case 'i':
-                    in_range = bjdata_ndarray_value_in_range<std::int8_t>(el);
-                    break;
-                case 'u':
-                    in_range = bjdata_ndarray_value_in_range<std::uint16_t>(el);
-                    break;
-                case 'I':
-                    in_range = bjdata_ndarray_value_in_range<std::int16_t>(el);
-                    break;
-                case 'm':
-                    in_range = bjdata_ndarray_value_in_range<std::uint32_t>(el);
-                    break;
-                case 'l':
-                    in_range = bjdata_ndarray_value_in_range<std::int32_t>(el);
-                    break;
-                case 'M':
-                    in_range = bjdata_ndarray_value_in_range<std::uint64_t>(el);
-                    break;
-                case 'L':
-                    in_range = bjdata_ndarray_value_in_range<std::int64_t>(el);
-                    break;
-                case 'd':
-                {
-                    const auto dval = el.template get<double>();
-                    in_range = !std::isfinite(dval) ||
-                               (dval >= static_cast<double>(std::numeric_limits<float>::lowest()) &&
-                                dval <= static_cast<double>((std::numeric_limits<float>::max)()));
-                    break;
-                }
-                default:
-                    // 'D' (double) already spans the full range of number_float_t
-                    break;
-            }
-            if (!in_range)
-            {
-                return true;
-            }
-        }
-
         oa->write_character('[');
         oa->write_character('$');
         oa->write_character(dtype);

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();

single_include/nlohmann/json.hpp

@@ -18655,20 +18655,6 @@ class binary_writer
         return 'D';  // float 64
     }
 
-    /*!
-    @brief checks whether a JSON number fits into @a TargetType
-    @param[in] el a JSON number of either the signed or unsigned integer kind
-    @return whether @a el's value can be represented by @a TargetType without
-            wrapping, regardless of which of the two kinds it is stored as
-    */
-    template<typename TargetType>
-    static bool bjdata_ndarray_value_in_range(const BasicJsonType& el)
-    {
-        return el.is_number_unsigned()
-               ? value_in_range_of<TargetType>(el.template get<std::uint64_t>())
-               : value_in_range_of<TargetType>(el.template get<std::int64_t>());
-    }
-
     /*!
     @return false if the object is successfully converted to a bjdata ndarray, true if the type or size is invalid
     */
@@ -18689,16 +18675,6 @@ class binary_writer
         }
         CharType dtype = it->second;
 
-        // the 'B' (byte) marker is only defined by BJData Draft 3; emitting it
-        // under the default Draft 2 mode would produce a stream that Draft 2
-        // readers reject, so such an object falls back to a plain object
-        // encoding instead (see the "Binary values" section of the BJData
-        // documentation)
-        if (dtype == 'B' && bjdata_version != bjdata_version_t::draft3)
-        {
-            return true;
-        }
-
         key = "_ArraySize_";
         // the dimensions are written verbatim as the header length below, so a
         // value that is not an array cannot produce a valid one: null emits 'Z'
@@ -18763,60 +18739,6 @@ class binary_writer
             }
         }
 
-        // every element is cast to the (possibly narrower) C++ type matching
-        // dtype below; a value that does not fit that type would silently
-        // wrap (integers) or overflow to infinity (the "single" precision
-        // float) instead of being reported, so such an object falls back to
-        // a plain object encoding as well
-        for (const auto& el : value.at(key))
-        {
-            bool in_range = true;
-            switch (dtype)
-            {
-                case 'U':
-                case 'C':
-                case 'B':
-                    in_range = bjdata_ndarray_value_in_range<std::uint8_t>(el);
-                    break;
-                case 'i':
-                    in_range = bjdata_ndarray_value_in_range<std::int8_t>(el);
-                    break;
-                case 'u':
-                    in_range = bjdata_ndarray_value_in_range<std::uint16_t>(el);
-                    break;
-                case 'I':
-                    in_range = bjdata_ndarray_value_in_range<std::int16_t>(el);
-                    break;
-                case 'm':
-                    in_range = bjdata_ndarray_value_in_range<std::uint32_t>(el);
-                    break;
-                case 'l':
-                    in_range = bjdata_ndarray_value_in_range<std::int32_t>(el);
-                    break;
-                case 'M':
-                    in_range = bjdata_ndarray_value_in_range<std::uint64_t>(el);
-                    break;
-                case 'L':
-                    in_range = bjdata_ndarray_value_in_range<std::int64_t>(el);
-                    break;
-                case 'd':
-                {
-                    const auto dval = el.template get<double>();
-                    in_range = !std::isfinite(dval) ||
-                               (dval >= static_cast<double>(std::numeric_limits<float>::lowest()) &&
-                                dval <= static_cast<double>((std::numeric_limits<float>::max)()));
-                    break;
-                }
-                default:
-                    // 'D' (double) already spans the full range of number_float_t
-                    break;
-            }
-            if (!in_range)
-            {
-                return true;
-            }
-        }
-
         oa->write_character('[');
         oa->write_character('$');
         oa->write_character(dtype);
@@ -25053,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();

tests/src/unit-bjdata.cpp

@@ -2586,12 +2586,7 @@ TEST_CASE("BJData")
                 CHECK(json::to_bjdata(json::from_bjdata(v_d), true, true) == v_d);
                 CHECK(json::to_bjdata(json::from_bjdata(v_D), true, true) == v_D);
                 CHECK(json::to_bjdata(json::from_bjdata(v_C), true, true) == v_C);
-                // v_B uses the Draft-3-only 'B' marker, so it round-trips only when
-                // Draft 3 is explicitly selected (see GitHub issue #5404); the
-                // default Draft 2 falls back to a plain object instead, covered by
-                // the "ndarray with _ArrayType_ "byte" is gated by the BJData draft
-                // version" section below
-                CHECK(json::to_bjdata(json::from_bjdata(v_B), true, true, json::bjdata_version_t::draft3) == v_B);
+                CHECK(json::to_bjdata(json::from_bjdata(v_B), true, true) == v_B);
             }
 
             SECTION("ndarray with data not matching _ArrayType_ is written as an object")
@@ -2634,10 +2629,8 @@ TEST_CASE("BJData")
                 // the C++ API stores an int literal as number_integer, so _ArrayType_
                 // names the wire type rather than the storage. Both storages have to
                 // produce the same typed array for every type.
-                // "byte" is checked separately below since it additionally requires
-                // BJData Draft 3 to be selected explicitly (see GitHub issue #5404).
                 for (const char* type :
-                        {"uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "char"
+                        {"uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "char", "byte"
                         })
                 {
                     CAPTURE(type);
@@ -2648,14 +2641,6 @@ TEST_CASE("BJData")
                     CHECK(from_text == json::to_bjdata(json({{"_ArrayType_", type}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}})));
                 }
 
-                {
-                    const std::string text = R"({"_ArrayType_":"byte","_ArraySize_":[2,3],"_ArrayData_":[1,2,3,4,5,6]})";
-                    const auto from_text = json::to_bjdata(json::parse(text), true, true, json::bjdata_version_t::draft3);
-                    CHECK(from_text.at(0) == '[');
-                    CHECK(from_text == json::to_bjdata(json({{"_ArrayType_", "byte"}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}}),
-                    true, true, json::bjdata_version_t::draft3));
-                }
-
                 // negative values under a signed type behave the same way
                 const auto from_neg = json::to_bjdata(json::parse(R"({"_ArrayType_":"int32","_ArraySize_":[2],"_ArrayData_":[-5,7]})"));
                 CHECK(from_neg.at(0) == '[');
@@ -2791,83 +2776,6 @@ TEST_CASE("BJData")
                 CHECK(out_num.at(0) == '{');
                 CHECK(json::from_bjdata(out_num) == j_num);
             }
-
-            SECTION("ndarray with out-of-range _ArrayData_ elements stays as object")
-            {
-                // each element is cast to the (possibly narrower) C++ type
-                // named by _ArrayType_ before being written; a value that
-                // does not fit that type would silently wrap instead of
-                // being reported, so such an object falls back to a plain
-                // object encoding that still round-trips (see GitHub issue #5403)
-
-                // an unsigned element that does not fit uint8
-                json const j_uint8 = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 256}}});
-                const auto out_uint8 = json::to_bjdata(j_uint8);
-                CHECK(out_uint8.at(0) == '{');
-                CHECK(json::from_bjdata(out_uint8) == j_uint8);
-
-                // a signed element that does not fit int8
-                json const j_int8 = json({{"_ArrayType_", "int8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 200}}});
-                const auto out_int8 = json::to_bjdata(j_int8);
-                CHECK(out_int8.at(0) == '{');
-                CHECK(json::from_bjdata(out_int8) == j_int8);
-
-                // a negative element is likewise out of range for an
-                // unsigned _ArrayType_
-                json const j_uint16_neg = json({{"_ArrayType_", "uint16"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, -1}}});
-                const auto out_uint16_neg = json::to_bjdata(j_uint16_neg);
-                CHECK(out_uint16_neg.at(0) == '{');
-                CHECK(json::from_bjdata(out_uint16_neg) == j_uint16_neg);
-
-                // a double element that overflows to infinity when narrowed
-                // to the "single" (float) precision named by _ArrayType_
-                json const j_single = json({{"_ArrayType_", "single"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1.5, 1e40}}});
-                const auto out_single = json::to_bjdata(j_single);
-                CHECK(out_single.at(0) == '{');
-                CHECK(json::from_bjdata(out_single) == j_single);
-
-                // in-range boundary values still use the compact ndarray encoding
-                json const j_uint8_ok = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {0, 255}}});
-                CHECK(json::to_bjdata(j_uint8_ok) == std::vector<uint8_t>({'[', '$', 'U', '#', '[', 'i', 2, ']', 0, 255}));
-
-                json const j_int8_ok = json({{"_ArrayType_", "int8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {-128, 127}}});
-                CHECK(json::to_bjdata(j_int8_ok) == std::vector<uint8_t>({'[', '$', 'i', '#', '[', 'i', 2, ']', 0x80, 0x7F}));
-
-                json const j_single_ok = json({{"_ArrayType_", "single"}, {"_ArraySize_", {1}}, {"_ArrayData_", {1.5}}});
-                const auto out_single_ok = json::to_bjdata(j_single_ok);
-                CHECK(out_single_ok.at(0) == '[');
-                CHECK(json::from_bjdata(out_single_ok) == json({1.5f}));
-            }
-
-            SECTION("ndarray with _ArrayType_ \"byte\" is gated by the BJData draft version")
-            {
-                // the 'B' (byte) marker used by _ArrayType_ "byte" is only defined
-                // by BJData Draft 3; Draft 2 (the default) has no such marker, so
-                // emitting it unconditionally produced a stream that a Draft 2
-                // reader could not parse as intended (see GitHub issue #5404).
-                // Two dimensions are used so that a successfully written ndarray
-                // round-trips back into the annotated object (a single dimension
-                // is, by the BJData ndarray convention, read back as a plain
-                // binary value rather than the annotated object, same as every
-                // other single-dimension ndarray of a non-"byte" type is read
-                // back as a plain array instead of the annotated object).
-                json const j_byte = json({{"_ArrayType_", "byte"}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}});
-
-                // default (Draft 2): falls back to a plain object and round-trips
-                const auto out_draft2 = json::to_bjdata(j_byte);
-                CHECK(out_draft2.at(0) == '{');
-                CHECK(json::from_bjdata(out_draft2) == j_byte);
-
-                // explicit Draft 2: same as the default
-                const auto out_draft2_explicit = json::to_bjdata(j_byte, true, true, json::bjdata_version_t::draft2);
-                CHECK(out_draft2_explicit.at(0) == '{');
-                CHECK(json::from_bjdata(out_draft2_explicit) == j_byte);
-
-                // Draft 3 explicitly selected: still uses the compact 'B' ndarray encoding
-                const auto out_draft3 = json::to_bjdata(j_byte, true, true, json::bjdata_version_t::draft3);
-                CHECK(out_draft3 == std::vector<uint8_t>({'[', '$', 'B', '#', '[', '$', 'i', '#', 'i', 2, 2, 3, 1, 2, 3, 4, 5, 6}));
-                CHECK(json::from_bjdata(out_draft3) == j_byte);
-            }
         }
     }
 

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);
+    }
+}