Repository navigation
Route hand-rolled diagnostic pragmas through Hedley - #5485
Merged
nlohmann merged 2 commits intoSep 9, 2026
Merged
Conversation
gregmarr
reviewed
Sep 8, 2026
nlohmann
added a commit
that referenced
this pull request
Sep 8, 2026
… the pragma they bracket Addresses review feedback from @gregmarr on PR #5485: the push/pop calls were unconditional, so compilers other than the one the ignored-pragma targets (e.g. MSVC, or GCC where the pair only applies under __clang__) now did a needless push/pop with nothing suppressed in between. Move the existing #ifdef __GNUC__ / #if defined(__clang__) guard to also cover the push/pop, restoring the original zero-overhead behavior on other compilers while still emitting the pragma itself through Hedley. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
gregmarr
approved these changes
Sep 8, 2026
nlohmann
added a commit
that referenced
this pull request
Sep 9, 2026
… the pragma they bracket Addresses review feedback from @gregmarr on PR #5485: the push/pop calls were unconditional, so compilers other than the one the ignored-pragma targets (e.g. MSVC, or GCC where the pair only applies under __clang__) now did a needless push/pop with nothing suppressed in between. Move the existing #ifdef __GNUC__ / #if defined(__clang__) guard to also cover the push/pop, restoring the original zero-overhead behavior on other compilers while still emitting the pragma itself through Hedley. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
force-pushed
the
issue-5409-hedley-diagnostic-pragmas
branch
from
September 9, 2026 07:46
c7d3d90 to
f5a205e
Compare
Several places in the library hand-roll compiler diagnostic suppression
with raw `#pragma`/`#ifdef __GNUC__`/`#ifdef __clang__` guards instead of
using the Hedley primitives already bundled and used elsewhere
(JSON_HEDLEY_DIAGNOSTIC_PUSH/POP, JSON_HEDLEY_PRAGMA, ...). Converted six
of the seven listed push/pop pairs to use those primitives instead of
raw `#pragma GCC diagnostic`/`#pragma clang diagnostic` text:
- include/nlohmann/json.hpp (~3770, ~3863): -Wfloat-equal
- include/nlohmann/detail/conversions/to_chars.hpp (~1078): -Wfloat-equal
- include/nlohmann/detail/output/binary_writer.hpp (~1844): -Wfloat-equal
- include/nlohmann/detail/iterators/iteration_proxy.hpp (~211): -Wmismatched-tags
- include/nlohmann/detail/exceptions.hpp (~36): -Wweak-vtables
iteration_proxy.hpp did not previously include macro_scope.hpp itself
(it only compiled because some other header included earlier in
json.hpp happened to pull macro_scope.hpp in first); it now includes it
directly like the other detail headers that use Hedley macros, so it is
self-contained.
Each push/pop pair now uses JSON_HEDLEY_DIAGNOSTIC_PUSH/POP
unconditionally (a no-op on compilers that don't need it) and wraps the
actual `#pragma ... diagnostic ignored` text in JSON_HEDLEY_PRAGMA so it
goes through Hedley's _Pragma()-based emission instead of a raw #pragma
line, while keeping the original `#ifdef __GNUC__` / `#if
defined(__clang__)` guard around the ignored-pragma itself.
Deviation from the issue's suggested transformation: the issue's example
replaces the `#ifdef __GNUC__` guard with `#if
JSON_HEDLEY_HAS_WARNING("-Wfloat-equal")`. JSON_HEDLEY_HAS_WARNING is
implemented purely via Clang's `__has_warning` builtin and evaluates to
0 on real GCC (`#define JSON_HEDLEY_HAS_WARNING(warning) (0)` when
`__has_warning` is not defined), so adopting it verbatim would silently
stop suppressing -Wfloat-equal on GCC -- a real regression, not just a
style change. The existing `#ifdef __GNUC__` / `#if defined(__clang__)`
guards were kept for the ignored-pragma to stay behavior-preserving, and
only the push/pop/pragma-emission mechanism was routed through Hedley.
Two of the seven locations from the issue (the -Wignored-attributes
push at the very top of json.hpp and its matching pop after
`#include <nlohmann/detail/macro_unscope.hpp>`) were intentionally left
unconverted:
- The push, at the very top of json.hpp, runs before
`detail/macro_scope.hpp` (and therefore hedley.hpp) has been included
anywhere in the translation unit, so JSON_HEDLEY_DIAGNOSTIC_PUSH is not
yet defined at that point.
- The pop runs after `macro_unscope.hpp`, which -- via hedley_undef.hpp
-- has already #undef'd every JSON_HEDLEY_* macro (by design, see
#5408) precisely so they don't leak to users, so JSON_HEDLEY_DIAGNOSTIC_POP
is no longer defined by the time the pop is reached either.
Making this one pair work would require either hoisting the ~2000
line vendored hedley.hpp to the very top of the amalgamated single
header (a much bigger structural change to single_include than a pure
mechanism swap) or special-casing this one pop ahead of the general
macro cleanup. Both are riskier than the mechanical, behavior-preserving
change requested, so this pair was left as-is.
## Validation
- Compiled include/nlohmann/json.hpp and single_include/nlohmann/json.hpp
with `-Wall -Wextra -Wfloat-equal -Wmismatched-tags -Wweak-vtables`
(clang, which self-identifies as __GNUC__ too): no warnings, same as
before the change.
- Compiled and ran tests/src/unit-to_chars.cpp, unit-conversions.cpp,
unit-iterators1.cpp, unit-iterators2.cpp, and unit-class_parser.cpp
against the fixed include/: all pass.
- Compiled unit-msgpack.cpp, unit-bjdata.cpp, and unit-ubjson.cpp (which
exercise binary_writer.hpp's write_compact_float extensively): all
compile cleanly; the vast majority of assertions pass (the only
failures are pre-existing environment issues unrelated to this change
-- missing generated test-data files, not code correctness).
- Ran `make amalgamate`; the single_include diff is limited to exactly
the lines touched in include/, with no unrelated reordering.
- No real (non-Apple) GCC was available in this environment to test
directly; the `_Pragma("GCC diagnostic ...")` text emitted by
JSON_HEDLEY_PRAGMA is byte-identical to the prior `#pragma GCC
diagnostic ...` text, and the `#ifdef __GNUC__` guard is unchanged, so
GCC's behavior is expected to be identical. CI covers the GCC matrix.
This PR is stacked on top of #5475 (issue-5408-hedley-undef-leak) since
both touch the same files; only the last commit here is new.
Fixes #5409.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
… the pragma they bracket Addresses review feedback from @gregmarr on PR #5485: the push/pop calls were unconditional, so compilers other than the one the ignored-pragma targets (e.g. MSVC, or GCC where the pair only applies under __clang__) now did a needless push/pop with nothing suppressed in between. Move the existing #ifdef __GNUC__ / #if defined(__clang__) guard to also cover the push/pop, restoring the original zero-overhead behavior on other compilers while still emitting the pragma itself through Hedley. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
force-pushed
the
issue-5409-hedley-diagnostic-pragmas
branch
from
September 9, 2026 07:47
f5a205e to
f07f5f6
Compare
nlohmann
added a commit
that referenced
this pull request
Sep 11, 2026
Match #5485, which moved the binary writer's hand-rolled diagnostic pragmas onto JSON_HEDLEY_PRAGMA (merged into develop while this branch was open). The devirtualization's -Wduplicated-branches suppression in write_compact_float was the one raw '#pragma GCC diagnostic' left; it now uses JSON_HEDLEY_PRAGMA like the adjacent -Wfloat-equal line, still guarded to GCC >= 7 and non-clang (the warning exists only there). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
added a commit
that referenced
this pull request
Sep 11, 2026
Match #5485, which moved the binary writer's hand-rolled diagnostic pragmas onto JSON_HEDLEY_PRAGMA (merged into develop while this branch was open). The devirtualization's -Wduplicated-branches suppression in write_compact_float was the one raw '#pragma GCC diagnostic' left; it now uses JSON_HEDLEY_PRAGMA like the adjacent -Wfloat-equal line, still guarded to GCC >= 7 and non-clang (the warning exists only there). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
added a commit
that referenced
this pull request
Sep 22, 2026
Match #5485, which moved the binary writer's hand-rolled diagnostic pragmas onto JSON_HEDLEY_PRAGMA (merged into develop while this branch was open). The devirtualization's -Wduplicated-branches suppression in write_compact_float was the one raw '#pragma GCC diagnostic' left; it now uses JSON_HEDLEY_PRAGMA like the adjacent -Wfloat-equal line, still guarded to GCC >= 7 and non-clang (the warning exists only there). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
added a commit
that referenced
this pull request
Sep 23, 2026
…coding (#5286) * Devirtualize binary_writer via a value-type output sink to_cbor/to_msgpack/to_ubjson/to_bjdata/to_bson wrote every byte through output_adapter_t, a shared_ptr<output_adapter_protocol> whose write_character/write_characters are virtual. Unlike the lexer (templated on a concrete InputAdapterType), the binary writer never got that treatment, so binary output paid a vtable lookup per byte and a make_shared per call. Template binary_writer on an OutputSinkType and give it two concrete, non-virtual sinks: - output_vector_sink: appends straight into a std::vector (push_back / insert), used by the vector-returning to_* convenience functions. No vtable, no shared_ptr; the writes inline. - output_adapter_sink: forwards to a type-erased output_adapter_t, so the existing to_*(j, output_adapter) overloads (streams, strings, custom adapters) keep working exactly as before -- one virtual call each, unchanged. binary_writer keeps a convenience constructor taking output_adapter_t (building the default output_adapter_sink), so the adapter overloads are untouched; only the convenience functions switch to the vector sink. The friend declaration and the basic_json binary_writer alias gain the new (defaulted) template parameter. Output is byte-for-byte identical: verified across ~3000 randomized values plus curated edge cases (all scalar widths, strings with invalid UTF-8, binary, nested arrays/objects) for CBOR, MessagePack, UBJSON (both size/type settings), BJData, and BSON, plus the output_adapter path, in C++11/17/20. Warning-clean under clang -Weverything and the gcc pedantic set; clang-tidy clean on the changed headers; make check-amalgamation clean. Throughput (g++ -O3, vs develop): scalar-dense binary output such as integer arrays ~1.4x; many small to_cbor calls ~1.04x (DOM traversal bound); string/blob-heavy output unchanged (already bulk-bound). No workload regressed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix CI failures from binary_writer output-sink change Four CI jobs failed on the initial commit; all are addressed here without changing any output (binary encodings remain byte-for-byte identical to develop across the differential corpus): 1. ci_test_gcc / cuda (-Werror=duplicated-branches): for number_float_t == float, static_cast<float>(n) is the identity, so write_compact_float's two branches are intentionally identical. Once the concrete vector sink is inlined, GCC constant-folds and diagnoses this (the type-erased path hid it behind a non-inlined virtual call). Silence -Wduplicated-branches for GCC (clang has no such warning) alongside the existing -Wfloat-equal pragma. 2. ci_static_analysis_clang (UBSan nonnull-attribute): binary_writer passes a null pointer with length 0 for empty strings/binary. output_vector_sink / output_adapter_sink declared write_characters JSON_HEDLEY_NON_NULL, so the sanitizer flagged the (harmless) zero-length call once the sink was called directly rather than through the attribute-free virtual base. Drop the attribute from both sinks, matching the pre-existing behavior. 3. ci_cpplint (build/include_what_you_use): output_adapter_sink uses std::move; add #include <utility>. 4. ci_cuda_example (nvcc 11.8): NVCC's front end rejects the default template argument on the binary_writer alias template. Revert the alias to its original single-parameter form (relying on binary_writer's own defaulted OutputSinkType) and spell out the full type in the vector-sink convenience functions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Encode big-endian numbers with a byte swap instead of std::reverse write_number() reordered multi-byte numbers for the big-endian formats (CBOR/MessagePack/UBJSON) with std::reverse over the byte array. GCC lowered only some sizes to a bswap; clang kept a scalar byte shuffle (0 bswap instructions in the CBOR number path). Replace the reverse with size-dispatched __builtin_bswap16/32/64 helpers (portable shift fallback for other compilers; std::reverse retained for exotic sizes such as a long double number_float_t). Codegen: the CBOR number path now emits bswap on both compilers (gcc 2 -> 16, clang 0 -> 4). Output is byte-for-byte identical to the previous implementation across the binary differential corpus. Throughput (isolated vs the std::reverse version, best of 9): CBOR int64 array gcc +7% clang +10% CBOR uint16 array gcc +27% clang flat Modest but consistent on number-dense encodings; negligible on string/blob-heavy output, as expected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Reserve output capacity up front for binary serialization The vector-returning to_cbor/to_msgpack/to_ubjson/to_bjdata/to_bson grew the output buffer purely by geometric reallocation. Reserving an estimate up front avoids the early reallocations, which is the dominant per-byte cost for array/object-heavy output. The estimate (binary_reserve_hint) is deliberately conservative and safe against untrusted input: it consults only the top-level element count (O(1), no walk of the DOM), guards the multiplication against overflow, and clamps the result to a fixed 1 MiB ceiling, so a large or hostile DOM can never force an oversized allocation here. The buffer still grows geometrically past the hint, so an underestimate only costs a few later reallocations; scalars/strings/binary are written in one shot and get no hint. Reserving capacity does not change the bytes produced. Throughput (g++/clang -O3, vs the previous commit): cbor int array +10% / +13% cbor object array +20% / +38% Output is byte-for-byte identical to develop across the binary differential corpus. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me> * 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> * Route the -Wduplicated-branches pragma through Hedley Match #5485, which moved the binary writer's hand-rolled diagnostic pragmas onto JSON_HEDLEY_PRAGMA (merged into develop while this branch was open). The devirtualization's -Wduplicated-branches suppression in write_compact_float was the one raw '#pragma GCC diagnostic' left; it now uses JSON_HEDLEY_PRAGMA like the adjacent -Wfloat-equal line, still guarded to GCC >= 7 and non-clang (the warning exists only there). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAYM1qhSA2FDaDcGfPW3fG Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5409.
Stacked on top of #5475 (
issue-5408-hedley-undef-leak) since both PRs touch some of the same files. Only the last commit on this branch (Route hand-rolled diagnostic pragmas through Hedley) is new; the base commit is #5475's fix. This PR should be reviewed/merged after #5475 (or rebased ontodeveloponce #5475 lands).Problem
Several places in the library hand-roll compiler diagnostic suppression with raw
#pragma/#ifdef __GNUC__/#ifdef __clang__guards, instead of using the Hedley primitives already bundled and used elsewhere (JSON_HEDLEY_LIKELY,WARN_UNUSED_RESULT,DEPRECATED_FOR, etc.).Changes
Converted 6 of the 7 listed push/pop pairs to use
JSON_HEDLEY_DIAGNOSTIC_PUSH/JSON_HEDLEY_DIAGNOSTIC_POPandJSON_HEDLEY_PRAGMA(...)instead of raw pragma text:include/nlohmann/json.hpp(~3770, ~3863):-Wfloat-equalinclude/nlohmann/detail/conversions/to_chars.hpp(~1078):-Wfloat-equalinclude/nlohmann/detail/output/binary_writer.hpp(~1844):-Wfloat-equalinclude/nlohmann/detail/iterators/iteration_proxy.hpp(~211):-Wmismatched-tagsinclude/nlohmann/detail/exceptions.hpp(~36):-Wweak-vtablesiteration_proxy.hppdidn't previously#include <nlohmann/detail/macro_scope.hpp>itself — it only compiled because some earlier header injson.hpp's include list happened to pull it in first. It now includes it directly, like the other detail headers that use Hedley macros, so it's self-contained.Each pair now calls
JSON_HEDLEY_DIAGNOSTIC_PUSH/POPunconditionally (a no-op on compilers that don't need it — that's exactly what the macro is for) and wraps the actual... diagnostic ignored "..."text inJSON_HEDLEY_PRAGMA(...), going through Hedley's_Pragma()-based emission instead of a raw#pragmaline — while keeping the original#ifdef __GNUC__/#if defined(__clang__)guard around the ignored-pragma itself.Deviation from the issue's suggested transformation
The issue's example replaces the
#ifdef __GNUC__guard with#if JSON_HEDLEY_HAS_WARNING("-Wfloat-equal"). I did not adopt that part:JSON_HEDLEY_HAS_WARNINGis implemented purely via Clang's__has_warningbuiltin —— so it evaluates to
0on real GCC (which doesn't define__has_warning). Adopting it verbatim would silently stop suppressing-Wfloat-equalon GCC, which is a real regression, not just a style change (the comment even says the guard exists specifically for GCC's behavior around-Wfloat-equalonNaN/0.0comparisons inconstexpr-friendly code). I kept the existing#ifdef __GNUC__/#if defined(__clang__)guards around the ignored-pragma so the change stays strictly behavior-preserving, and only routed the push/pop/pragma-emission mechanism itself through Hedley, per the issue's actual stated goal ("This must be behavior-preserving").Two locations intentionally left unconverted
The
-Wignored-attributespush at the very top ofjson.hppand its matching pop after#include <nlohmann/detail/macro_unscope.hpp>(the workaround for #5103) were not converted:detail/macro_scope.hpp(and thereforehedley.hpp) has been included anywhere in the translation unit — it's literally the first thing in the file.JSON_HEDLEY_DIAGNOSTIC_PUSHis not yet defined at that point.macro_unscope.hpp, which — viahedley_undef.hpp— has already#undef'd everyJSON_HEDLEY_*macro (by design, so they don't leak to users, see Four JSON_HEDLEY_* macros leak into user code (PRAGMA, PREDICT_TRUE, PREDICT_FALSE, CLANG_HAS_DECLSPEC_ATTRIBUTE) — not undefined by hedley_undef.hpp #5408/Undefine the four JSON_HEDLEY_* macros that leak after including json.hpp #5475).JSON_HEDLEY_DIAGNOSTIC_POPis no longer defined by the time the pop is reached either.Making this one pair route through Hedley would require either hoisting the ~2000-line vendored
hedley.hppto the very top of the amalgamated single header (a much bigger structural change tosingle_includethan a pure mechanism swap) or special-casing this one pop ahead of the general macro cleanup at the end of the file. Both are riskier than the mechanical, behavior-preserving change the issue asks for, so I left this one pair as raw pragmas.Validation
include/nlohmann/json.hppandsingle_include/nlohmann/json.hppwith-Wall -Wextra -Wfloat-equal -Wmismatched-tags -Wweak-vtables(Clang, which also self-identifies as__GNUC__): no warnings, same as before the change.unit-to_chars.cpp,unit-conversions.cpp,unit-iterators1.cpp,unit-iterators2.cpp, andunit-class_parser.cpp: all pass.unit-msgpack.cpp,unit-bjdata.cpp, andunit-ubjson.cpp(which exercisebinary_writer.hpp'swrite_compact_floatextensively): all compile cleanly, and the overwhelming majority of assertions pass (the only failures are pre-existing, unrelated to this change — missing generated test-data files in this offline environment).make amalgamatewas run; thesingle_includediff is limited to exactly the lines touched ininclude/, with no unrelated reordering._Pragma("GCC diagnostic ...")textJSON_HEDLEY_PRAGMAemits is byte-identical to the previous raw#pragma GCC diagnostic ...text, and the surrounding#ifdef __GNUC__guard is unchanged, so GCC's behavior should be identical; CI covers the full GCC matrix.Breaking change?
No breaking changes. This is purely an internal mechanism change (compiler-diagnostic suppression plumbing); no public API, behavior, or emitted warnings change for any user of the library.
— opened by Claude Code on behalf of @nlohmann