Skip to content

CI: enforce clang-tidy with a pinned toolchain and a curated check list #88

Description

@martinus

What

The repository maintains a .clang-tidy and the header carries curated NOLINT comments, but no CI job has ever run clang-tidy. Make it enforced — which requires narrowing the check list and pinning the toolchain first.

Why

The config is maintained but unenforced. .clang-tidy exists with a deliberate, curated exclusion list, and include/ankerl/svector.h carries 13 NOLINT comments written to satisfy specific checks (cppcoreguidelines-init-variables, cppcoreguidelines-pro-type-const-cast, and others). Someone clearly ran this and cleaned up against it. Nothing keeps it that way: scripts/lint/lint-all.py globs lint-* and only finds lint-clang-format.py and lint-version.py.

It has drifted. Running clang-tidy 22 over a TU that includes the header produces 25+ warnings in svector.h alone. A sample:

  • cppcoreguidelines-rvalue-reference-param-not-moved — parameter other never moved from (svector.h:995)
  • cppcoreguidelines-missing-std-forward — forwarding reference args never forwarded (svector.h:904)
  • bugprone-sizeof-expression and bugprone-multi-level-implicit-pointer-conversion on the memcpy calls in indirect() / set_indirect() (svector.h:649, 669)
  • cppcoreguidelines-special-member-functionsstorage_guard has a destructor and copy operations but no move operations (svector.h:536)
  • performance-enum-sizeenum class direction uses a 4-byte base type (svector.h:583)
  • plus a long tail of readability-redundant-inline-specifier, modernize-use-trailing-return-type and modernize-type-traits

Most of these are style churn or false positives against a container that deliberately does unusual things with raw bytes — the sizeof/memcpy ones in particular are correct code being flagged. But a few are worth a real look, especially the missing-std::forward on an emplace path and storage_guard's special members, which is ownership code.

The reason this is not already in CI is real and must be handled. The config starts with Checks: "*", which auto-enables every check any future clang-tidy adds. Wired up naively, the job would go red on every toolchain bump for reasons that have nothing to do with a change. That makes this a curation task, not a one-line workflow addition.

Work

  • Decide the enforcement model. Recommended: pin an exact clang-tidy version (a container image or an explicit apt version pin), so the check set only changes when the pin is deliberately bumped.
  • Replace Checks: "*" with an explicit list of check groups worth enforcing here. bugprone-*, cppcoreguidelines-*, misc-*, performance-* and readability-* minus a curated deny list is a reasonable starting point and keeps future churn bounded.
  • Triage the current 25+ warnings into three buckets: fix, suppress with a justified NOLINT explaining why, or disable the check project-wide. Do not blanket-NOLINT to get to green.
  • Look properly at the two that are plausibly substantive rather than stylistic: missing-std-forward at svector.h:904 and special-member-functions on storage_guard at svector.h:536.
  • Add scripts/lint/lint-clang-tidy.py following the existing pattern so lint-all.py picks it up automatically and it runs locally the same way it runs in CI. Generate a compilation database from a meson build directory rather than hardcoding flags.
  • Run it in the existing lint job, which currently takes 0.2 minutes and has room.

Acceptance criteria

  • ./scripts/lint/lint-all.py runs clang-tidy and exits zero on a clean tree.
  • The check list is explicit, so a new clang-tidy release cannot spontaneously fail CI.
  • Every remaining suppression carries a reason.
  • The job runs against the same pinned version locally and in CI, so contributors can reproduce a failure.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions