Address review findings on the binary writer output sinks - binary_reserve_hint(): the 4-bytes-per-element estimate over-reserved by up to 4x for arrays of small scalars (CBOR encodes 0..23 in one byte), and the returned vector kept that capacity. Make the hint a strict lower bound on the encoded size instead, which also removes the 1 MiB clamp whose branch no test could reach (the largest container in the suite has 65793 elements). - Guard the -Wduplicated-branches pragma with __GNUC__ >= 7. The warning does not exist before GCC 7, so naming it made GCC 4.8/4.9/5/6 - which the CI matrix still builds - warn under -Wpragmas on every including translation unit, breaking downstream -Werror builds. - Constrain the adapter constructor of binary_writer with the enable_if its documentation already claimed, so a writer over some other sink type is no longer advertised as constructible from an output adapter. - Let output_vector_adapter wrap output_vector_sink rather than duplicating the append logic, so the type-erased and templated paths share one implementation. - Collapse the three copies of the memcpy/byte_swap/memcpy dance into a single byte_swap_buffer() helper, and add the MSVC _byteswap_* intrinsics so MSVC no longer falls back to the scalar shuffle this change exists to eliminate. - Add a vector_writer() helper for the five vector-returning to_* overloads instead of spelling out the writer type at each call site, and drop a dead default member initializer on output_adapter_sink. - New tests: the vector sink and the adapter sink must produce identical bytes for every format (the two to_* overloads no longer delegate to each other and could otherwise drift), and binary_reserve_hint() must never exceed the size actually written. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index 27122a2..98a2e23 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp
@@ -16,9 +16,14 @@ #include <cstring> // memcpy #include <limits> // numeric_limits #include <string> // string +#include <type_traits> // enable_if, is_constructible #include <utility> // move #include <vector> // vector +#ifdef _MSC_VER + #include <cstdlib> // _byteswap_ushort, _byteswap_ulong, _byteswap_uint64 +#endif + #include <nlohmann/detail/input/binary_reader.hpp> #include <nlohmann/detail/macro_scope.hpp> #include <nlohmann/detail/output/output_adapters.hpp> @@ -40,31 +45,33 @@ /////////////////// /*! -@brief conservative capacity hint for binary serialization into a std::vector +@brief capacity hint for binary serialization into a std::vector -Returns an approximate number of bytes to reserve up front so that serializing -an array/object of many elements does not repeatedly reallocate the output -buffer. Only the top-level element count is consulted (O(1), no walk of the -DOM), and the result is clamped to a fixed ceiling: a large or untrusted DOM can -therefore never trigger an oversized allocation here, and the multiplication -cannot overflow. The buffer still grows geometrically beyond the hint, so a hint -that is too small only costs a few later reallocations. A single scalar, string, -or binary value is written in one shot and needs no hint. +Returns a *lower* bound on the number of bytes the serialization will produce, +so that writing an array/object of many elements does not start reallocating +from an empty buffer. Every array element occupies at least one byte in every +supported binary format, and every object entry at least two (a key of at least +one byte plus a value of at least one), plus one byte for the container header, +so the hint can never exceed the final size and the returned vector is never +left holding capacity the caller did not ask for. The buffer still grows +geometrically past the hint, so under-reserving only costs a few later +reallocations. Only the top-level element count is consulted (O(1), no walk of +the DOM); a single scalar, string, or binary value is written in one shot and +needs no hint. */ template<typename BasicJsonType> std::size_t binary_reserve_hint(const BasicJsonType& j) { - constexpr std::size_t max_hint = static_cast<std::size_t>(1) << 20; // 1 MiB - if (j.is_array() || j.is_object()) + if (j.is_array()) { - const std::size_t elements = j.size(); - // guard the multiplication against overflow and cap the reservation - if (elements > max_hint / 4) - { - return max_hint; - } - return (elements * 4) + 2; + return j.size() + 1; } + + if (j.is_object()) + { + return (j.size() * 2) + 1; + } + return 0; } @@ -94,12 +101,15 @@ Convenience constructor for the default (output_adapter_sink) sink so the `output_adapter`-based overloads keep constructing the writer directly from - an adapter. Only participates in overload resolution when the sink can be - built from an adapter. + an adapter. Constrained to sinks that can actually be built from an adapter, + so that a writer over some other sink type is not advertised as constructible + from one. @param[in] adapter output adapter to write to */ - explicit binary_writer(output_adapter_t<CharType> adapter) : oa(OutputSinkType(std::move(adapter))) + template < typename SinkType = OutputSinkType, + typename std::enable_if < std::is_constructible<SinkType, output_adapter_t<CharType>>::value, int >::type = 0 > + explicit binary_writer(output_adapter_t<CharType> adapter) : oa(SinkType(std::move(adapter))) {} /*! @@ -1869,6 +1879,8 @@ { #if defined(__GNUC__) || defined(__clang__) return __builtin_bswap16(x); +#elif defined(_MSC_VER) + return _byteswap_ushort(x); #else return static_cast<std::uint16_t>((x >> 8) | (x << 8)); #endif @@ -1878,6 +1890,8 @@ { #if defined(__GNUC__) || defined(__clang__) return __builtin_bswap32(x); +#elif defined(_MSC_VER) + return _byteswap_ulong(x); #else return ((x & 0x000000FFu) << 24) | ((x & 0x0000FF00u) << 8) | ((x & 0x00FF0000u) >> 8) | ((x & 0xFF000000u) >> 24); @@ -1888,6 +1902,8 @@ { #if defined(__GNUC__) || defined(__clang__) return __builtin_bswap64(x); +#elif defined(_MSC_VER) + return _byteswap_uint64(x); #else x = ((x & 0x00000000FFFFFFFFull) << 32) | ((x & 0xFFFFFFFF00000000ull) >> 32); x = ((x & 0x0000FFFF0000FFFFull) << 16) | ((x & 0xFFFF0000FFFF0000ull) >> 16); @@ -1896,30 +1912,40 @@ #endif } + /*! + @brief reverse the bytes of a buffer by byte-swapping it as UIntType + + Loading the buffer into an unsigned integer of the same width and swapping + that is what lets the compiler emit a single bswap/rev/movbe; reversing the + buffer element by element does not reliably get there (clang keeps a scalar + shuffle). The two memcpy calls are the only portable way to reinterpret the + bytes and are folded away by every optimizer. + */ + template<typename UIntType, std::size_t N> + static void byte_swap_buffer(std::array<CharType, N>& a) noexcept + { + static_assert(sizeof(UIntType) == N, "swap width must match the buffer size"); + UIntType v{}; + std::memcpy(&v, a.data(), sizeof(v)); + v = byte_swap(v); + std::memcpy(a.data(), &v, sizeof(v)); + } + // reverse the bytes of a fixed-size buffer; a single byte_swap() for the // common 2/4/8-byte number payloads, std::reverse for any other size static void reverse_bytes(std::array<CharType, 2>& a) noexcept { - std::uint16_t v{}; - std::memcpy(&v, a.data(), sizeof(v)); - v = byte_swap(v); - std::memcpy(a.data(), &v, sizeof(v)); + byte_swap_buffer<std::uint16_t>(a); } static void reverse_bytes(std::array<CharType, 4>& a) noexcept { - std::uint32_t v{}; - std::memcpy(&v, a.data(), sizeof(v)); - v = byte_swap(v); - std::memcpy(a.data(), &v, sizeof(v)); + byte_swap_buffer<std::uint32_t>(a); } static void reverse_bytes(std::array<CharType, 8>& a) noexcept { - std::uint64_t v{}; - std::memcpy(&v, a.data(), sizeof(v)); - v = byte_swap(v); - std::memcpy(a.data(), &v, sizeof(v)); + byte_swap_buffer<std::uint64_t>(a); } template<std::size_t N> @@ -1955,7 +1981,9 @@ // both branches below are intentionally identical (the "compact" float // representation is the value itself). Only GCC diagnoses this, and only // when the sink calls are inlined; clang has no such warning. -#if defined(__GNUC__) && !defined(__clang__) + // (-Wduplicated-branches only exists from GCC 7 on; naming it on an older + // GCC would itself warn under -Wpragmas) +#if defined(__GNUC__) && !defined(__clang__) && (__GNUC__ >= 7) #pragma GCC diagnostic ignored "-Wduplicated-branches" #endif if (!std::isfinite(n) || ((static_cast<double>(n) >= static_cast<double>(std::numeric_limits<float>::lowest()) &&
diff --git a/include/nlohmann/detail/output/output_adapters.hpp b/include/nlohmann/detail/output/output_adapters.hpp index 497bde7..7eb7312 100644 --- a/include/nlohmann/detail/output/output_adapters.hpp +++ b/include/nlohmann/detail/output/output_adapters.hpp
@@ -45,22 +45,32 @@ template<typename CharType> using output_adapter_t = std::shared_ptr<output_adapter_protocol<CharType>>; -/// output adapter for byte vectors +/// @brief non-virtual output sink writing into a std::vector +/// +/// This sink is not part of the virtual output_adapter_protocol hierarchy: it is +/// passed to binary_writer by value as a template parameter, so +/// write_character()/write_characters() are ordinary (inlinable) calls with no +/// vtable lookup and no shared_ptr. It is used for the common +/// `to_cbor`/`to_msgpack`/... into a std::vector. output_vector_adapter below +/// wraps this same sink to provide the virtual interface. template<typename CharType, typename AllocatorType = std::allocator<CharType>> -class output_vector_adapter : public output_adapter_protocol<CharType> +class output_vector_sink { public: - explicit output_vector_adapter(std::vector<CharType, AllocatorType>& vec) noexcept + explicit output_vector_sink(std::vector<CharType, AllocatorType>& vec) noexcept : v(vec) {} - void write_character(CharType c) override + void write_character(CharType c) { v.push_back(c); } - JSON_HEDLEY_NON_NULL(2) - void write_characters(const CharType* s, std::size_t length) override + // no JSON_HEDLEY_NON_NULL here: binary_writer legitimately passes a null + // pointer with length 0 for empty strings/binary values. Appending an empty + // range is a no-op; the type-erased path tolerates this via the (unattributed) + // virtual base, and the concrete sink must do the same. + void write_characters(const CharType* s, std::size_t length) { v.insert(v.end(), s, s + length); } @@ -69,6 +79,34 @@ std::vector<CharType, AllocatorType>& v; }; +/// output adapter for byte vectors +/// +/// The appending itself lives in output_vector_sink; this class only adds the +/// virtual output_adapter_protocol interface on top of it, so both the +/// type-erased and the templated path share one implementation. +template<typename CharType, typename AllocatorType = std::allocator<CharType>> +class output_vector_adapter : public output_adapter_protocol<CharType> +{ + public: + explicit output_vector_adapter(std::vector<CharType, AllocatorType>& vec) noexcept + : sink(vec) + {} + + void write_character(CharType c) override + { + sink.write_character(c); + } + + JSON_HEDLEY_NON_NULL(2) + void write_characters(const CharType* s, std::size_t length) override + { + sink.write_characters(s, length); + } + + private: + output_vector_sink<CharType, AllocatorType> sink; +}; + #ifndef JSON_NO_IO /// output adapter for output streams template<typename CharType> @@ -119,39 +157,6 @@ StringType& str; }; -/// @brief non-virtual output sink writing into a std::vector -/// -/// Unlike output_vector_adapter, this sink is not part of the virtual -/// output_adapter_protocol hierarchy: it is passed to binary_writer by value as -/// a template parameter, so write_character()/write_characters() are ordinary -/// (inlinable) calls with no vtable lookup and no shared_ptr. It is used for the -/// common `to_cbor`/`to_msgpack`/... into a std::vector. -template<typename CharType, typename AllocatorType = std::allocator<CharType>> -class output_vector_sink -{ - public: - explicit output_vector_sink(std::vector<CharType, AllocatorType>& vec) noexcept - : v(vec) - {} - - void write_character(CharType c) - { - v.push_back(c); - } - - // no JSON_HEDLEY_NON_NULL here: binary_writer legitimately passes a null - // pointer with length 0 for empty strings/binary values. Appending an empty - // range is a no-op; the type-erased path tolerates this via the (unattributed) - // virtual base, and the concrete sink must do the same. - void write_characters(const CharType* s, std::size_t length) - { - v.insert(v.end(), s, s + length); - } - - private: - std::vector<CharType, AllocatorType>& v; -}; - /// @brief output sink forwarding to a type-erased output adapter /// /// Wraps the polymorphic output_adapter_t so the same binary_writer template can @@ -182,7 +187,7 @@ } private: - output_adapter_t<CharType> oa = nullptr; + output_adapter_t<CharType> oa; }; template<typename CharType, typename StringType = std::basic_string<CharType>>
diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 002399b..128015f 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp
@@ -187,6 +187,14 @@ template<typename InputType> using binary_reader = ::nlohmann::detail::binary_reader<basic_json, InputType>; template<typename CharType> using binary_writer = ::nlohmann::detail::binary_writer<basic_json, CharType>; + // binary_writer over a concrete (non-virtual) sink appending into a std::vector, + // used by the vector-returning to_* overloads + template<typename CharType> using vector_binary_writer = + ::nlohmann::detail::binary_writer<basic_json, CharType, ::nlohmann::detail::output_vector_sink<CharType>>; + template<typename CharType> static vector_binary_writer<CharType> vector_writer(std::vector<CharType>& v) + { + return vector_binary_writer<CharType>(::nlohmann::detail::output_vector_sink<CharType>(v)); + } JSON_PRIVATE_UNLESS_TESTED: using serializer = ::nlohmann::detail::serializer<basic_json>; @@ -4341,8 +4349,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_cbor(j); + vector_writer(result).write_cbor(j); return result; } @@ -4366,8 +4373,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_msgpack(j); + vector_writer(result).write_msgpack(j); return result; } @@ -4393,8 +4399,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_ubjson(j, use_size, use_type); + vector_writer(result).write_ubjson(j, use_size, use_type); return result; } @@ -4423,8 +4428,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_ubjson(j, use_size, use_type, true, true, version); + vector_writer(result).write_ubjson(j, use_size, use_type, true, true, version); return result; } @@ -4452,8 +4456,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_bson(j); + vector_writer(result).write_bson(j); return result; }
diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index c9cbb98..4755253 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp
@@ -3722,71 +3722,71 @@ // SPDX-License-Identifier: MIT #ifndef INCLUDE_NLOHMANN_JSON_FWD_HPP_ -#define INCLUDE_NLOHMANN_JSON_FWD_HPP_ + #define INCLUDE_NLOHMANN_JSON_FWD_HPP_ -#include <cstdint> // int64_t, uint64_t -#include <map> // map -#include <memory> // allocator -#include <string> // string -#include <vector> // vector + #include <cstdint> // int64_t, uint64_t + #include <map> // map + #include <memory> // allocator + #include <string> // string + #include <vector> // vector -// #include <nlohmann/detail/abi_macros.hpp> + // #include <nlohmann/detail/abi_macros.hpp> -/*! -@brief namespace for Niels Lohmann -@see https://github.com/nlohmann -@since version 1.0.0 -*/ -NLOHMANN_JSON_NAMESPACE_BEGIN + /*! + @brief namespace for Niels Lohmann + @see https://github.com/nlohmann + @since version 1.0.0 + */ + NLOHMANN_JSON_NAMESPACE_BEGIN -/*! -@brief default JSONSerializer template argument + /*! + @brief default JSONSerializer template argument -This serializer ignores the template arguments and uses ADL -([argument-dependent lookup](https://en.cppreference.com/w/cpp/language/adl)) -for serialization. -*/ -template<typename T = void, typename SFINAE = void> -struct adl_serializer; + This serializer ignores the template arguments and uses ADL + ([argument-dependent lookup](https://en.cppreference.com/w/cpp/language/adl)) + for serialization. + */ + template<typename T = void, typename SFINAE = void> + struct adl_serializer; -/// a class to store JSON values -/// @sa https://json.nlohmann.me/api/basic_json/ -template<template<typename U, typename V, typename... Args> class ObjectType = - std::map, - template<typename U, typename... Args> class ArrayType = std::vector, - class StringType = std::string, class BooleanType = bool, - class NumberIntegerType = std::int64_t, - class NumberUnsignedType = std::uint64_t, - class NumberFloatType = double, - template<typename U> class AllocatorType = std::allocator, - template<typename T, typename SFINAE = void> class JSONSerializer = - adl_serializer, - class BinaryType = std::vector<std::uint8_t>, // cppcheck-suppress syntaxError - class CustomBaseClass = void> -class basic_json; + /// a class to store JSON values + /// @sa https://json.nlohmann.me/api/basic_json/ + template<template<typename U, typename V, typename... Args> class ObjectType = + std::map, + template<typename U, typename... Args> class ArrayType = std::vector, + class StringType = std::string, class BooleanType = bool, + class NumberIntegerType = std::int64_t, + class NumberUnsignedType = std::uint64_t, + class NumberFloatType = double, + template<typename U> class AllocatorType = std::allocator, + template<typename T, typename SFINAE = void> class JSONSerializer = + adl_serializer, + class BinaryType = std::vector<std::uint8_t>, // cppcheck-suppress syntaxError + class CustomBaseClass = void> + class basic_json; -/// @brief JSON Pointer defines a string syntax for identifying a specific value within a JSON document -/// @sa https://json.nlohmann.me/api/json_pointer/ -template<typename RefStringType> -class json_pointer; + /// @brief JSON Pointer defines a string syntax for identifying a specific value within a JSON document + /// @sa https://json.nlohmann.me/api/json_pointer/ + template<typename RefStringType> + class json_pointer; -/*! -@brief default specialization -@sa https://json.nlohmann.me/api/json/ -*/ -using json = basic_json<>; + /*! + @brief default specialization + @sa https://json.nlohmann.me/api/json/ + */ + using json = basic_json<>; -/// @brief a minimal map-like container that preserves insertion order -/// @sa https://json.nlohmann.me/api/ordered_map/ -template<class Key, class T, class IgnoredLess, class Allocator> -struct ordered_map; + /// @brief a minimal map-like container that preserves insertion order + /// @sa https://json.nlohmann.me/api/ordered_map/ + template<class Key, class T, class IgnoredLess, class Allocator> + struct ordered_map; -/// @brief specialization that maintains the insertion order of object keys -/// @sa https://json.nlohmann.me/api/ordered_json/ -using ordered_json = basic_json<nlohmann::ordered_map>; + /// @brief specialization that maintains the insertion order of object keys + /// @sa https://json.nlohmann.me/api/ordered_json/ + using ordered_json = basic_json<nlohmann::ordered_map>; -NLOHMANN_JSON_NAMESPACE_END + NLOHMANN_JSON_NAMESPACE_END #endif // INCLUDE_NLOHMANN_JSON_FWD_HPP_ @@ -5873,7 +5873,7 @@ // #include <nlohmann/detail/macro_scope.hpp> - // JSON_HAS_CPP_17 +// JSON_HAS_CPP_17 #ifdef JSON_HAS_CPP_17 #include <optional> // optional #endif @@ -16807,9 +16807,14 @@ #include <cstring> // memcpy #include <limits> // numeric_limits #include <string> // string +#include <type_traits> // enable_if, is_constructible #include <utility> // move #include <vector> // vector +#ifdef _MSC_VER + #include <cstdlib> // _byteswap_ushort, _byteswap_ulong, _byteswap_uint64 +#endif + // #include <nlohmann/detail/input/binary_reader.hpp> // #include <nlohmann/detail/macro_scope.hpp> @@ -16863,22 +16868,32 @@ template<typename CharType> using output_adapter_t = std::shared_ptr<output_adapter_protocol<CharType>>; -/// output adapter for byte vectors +/// @brief non-virtual output sink writing into a std::vector +/// +/// This sink is not part of the virtual output_adapter_protocol hierarchy: it is +/// passed to binary_writer by value as a template parameter, so +/// write_character()/write_characters() are ordinary (inlinable) calls with no +/// vtable lookup and no shared_ptr. It is used for the common +/// `to_cbor`/`to_msgpack`/... into a std::vector. output_vector_adapter below +/// wraps this same sink to provide the virtual interface. template<typename CharType, typename AllocatorType = std::allocator<CharType>> -class output_vector_adapter : public output_adapter_protocol<CharType> +class output_vector_sink { public: - explicit output_vector_adapter(std::vector<CharType, AllocatorType>& vec) noexcept + explicit output_vector_sink(std::vector<CharType, AllocatorType>& vec) noexcept : v(vec) {} - void write_character(CharType c) override + void write_character(CharType c) { v.push_back(c); } - JSON_HEDLEY_NON_NULL(2) - void write_characters(const CharType* s, std::size_t length) override + // no JSON_HEDLEY_NON_NULL here: binary_writer legitimately passes a null + // pointer with length 0 for empty strings/binary values. Appending an empty + // range is a no-op; the type-erased path tolerates this via the (unattributed) + // virtual base, and the concrete sink must do the same. + void write_characters(const CharType* s, std::size_t length) { v.insert(v.end(), s, s + length); } @@ -16887,6 +16902,34 @@ std::vector<CharType, AllocatorType>& v; }; +/// output adapter for byte vectors +/// +/// The appending itself lives in output_vector_sink; this class only adds the +/// virtual output_adapter_protocol interface on top of it, so both the +/// type-erased and the templated path share one implementation. +template<typename CharType, typename AllocatorType = std::allocator<CharType>> +class output_vector_adapter : public output_adapter_protocol<CharType> +{ + public: + explicit output_vector_adapter(std::vector<CharType, AllocatorType>& vec) noexcept + : sink(vec) + {} + + void write_character(CharType c) override + { + sink.write_character(c); + } + + JSON_HEDLEY_NON_NULL(2) + void write_characters(const CharType* s, std::size_t length) override + { + sink.write_characters(s, length); + } + + private: + output_vector_sink<CharType, AllocatorType> sink; +}; + #ifndef JSON_NO_IO /// output adapter for output streams template<typename CharType> @@ -16937,39 +16980,6 @@ StringType& str; }; -/// @brief non-virtual output sink writing into a std::vector -/// -/// Unlike output_vector_adapter, this sink is not part of the virtual -/// output_adapter_protocol hierarchy: it is passed to binary_writer by value as -/// a template parameter, so write_character()/write_characters() are ordinary -/// (inlinable) calls with no vtable lookup and no shared_ptr. It is used for the -/// common `to_cbor`/`to_msgpack`/... into a std::vector. -template<typename CharType, typename AllocatorType = std::allocator<CharType>> -class output_vector_sink -{ - public: - explicit output_vector_sink(std::vector<CharType, AllocatorType>& vec) noexcept - : v(vec) - {} - - void write_character(CharType c) - { - v.push_back(c); - } - - // no JSON_HEDLEY_NON_NULL here: binary_writer legitimately passes a null - // pointer with length 0 for empty strings/binary values. Appending an empty - // range is a no-op; the type-erased path tolerates this via the (unattributed) - // virtual base, and the concrete sink must do the same. - void write_characters(const CharType* s, std::size_t length) - { - v.insert(v.end(), s, s + length); - } - - private: - std::vector<CharType, AllocatorType>& v; -}; - /// @brief output sink forwarding to a type-erased output adapter /// /// Wraps the polymorphic output_adapter_t so the same binary_writer template can @@ -17000,7 +17010,7 @@ } private: - output_adapter_t<CharType> oa = nullptr; + output_adapter_t<CharType> oa; }; template<typename CharType, typename StringType = std::basic_string<CharType>> @@ -17050,31 +17060,33 @@ /////////////////// /*! -@brief conservative capacity hint for binary serialization into a std::vector +@brief capacity hint for binary serialization into a std::vector -Returns an approximate number of bytes to reserve up front so that serializing -an array/object of many elements does not repeatedly reallocate the output -buffer. Only the top-level element count is consulted (O(1), no walk of the -DOM), and the result is clamped to a fixed ceiling: a large or untrusted DOM can -therefore never trigger an oversized allocation here, and the multiplication -cannot overflow. The buffer still grows geometrically beyond the hint, so a hint -that is too small only costs a few later reallocations. A single scalar, string, -or binary value is written in one shot and needs no hint. +Returns a *lower* bound on the number of bytes the serialization will produce, +so that writing an array/object of many elements does not start reallocating +from an empty buffer. Every array element occupies at least one byte in every +supported binary format, and every object entry at least two (a key of at least +one byte plus a value of at least one), plus one byte for the container header, +so the hint can never exceed the final size and the returned vector is never +left holding capacity the caller did not ask for. The buffer still grows +geometrically past the hint, so under-reserving only costs a few later +reallocations. Only the top-level element count is consulted (O(1), no walk of +the DOM); a single scalar, string, or binary value is written in one shot and +needs no hint. */ template<typename BasicJsonType> std::size_t binary_reserve_hint(const BasicJsonType& j) { - constexpr std::size_t max_hint = static_cast<std::size_t>(1) << 20; // 1 MiB - if (j.is_array() || j.is_object()) + if (j.is_array()) { - const std::size_t elements = j.size(); - // guard the multiplication against overflow and cap the reservation - if (elements > max_hint / 4) - { - return max_hint; - } - return (elements * 4) + 2; + return j.size() + 1; } + + if (j.is_object()) + { + return (j.size() * 2) + 1; + } + return 0; } @@ -17104,12 +17116,15 @@ Convenience constructor for the default (output_adapter_sink) sink so the `output_adapter`-based overloads keep constructing the writer directly from - an adapter. Only participates in overload resolution when the sink can be - built from an adapter. + an adapter. Constrained to sinks that can actually be built from an adapter, + so that a writer over some other sink type is not advertised as constructible + from one. @param[in] adapter output adapter to write to */ - explicit binary_writer(output_adapter_t<CharType> adapter) : oa(OutputSinkType(std::move(adapter))) + template < typename SinkType = OutputSinkType, + typename std::enable_if < std::is_constructible<SinkType, output_adapter_t<CharType>>::value, int >::type = 0 > + explicit binary_writer(output_adapter_t<CharType> adapter) : oa(SinkType(std::move(adapter))) {} /*! @@ -18879,6 +18894,8 @@ { #if defined(__GNUC__) || defined(__clang__) return __builtin_bswap16(x); +#elif defined(_MSC_VER) + return _byteswap_ushort(x); #else return static_cast<std::uint16_t>((x >> 8) | (x << 8)); #endif @@ -18888,6 +18905,8 @@ { #if defined(__GNUC__) || defined(__clang__) return __builtin_bswap32(x); +#elif defined(_MSC_VER) + return _byteswap_ulong(x); #else return ((x & 0x000000FFu) << 24) | ((x & 0x0000FF00u) << 8) | ((x & 0x00FF0000u) >> 8) | ((x & 0xFF000000u) >> 24); @@ -18898,6 +18917,8 @@ { #if defined(__GNUC__) || defined(__clang__) return __builtin_bswap64(x); +#elif defined(_MSC_VER) + return _byteswap_uint64(x); #else x = ((x & 0x00000000FFFFFFFFull) << 32) | ((x & 0xFFFFFFFF00000000ull) >> 32); x = ((x & 0x0000FFFF0000FFFFull) << 16) | ((x & 0xFFFF0000FFFF0000ull) >> 16); @@ -18906,30 +18927,40 @@ #endif } + /*! + @brief reverse the bytes of a buffer by byte-swapping it as UIntType + + Loading the buffer into an unsigned integer of the same width and swapping + that is what lets the compiler emit a single bswap/rev/movbe; reversing the + buffer element by element does not reliably get there (clang keeps a scalar + shuffle). The two memcpy calls are the only portable way to reinterpret the + bytes and are folded away by every optimizer. + */ + template<typename UIntType, std::size_t N> + static void byte_swap_buffer(std::array<CharType, N>& a) noexcept + { + static_assert(sizeof(UIntType) == N, "swap width must match the buffer size"); + UIntType v{}; + std::memcpy(&v, a.data(), sizeof(v)); + v = byte_swap(v); + std::memcpy(a.data(), &v, sizeof(v)); + } + // reverse the bytes of a fixed-size buffer; a single byte_swap() for the // common 2/4/8-byte number payloads, std::reverse for any other size static void reverse_bytes(std::array<CharType, 2>& a) noexcept { - std::uint16_t v{}; - std::memcpy(&v, a.data(), sizeof(v)); - v = byte_swap(v); - std::memcpy(a.data(), &v, sizeof(v)); + byte_swap_buffer<std::uint16_t>(a); } static void reverse_bytes(std::array<CharType, 4>& a) noexcept { - std::uint32_t v{}; - std::memcpy(&v, a.data(), sizeof(v)); - v = byte_swap(v); - std::memcpy(a.data(), &v, sizeof(v)); + byte_swap_buffer<std::uint32_t>(a); } static void reverse_bytes(std::array<CharType, 8>& a) noexcept { - std::uint64_t v{}; - std::memcpy(&v, a.data(), sizeof(v)); - v = byte_swap(v); - std::memcpy(a.data(), &v, sizeof(v)); + byte_swap_buffer<std::uint64_t>(a); } template<std::size_t N> @@ -18965,7 +18996,9 @@ // both branches below are intentionally identical (the "compact" float // representation is the value itself). Only GCC diagnoses this, and only // when the sink calls are inlined; clang has no such warning. -#if defined(__GNUC__) && !defined(__clang__) + // (-Wduplicated-branches only exists from GCC 7 on; naming it on an older + // GCC would itself warn under -Wpragmas) +#if defined(__GNUC__) && !defined(__clang__) && (__GNUC__ >= 7) #pragma GCC diagnostic ignored "-Wduplicated-branches" #endif if (!std::isfinite(n) || ((static_cast<double>(n) >= static_cast<double>(std::numeric_limits<float>::lowest()) && @@ -21700,10 +21733,10 @@ const bool allow_exceptions = true, const bool ignore_comments = false, const bool ignore_trailing_commas = false - ) + ) { return ::nlohmann::detail::parser<basic_json, InputAdapterType>(std::move(adapter), - std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas); + std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas); } private: @@ -21722,6 +21755,14 @@ template<typename InputType> using binary_reader = ::nlohmann::detail::binary_reader<basic_json, InputType>; template<typename CharType> using binary_writer = ::nlohmann::detail::binary_writer<basic_json, CharType>; + // binary_writer over a concrete (non-virtual) sink appending into a std::vector, + // used by the vector-returning to_* overloads + template<typename CharType> using vector_binary_writer = + ::nlohmann::detail::binary_writer<basic_json, CharType, ::nlohmann::detail::output_vector_sink<CharType>>; + template<typename CharType> static vector_binary_writer<CharType> vector_writer(std::vector<CharType>& v) + { + return vector_binary_writer<CharType>(::nlohmann::detail::output_vector_sink<CharType>(v)); + } JSON_PRIVATE_UNLESS_TESTED: using serializer = ::nlohmann::detail::serializer<basic_json>; @@ -22401,8 +22442,8 @@ detail::enable_if_t < !detail::is_basic_json<U>::value && detail::is_compatible_type<basic_json_t, U>::value, int > = 0 > basic_json(CompatibleType && val) noexcept(noexcept( // NOLINT(bugprone-forwarding-reference-overload,bugprone-exception-escape) - JSONSerializer<U>::to_json(std::declval<basic_json_t&>(), - std::forward<CompatibleType>(val)))) + JSONSerializer<U>::to_json(std::declval<basic_json_t&>(), + std::forward<CompatibleType>(val)))) { JSONSerializer<U>::to_json(*this, std::forward<CompatibleType>(val)); set_parents(); @@ -23205,7 +23246,7 @@ detail::has_from_json<basic_json_t, ValueType>::value, int > = 0 > ValueType get_impl(detail::priority_tag<0> /*unused*/) const noexcept(noexcept( - JSONSerializer<ValueType>::from_json(std::declval<const basic_json_t&>(), std::declval<ValueType&>()))) + JSONSerializer<ValueType>::from_json(std::declval<const basic_json_t&>(), std::declval<ValueType&>()))) { auto ret = ValueType(); JSONSerializer<ValueType>::from_json(*this, ret); @@ -23247,7 +23288,7 @@ detail::has_non_default_from_json<basic_json_t, ValueType>::value, int > = 0 > ValueType get_impl(detail::priority_tag<1> /*unused*/) const noexcept(noexcept( - JSONSerializer<ValueType>::from_json(std::declval<const basic_json_t&>()))) + JSONSerializer<ValueType>::from_json(std::declval<const basic_json_t&>()))) { return JSONSerializer<ValueType>::from_json(*this); } @@ -23397,7 +23438,7 @@ detail::has_from_json<basic_json_t, ValueType>::value, int > = 0 > ValueType & get_to(ValueType& v) const noexcept(noexcept( - JSONSerializer<ValueType>::from_json(std::declval<const basic_json_t&>(), v))) + JSONSerializer<ValueType>::from_json(std::declval<const basic_json_t&>(), v))) { JSONSerializer<ValueType>::from_json(*this, v); return v; @@ -25876,8 +25917,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_cbor(j); + vector_writer(result).write_cbor(j); return result; } @@ -25901,8 +25941,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_msgpack(j); + vector_writer(result).write_msgpack(j); return result; } @@ -25928,8 +25967,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_ubjson(j, use_size, use_type); + vector_writer(result).write_ubjson(j, use_size, use_type); return result; } @@ -25958,8 +25996,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_ubjson(j, use_size, use_type, true, true, version); + vector_writer(result).write_ubjson(j, use_size, use_type, true, true, version); return result; } @@ -25987,8 +26024,7 @@ { std::vector<std::uint8_t> result; result.reserve(detail::binary_reserve_hint(j)); - detail::binary_writer<basic_json, std::uint8_t, detail::output_vector_sink<std::uint8_t>>( - detail::output_vector_sink<std::uint8_t>(result)).write_bson(j); + vector_writer(result).write_bson(j); return result; }
diff --git a/tests/src/unit-binary_writer_sinks.cpp b/tests/src/unit-binary_writer_sinks.cpp new file mode 100644 index 0000000..f60e1bf --- /dev/null +++ b/tests/src/unit-binary_writer_sinks.cpp
@@ -0,0 +1,198 @@ +// __ _____ _____ _____ +// __| | __| | | | JSON for Modern C++ (supporting code) +// | | |__ | | | | | | version 3.12.0 +// |_____|_____|_____|_|___| https://github.com/nlohmann/json +// +// SPDX-FileCopyrightText: 2013-2026 Niels Lohmann <https://nlohmann.me> +// SPDX-License-Identifier: MIT + +#include "doctest_compatibility.h" + +#include <nlohmann/json.hpp> +using nlohmann::json; + +#include <cstdint> +#include <string> +#include <vector> + +namespace +{ + +// a spread of values exercising every writer path: scalars of each width, the +// float paths, strings, binary, and containers big enough to reallocate +std::vector<json> test_values() +{ + json big_array = json::array(); + for (int i = 0; i < 5000; ++i) + { + big_array.push_back(i); + } + + json big_object = json::object(); + for (int i = 0; i < 1000; ++i) + { + big_object[std::to_string(i)] = i; + } + + return + { + json(nullptr), json(true), json(false), + json(0), json(-1), json(255), json(-129), json(65535), json(-32769), + json(4294967295U), json(-2147483649LL), json(18446744073709551615ULL), + json(0.0), json(-0.5), json(3.1415926535897932), + json(""), json("hello"), json(std::string(1000, 'x')), + json::binary({0x00, 0x01, 0x02}, 42), + json::array(), json::object(), + json::array({1, 2, 3}), json({{"a", 1}, {"b", nullptr}}), + json({{"nested", {{"deep", json::array({1, "two", 3.0, nullptr})}}}}), + big_array, big_object + }; +} + +// values to_bson() accepts: the document must be an object +std::vector<json> bson_values() +{ + json big_object = json::object(); + for (int i = 0; i < 1000; ++i) + { + big_object[std::to_string(i)] = i; + } + + return + { + json::object(), + json({{"a", 1}, {"b", nullptr}, {"c", true}, {"d", 2.5}, {"e", "text"}}), + json({{"arr", json::array({1, 2, 3})}, {"obj", {{"k", "v"}}}}), + big_object + }; +} + +} // namespace + +// The vector-returning to_*(j) overloads write through the non-virtual +// output_vector_sink, while to_*(j, adapter) goes through output_adapter_sink. +// The two are separate code paths that must stay byte-for-byte identical; these +// checks fail if either overload is ever changed without the other. +TEST_CASE("binary writer output sinks") +{ + SECTION("vector sink and adapter sink agree") + { + // note: no SUBCASE inside these loops - doctest keys subcases by + // name/file/line, so a subcase in a loop body would only ever run for + // the first iteration + for (const auto& j : test_values()) + { + CAPTURE(j.dump(-1, ' ', false, json::error_handler_t::replace)); + + std::vector<std::uint8_t> cbor; + json::to_cbor(j, cbor); + CHECK(json::to_cbor(j) == cbor); + + std::vector<std::uint8_t> msgpack; + json::to_msgpack(j, msgpack); + CHECK(json::to_msgpack(j) == msgpack); + + for (const bool use_size : + { + false, true + }) + { + for (const bool use_type : + { + false, true + }) + { + if (use_type && !use_size) + { + continue; // not a supported combination + } + CAPTURE(use_size); + CAPTURE(use_type); + std::vector<std::uint8_t> ubjson; + json::to_ubjson(j, ubjson, use_size, use_type); + CHECK(json::to_ubjson(j, use_size, use_type) == ubjson); + } + } + + for (const auto version : + { + json::bjdata_version_t::draft2, json::bjdata_version_t::draft3 + }) + { + std::vector<std::uint8_t> bjdata; + json::to_bjdata(j, bjdata, false, false, version); + CHECK(json::to_bjdata(j, false, false, version) == bjdata); + } + } + + for (const auto& j : bson_values()) + { + CAPTURE(j.dump()); + std::vector<std::uint8_t> bson; + json::to_bson(j, bson); + CHECK(json::to_bson(j) == bson); + } + } + + SECTION("the char adapter produces the same bytes") + { + for (const auto& j : test_values()) + { + CAPTURE(j.dump(-1, ' ', false, json::error_handler_t::replace)); + + const std::vector<std::uint8_t> expected = json::to_cbor(j); + std::vector<char> as_char; + json::to_cbor(j, as_char); + + REQUIRE(as_char.size() == expected.size()); + std::vector<std::uint8_t> as_bytes; + as_bytes.reserve(as_char.size()); + for (const char c : as_char) + { + as_bytes.push_back(static_cast<std::uint8_t>(c)); + } + CHECK(as_bytes == expected); + } + } +} + +// binary_reserve_hint() is documented as a *lower* bound on the serialized size, +// so that reserving it up front can never leave the returned vector holding +// capacity beyond what the value actually needs. +TEST_CASE("binary_reserve_hint never over-reserves") +{ + for (const auto& j : test_values()) + { + CAPTURE(j.dump(-1, ' ', false, json::error_handler_t::replace)); + + const std::size_t hint = nlohmann::detail::binary_reserve_hint(j); + + CHECK(hint <= json::to_cbor(j).size()); + CHECK(hint <= json::to_msgpack(j).size()); + CHECK(hint <= json::to_ubjson(j).size()); + CHECK(hint <= json::to_ubjson(j, true, true).size()); + CHECK(hint <= json::to_bjdata(j).size()); + } + + for (const auto& j : bson_values()) + { + CAPTURE(j.dump()); + CHECK(nlohmann::detail::binary_reserve_hint(j) <= json::to_bson(j).size()); + } + + SECTION("scalars get no hint") + { + CHECK(nlohmann::detail::binary_reserve_hint(json(nullptr)) == 0); + CHECK(nlohmann::detail::binary_reserve_hint(json(42)) == 0); + CHECK(nlohmann::detail::binary_reserve_hint(json("a string")) == 0); + CHECK(nlohmann::detail::binary_reserve_hint(json::binary({0x01})) == 0); + } + + SECTION("containers are hinted from their element count") + { + CHECK(nlohmann::detail::binary_reserve_hint(json::array()) == 1); + CHECK(nlohmann::detail::binary_reserve_hint(json::array({1, 2, 3})) == 4); + CHECK(nlohmann::detail::binary_reserve_hint(json::object()) == 1); + CHECK(nlohmann::detail::binary_reserve_hint(json({{"a", 1}, {"b", 2}})) == 5); + } +}