amorfo77/json
1
0
Fork 0
mirror of https://github.com/nlohmann/json.git synced 2026-09-01 14:07:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address review findings on the binary writer output sinks

Browse Source 
- 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>
This commit is contained in:
Niels Lohmann
2026-08-31 23:24:37 +00:00
committed by Claude
parent 1d51b00830
commit aad5fa9a29
5 changed files with 499 additions and 229 deletions
Download Patch File Download Diff File Expand all files Collapse all files
include/nlohmann/detail/output/binary_writer.hpp
+62 -34
View File
@@ -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 @@ enum class bjdata_version_t
///////////////////
/*!
@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 @@ class binary_writer
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 @@ class binary_writer
{
#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 @@ class binary_writer
{
#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 @@ class binary_writer
{
#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 @@ class binary_writer
#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 @@ class binary_writer
// 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()) &&