Skip to content

Fix GCC C++20 modules compilation #5103 - #5164

Merged
nlohmann merged 5 commits into
nlohmann:developfrom
hariomphulre:fix/issue-5103-build-issue-when-compiling-with-gcc
May 15, 2026
Merged

nlohmann merged 5 commits into
nlohmann:developfrom
hariomphulre:fix/issue-5103-build-issue-when-compiling-with-gcc

Conversation

@hariomphulre

Copy link
Copy Markdown
Contributor

This PR fixes GCC C++20 modules compilation errors (issue #5103) that occur when nlohmann/json.hpp is included in a module preamble after importing other modules.

What Changed

  • include/nlohmann/json.hpp: Added GCC diagnostic pragmas to suppress false-positive warnings in C++20 module preambles
  • src/modules/json.cppm: Added GCC-specific documentation with workarounds; removed export namespace std to avoid symbol conflicts
  • src/modules/CMakeLists.txt: Simplified module structure (removed partition file complexity)

Why Changed

GCC has limitations handling template instantiations in module preambles when other modules are already imported. The pragmas suppress spurious template redefinition errors without affecting actual warnings. Documentation helps users understand the limitation and provides three workarounds.

Testing

  • ✅ All 10,000+ existing tests pass
  • ✅ Module compiles without errors on GCC 15.2.1
  • ✅ Zero regressions detected
  • ✅ Tested with problematic include order (STL headers before json.hpp)

@hariomphulre
hariomphulre requested a review from nlohmann as a code owner May 11, 2026 14:54
@github-actions github-actions Bot added the M label May 11, 2026
@github-actions

Copy link
Copy Markdown

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @hariomphulre
Please read and follow the Contribution Guidelines.

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please amalgamate and check the DCO requirements.

…nn#5103)

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>
@hariomphulre
hariomphulre force-pushed the fix/issue-5103-build-issue-when-compiling-with-gcc branch from 6ec74dc to 9a60ee5 Compare May 11, 2026 17:57
Comment thread include/nlohmann/json.hpp
Comment thread src/modules/json.cppm
@github-actions

Copy link
Copy Markdown

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @hariomphulre
Please read and follow the Contribution Guidelines.

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>
@github-actions github-actions Bot added L and removed M labels May 11, 2026
@github-actions

Copy link
Copy Markdown

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @hariomphulre
Please read and follow the Contribution Guidelines.

…le astyle preprocessor indentation

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>
Signed-off-by: hariomphulre <hariiomphullre@gmail.com>
@github-actions github-actions Bot added the tests label May 12, 2026
@hariomphulre

Copy link
Copy Markdown
Contributor Author

@nlohmann plz approve workflow , i want to see amalgamation check.

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something went wrong during the amalgamation - all files are indented differently now. Please revert these whitespace changes.

Also, please check the DCO requirements.

@hariomphulre

Copy link
Copy Markdown
Contributor Author

Something went wrong during the amalgamation - all files are indented differently now. Please revert these whitespace changes.

Also, please check the DCO requirements.

@nlohmann Actually you can see in changes their is unnecessary indentations. in some files, which cause amalgamation issues. Now the structure is correct.

@nlohmann

Copy link
Copy Markdown
Owner

Please revert the unrelated changes.

@hariomphulre

Copy link
Copy Markdown
Contributor Author

Please revert the unrelated changes.

You are taking about indentation ?

@nlohmann

Copy link
Copy Markdown
Owner

Please revert the unrelated changes.

You are taking about indentation ?

Yes, I mean all whitespace changes in unrelated files.

@hariomphulre
hariomphulre force-pushed the fix/issue-5103-build-issue-when-compiling-with-gcc branch from 0073479 to 87a0c18 Compare May 13, 2026 13:01
@github-actions github-actions Bot removed the tests label May 13, 2026
@hariomphulre
hariomphulre force-pushed the fix/issue-5103-build-issue-when-compiling-with-gcc branch from 87a0c18 to e1f90c0 Compare May 13, 2026 13:10
@hariomphulre

Copy link
Copy Markdown
Contributor Author

@nlohmann again approval pls 🥲

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert the changes to the astyle config.

@hariomphulre
hariomphulre force-pushed the fix/issue-5103-build-issue-when-compiling-with-gcc branch from e1f90c0 to bd18ac2 Compare May 13, 2026 13:28
@hariomphulre

Copy link
Copy Markdown
Contributor Author

Please revert the changes to the astyle config.

@nlohmann Done! now approve workflows

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still hundreds of unrelated files with whitespace changes. Please revert this.

@github-actions

Copy link
Copy Markdown

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @hariomphulre
Please read and follow the Contribution Guidelines.

@hariomphulre
hariomphulre force-pushed the fix/issue-5103-build-issue-when-compiling-with-gcc branch from bd18ac2 to 285f2ab Compare May 13, 2026 14:45
@hariomphulre

hariomphulre commented May 13, 2026 •

Copy link
Copy Markdown
Contributor Author

There are still hundreds of unrelated files with whitespace changes. Please revert this.

But i revert my all "white space" changes.
@nlohmann approve workflows 🙂

@github-actions github-actions Bot added the tests label May 13, 2026
@nlohmann

Copy link
Copy Markdown
Owner
image

I mean these changes - they are unrelated to your PR.

@github-actions

Copy link
Copy Markdown

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @hariomphulre
Please read and follow the Contribution Guidelines.

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>
@hariomphulre
hariomphulre force-pushed the fix/issue-5103-build-issue-when-compiling-with-gcc branch from 285f2ab to 4015c54 Compare May 14, 2026 14:43
@github-actions github-actions Bot added M and removed L tests labels May 14, 2026
@hariomphulre

hariomphulre commented May 14, 2026 •

Copy link
Copy Markdown
Contributor Author
image I mean these changes - they are unrelated to your PR.

@nlohmann Now i revert my all whitespaces chnages, now pls approve workflows fast.
I dont know why Amalgamation check fails, althought i amalgamte and run tests properly.

Lets see now

@hariomphulre

Copy link
Copy Markdown
Contributor Author

🙃 @nlohmann whats happened with appveyor , its taking too much time

@hariomphulre
hariomphulre requested a review from nlohmann May 14, 2026 23:11
@hariomphulre

Copy link
Copy Markdown
Contributor Author

@nlohmann , review and merge plz. I am waiting

@nlohmann nlohmann modified the milestone: 5103 May 15, 2026
@hariomphulre

Copy link
Copy Markdown
Contributor Author

Their is any thing else, to do with this PR from my side ?

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann added this to the Release 3.12.1 milestone May 15, 2026
@nlohmann
nlohmann merged commit bb5404b into nlohmann:develop May 15, 2026
143 checks passed
@nlohmann

Copy link
Copy Markdown
Owner

Thanks!

@hariomphulre

Copy link
Copy Markdown
Contributor Author

Thanks!

Lets move forward to other issue.

If you want to assign me any issue which is more important to resolve first, then you can tell me!

GhostVaibhav pushed a commit to GhostVaibhav/json that referenced this pull request May 19, 2026
* Fix: Add GCC diagnostic pragmas for C++ modules support (issue nlohmann#5103)

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Fix: GCC C++20 modules with __cplusplus >= 202002L check (nlohmann#5103)

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Fix: Remove indentation from nested preprocessor directives and disable astyle preprocessor indentation

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Update amalgamated files with correct preprocessor directive indentation

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Fix GCC build for issue nlohmann#5103; refresh amalgamated header

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

---------

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>
Signed-off-by: Vaibhav Sharma <48472541+GhostVaibhav@users.noreply.github.com>
jwnimmer-tri pushed a commit to jwnimmer-tri/json that referenced this pull request Sep 21, 2026
* Fix: Add GCC diagnostic pragmas for C++ modules support (issue nlohmann#5103)

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Fix: GCC C++20 modules with __cplusplus >= 202002L check (nlohmann#5103)

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Fix: Remove indentation from nested preprocessor directives and disable astyle preprocessor indentation

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Update amalgamated files with correct preprocessor directive indentation

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

* Fix GCC build for issue nlohmann#5103; refresh amalgamated header

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>

---------

Signed-off-by: hariomphulre <hariiomphullre@gmail.com>
nlohmann added a commit that referenced this pull request Sep 30, 2026
The pragma (added in #5164) claimed to work around the C++ modules
redefinition errors of #5103, but #5103 is about hard errors (e.g.
"redefinition of std::__is_constant_evaluated()", conflicting
std::integral_constant) that ignoring a warning cannot suppress; they
are traced to GCC PR 124430 and reproduce with <map> or <string>
instead of json.hpp too. A GCC 16.2 -std=gnu++20 -fmodules build
following #5103's repro steps still fails with the pragma in place,
and a build of all test TUs with GCC_CXXFLAGS (which enable
-Wignored-attributes) and the pragma removed produces no such
warning. The block only hid a warning class from GCC C++20 users
while suggesting #5103 was handled.

Overlaps #5610, whose hunks touch the closing half of this pragma to
insert the json_literals.hpp include.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5725 item 9
nlohmann added a commit that referenced this pull request Oct 1, 2026
…5737)

* Drop stale LCOV_EXCL_LINE from the json_pointer out_of_range.410 throw

The comment said the size_type overflow check in array_index() is only
triggered on special platforms like 32-bit, and the throw was excluded
from coverage. On 64-bit platforms the check is true for SIZE_MAX
itself, and unit-json_pointer.cpp has asserted that case four times
since #5395, so the line is executed in the coverage job. Reword the
comment and remove the exclusion marker so the coverage report notices
if the tests stop reaching it.

Part of #5725

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Name all three C-array check aliases in the enum-macro NOLINTs

NLOHMANN_JSON_SERIALIZE_ENUM(_STRICT) suppressed the c-array warning
under modernize-avoid-c-arrays only, but clang-tidy emits the same
diagnostic under the aliases cppcoreguidelines-avoid-c-arrays and
hicpp-avoid-c-arrays too. Any user running those checks got a false
positive at every macro expansion, and our own tests needed a local
NOLINT at each call site to work around it.

Name all three aliases in the four macro comments instead, and drop
the now-redundant c-array names from the five test call-site NOLINTs.
Comment-only change; behavior, the public API, and the ABI do not
change. Ran make amalgamate.

Part of #5725

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Include doctest as a SYSTEM directory instead of disabling warnings for all tests

test_main added -Wno-deprecated and -Wno-float-equal as PUBLIC compile
options for every non-MSVC compiler, so they were applied to every
translation unit, library headers included, and silenced the CI
warnings meant to check the library's own -Wfloat-equal pragmas. The
only code that actually needed the suppression was the vendored
doctest.h, which was included as a normal (non-SYSTEM) directory.

Include thirdparty/doctest as SYSTEM for test_main, matching what
tests/abi/CMakeLists.txt already does, and drop the two suppressions
from both targets. Verified locally that unit-comparison,
unit-conversions and unit-constructor1 compile clean with
-Werror -Weverything and doctest as -isystem, and that CMake still
configures with JSON_BuildTests=ON.

Part of #5725

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Remove the no-op ci_clang_analyze target

ci_clang_analyze configured the build with the real compiler and only
then wrapped ninja with scan-build. scan-build intercepts compiles by
overriding CC/CXX, but build.ninja already had the compiler path baked
in from the configure step, so every run bypassed the analyzer: CI
logs show "No bugs found" after a normal build, never an analysis.
The job also used Debian's frozen clang-tools-14 rather than the
image's own clang, and CLANG_ANALYZER_CHECKS still named three
valist.* checkers that current clang merged into security.VAList.

ci_clang_tidy already runs every clang-analyzer-* check (via
.clang-tidy's "Checks: '*'") with warnings as errors, so nothing is
lost by removing the dead job. Delete ci_clang_analyze,
CLANG_ANALYZER_CHECKS and the SCAN_BUILD_TOOL lookup from
cmake/ci.cmake, drop it from the ubuntu.yml ci_static_analysis_clang
matrix, and drop the now-unused clang-tools apt package (iwyu stays
for ci_single_binaries). Reword quality_assurance.md and
assurance_case.md, which described the dead job as a working control,
to say the Clang Static Analyzer checks run through clang-tidy.

Verified that `cmake -DJSON_CI=ON` still configures cleanly and that
ci_clang_analyze no longer appears in the generated build or in any
CMake/workflow file.

Part of #5725

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Re-enable portability-template-virtual-member-function; remove redundant forwards

.clang-tidy disabled three checks "to get the CI going" (#4489,
2024-11-13): portability-template-virtual-member-function,
bugprone-use-after-move and its alias hicpp-invalid-access-moved.

portability-template-virtual-member-function only flagged
output_stream_adapter::write_character/write_characters; annotate
both with NOLINT and re-enable the check.

bugprone-use-after-move flagged several double forwards that have no
effect at runtime:
- from_json.hpp calls std::forward<BasicJsonType>(j).at(Idx) inside
  pack expansions; at() has no ref-qualified overloads and always
  returns an lvalue reference, so the forward is a no-op. Replace with
  plain j.at(Idx) in all four places.
- the move constructor forwards the whole object to its base class
  and then reads other's members. That is item 9 of #5724 (together
  with its cppcheck suppressions) and is left to that change.
- input_adapters.hpp forwards the container twice on purpose, so the
  begin/end iterator types match adapter_type; annotate with NOLINT
  and a comment instead of changing behavior.

The check still flags the move constructor (see above) and two sites
in at(KeyType&&) (both overloads, json.hpp, in the throw's
string_t(std::forward<KeyType>(key)) after
find(std::forward<KeyType>(key))). Open PR #5689 rewrites that hunk,
so bugprone-use-after-move (and hicpp-invalid-access-moved)
stay disabled for now, with a comment explaining why; re-enable them
once #5689 and the #5724 move-constructor change have landed.

Also resolve the portability-avoid-pragma-once TODO: single_include
never has #pragma once (amalgamate.py strips it) and every supported
compiler accepts it in include/, so keep it disabled with an
explanatory comment instead of a TODO. Fix the stale "json.hpp,
around line 1265" comment in unit-class_parser.cpp, which now points
at the move constructor's actual line.

Behavior, the public API and the ABI do not change. Verified with
clang-tidy 22.1.8 that portability-template-virtual-member-function
now reports nothing, that bugprone-use-after-move/
hicpp-invalid-access-moved report only the known at(KeyType&&) and
move-constructor sites, and that unit-custom-base-class, unit-constructor1,
unit-conversions, unit-element_access2, unit-class_parser and
unit-diagnostic-positions (JSON_DIAGNOSTIC_POSITIONS=1) compile
under ASan/UBSan and pass with the same assertion counts as before.
Ran make amalgamate.

Part of #5725

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Fix stale and malformed NOLINT comments

json_sax.hpp named "-warnings-as-errors" in the NOLINT list on the two
JSON_ASSERT(false) lines; that is the suffix clang-tidy appends to a
diagnostic tag under WarningsAsErrors, not a check name, and every
other JSON_ASSERT(false) omits it.

unit-capacity.cpp carried 30 "// NOLINT(misc-const-correctness)"
comments on "json j = ...;" declarations that are all used with
non-const members afterwards, so the check has nothing to report
there.

unit-constructor2.cpp used a blanket "// NOLINT: access after move is
OK here" on a use-after-move that hides every check on the line;
naming bugprone-use-after-move and hicpp-invalid-access-moved keeps
the intent once those checks are re-enabled (#5724).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5725 item 10

* Remove stale .clang-tidy entries

-google-runtime-references disabled a check that neither clang-tidy
22.1.8 nor 23.1.2 lists under --list-checks -checks='*'; it was
removed upstream. The commented-out HeaderFilterRegex line has been
unused since the active HeaderFilterRegex was introduced in #2561
(2021).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5725 item 11

* Remove the GCC C++20 -Wignored-attributes pragma in json.hpp

The pragma (added in #5164) claimed to work around the C++ modules
redefinition errors of #5103, but #5103 is about hard errors (e.g.
"redefinition of std::__is_constant_evaluated()", conflicting
std::integral_constant) that ignoring a warning cannot suppress; they
are traced to GCC PR 124430 and reproduce with <map> or <string>
instead of json.hpp too. A GCC 16.2 -std=gnu++20 -fmodules build
following #5103's repro steps still fails with the pragma in place,
and a build of all test TUs with GCC_CXXFLAGS (which enable
-Wignored-attributes) and the pragma removed produces no such
warning. The block only hid a warning class from GCC C++20 users
while suggesting #5103 was handled.

Overlaps #5610, whose hunks touch the closing half of this pragma to
insert the json_literals.hpp include.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5725 item 9

* Fix stale doxygen comments hidden by the -Wdocumentation pragma

macro_scope.hpp ignores -Wdocumentation and -Wdocumentation-unknown-command
for the whole library, which also hides genuine documentation mistakes:

- detail::unescape() documented "@return unescaped string" but returns
  void and unescapes its argument in place; reworded to
  "@PARAM[in,out] s string to unescape in place" and dropped the
  bogus @return.
- basic_json::get()'s copy-conversion overload wrote "converted to
  @tparam ValueType" inside @return, which Doxygen and Clang parse as
  a second, malformed @tparam; changed to "@A ValueType", matching the
  two other get() overloads a few lines above that already use it.

This narrows the gap the -Wdocumentation pragma needs to cover; fully
replacing the Doxygen-only commands it also hides (item 2c) is left
for after #5267.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5725 item 2

* Fix -Wextra-semi-stmt at its actual source, not assert()

clang_flags.cmake blamed the global -Wno-extra-semi-stmt on assert(),
but assert() expands to an expression under glibc and libc++ and does
not trigger this warning. unit-assert_macro.cpp overrides JSON_ASSERT
with "{if (!(x)) ++assert_counter; }", a bare block followed by a
semicolon at every JSON_ASSERT(...) call site in the library; that
was the actual source of 151 of the 208 -Wextra-semi-stmt sites found
in a Clang 22 -Weverything sweep of the test suite with the flag
removed. Switched to the standard do/while(false) macro idiom, which
does not expand to a statement-plus-semicolon, and corrected the
comment to name the remaining source instead: vendored Doctest's
CAPTURE(x) shim, which already ends in a semicolon.

Verified with clang++ -Wextra-semi-stmt (plus the file's other CI
ignores) that unit-assert_macro.cpp now compiles without any
-Wextra-semi-stmt diagnostic.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5725 item 8 (step 1 of 2; step 2 covers the CAPTURE() call sites)

* Drop the redundant semicolon from CAPTURE() call sites; remove -Wno-extra-semi-stmt

doctest_compatibility.h defines CAPTURE(x) as DOCTEST_CAPTURE(x); (with
a trailing semicolon baked into the macro), specifically so call sites
do not need to add one themselves; most of the ~267 call sites already
follow that convention. The remaining 64 call sites across 20 files
wrote "CAPTURE(x);" anyway, turning into a statement plus an empty
statement and triggering -Wextra-semi-stmt. Dropped the redundant
semicolon at each of those sites.

With item 6 having already made vendored Doctest a SYSTEM include, and
this the last known source of -Wextra-semi-stmt findings, removed the
flag from clang_flags.cmake entirely.

Verified with clang++ -Wextra-semi-stmt (plus the file's other CI
ignores) that all 20 touched files, plus a file with no CAPTURE() use
(unit-json_pointer.cpp), compile without any -Wextra-semi-stmt
diagnostic.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5725 item 8 (step 2 of 2)

* Switch ci_static_analysis_clang off the frozen LLVM 22 dev image

ubuntu.yml pinned the clang-tidy/clang-tidy-sanitizer/single-binaries
job to silkeh/clang:dev, a tag last pushed 2026-02-18 that reports
"clang version 22.0.0 (...+20251015...)", a pre-release snapshot from
before the LLVM 22 release; the maintainer now updates dev-unstable,
22, and latest instead. Switched to silkeh/clang:22, matching the
other clang jobs on :latest.

Verified with clang-tidy 22.1.8 (the image's actual version) against
this repository's .clang-tidy and library headers what the release
image newly reports compared to :dev:

- readability-redundant-typename fires at ~250 sites across the
  _cpp20-relevant conversion/to_chars headers; the library targets
  C++11 and keeps the typenames, so the check is disabled in
  .clang-tidy, matching how the file already handles checks that
  don't fit a C++11 codebase.
- misc-anonymous-namespace-in-header fires on the two anonymous
  namespaces in from_json.hpp and to_json.hpp; added the alias to
  their existing NOLINT (cert-dcl59-cpp, fuchsia-header-anon-namespaces,
  google-build-namespaces).
- bugprone-std-namespace-modification fires on every addition to
  namespace std: the std::hash, std::formatter and std::swap
  overloads in json.hpp, and the std::tuple_size/std::tuple_element
  specializations in iteration_proxy.hpp (this last file is not named
  in #5725's item 5, found by actually running clang-tidy 22.1.8
  against the current tree). All six are legal, deliberate additions
  to namespace std (explicit/partial specializations of std types, or
  the pre-C++20 std::swap overload); annotated each with the check
  name next to its existing cert-dcl58-cpp NOLINT.
- modernize-avoid-c-style-cast reported nothing new.

Also added clang++-22/21, clang-tidy-22/21, g++-16 and gcov-16 to the
find_program search lists in ci.cmake so a local "maximal warnings"
configure prefers the current toolchain version over an older one on
PATH.

#5725 item 5

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Regenerate cmake/gcc_flags.cmake for GCC 16.2.0

GCC_CXXFLAGS was generated for GCC 15.1.0, but ci_test_gcc and
ci_test_gcc_cxx{11..26} now run in gcc:latest, currently GCC 16.2.0,
so the "maximal warnings" job was missing warnings introduced since
15.1.0 while carrying entries GCC 16 treats as duplicates or no-ops.

Regenerated with https://github.com/nlohmann/gcc_flags (patched
locally to not crash on an option whose "-x c++ <opt> -" probe fails
before it reads stdin, e.g. -Wabi=; the tool otherwise raises
BrokenPipeError instead of recording the option as an error) run
against g++ 16.2.0 in the official gcc:16 Docker image, keeping the
documented -Wno-* exclusions and the same alphabetical placement
scheme as before.

Also added three GCC 16 warnings the generator cannot discover on its
own because it only probes value ranges/lists it finds in the -Q
option name itself, not in the enum choices --help=warnings documents
separately:

- -Wbidi-chars=any, -Wleading-whitespace=spaces: manually verified
  these compile cleanly with g++ 16.2.0.
- -Wstrict-flex-arrays: deliberately NOT added, unlike the other two.
  Without -fstrict-flex-arrays (which the library does not enable, as
  it would change codegen for flexible array members), GCC prints
  "'-Wstrict-flex-arrays' is ignored when '-fstrict-flex-arrays' is
  not present" on every translation unit, and under our -Werror that
  note itself aborts the build. This differs from the harmless
  no-op warnings already kept in the file (-Whsa, -Wsynth,
  -Wunreachable-code, -Wunsafe-loop-optimizations), which emit
  nothing; #5725 item 7 named -Wstrict-flex-arrays as one of the
  flags GCC 16 adds, but did not anticipate this failure mode.

Verified: compiled the library header and a representative set of
test translation units (including ones touched by items 1, 3, 8, 9,
10 of this issue) with the regenerated GCC_CXXFLAGS plus -Werror
under g++ 16.2.0 at -std=c++11 through -std=c++26, with zero warnings;
ran the full local test suite (129/129 passing, unrelated to this
compiler) as a regression check. CI must still confirm the actual
ci_test_gcc / ci_test_standards_gcc targets end to end, since this was
verified with direct g++ invocations rather than through the CMake/
CXXFLAGS environment-variable plumbing in ci.cmake.

#5725 item 7

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Avoid std::basic_string<CharType> for non-character output_adapter CharType

output_adapter<CharType, StringType> defaulted StringType to
std::basic_string<CharType>, and (with JSON_NO_IO undefined) always
declared a std::basic_ostream<CharType>&-taking constructor. For
CharType with no non-deprecated std::char_traits specialization (only
std::uint8_t is ever used this way, by the binary writers), simply
naming either type - as an unused default template argument, or as an
unused, never-called constructor's parameter type - instantiates
std::char_traits<CharType> merely to name it, which some standard
libraries mark deprecated: with the library-wide -Wdocumentation
pragma (item 2's other half, left for a later commit) temporarily
removed, an Apple clang 21 / libc++ TU calling json::to_cbor(j, vec)
with std::vector<std::uint8_t>& got one -Wdeprecated-declarations
warning per binary writer at the old output_adapters.hpp:193.

Replaced the eager std::basic_string<CharType> / std::basic_ostream
<CharType> defaults with a bool-tagged partial specialization (not
std::conditional, which requires naming both branches' types up
front regardless of which is selected, reproducing the same warning)
that only ever names std::basic_string<CharType> / std::basic_ostream
<CharType> when CharType is actually one of char, wchar_t, char16_t,
char32_t, or (with __cpp_lib_char8_t) char8_t. For any other
CharType, output_adapter's StringType and ostream-constructor
parameter fall back to two distinct empty placeholder types, kept
distinct so the two constructor overloads do not collide into a
single redeclaration.

Public API / behavior: passing a std::basic_string<std::uint8_t>& or
std::basic_ostream<std::uint8_t>& directly to a binary writer's
output_adapter now fails to compile instead of compiling with a
deprecation warning; this was neither documented nor tested. All
documented uses (std::vector<CharType>, std::basic_ostream<CharType>
and StringType for character CharType) are unaffected.

Verified with Apple clang 21 / libc++, with the two -Wdocumentation*
"ignored" pragma lines in macro_scope.hpp temporarily removed and
-std=c++11/c++20 plus the project's -Weverything flag set: calling
to_cbor/to_msgpack/to_ubjson/to_bjdata/to_bson/to_bon8 on a
std::vector<std::uint8_t> now produces no char_traits<unsigned char>
(or any other) deprecation warning, while the char-based string- and
ostream-adapter paths, and a to_cbor/from_cbor round trip, still
compile and run correctly; also verified with GCC 16.2.0. Ran the
full local test suite, including the binary-format unit tests
(unit-cbor, unit-msgpack, unit-ubjson, unit-bjdata, unit-bson,
unit-bon8, unit-binary_writer_sinks, unit-binary_formats,
unit-custom-binary-type): 129/129 passing.

#5725 item 2 (step a)

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Remove the library-wide -Wdocumentation pragma; fix what it hid

macro_scope.hpp / macro_unscope.hpp pushed and popped a Clang
diagnostic region over the entire library that ignored -Wdocumentation
and -Wdocumentation-unknown-command. Removed both pragmas and fixed
every finding a full -Wdocumentation (which implies
-Wdocumentation-unknown-command and -Wdocumentation-deprecated-sync)
build reports, so the library now compiles clean under Clang's
documentation checks without a blanket suppression. Overlaps #5267,
which is still open and edits a nearby doc block (json.hpp's
get()/get_impl() @return, already fixed in the item 2 step (b) commit
of this branch); this commit does not touch that block again.

Unknown Doxygen alias commands (Doxyfile removed in #3071, so these
were never rendered by anything) rewritten as plain prose, keeping the
same information:
- @requirement REQ-JSON-01 / REQ-JSON-02 (iter_impl.hpp,
  json_reverse_iterator.hpp): now "This class satisfies the following
  concept requirements (REQ-JSON-0N):".
- @liveexample{prose,example-id} (three sites in json.hpp): kept the
  prose, dropped the command wrapper and the trailing example-id
  (docs/mkdocs/docs/examples/*.cpp still exist and are used directly
  by the rendered docs, not through this in-header alias) and
  unescaped the "\," commas that were only needed for the old alias's
  comma-separated argument syntax.
- @Complexity X (json.hpp x4, json_pointer.hpp x2, serializer.hpp x1):
  now "Complexity: X".

Backslash sequences Clang's comment lexer tried to parse as commands,
escaped to render as literal backslashes:
- lexer.hpp get_codepoint(): two `\u` occurrences.
- binary_reader.hpp get_bson_cstr() / get_bson_cstr_bulk(): two
  `\x00` occurrences.
- serializer.hpp: three `\uXXXX` occurrences (constructor @PARAM,
  append_codepoint_to_string_buffer() @brief, and the ensure_ascii
  member comment).

One finding remained after all of the above: Clang reports
"declaration is marked with '@deprecated' command but does not have a
deprecation attribute" on the deprecated sax_parse(span_input_adapter&&, ...)
overload, even though JSON_HEDLEY_DEPRECATED_FOR does expand to
__attribute__((deprecated(...))) for Clang. Several isolated
reproductions of this exact declaration shape - doc comment,
template<>, two stacked __attribute__ macros, an overload set sharing
the name - did not reproduce the warning, so this looks like a
Clang comment/declaration-association quirk specific to this overload
inside the much larger basic_json class template, not an actual
documentation defect. Rather than keep the pragma library-wide for one
Clang false positive, added a tightly scoped
-Wdocumentation-deprecated-sync push/pop around just that overload.

Verified with Apple clang 21 and the project's actual -Weverything
flag set (cmake/clang_flags.cmake) on the full header at -std=c++11
and -std=c++20: zero -Wdocumentation* diagnostics. Also compiled
clean with GCC 16.2.0 (the pragmas are already __clang__-gated, so
this only confirms no unrelated breakage). Ran make check-amalgamation
and the full local test suite: 129/129 passing.

#5725 item 2 (step c)

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Take the JSON value by const reference in the array and tuple from_json paths

Review feedback on #5737 (gregmarr): once the no-op std::forward calls
are gone, the forwarding references have no purpose. from_json_fn
passes the value as const BasicJsonType&, so these functions were only
ever instantiated with a const lvalue anyway.

The std::array, std::pair and std::tuple overloads of from_json and
their helpers now take const BasicJsonType& and pass j on unchanged.
Because the deduced BasicJsonType is now the plain type, tuple_type and
the static_assert name const BasicJsonType& explicitly, so the
reference checks are unchanged: get<std::tuple<const std::string&>>()
still works, and get<std::tuple<std::string&>>() still fails the same
static_assert. from_json_tuple_get_impl keeps its forwarding reference,
since tuple_type calls it through std::declval.

Behavior, the public API and the ABI do not change. unit-conversions,
unit-constructor1, unit-udt, unit-udt_macro, unit-regression1/2/3,
unit-deserialization, unit-noexcept, unit-items, unit-allocator,
unit-custom-object-type, unit-ordered_json2 and
unit-brace-init-copy-semantics pass at C++11, C++17 and C++20 with
unchanged assertion counts. Ran make amalgamate.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build issue when compiling with gcc (any modules-capable version) in a C++ modules project

2 participants