Run clang-tidy 21 on pull-request changed lines#4982
Open
matthargett wants to merge 2 commits into
Open
Conversation
lum1n0us
reviewed
Jul 8, 2026
lum1n0us
left a comment
Contributor
There was a problem hiding this comment.
Before turning this workflow into an enforced check, should we run a full clang-tidy scan on the current codebase and clean up any existing issues first? Otherwise, contributors may end up having to deal with pre-existing diagnostics while making unrelated changes in their PRs.
This was referenced Jul 10, 2026
c367072 to
8b0d482
Compare
The repository already ships a strict .clang-tidy (bugprone-*, cert-*, clang-analyzer-*, ... with WarningsAsErrors) but no workflow runs it, so the bug class it catches is currently caught only by a reviewer reading the diff. Add a pull_request workflow that runs clang-tidy on the lines a PR changes (clang-tidy-diff.py driven from the base SHA, the same diff-scoping approach as the git-clang-format gate in coding_guidelines.yml), so a PR is only flagged for issues it introduces on the lines it touches. Like that gate it needs only contents:read and computes the diff from the pull_request base SHA, so it behaves identically for in-repo and forked pull requests. The job runs on ubuntu-24.04 and installs clang-tidy-21 from apt.llvm.org (a small package, not a full LLVM toolchain download). clang-tidy 21 matches the LLVM shipped in Xcode 26/27 and the clang-format-21 gate, so lint results are consistent for contributors building on macOS/Xcode and on other LLVM-built platforms. To keep it correct and cheap: - compile_commands.json comes from configuring the default linux iwasm build (interp + AOT runtime, no LLVM required); the database is emitted at configure time, so no build step is needed. - Only changed sources that are in compile_commands.json are analyzed, each with its real compile flags. A changed file not in the default build (or a header, which has no translation unit of its own) is skipped rather than analyzed without context, which would otherwise produce false failures from missing includes or the wrong #ifdef branch. - A PR that changes no C/C++ source skips install / configure / lint entirely, so docs-, test- and CI-only PRs cost almost no Action minutes while the job still reports success.
Running clang-tidy-21 instead of clang-tidy-14 surfaces checks that did
not exist in 14. Five of them are noise or style for this C codebase
rather than bug-catchers; on a full-tree scan they account for the
overwhelming majority of findings with no correctness signal:
- misc-include-cleaner IWYU-style; ~1750 findings, unusable noise
- misc-header-include-cycle structural noise
- modernize-macro-to-enum opinionated for a C codebase
- readability-inconsistent-declaration-parameter-name cosmetic
- bugprone-assignment-in-if-condition flags WAMR's deliberate
assignment-in-condition idiom
Disable only those five. The bug-catching checks that have historically
caught real portability / correctness bugs stay on, including
bugprone-narrowing-conversions, bugprone-sizeof-expression,
bugprone-implicit-widening-of-multiplication-result,
bugprone-multi-level-implicit-pointer-conversion, performance-*,
clang-analyzer-* and cert-*.
The gate added in the previous commit is diff-scoped, so these disables
only affect the handful of new lines a PR touches; residual findings on
existing code are addressed separately in per-subsystem cleanup PRs.
8b0d482 to
7ec71c1
Compare
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
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.
What
Run the repository's existing
.clang-tidyon pull requests — but only on the lines the PR changes.The root
.clang-tidyis already a maintained, strict config (bugprone-*,cert-*,clang-analyzer-*, ... withWarningsAsErrors), but no workflow runs it today, so the null-deref / narrowing-conversion / use-after-move class of bug it catches is currently caught only by a reviewer reading the diff.How
Mirrors the diff-scoping already used by the
git-clang-formatgate incoding_guidelines.yml:permissions: contents: read,fetch-depth: 0, diff computed from${{ github.event.pull_request.base.sha }}..HEAD.clang-tidy-diff.pyturns the changed-line ranges into-line-filter, so a PR is only flagged for issues it introduces on the lines it touches — pre-existing issues on untouched lines stay silent.compile_commands.jsoncomes from configuring the default linuxiwasmbuild (interp + AOT runtime, no LLVM required); the database is emitted at configure time, so no build step is needed.compile_commands.jsonare analyzed, each with its real compile flags. A changed file not in the default build — or a header, which has no translation unit of its own — is skipped rather than analyzed without context (which would otherwise produce false failures from missing includes or the wrong#ifdefbranch).clang-tidy version
Runs clang-tidy 21 on
ubuntu-24.04, installed from apt.llvm.org (a small ~50 MB package, not a full LLVM toolchain download). clang-tidy 21 matches the LLVM shipped in Xcode 26/27 and the clang-format-21 gate proposed in #4992, so lint results are consistent whether a contributor builds/auto-formats on macOS/Xcode or on another LLVM-built platform.Curated
.clang-tidyMoving from clang-tidy 14 to 21 surfaces newer checks. Five are noise/style for this C codebase rather than bug-catchers, so they are disabled (on a full-tree scan they otherwise dominate the output with no correctness signal):
misc-include-cleanermisc-header-include-cyclemodernize-macro-to-enumreadability-inconsistent-declaration-parameter-namebugprone-assignment-in-if-conditionThe bug-catchers stay on, including
bugprone-narrowing-conversions,bugprone-sizeof-expression,bugprone-implicit-widening-of-multiplication-result,bugprone-multi-level-implicit-pointer-conversion,performance-*,clang-analyzer-*andcert-*— the classes that have historically caught real portability / correctness bugs on this project.Residual on existing code (why the gate can land now)
A full-tree scan with this config reports roughly 600 diagnostics on the default-config translation units — dominated by
bugprone-narrowing-conversions— concentrated incore/iwasm/commonandcore/iwasm/interpreter. None of these block contributors: the gate is diff-scoped, so they only surface when a PR actually edits those lines. The residual is cleaned up incrementally in per-subsystem follow-up PRs (ordered stack in #4983).Fork / upstream safety
Same properties as
coding_guidelines.yml: onlycontents: read, diff from the base SHA — so it behaves identically for in-repo and forked pull requests.Notes
pull_requestonly: the gate diffs against the PR base, so a manual (workflow_dispatch) run would have no meaningful base.Verification
Verified end-to-end against real CI on a throwaway fork PR:
bugprone-narrowing-conversionson a changed line of an in-database translation unit fails the job — confirming both that configure-only still parses the linux sources and that the diff-scoped fail path works;compile_commands.json).