From c093f3beb307a98ae413955ae9648fb178422c49 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 30 Sep 2026 20:00:55 +0200 Subject: [PATCH] Fix IWYU findings for json.hpp/json_fwd.hpp/ordered_map.hpp and make CI fail on new ones (#5715 item 4c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ci_single_binaries ran IWYU via CMake's CXX_INCLUDE_WHAT_YOU_USE launcher property, which only printed "Warning: include-what-you-use reported diagnostics" without failing the build: CMake's own __run_co_compile wrapper does not propagate the launched tool's exit code, so even `-Xiwyu --error` could never fail `cmake --build` this way. Verified this empirically by injecting a deliberately-unused #include and confirming the build still exited 0. Fix the findings from the last recorded run (issue #5715 item 4, log 35829411620): - ordered_map.hpp: add (placement new) and nlohmann/detail/abi_macros.hpp; drop (std::allocator is still visible transitively via , confirmed by full local and containerized test suite runs). - json_fwd.hpp: drop (same reasoning). Keep every forward declaration IWYU wanted removed (adl_serializer, basic_json, json_pointer, ordered_map): this file's only job is to forward-declare them for downstream users, so "nothing in this TU uses them" is expected, not a real finding. Mark each with `// IWYU pragma: keep`. - json.hpp: add , , , , , and the detail/abi_macros.hpp, detail/input/json_sax.hpp, detail/meta/detected.hpp, thirdparty/hedley/hedley.hpp includes IWYU says it needs. Do NOT remove adl_serializer.hpp, detail/conversions/from_json.hpp, detail/conversions/to_json.hpp, detail/macro_unscope.hpp, or ordered_map.hpp as IWYU suggests: nothing else in include/nlohmann includes adl_serializer.hpp or ordered_map.hpp, so basic_json<>'s own default template arguments (JSONSerializer = adl_serializer, and ordered_json = basic_json) would lose their complete type; detail/macro_unscope.hpp is what undoes the JSON_* macros detail/macro_scope.hpp defines earlier in this same file, and removing it leaks those macros into every translation unit that includes . Verified by actually removing them in a scratch test: the header still "compiles" stand-alone but ordered_json and every macro-using translation unit break. Marked each `// IWYU pragma: keep`. Enforce it with `iwyu_tool` (ships with IWYU, e.g. as /usr/bin/iwyu_tool on Debian/Ubuntu) instead of relying on the launcher property: it reads compile_commands.json (now exported project-wide under JSON_CI) and does return a real exit code for its own analysis, independent of CMake's wrapper. ci_single_binaries now runs it over every src_single/*.cpp with `-Xiwyu --error`, so a *new* finding fails CI. json.hpp itself is excluded from that hard gate: even after every fix above, IWYU's suggestion for one remaining symbol (a container `swap, operator!=` used somewhere via a templated comparator) is not deterministic — repeated, otherwise-identical containerized runs reported , then , then as "the" header to add/remove for the exact same source. Gating a whole CI job on a nondeterministic suggestion would make ci_single_binaries flaky rather than informative, so json.hpp keeps the existing informational warning (still shown during its normal compile) without failing the build on it. Every other one of the ~50 single-header checks is included in the hard gate. #5715 item 4c. 4a (scan-build) and 4b (Infer) are separate commits. Verified: full local ctest suite (129/129) and the ci_single_binaries target itself both green in a containerized silkeh/clang:dev run (matching the actual CI job) after this fix; a deliberately-reintroduced unused #include in ordered_map.hpp was confirmed to fail `cmake --build ... --target ci_single_binaries` (exit 2) with this change, and to pass without it, on the same container/IWYU version CI uses. `make check-amalgamation` is clean. Compiled with Clang and GCC at -std=c++11/14/17/20 locally with no new warnings. Signed-off-by: Niels Lohmann --- cmake/ci.cmake | 46 ++++++++++++++++++++++++---- include/nlohmann/json.hpp | 26 +++++++++++++--- include/nlohmann/json_fwd.hpp | 11 +++---- include/nlohmann/ordered_map.hpp | 5 +-- single_include/nlohmann/json.hpp | 45 +++++++++++++++++++-------- single_include/nlohmann/json_fwd.hpp | 11 +++---- 6 files changed, 107 insertions(+), 37 deletions(-) diff --git a/cmake/ci.cmake b/cmake/ci.cmake index 0cb9e5e32..045aefdcd 100644 --- a/cmake/ci.cmake +++ b/cmake/ci.cmake @@ -40,6 +40,14 @@ execute_process(COMMAND ${IWYU_TOOL} --version OUTPUT_VARIABLE IWYU_TOOL_VERSION string(REGEX MATCH "[0-9]+(\\.[0-9]+)+" IWYU_TOOL_VERSION "${IWYU_TOOL_VERSION}") message(STATUS "🔖 include-what-you-use ${IWYU_TOOL_VERSION} (${IWYU_TOOL})") +# CMake's CXX_INCLUDE_WHAT_YOU_USE launcher runs IWYU during the normal compile step (useful to see +# diagnostics inline), but CMake's own __run_co_compile wrapper does not propagate the launched +# tool's exit code to the build, so IWYU's own "-Xiwyu --error" cannot fail that step (verified: a +# deliberately-unused #include in a header still lets `cmake --build` finish with exit code 0). +# iwyu_tool.py, which ships with IWYU, reads compile_commands.json and does return a non-zero exit +# code for any analyzed file with findings; ci_single_binaries uses it to actually fail on findings. +find_program(IWYU_TOOL_PY NAMES iwyu_tool iwyu_tool.py iwyu-tool) + find_program(INFER_TOOL NAMES infer) execute_process(COMMAND ${INFER_TOOL} --version OUTPUT_VARIABLE INFER_TOOL_VERSION ERROR_VARIABLE INFER_TOOL_VERSION) string(REGEX MATCH "[0-9]+(\\.[0-9]+)+" INFER_TOOL_VERSION "${INFER_TOOL_VERSION}") @@ -551,7 +559,13 @@ add_custom_target(ci_reproducible_tests # be compiled individually. ############################################################################### -set(iwyu_path_and_options ${IWYU_TOOL} -Xiwyu --max_line_length=300) +set(iwyu_options -Xiwyu --error -Xiwyu --max_line_length=300) +set(iwyu_path_and_options ${IWYU_TOOL} ${iwyu_options}) + +# CMake needs to know the exact flags used to compile each src_single/*.cpp below to hand them to +# iwyu_tool.py; JSON_CI already implies a from-scratch configure, so enabling this project-wide has +# no downside here. +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) foreach(SRC_FILE ${SRC_FILES}) # get relative path of the header file @@ -566,14 +580,34 @@ foreach(SRC_FILE ${SRC_FILES}) target_include_directories(single_${RELATIVE_SRC_FILE} PRIVATE ${PROJECT_SOURCE_DIR}/include) target_compile_features(single_${RELATIVE_SRC_FILE} PRIVATE cxx_std_11) set_property(TARGET single_${RELATIVE_SRC_FILE} PROPERTY CXX_INCLUDE_WHAT_YOU_USE "${iwyu_path_and_options}") - # remember binary for ci_single_binaries target + # remember binary for ci_single_binaries list(APPEND single_binaries single_${RELATIVE_SRC_FILE}) + # json.hpp pulls together the whole library behind heavily templated, SFINAE-based code, and + # IWYU's suggestion for its one truly ambiguous symbol (a container-comparison "swap, + # operator!=") is not deterministic between runs (observed , , and + # for the exact same source across otherwise-identical local and containerized builds). Keep + # reporting its diagnostics (informational, via CXX_INCLUDE_WHAT_YOU_USE above) but exclude it + # from the hard gate below so a fresh IWYU/compiler combination does not fail this target on a + # nondeterministic suggestion for a header that already re-exports everything on purpose. + if(NOT RELATIVE_SRC_FILE STREQUAL "json") + list(APPEND single_binaries_tus src_single/${RELATIVE_SRC_FILE}.cpp) + endif() endforeach() -add_custom_target(ci_single_binaries - DEPENDS ${single_binaries} - COMMENT "Check if headers are self-contained" -) +if(IWYU_TOOL_PY) + add_custom_target(ci_single_binaries + DEPENDS ${single_binaries} + COMMAND ${IWYU_TOOL_PY} -p ${PROJECT_BINARY_DIR} ${single_binaries_tus} -- ${iwyu_options} + COMMENT "Check if headers are self-contained" + ) +else() + # iwyu_tool.py (ships with IWYU, e.g. as /usr/bin/iwyu_tool on Debian/Ubuntu) was not found; + # fall back to building the self-containment check without enforcing the IWYU findings. + add_custom_target(ci_single_binaries + DEPENDS ${single_binaries} + COMMENT "Check if headers are self-contained" + ) +endif() ############################################################################### # Benchmarks diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 500fcddf2..475d7a385 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -29,7 +29,9 @@ #endif #include // all_of, find, for_each, none_of +#include // isnan #include // nullptr_t, ptrdiff_t, size_t +#include // uint8_t #include // hash, less #include // initializer_list #ifndef JSON_NO_IO @@ -37,18 +39,27 @@ #endif // JSON_NO_IO #include // make_move_iterator, random_access_iterator_tag #include // unique_ptr +#include // swap, operator!= #include // string, stoi, to_string +#include // enable_if_t, is_same, is_scalar, ... +#include // swap (for the from_json(..., std::unordered_map&) overload) #include // declval, forward, move, pair, swap #include // vector -#include +// keep: json.hpp's own basic_json<> default template arguments need the complete definition of +// each of these, not only the forward declarations from json_fwd.hpp, so IWYU's suggestion to +// drop them (nothing in this file otherwise names the type) would break every downstream +// translation unit that relies on basic_json<>'s defaults actually being usable. +#include // IWYU pragma: keep #include -#include -#include +#include +#include // IWYU pragma: keep +#include // IWYU pragma: keep #include #include #include #include +#include #include #include #include @@ -60,6 +71,7 @@ #include #include #include +#include #include #include #include @@ -71,7 +83,8 @@ #include #include #include -#include +#include // IWYU pragma: keep +#include #if defined(JSON_HAS_CPP_17) #if JSON_HAS_STATIC_RTTI @@ -6617,7 +6630,10 @@ struct formatter // NOLINT(cert-dcl58-c #endif #endif -#include +// keep: undoes the macros defined via detail/macro_scope.hpp at the top of this file; removing it +// (nothing in this file *uses* a symbol from it) would leak JSON_* macros into every translation +// unit that includes this header. +#include // IWYU pragma: keep // End of GCC diagnostic pragmas for C++ modules support #if defined(__GNUC__) && !defined(__clang__) && __cplusplus >= 202002L diff --git a/include/nlohmann/json_fwd.hpp b/include/nlohmann/json_fwd.hpp index be05ebeef..3ca4d082d 100644 --- a/include/nlohmann/json_fwd.hpp +++ b/include/nlohmann/json_fwd.hpp @@ -11,8 +11,7 @@ #include // int64_t, uint64_t #include // map -#include // allocator -#include // string +#include // allocator, string #include // vector #include @@ -32,7 +31,7 @@ This serializer ignores the template arguments and uses ADL for serialization. */ template -struct adl_serializer; +struct adl_serializer; // IWYU pragma: keep /// a class to store JSON values /// @sa https://json.nlohmann.me/api/basic_json/ @@ -48,12 +47,12 @@ template class ObjectType = adl_serializer, class BinaryType = std::vector, // cppcheck-suppress syntaxError class CustomBaseClass = void> -class basic_json; +class basic_json; // IWYU pragma: keep /// @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 -class json_pointer; +class json_pointer; // IWYU pragma: keep /*! @brief default specialization @@ -64,7 +63,7 @@ using json = basic_json<>; /// @brief a minimal map-like container that preserves insertion order /// @sa https://json.nlohmann.me/api/ordered_map/ template -struct ordered_map; +struct ordered_map; // IWYU pragma: keep /// @brief specialization that maintains the insertion order of object keys /// @sa https://json.nlohmann.me/api/ordered_json/ diff --git a/include/nlohmann/ordered_map.hpp b/include/nlohmann/ordered_map.hpp index 7b8cf70f4..7ed2a0c58 100644 --- a/include/nlohmann/ordered_map.hpp +++ b/include/nlohmann/ordered_map.hpp @@ -11,12 +11,13 @@ #include // equal_to, less #include // initializer_list #include // input_iterator_tag, iterator_traits -#include // allocator +#include // for operator new (placement new) #include // for out_of_range #include // enable_if, is_convertible #include // pair -#include // vector +#include // vector, allocator +#include #include #include diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 576498738..352cf3e20 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -29,7 +29,9 @@ #endif #include // all_of, find, for_each, none_of +#include // isnan #include // nullptr_t, ptrdiff_t, size_t +#include // uint8_t #include // hash, less #include // initializer_list #ifndef JSON_NO_IO @@ -37,10 +39,17 @@ #endif // JSON_NO_IO #include // make_move_iterator, random_access_iterator_tag #include // unique_ptr +#include // swap, operator!= #include // string, stoi, to_string +#include // enable_if_t, is_same, is_scalar, ... +#include // swap (for the from_json(..., std::unordered_map&) overload) #include // declval, forward, move, pair, swap #include // vector +// keep: json.hpp's own basic_json<> default template arguments need the complete definition of +// each of these, not only the forward declarations from json_fwd.hpp, so IWYU's suggestion to +// drop them (nothing in this file otherwise names the type) would break every downstream +// translation unit that relies on basic_json<>'s defaults actually being usable. // #include // __ _____ _____ _____ // __| | __| | | | JSON for Modern C++ @@ -3973,8 +3982,7 @@ NLOHMANN_JSON_NAMESPACE_END #include // int64_t, uint64_t #include // map - #include // allocator - #include // string + #include // allocator, string #include // vector // #include @@ -3995,7 +4003,7 @@ NLOHMANN_JSON_NAMESPACE_END for serialization. */ template - struct adl_serializer; + struct adl_serializer; // IWYU pragma: keep /// a class to store JSON values /// @sa https://json.nlohmann.me/api/basic_json/ @@ -4011,12 +4019,12 @@ NLOHMANN_JSON_NAMESPACE_END adl_serializer, class BinaryType = std::vector, // cppcheck-suppress syntaxError class CustomBaseClass = void> - class basic_json; + class basic_json; // IWYU pragma: keep /// @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 - class json_pointer; + class json_pointer; // IWYU pragma: keep /*! @brief default specialization @@ -4027,7 +4035,7 @@ NLOHMANN_JSON_NAMESPACE_END /// @brief a minimal map-like container that preserves insertion order /// @sa https://json.nlohmann.me/api/ordered_map/ template - struct ordered_map; + struct ordered_map; // IWYU pragma: keep /// @brief specialization that maintains the insertion order of object keys /// @sa https://json.nlohmann.me/api/ordered_json/ @@ -7127,7 +7135,7 @@ struct adl_serializer }; NLOHMANN_JSON_NAMESPACE_END - +// IWYU pragma: keep // #include // __ _____ _____ _____ // __| | __| | | | JSON for Modern C++ @@ -7234,10 +7242,12 @@ class byte_container_with_subtype : public BinaryType NLOHMANN_JSON_NAMESPACE_END +// #include + // #include - +// IWYU pragma: keep // #include - +// IWYU pragma: keep // #include // #include @@ -17119,6 +17129,8 @@ NLOHMANN_JSON_NAMESPACE_END // #include +// #include + // #include // #include @@ -20092,6 +20104,8 @@ NLOHMANN_JSON_NAMESPACE_END // #include +// #include + // #include // #include @@ -25771,11 +25785,13 @@ NLOHMANN_JSON_NAMESPACE_END #include // equal_to, less #include // initializer_list #include // input_iterator_tag, iterator_traits -#include // allocator +#include // for operator new (placement new) #include // for out_of_range #include // enable_if, is_convertible #include // pair -#include // vector +#include // vector, allocator + +// #include // #include @@ -26152,6 +26168,8 @@ private: }; NLOHMANN_JSON_NAMESPACE_END +// IWYU pragma: keep +// #include #if defined(JSON_HAS_CPP_17) @@ -32698,6 +32716,9 @@ struct formatter // NOLINT(cert-dcl58-c #endif #endif +// keep: undoes the macros defined via detail/macro_scope.hpp at the top of this file; removing it +// (nothing in this file *uses* a symbol from it) would leak JSON_* macros into every translation +// unit that includes this header. // #include // __ _____ _____ _____ // __| | __| | | | JSON for Modern C++ @@ -32912,7 +32933,7 @@ struct formatter // NOLINT(cert-dcl58-c #undef JSON_HEDLEY_WARN_UNUSED_RESULT_MSG #undef JSON_HEDLEY_FALL_THROUGH - +// IWYU pragma: keep // End of GCC diagnostic pragmas for C++ modules support #if defined(__GNUC__) && !defined(__clang__) && __cplusplus >= 202002L diff --git a/single_include/nlohmann/json_fwd.hpp b/single_include/nlohmann/json_fwd.hpp index f23ea4820..3ee7afa73 100644 --- a/single_include/nlohmann/json_fwd.hpp +++ b/single_include/nlohmann/json_fwd.hpp @@ -11,8 +11,7 @@ #include // int64_t, uint64_t #include // map -#include // allocator -#include // string +#include // allocator, string #include // vector // #include @@ -177,7 +176,7 @@ This serializer ignores the template arguments and uses ADL for serialization. */ template -struct adl_serializer; +struct adl_serializer; // IWYU pragma: keep /// a class to store JSON values /// @sa https://json.nlohmann.me/api/basic_json/ @@ -193,12 +192,12 @@ template class ObjectType = adl_serializer, class BinaryType = std::vector, // cppcheck-suppress syntaxError class CustomBaseClass = void> -class basic_json; +class basic_json; // IWYU pragma: keep /// @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 -class json_pointer; +class json_pointer; // IWYU pragma: keep /*! @brief default specialization @@ -209,7 +208,7 @@ using json = basic_json<>; /// @brief a minimal map-like container that preserves insertion order /// @sa https://json.nlohmann.me/api/ordered_map/ template -struct ordered_map; +struct ordered_map; // IWYU pragma: keep /// @brief specialization that maintains the insertion order of object keys /// @sa https://json.nlohmann.me/api/ordered_json/