feat: S24.01 IWYU CI gate for include hygiene#260
Merged
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 163 files, which is 13 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (163)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1014 passed, 🙈 3 skipped) 🚧 Error MessagesCreated by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
New `iwyu` CMake preset runs include-what-you-use against the active build's compile_commands.json, piped through a project-specific filter that drops findings IWYU gets wrong on this codebase (redundant struct forward-decls, dual-language stdbool.h confusion, CppUTest macro invisibility). A custom CppUTest mapping file directs IWYU to suggest the public TestHarness.h umbrella over the private internals. Also disables modernize-deprecated-headers in clang-tidy: the library is C and IWYU naturally suggests <stdint.h>-style names; forcing C++ wrappers on tests creates friction that doesn't earn its keep for an embedded-friendly library. This commit adds only the infrastructure — running the gate against the existing tree would surface findings, fixed in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e, Tests
Mechanical apply of IWYU's surviving findings (after the project filter)
via fix_includes.py, then clang-format. Three classes of change:
- Public Interface headers narrowed: 30 headers stop transitively re-
exporting other public headers. Audience contract from CLAUDE.md
preserved (app/service code = 1 header; setup code = N headers, one
per type constructed).
- Source files made self-sufficient: each TU now includes the headers
for symbols it directly names rather than relying on transitive
includes from its primary header.
- Test code follow-on: 13 test files needed manual additions where
IWYU couldn't parse them due to compile errors caused by the
narrowing (one-time bootstrap cost). 1021 spurious blank lines
fix_includes.py inserted between TEST(...) and { stripped via script.
Two pragma annotations for genuinely dual-language (C+C++) headers:
- Tests/Support/OpenSslFake.h keeps <stdbool.h>
- Tests/TestUtils.h relocates the CppUTest include inside #ifdef __cplusplus
One structural reorder in Tests/BlockSequenceTest.cpp moves the
TestHarness include out of an extern "C" block where fix_includes.py
incorrectly placed it.
All build presets green (debug, clang-debug, sanitize, tidy, cppcheck,
coverage at 100% lines/functions). All 1009 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Windows-only test files weren't in IWYU's Linux scope (they don't appear in compile_commands.json on Linux), so the same hand-fix that was applied to the POSIX equivalents is needed here. Caught by CI build-windows-msvc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
S18.03 expanded BlockSequenceTest with std::map / std::vector usage; the post-rebase IWYU pass picked up the new dependencies on <cstddef> and <utility>. Also strips fix_includes.py's spurious blank-lines-after-TEST on this one file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DavidCozens
force-pushed
the
feat/iwyu-ci-gate
branch
from
88b908f to
d1f6ddf
Compare
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1025 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
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
1 participant
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.
Purpose
Adds include-what-you-use (IWYU) as an opt-in CMake preset (
iwyu) that surfacesinclude-hygiene drift across Core, Platform, Example, and Tests. The preset can
be promoted to a required CI status check in a follow-up PR once any in-flight
PRs have merged.
Also lands the cleanup that the gate now finds clean: 30 public headers
narrowed to stop transitively re-exporting other headers, plus the test-side
follow-on.
Closes #255.
Change Description
Gate infrastructure (commit 1):
cmake/Iwyu.cmake—iwyuandiwyu-applycustom targets, driven viaiwyu_tool.pyovercompile_commands.json. CMake's launcher mechanism(
CMAKE_<LANG>_INCLUDE_WHAT_YOU_USE) ignores IWYU's exit code since 3.11,so the launcher pattern can't enforce findings — the custom-target pipeline
pipes IWYU output through a project filter and uses that as the gate.
scripts/iwyu_filter.py— drops three classes of finding IWYU gets wrongon this codebase: redundant
struct Foo;forward-decls (kept forMISRA/readability),
<stdbool.h>removals on dual-language C+C++ headers(IWYU only sees the C++ side), and
CppUTest/TestHarness.hremovals (IWYUdoesn't model
TEST(...)macro expansion). Each rule documented inline.cmake/cpputest.imp— directs IWYU to suggest the public CppUTest umbrellaheader rather than internal headers like
<UtestMacros.h>..clang-tidyadds-modernize-deprecated-headersto the disable list. IWYUnatively suggests C-style header names which is right for a C library;
forcing C++ wrappers in tests was the two tools fighting each other.
Apply (commit 2):
it directly names).
(
SolidSyslog.h), service-code = 1 header, setup-code = N headers, one pertype constructed.
to the cascade of compile errors from the production-side narrowing
(one-time bootstrap cost; the rest IWYU/fix_includes drove automatically).
TEST(...)and
{stripped via script across 46 test files.Tests/Support/OpenSslFake.hkeeps<stdbool.h>;Tests/TestUtils.hhasthe CppUTest include relocated inside
#ifdef __cplusplus.Tests/BlockSequenceTest.cppmoves the TestHarnessinclude out of an
extern "C"block.Container: IWYU is built from source against clang-19 in
CppUTestDockerClang(separate repo). The container SHA is not bumped inthis PR — the active feature-work clone is using the existing pinned SHA via
docker-compose, and bumping mid-flight would disrupt that. The follow-up PR
that flips
analyze-iwyuto required will bump the SHA at the same time.Test Evidence
All build presets verified locally via
docker run:debug(gcc)clang-debugsanitize(ASan + UBSan)coverageanalyze-tidy-modernize-deprecated-headers)analyze-cppcheckanalyze-formatiwyuSanity-check on the gate itself: synthetic
#include <math.h>inCore/Source/SolidSyslog.cis detected and the gate exits non-zero.The gate is correctly limited to "when the code compiles, are its includes
minimal and complete?" — it can't catch missing-include cases that fail to
compile (IWYU can't parse broken files), but the existing build gates catch
those anyway.
Areas Affected
Core/Interface/,Core/Source/— public headers narrowed; source filesnow include their own dependencies directly. No API behaviour changes.
Platform/Posix/,Platform/OpenSsl/,Platform/Atomics/— same shape:narrowed public headers, self-sufficient sources.
Example/— net-zero churn in setup-code header counts; mostly stdlibreordering and a few
structforward-decls.Tests/— 13 manual test fixes, 65 auto-driven test fixes, blank-linecleanup. Tier-4 per CLAUDE.md but in scope for the gate because narrowed
production headers expose test consumers that relied on transitive includes.
iwyu+iwyu-applypreset,cmake/Iwyu.cmake,cmake/cpputest.imp,scripts/iwyu_filter.py..clang-tidyexclusion listgrows by one rule.
🤖 Generated with Claude Code