tooling: fix the compile DB so clang-tidy-22 parses clean - #405
Merged
Conversation
+ xff fix) clang-tidy-22 aborted on every TU. Two causes, both fixed: 1) compile_commands-update.sh ran `refresh_all --config=clang`, but that flag only configures the extractor tool's own build, never the internal aquery, so every recorded command named the autodetected Apple clang, not the hermetic toolchain (-> 'concepts'/'time.h' not found). Port mbo #270: bump the extractor pin to 6eb3ff1 (adds --bcce-prefer-target-config, dropping 227 duplicate exec-config entries), resolve the hermetic clang++ and pass --bcce-compiler + --bcce-prefer-target-config after --, plus Darwin-only --bcce-copt=-isysroot. Add //tools:show_compiler as the probe target that materializes the toolchain on a fresh checkout. 2) xff-specific: mbo is on the -isystem search path and ships a plain-text file named `version` at its root, which shadows libc++'s <version> (an explicit -isystem beats the compiler's builtin libc++). clang_tidy.sh now prepends the hermetic libc++ dir via --extra-arg-before=-isystem so the real <version> wins (hermetic libc++, matching the DB's clang, not the SDK's). Also fix the dead .clang-tidy option (CharTypdefsToIgnore -> CharTypedefsToIgnore). clang-tidy-22 now parses clean (real findings only, no abort). The hook stays stages:[manual]; the report-only CI job + the residual-finding sweep are the follow-ons (#166).
helly25
enabled auto-merge (squash)
August 8, 2026 16:19
This was referenced Aug 8, 2026
helly25
added a commit
that referenced
this pull request
Aug 8, 2026
The #405 comment narrated the fork's own changelog (which PR added which --bcce flag); that is the referenced module's history, not xff's to carry. Keep only the xff-relevant fact: it is the helly25 fork and our script uses its --bcce-* flags.
helly25
added a commit
that referenced
this pull request
Aug 8, 2026
… a hard gate (#413) * clang-tidy #166: sweep the test tree clean and flip clang-tidy to a hard gate Clears the ~100 clang-tidy findings in the _test.cc files (the report-only CI job surfaced them), so the whole tree is now clang-tidy-clean, then promotes the CI clang-tidy job from report-only to a hard gate. Test findings, fixed the genuine way (no blanket suppressions): - optional access: EXPECT_THAT(opt, Optional(matcher)) for value-matches (fields); an if-guarded RenderIndex() helper for render-then-check cases (help_render); compute the expected instant with absl::FromCivil instead of deref (datetime). - const-correctness (ignore/repo/license locals); use the unqualified matchers via the using-decls (hash IsTrue/IsFalse); std::ranges::is_sorted (license); (void)std::remove for best-effort cleanup (cert-err33); bugprone-argument-comment names matched to the real parameters (color); short test locals renamed (t->now, c->cfg, b->parts, lambda p->path); walk_test perf (const methods, reserve, std::cmp sign compare, emplace_back). - Documented file-scoped suppressions only where the finding fights a mandated test idiom: gtest struct-fixture SetUp/TearDown visibility + `_`-suffixed members + data-heavy TEST_F cognitive-complexity (pager/walk), and single-threaded env-mutating fixtures (setenv/getenv). Gate: - .github/workflows/main.yml: drop `continue-on-error` from the clang-tidy step so any finding fails the job (and `done`). The compile-DB build already gated. - .pre-commit-config.yaml: refresh the clang-tidy hook description (it is now the CI gate, not local-only; the compile-DB parse abort was fixed in #405). Keeps `stages: [manual]` so the CI job's `--hook-stage manual` invocation still runs it and local commits are not slowed. Also: STYLE_CPP.md - rewrite the stale clang-tidy toolchain paragraph (it still described trunk + WarningsAsErrors:true + "CI does not run it"; now the local tools/clang_tidy.sh hook + compile_commands-update.sh + the CI clang-tidy gate). Verified: full //xff/... suite green (92); the whole tree is clang-tidy-clean under the hermetic clang-22 (confirmed by re-sweep). * clang-tidy #166: fix Linux __config_site parse breakage + adopt mbo's test-pass split The gate CI run surfaced two things, both in the tooling not the tree: 1. Linux libc++ parse collapse. tools/clang_tidy.sh prepends the hermetic `-isystem .../include/c++/v1` (to beat mbo's root `version` file winning `#include <version>`). libc++'s <__config> does `#include <__config_site>`, a generated per-target header. On Linux the hermetic install puts it in a target-triple subdir (include/<triple>/c++/v1/__config_site), NOT next to <__config>; macOS puts both in c++/v1. Prepending only c++/v1 then won the search for <__config> but hid <__config_site>, so libc++ failed to configure and the whole parse collapsed - std::string_view resolved to `int` and clang-tidy emitted a flood of spurious findings (member-init, convert-to-static on override methods, implicit-bool 'string_view -> int'). That is the "'__config_site' file not found" in the CI log. Fix: also prepend the directory that actually holds __config_site, located dynamically so it works on both the macOS (same dir, skipped) and Linux (triple subdir) layouts. 2. Adopt helly25/mbo's clang_tidy.sh test-pass split instead of scattering NOLINTs. Sources and *_test.cc run as two passes; the test pass appends `--checks=-readability-function-cognitive-complexity,-misc-override-with-different-visibility` (which APPENDS to .clang-tidy, removing just those two): gtest EXPECT/ASSERT macros inflate cognitive-complexity, and our mandated `struct` fixtures make SetUp/TearDown overrides public vs ::testing::Test's protected. One statement of each rule, covering new tests automatically. concurrency-mt-unsafe is deliberately NOT blanket-disabled: env-touching tests keep a targeted NOLINT. Verified locally (hermetic clang-22, correct parse): all 43 _test.cc clean under the two-pass; globals_test clean in isolation. * clang-tidy #166: pin libc++ in the compile DB (fix the Linux stdlib mix) The real root cause of the Linux-only phantom findings: the hermetic clang compiles against libc++ on macOS but the system libstdc++ on Linux (the toolchains_llvm default). tools/clang_tidy.sh prepends the hermetic libc++ `include/c++/v1` so `#include <version>` beats helly25_mbo's plain-text `version` file (its -isystem'd repo root otherwise shadows it) - which on Linux injected libc++ headers into a libstdc++ parse. That stdlib mix silently corrupted clang-tidy's semantic analysis and produced a flood of contradictory phantom findings: std::string_view resolving to `int`, misc-const-correctness on a variable that IS mutated, readability-convert-member-functions-to-static on `override` methods, member-init on aggregates. None reproduce on macOS (all libc++), which is why the local sweep was clean. Fix: always parse with libc++. - compile_commands-update.sh: add `--bcce-copt=-stdlib=libc++` so every recorded command uses the hermetic clang's own libc++ on every platform. clang-tidy is then internally all-libc++ and the `<version>` prepend is consistent (as on macOS), so the phantom findings disappear at the source. - tools/clang_tidy.sh: revert the __config_site triple-dir hunt from the previous commit; with libc++ actually active its builtin provides <__config_site>, and the hunt risked picking a wrong-triple config. - .clang-tidy: disable cppcoreguidelines-pro-type-member-init + its alias hicpp-member-init. It forces blind initialization of every field rather than flagging a real defect; genuinely-unused members/vars are a different, useful check and stay on. Verified: repo_test + walk_test clean on macOS with the libc++-pinned DB (the earlier phantom findings in both were the stdlib mix). Linux CI on this PR is the proof. * clang-tidy #166: build the headers the compile DB references before linting With libc++ pinned the Linux parse is correct, and the only remaining CI errors were generated / virtual-include headers not on disk: 'xff/license/notice.h' file not found 'xff/regex/backend.h' file not found Both come from the local @xff_extras_api module and are served through bazel-out `_virtual_includes` symlink forests that exist only once the owning cc_library is built. compile_commands-update.sh builds the DB but not those outputs, so a fresh runner had the include paths recorded but the files absent - which surfaced as the "some sources were not generated" symptom. Fix: the clang-tidy CI job now runs `bazel build //...` before generating the DB, materializing every genrule output and _virtual_includes forest. It is a plain default-config build (NOT --config=clang): refresh_all records the DB in the default config (`<cpu>-fastbuild`), so the forests must land there; --config=clang would place them under a different output dir and miss. Headers do not depend on which compiler compiles the .cc, so the default toolchain is correct for materializing them. (helly25/mbo builds only the `kind("generated file")` targets with --config=clang; that works there because mbo's are config-invariant genrule outputs. xff's are cc_library virtual-includes from an external module, so it needs the default-config whole build instead.)
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.
Ports the compile-DB fix from mbo #270 and adds the xff-specific
<version>-shadow fix, soclang-tidy-22parses cleanly again (it previously aborted on every TU). Unblocks #166.Two root causes, both fixed
compile_commands-update.shranrefresh_all --config=clang, but--config=clangonly configures the extractor tool's own build and never reaches the internalaquery, so every recorded command named the autodetected Apple clang, not the hermetic toolchain →'concepts'/'time.h' file not found. Port: bump the extractor pin to6eb3ff1(adds--bcce-prefer-target-config, which drops 227 duplicate exec-config entries), resolve the hermeticclang++and pass--bcce-compiler+--bcce-prefer-target-configafter--, plus Darwin-only--bcce-copt=-isysroot. New//tools:show_compilerprobe materializes the toolchain on a fresh checkout.<version>shadow. mbo is on the-isystemsearch path and ships a plain-text file literally namedversionat its root, which shadows libc++'s<version>(an explicit-isystembeats the compiler's builtin libc++).clang_tidy.shnow prepends the hermetic libc++ dir via--extra-arg-before=-isystemso the real<version>wins.Also fixes a dead
.clang-tidyoption (CharTypdefsToIgnore→CharTypedefsToIgnore, surfaced by--verify-config).Result
tools/clang_tidy.shon a TU now yields real findings only - noconcepts/versionabort, no "too many errors". Tooling-only change (no//...target affected). The hook staysstages: [manual].Follow-ons (#166)
continue-on-error) that builds the DB + runs the hook.cppcoreguidelines-pro-bounds-avoid-unchecked-container-access), then dropstages: [manual]to gate.