bazel: add hermetic clang-tidy via aspect_rules_lint (--config=lint)

[openroad-ci]

Summary

Adds a hermetic, opt-in clang-tidy path via Bazel: bazel build --config=lint //…. Uses aspect_rules_lint 2.5.2 to drive clang-tidy 20.1.8 (from @llvm_toolchain) against every cc target's own CcInfo flags. Output lands as *.AspectRulesLintClangTidy.out files under bazel-bin/.

This PR is purely additive. No CI behavior changes, no existing local-lint flow is removed, and nothing runs unless a developer or future workflow passes --config=lint. The intent is to (eventually) replace the The-OpenROAD-Project/clang-tidy-review fork in CI with reviewdog-style PR-comment generation that consumes the aspect's output, scoped to changed lines.

Why another lint path

There are three existing systems; this is a fourth with a distinct purpose. Quick map:

System	Source of truth	Purpose	Pinned tool version?
3a — CI workflow (clang-tidy-review fork)	cmake compile_commands.json	Gatekeeping PRs	No — system clang-tidy 19
3b — etc/run-clang-tidy.sh (bant)	universal compile_flags.txt	IDE/clangd + mass cleanup	Yes — @llvm_toolchain 20.1.8
3c — //:lint_test umbrella	per-tool sh_test	TCL / buildifier	Yes
4 — --config=lint aspect (this PR)	per-target CcInfo	Pre-PR lint + future CI source-of-truth	Yes — @llvm_toolchain 20.1.8

Why this specifically:

1. Version pinning. @llvm_toolchain 20.1.8 — same toolchain 3b already uses, byte-identical binary (sha confirmed). Closes the gap @maliberty raised in Feature Request: Migrate clang-format to Bazel Lint Umbrella #9860 about "format fights" between unpinned linter versions.
2. Per-TU flag accuracy. Each cc target's own includes/defines drive its lint invocation. Fixes the graphics.h-style ambiguity @maliberty flagged on PR Provide a way to create a compilation DB in the bazel project #9122 (universal compile_flags.txt cannot distinguish three graphics.h files in mpl/, fin/, exa/).
3. Action-graph native. The aspect participates in Bazel's action cache, including the project's remote cache. Lint actions are content-addressed: change one file, only that file's lint action reruns. Re-running a clean lint pass after a remote-cache warm-up is effectively free.
4. No compile-db memory blow-up. @hzeller's PR Provide a way to create a compilation DB in the bazel project #9122 rejection of compile_commands.json was based on its ~hundreds-of-MB size + GB-scale RAM consumption on parallel runs. The aspect emits no compile-db; each invocation is a small per-action argv. The memory concern doesn't apply.
5. Submodules left untouched. src/sta (OpenSTA) and third-party/abc (ABC) are upstream-managed submodules. Both are excluded by invocation pattern (-//src/sta/..., -//third-party/abc/...), not by no-lint tagging — that way the submodule trees stay unmodified and lint findings against them aren't surfaced (they'd belong upstream, not here).

Non-goals (deliberately out of scope)

• Bumping CI tool version 19 → 20. Will be a separate CI-swap PR using reviewdog against this aspect's output.
• Replacing etc/run-clang-tidy.sh / etc/bazel-make-compilation-db.sh. They serve clangd IDE integration and large-scale cleanup workflows that this aspect does not.
• Wiring into //:lint_test. That umbrella is sh_test-based and reruns the full script every invocation; using it for C++ would defeat Bazel's per-action lint cache. Aspects are bazel-native for this exact reason. The umbrella is right for tools without aspect equivalents (TCL, buildifier).

Relationship to existing local lint (etc/run-clang-tidy.sh)

etc/run-clang-tidy.sh (system 3b) and this aspect produce different findings by design:

• Same clang-tidy binary (20.1.8, sha c9bbe43a…), so tool version is in parity.
• Compile flags diverge: 3b uses a universal compile_flags.txt; the aspect uses per-target flags. Concrete deltas:
  • 3b lacks -stdlib=libc++, -fopenmp, per-target -D macros, -DSPDLOG_FMT_EXTERNAL, and per-target include paths.
  • 3b cannot disambiguate same-named headers across modules (the graphics.h issue).

Recommended user-facing story (added to docs/agents/ci.md):

Goal	Command
Reproduce the exact CI lint result (once CI uses the aspect)	bazel build --config=lint //affected:target
Power clangd / IDE language server	etc/run-clang-tidy.sh
Mass-cleanup across the codebase	etc/run-clang-tidy.sh cached runner

The headline guarantee: once CI runs the aspect, bazel build --config=lint locally produces the same findings as CI, byte for byte, by sharing the action cache.

Note: divergence between local lint (3b, ct20) and current CI (3a, ct19) already exists today and is unchanged by this PR. If anything, this PR narrows the gap by introducing a path with tool-version parity to 3b.

Pre-emptive responses to likely review feedback

• "This is an internals hack against bazel actions" — aspect_rules_lint 2.5.2 is a maintained Aspect.dev product, not action-internals scraping. It exposes a sanctioned API.
• "Why not the //:lint_test umbrella?" — see "Non-goals" above; aspects ≠ sh_test. Aspects cache per-target; sh_test reruns the full script. Wrong tool for C++ lint at this codebase's size.
• "Version pinned?" — yes, @llvm_toolchain 20.1.8.
• "compile_commands.json memory blow-up?" — N/A. The aspect emits no compile-db.
• "graphics.h-style universal-flag ambiguity?" — solved. Per-target CcInfo is the input.

Test plan

• bazel build --config=lint //src/utl/... — clean run, produces .AspectRulesLintClangTidy.out files
• bazel build --config=lint -- //src/... //third-party/... -//src/sta/... -//third-party/abc/... — runs across all eligible cc targets
• Hermeticity check: byte-identical output across two clean runs (cache bypassed)
• HeaderFilterRegex from //:.clang-tidy is honored (53k+ external warnings suppressed; only user-code findings reported)
• Generated files (SWIG/bison/flex outputs) are auto-skipped via the aspect's generated-file detection
• Qt targets resolve via the Bazel dep graph (no missing-header failures on gui_qt_headless and friends)
• Diagnostic parity vs the current CI fork (system 3a, ct19+cmake) on 5 representative files across odb/grt/gpl/rsz/dpl: no user-code diagnostic that the fork catches is silently missed by the aspect. All A-vs-D deltas were traced to deliberate toolchain choices (-stdlib=libc++, hermetic __DATE__, external fmt, bundled omp.h), not aspect bugs.

Files

.bazelrc                          # --config=lint definition
MODULE.bazel                      # aspect_rules_lint 2.5.2 (dev_dep); exposes @llvm_toolchain_llvm
BUILD.bazel                       # exports //:.clang-tidy
tools/lint/BUILD.bazel            # native_binary wrapping clang-tidy
tools/lint/linters.bzl            # lint_clang_tidy_aspect (defers HeaderFilterRegex to .clang-tidy)
third-party/{gif-h,lodepng,stb_truetype}/BUILD*   # tag no-lint on vendored single-header libs
docs/agents/ci.md                 # contributor docs

---

Parity vs etc/run-clang-tidy.sh (bant compile_flags.txt) — measured

Captured clang-tidy 20.1.8 output for one representative .cpp/.cc per module under both paths and diffed the user-code findings:

File	aspect (--config=lint)	bant (run-clang-tidy.sh)	Aspect-only findings	bant-only findings
odb/src/db/dbBTerm.cpp	3	3	0	0
grt/src/Pin.cpp	0	0	0	0
gpl/src/initialPlace.cpp	4	4	0	0
rsz/src/PreChecks.cc	1	1	0	0
dpl/src/dbToOpendp.cpp	1	2	0	1

The single bant-only finding on dbToOpendp.cpp is in vendored boost (boost/polygon/rectangle_concept.hpp:791, clang-diagnostic-bitwise-instead-of-logical), not user code. The aspect's per-target compile context filters it out correctly; bant's universal flag set surfaces it as noise. The aspect is silent where it should be silent; bant is noisier than CI will be. This is the safe direction of divergence — a developer running etc/run-clang-tidy.sh locally might see findings CI does not, but CI will never flag something the aspect-driven local lint missed.

Diagnostic deltas vs the current CI fork (system 3a: clang-tidy 19 + cmake compile_commands.json) were also measured and are explained entirely by deliberate toolchain choices in the hermetic Bazel environment (-stdlib=libc++, hermetic __DATE__/__TIME__, external fmt via -DSPDLOG_FMT_EXTERNAL, bundled openmp). No silent regressions in user-code diagnostics were observed.

[gemini-code-assist]

Code Review

This pull request integrates hermetic clang-tidy analysis into the Bazel build system using aspect_rules_lint. Key changes include the addition of a lint configuration in .bazelrc, the definition of a linting aspect in tools/lint/, and updated documentation for developers. Review feedback correctly identifies a breaking configuration error in tools/lint/linters.bzl regarding the lint_clang_tidy_aspect parameters and suggests simplifying a redundant select statement in the linter's binary definition.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[hzeller]

FYI A bit of context.
Note, the reason why we use compile_flags.txt instead of a compilation db json is because the latter would be very huge, resulting in memory issues when running clang-tidy on 128 cores... it would of course have the advantage that we can use target specific defines and includes.

Relying on that, though, is also very fragile. The include paths in this project are super-leaky.

The problem with the ambiguity of headers can not be resolved by using the fully qualified paths from project root (#include "src/exa/src/graphics.h") which we unfortunately have to hold off right now while using cmake.

Having said that, using the aspect-based linting will be beneficial as each file gets its own 'tailored' compile_flags.txt, so the memory consumption will not be a problem. So it can be more targeted. Also, bazel caching will help, similar to run-clang-tidy-cached.cc, to only run on changes.
(as long as I can still use etc/run-clang-tidy.sh as I need that for my bulk-cleanup workflow)

[maliberty]

Note, the reason why we use compile_flags.txt instead of a compilation db json is because the latter would be very huge, resulting in memory issues when running clang-tidy on 128 cores... it would of course have the advantage that we can use target specific defines and includes.

It isn't huge when using cmake (~4Mb). Why would that be different with Bazel?

[hzeller]

It isn't huge when using cmake (~4Mb). Why would that be different with Bazel?

Because cmake essentially has to add /usr/include and /usr/local/include
In bazel, there is a different path for every sub project we use.

[sombraSoft]

Thanks for the context @hzeller — agree on all points.

To confirm the two practical concerns:

1. etc/run-clang-tidy.sh is explicitly preserved. It's listed in the PR's "Non-goals" section and the change set doesn't touch it, the bant compile-flags script, or run-clang-tidy-cached.cc. The aspect is a fourth, opt-in path alongside it, not a replacement.

2. The "tailored compile_flags.txt per file" characterization matches the implementation exactly — each lint action carries its target's own CcInfo flags as a per-action argv, so there's no aggregated JSON at any scale. As a data point on what this looks like in practice, I ran the full-repo lint scope end-to-end:

   • 851 lint reports produced, 5.6 MB total
   • 4,525 actions, of which 463 actually ran clang-tidy; the rest (2,668 disk + 186 remote + 207 action cache hits, plus 1,208 internal) were cache hits or scaffolding
   • First run wall time: 17:39
   • Warm-cache rerun: 3.7 s, all hits
   • 5,842 findings in user code across 12 check categories

   Once CI runs this with --config=ci --config=lint and populates the remote cache, a developer's first lint pass should be mostly cache-fetches rather than re-execution.

[hzeller]

Nice. And can we use that then for the clang-tidy comments in pull requests that are now coming via a different clang-tidy on the cmake compilation db ?

[sombraSoft]

@maliberty — looking for your input on the rollout plan. This PR sets up something you've explicitly asked for (PR #9122 closeout: "it would be nice to have a clang-tidy run option available from bazel"), and the medium-term goal is to deprecate the current clang-tidy CI workflow that depends on our clang-tidy-review fork.

Why deprecate the fork-based workflow

The fork-based path has produced recurring transient issues tied to runner-environment drift: the libyaml-cpp-dev apt fix from #8487, clang-tidy version skew between local installs and CI runners, and general non-hermeticity across developer machines vs the runner image. The fork pins clang-tidy 19 system-installed; bumping it requires fork-level coordination on top of runner-image changes. The aspect path pulls clang-tidy from @llvm_toolchain — the binary is pinned in MODULE.bazel and every developer and CI runner uses bit-for-bit the same one.

How prior concerns map onto this design

Concern	Where you raised it	How this PR handles it
Linter version pinning ("format fights")	#9860	Pinned at clang-tidy 20.1.8 via @llvm_toolchain
Universal-include ambiguity (same-name graphics.h in mpl//fin//exa/)	PR #9122 review	Aspect uses each cc target's own CcInfo; per-TU includes resolve correctly
PR validation under a version bump	#9860	Step 3 below keeps the changed-lines-only scoping, same as today
Upgrade clang-tidy (closed stale)	#8983	Addressed for the new path here; CI swap completes the upgrade

@hzeller has endorsed the per-target compile-context approach and just asked directly whether the aspect output can power the PR-comment workflow — which is exactly step 3 of the rollout.

Proposed rollout

1. This PR — lands the aspect as an opt-in --config=lint. Doesn't touch CI. Doesn't replace etc/run-clang-tidy.sh. Pure addition.

2. Optional, incremental cleanup PRs — clear v19→v20 regressions in modules where they actually obstruct ongoing work. Not a goal to clear the full v20 backlog: the repo is large, linter version bumps have happened before, and CI's changed-lines-only scoping means pre-existing findings don't gate any specific PR. Cleanup happens when and where it pays for itself.

3. CI-swap PR — replace the fork-based workflow with one consuming the aspect output. Comments stay scoped to changed lines only, matching current behavior. A developer who only touches a few lines sees only the findings on those lines, regardless of the v20 backlog elsewhere. This is the same operational stance the project has historically taken on linter version bumps: change the tool, keep the diff-scoping, let pre-existing findings sit until they're touched.

   Tool choice for the swap is still open. We've looked at reviewdog as the most obvious off-the-shelf candidate (mature, with ready-made changed-lines filtering and GitHub PR-review-comment posting), but the aspect's *.AspectRulesLintClangTidy.out files are standard clang-tidy text format — anything that consumes clang-tidy output plumbs in, including a small custom GitHub Actions script that filters against git diff and posts comments via gh. Final decision is for the swap PR; this PR is just the foundation that makes all of those options viable.

Questions for you

1. Does the staged rollout match what you'd want?
2. Anything blocking step 3 from your side before we open it?

[sombraSoft]

@hzeller — re:

Nice. And can we use that then for the clang-tidy comments in pull requests that are now coming via a different clang-tidy on the cmake compilation db?

Yes — that's exactly the goal. Step 3 of the rollout I just laid out in the comment to @maliberty: replace the fork-based clang-tidy-review workflow with one that consumes the aspect's *.AspectRulesLintClangTidy.out files, keeping the changed-lines-only scoping the fork has today.

Tool for the swap is still open — reviewdog is the obvious off-the-shelf candidate, but since the aspect output is plain clang-tidy text, any consumer works (including a small custom GHA script filtering against git diff and posting via gh). Waiting on Matt's go on the rollout staging before opening the swap PR.