-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[TEST] Add clang tidy premerge CI [WIP] #154223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesWIP, do not review! Patch is 43.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/154223.diff 5 Files Affected:
diff --git a/.github/workflows/pr-code-lint.yml b/.github/workflows/pr-code-lint.yml
new file mode 100644
index 0000000000000..5b3f9ea624651
--- /dev/null
+++ b/.github/workflows/pr-code-lint.yml
@@ -0,0 +1,120 @@
+name: "Code lint"
+
+permissions:
+ contents: read
+
+on:
+ pull_request:
+ branches:
+ - main
+ - 'users/**'
+ - add-clang-tidy-ci
+ paths:
+ - 'clang-tools-extra/clang-tidy/**'
+ - '.github/workflows/clang-tidy-self-check.yml'
+
+jobs:
+ code_linter:
+ # if: github.repository_owner == 'llvm'
+ runs-on: ubuntu-24.04
+ container:
+ image: 'ghcr.io/llvm/ci-ubuntu-24.04:latest'
+ timeout-minutes: 60
+ concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
+ steps:
+ - name: Fetch LLVM sources
+ uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+ with:
+ fetch-depth: 2
+
+ - name: Get changed files
+ id: changed-files
+ uses: step-security/changed-files@3dbe17c78367e7d60f00d78ae6781a35be47b4a1 # v45.0.1
+ with:
+ separator: ","
+ skip_initial_fetch: true
+ base_sha: 'HEAD~1'
+ sha: 'HEAD'
+
+ - name: Listed files
+ env:
+ CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
+ run: |
+ echo "Changed files:"
+ echo "$CHANGED_FILES"
+
+ - name: Fetch code linting utils
+ uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
+ with:
+ repository: ${{ github.repository }}
+ ref: ${{ github.head_ref }} # FIXME: github.base_ref
+ sparse-checkout: |
+ llvm/utils/git/code-lint-helper.py
+ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+ sparse-checkout-cone-mode: false
+ path: code-lint-tools
+
+ # FIXME: add setup of sccache
+
+ - name: Setup Python env
+ uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0
+ with:
+ python-version: '3.11'
+ cache: 'pip'
+ cache-dependency-path: 'code-format-tools/llvm/utils/git/requirements_linting.txt'
+
+ - name: Install python dependencies
+ run: pip install -r code-format-tools/llvm/utils/git/requirements_linting.txt
+
+ - name: Install clang-tidy
+ uses: aminya/setup-cpp@17c11551771948abc5752bbf3183482567c7caf0 # v1.1.1
+ with:
+ clang-tidy: 20.1.8
+
+ # FIXME: create special mapping for 'gen' targets, for now build predefined set
+ - name: Configure and Build
+ run: |
+ source <(git diff --name-only HEAD~1..HEAD | python3 .ci/compute_projects.py)
+
+ if [[ "${projects_to_build}" == "" ]]; then
+ echo "No projects to build"
+ exit 0
+ fi
+
+ echo "Building projects: ${projects_to_build}"
+ echo "Running project checks targets: ${project_check_targets}"
+
+ cmake -G Ninja \
+ -B build \
+ -S llvm \
+ -DLLVM_ENABLE_ASSERTIONS=OFF \
+ -DLLVM_ENABLE_PROJECTS="${projects_to_build}" \
+ -DCMAKE_CXX_COMPILER=clang++ \
+ -DCMAKE_C_COMPILER=clang \
+ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
+ -DLLVM_INCLUDE_TESTS=OFF \
+ -DCLANG_INCLUDE_TESTS=OFF \
+ -DCMAKE_BUILD_TYPE=Release
+
+ ninja -C build clang-tablegen-targets intrinsics_gen genconfusable
+
+ - name: Run code linter
+ env:
+ CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
+ run: |
+ echo "[]" > comments &&
+ python ./code-lint-tools/llvm/utils/git/code-lint-helper.py \
+ --write-comment-to-file \
+ --start-rev HEAD~1 \
+ --end-rev HEAD \
+ --changed-files "$CHANGED_FILES"
+
+ - name: Upload results
+ uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 #v4.3.0
+ if: always()
+ with:
+ name: workflow-args
+ path: |
+ comments
\ No newline at end of file
diff --git a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
index f458e26d964b0..283063dd69d5f 100644
--- a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
@@ -19,7 +19,7 @@ static void replaceCallWithArg(const CallExpr *Call, DiagnosticBuilder &Diag,
const LangOptions &LangOpts) {
const Expr *Arg = Call->getArg(0);
- CharSourceRange BeforeArgumentsRange = Lexer::makeFileCharRange(
+ CharSourceRange beforeArgumentsRange = Lexer::makeFileCharRange(
CharSourceRange::getCharRange(Call->getBeginLoc(), Arg->getBeginLoc()),
SM, LangOpts);
CharSourceRange AfterArgumentsRange = Lexer::makeFileCharRange(
@@ -27,8 +27,8 @@ static void replaceCallWithArg(const CallExpr *Call, DiagnosticBuilder &Diag,
Call->getEndLoc().getLocWithOffset(1)),
SM, LangOpts);
- if (BeforeArgumentsRange.isValid() && AfterArgumentsRange.isValid()) {
- Diag << FixItHint::CreateRemoval(BeforeArgumentsRange)
+ if (beforeArgumentsRange.isValid() && AfterArgumentsRange.isValid()) {
+ Diag << FixItHint::CreateRemoval(beforeArgumentsRange)
<< FixItHint::CreateRemoval(AfterArgumentsRange);
}
}
diff --git a/llvm/utils/git/code-lint-helper.py b/llvm/utils/git/code-lint-helper.py
new file mode 100755
index 0000000000000..d8e02059a513e
--- /dev/null
+++ b/llvm/utils/git/code-lint-helper.py
@@ -0,0 +1,396 @@
+#!/usr/bin/env python3
+#
+# ====- clang-tidy-helper, runs clang-tidy from the ci --*- python -*--==#
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+# ==--------------------------------------------------------------------------------------==#
+
+import argparse
+import os
+import re
+import shlex
+import subprocess
+import sys
+from typing import List, Optional
+
+"""
+This script is run by GitHub actions to ensure that the code in PR's conform to
+the coding style of LLVM. The canonical source of this script is in the LLVM
+source tree under llvm/utils/git.
+
+You can learn more about the LLVM coding style on llvm.org:
+https://llvm.org/docs/CodingStandards.html
+
+You can control the exact path to clang-tidy with the following environment
+variables: $CLANG_TIDY_PATH.
+"""
+
+
+class TidyArgs:
+ start_rev: str = None
+ end_rev: str = None
+ repo: str = None
+ changed_files: List[str] = []
+ token: str = None
+ issue_number: int = 0
+ write_comment_to_file: bool = False
+
+ def __init__(self, args: argparse.Namespace = None) -> None:
+ if not args is None:
+ self.start_rev = args.start_rev
+ self.end_rev = args.end_rev
+ self.repo = args.repo
+ #self.token = args.token
+ self.changed_files = args.changed_files
+ #self.issue_number = args.issue_number
+ self.write_comment_to_file = args.write_comment_to_file
+
+
+class TidyHelper:
+ COMMENT_TAG = "<!--LLVM CLANG-TIDY COMMENT: {fmt}-->"
+ name: str
+ friendly_name: str
+ comment: dict = None
+
+ @property
+ def comment_tag(self) -> str:
+ return self.COMMENT_TAG.replace("fmt", self.name)
+
+ @property
+ def instructions(self) -> str:
+ raise NotImplementedError()
+
+ def has_tool(self) -> bool:
+ raise NotImplementedError()
+
+ def lint_run(self, changed_files: List[str], args: TidyArgs) -> Optional[str]:
+ raise NotImplementedError()
+
+ def pr_comment_text_for_diff(self, diff: str) -> str:
+ return f"""
+:warning: {self.friendly_name}, {self.name} found issues in your code. :warning:
+
+<details>
+<summary>
+You can test this locally with the following command:
+</summary>
+
+``````````bash
+{self.instructions}
+``````````
+
+</details>
+
+<details>
+<summary>
+View the diff from {self.name} here.
+</summary>
+
+``````````diff
+{diff}
+``````````
+
+</details>
+"""
+
+ # TODO: any type should be replaced with the correct github type, but it requires refactoring to
+ # not require the github module to be installed everywhere.
+ """
+ def find_comment(self, pr: any) -> any:
+ for comment in pr.as_issue().get_comments():
+ if self.comment_tag in comment.body:
+ return comment
+ return None
+
+ def update_pr(self, comment_text: str, args: TidyArgs, create_new: bool) -> None:
+ import github
+ from github import IssueComment, PullRequest
+
+ repo = github.Github(args.token).get_repo(args.repo)
+ pr = repo.get_issue(args.issue_number).as_pull_request()
+
+ comment_text = self.comment_tag + "\n\n" + comment_text
+
+ existing_comment = self.find_comment(pr)
+
+ if args.write_comment_to_file:
+ if create_new or existing_comment:
+ self.comment = {"body": comment_text}
+ if existing_comment:
+ self.comment["id"] = existing_comment.id
+ return
+
+ if existing_comment:
+ existing_comment.edit(comment_text)
+ elif create_new:
+ pr.as_issue().create_comment(comment_text)
+ """
+
+ def run(self, changed_files: List[str], args: TidyArgs) -> bool:
+ changed_files = [arg for arg in changed_files if "third-party" not in arg]
+ print(f"got changed_files: {changed_files}")
+ diff = self.lint_run(changed_files, args)
+ print(f"got diff {diff}")
+ should_update_gh = True
+ #args.token is not None and args.repo is not None
+
+ if diff is None:
+ if should_update_gh:
+ comment_text = (
+ ":white_check_mark: With the latest revision "
+ f"this PR passed the {self.friendly_name}."
+ )
+ print(comment_text)
+ # self.update_pr(comment_text, args, create_new=False)
+ return True
+ elif len(diff) > 0:
+ if should_update_gh:
+ comment_text = self.pr_comment_text_for_diff(diff)
+ print(comment_text)
+ # self.update_pr(comment_text, args, create_new=True)
+ else:
+ print(
+ f"Warning: {self.friendly_name}, {self.name} detected "
+ "some issues with your code formatting..."
+ )
+ return False
+ else:
+ # The formatter failed but didn't output a diff (e.g. some sort of
+ # infrastructure failure).
+ comment_text = (
+ f":warning: The {self.friendly_name} failed without printing "
+ "a diff. Check the logs for stderr output. :warning:"
+ )
+ print(comment_text)
+ # self.update_pr(comment_text, args, create_new=False)
+ return False
+
+
+class ClangTidyDiffHelper(TidyHelper):
+ name = "clang-tidy"
+ friendly_name = "C/C++ code linter"
+
+ def __init__(self, build_path: str = "build", clang_tidy_binary: str = "clang-tidy"):
+ self.build_path = build_path
+ self.clang_tidy_binary = clang_tidy_binary
+ self.cpp_files = []
+
+ @property
+ def instructions(self) -> str:
+ return f"""
+git diff -U0 origin/main..HEAD -- {self.cpp_files} |
+python3 clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py \\
+ -path {self.build_path} -p1
+
+# See https://clang.llvm.org/extra/clang-tidy/#using-clang-tidy for more
+# instructions on how to use clang-tidy"""
+
+ def filter_changed_files(self, changed_files: List[str]) -> List[str]:
+ filtered_files = []
+ print(f"filtering changed files: {filtered_files}")
+ for path in changed_files:
+ print(f"file: {path}")
+ if not path.startswith("clang-tools-extra/clang-tidy/"):
+ print("continue")
+ continue
+ _, ext = os.path.splitext(path)
+ if ext in (".cpp", ".c", ".h", ".hpp", ".hxx", ".cxx"):
+ if os.path.exists(path):
+ print("appending")
+ filtered_files.append(path)
+ else:
+ print("skipped")
+ else:
+ print(f"wrong ext {ext}")
+ return filtered_files
+
+ @property
+ def clang_tidy_path(self) -> str:
+ if "CLANG_TIDY_PATH" in os.environ:
+ return os.environ["CLANG_TIDY_PATH"]
+ return self.clang_tidy_binary
+
+ def has_tool(self) -> bool:
+ cmd = [self.clang_tidy_path, "--version"]
+ proc = None
+ try:
+ proc = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ except:
+ return False
+ return proc.returncode == 0
+
+ def lint_run(self, changed_files: List[str], args: TidyArgs) -> Optional[str]:
+ cpp_files = self.filter_changed_files(changed_files)
+ if not cpp_files:
+ print("no cpp files!")
+ return None
+
+ print(f"Generating diff: begin: {args.start_rev}, end: {args.end_rev}")
+ git_diff_cmd = [
+ "git", "diff", "-U0",
+ f"{args.start_rev}..{args.end_rev}",
+ "--"
+ ] + cpp_files
+
+ print(f"Generating diff: {' '.join(git_diff_cmd)}")
+
+ diff_proc = subprocess.run(
+ git_diff_cmd,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ text=True,
+ check=False
+ )
+
+ if diff_proc.returncode != 0:
+ print(f"Git diff failed: {diff_proc.stderr}")
+ return None
+
+ diff_content = diff_proc.stdout
+ if not diff_content.strip():
+ print("No diff content found")
+ return None
+
+ # Run clang-tidy-diff.py
+ tidy_diff_cmd = [
+ "code-lint-tools/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py",
+ "-path", self.build_path,
+ "-p1",
+ "-quiet"
+ ]
+
+ print(f"Running clang-tidy-diff: {' '.join(tidy_diff_cmd)}")
+
+ proc = subprocess.run(
+ tidy_diff_cmd,
+ input=diff_content,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ text=True,
+ check=False
+ )
+
+ output = proc.stdout.strip()
+ if output:
+ # Check if clang-tidy-diff found no relevant changes
+ if output == "No relevant changes found.":
+ return None # No issues found, this is success
+ # Filter out summary lines like "N warnings generated"
+ lines = output.split('\n')
+ filtered_lines = []
+ for line in lines:
+ if line.strip() and not line.endswith('generated.'):
+ filtered_lines.append(line)
+ if filtered_lines:
+ return '\n'.join(filtered_lines)
+
+ return None
+
+ def pr_comment_text_for_diff(self, warnings: str) -> str:
+ return f"""
+:warning: {self.friendly_name} found issues in your code. :warning:
+
+<details>
+<summary>
+You can test this locally with the following command:
+</summary>
+
+``````````bash
+{self.instructions}
+``````````
+
+</details>
+
+<details>
+<summary>
+View the warnings from {self.name} here.
+</summary>
+
+``````````
+{warnings}
+``````````
+
+</details>
+"""
+
+
+if __name__ == "__main__":
+ parser = argparse.ArgumentParser()
+ # parser.add_argument(
+ # "--token", type=str, required=True, help="GitHub authentiation token"
+ # )
+ parser.add_argument(
+ "--repo",
+ type=str,
+ default=os.getenv("GITHUB_REPOSITORY", "llvm/llvm-project"),
+ help="The GitHub repository that we are working with in the form of <owner>/<repo> (e.g. llvm/llvm-project)",
+ )
+ # parser.add_argument("--issue-number", type=int, required=True)
+ parser.add_argument(
+ "--start-rev",
+ type=str,
+ required=True,
+ help="Compute changes from this revision.",
+ )
+ parser.add_argument(
+ "--end-rev", type=str, required=True, help="Compute changes to this revision"
+ )
+ parser.add_argument(
+ "--changed-files",
+ type=str,
+ help="Comma separated list of files that has been changed",
+ )
+ parser.add_argument(
+ "--write-comment-to-file",
+ action="store_true",
+ help="Don't post comments on the PR, instead write the comments and metadata a file called 'comment'",
+ )
+ parser.add_argument(
+ "--build-path",
+ type=str,
+ default="build",
+ help="Path to build directory with compile_commands.json"
+ )
+ parser.add_argument(
+ "--clang-tidy-binary",
+ type=str,
+ default="clang-tidy",
+ help="Path to clang-tidy binary"
+ )
+
+ parsed_args = parser.parse_args()
+ args = TidyArgs(parsed_args)
+
+ changed_files = []
+ if args.changed_files:
+ changed_files = args.changed_files.split(",")
+
+ failed_linters = []
+ comments = []
+ clang_tidy_runner = ClangTidyDiffHelper(
+ build_path=parsed_args.build_path,
+ clang_tidy_binary=parsed_args.clang_tidy_binary
+ )
+
+ print("running tool")
+ if not clang_tidy_runner.run(changed_files, args):
+ print("adding failed")
+ failed_linters.append(clang_tidy_runner.name)
+ if clang_tidy_runner.comment:
+ print("adding comment")
+ comments.append(clang_tidy_runner.comment)
+
+ if len(comments) > 0:
+ print("dumping comments")
+ with open("comments", "w") as f:
+ import json
+
+ json.dump(comments, f)
+ print("dumping done!")
+
+ if len(failed_linters) > 0:
+ print(f"error: some linters failed: {' '.join(failed_linters)}")
+ sys.exit(1)
diff --git a/llvm/utils/git/requirements_linting.txt b/llvm/utils/git/requirements_linting.txt
new file mode 100644
index 0000000000000..2a18fe654fdc2
--- /dev/null
+++ b/llvm/utils/git/requirements_linting.txt
@@ -0,0 +1,324 @@
+#
+# This file is autogenerated by pip-compile with Python 3.11
+# by the following command:
+#
+# pip-compile --generate-hashes --output-file=requirements_linting.txt requirements_linting.txt.in
+#
+certifi==2025.8.3 \
+ --hash=sha256:e564105f78ded564e3ae7c923924435e1daa7463faeab5bb932bc53ffae63407 \
+ --hash=sha256:f6c12493cfb1b06ba2ff328595af9350c65d6644968e5d3a2ffd78699af217a5
+ # via requests
+cffi==1.17.1 \
+ --hash=sha256:045d61c734659cc045141be4bae381a41d89b741f795af1dd018bfb532fd0df8 \
+ --hash=sha256:0984a4925a435b1da406122d4d7968dd861c1385afe3b45ba82b750f229811e2 \
+ --hash=sha256:0e2b1fac190ae3ebfe37b979cc1ce69c81f4e4fe5746bb401dca63a9062cdaf1 \
+ --hash=sha256:0f048dcf80db46f0098ccac01132761580d28e28bc0f78ae0d58048063317e15 \
+ --hash=sha256:1257bdabf294dceb59f5e70c64a3e2f462c30c7ad68092d01bbbfb1c16b1ba36 \
+ --hash=sha256:1c39c6016c32bc48dd54561950ebd6836e1670f2ae46128f67cf49e789c52824 \
+ --hash=sha256:1d599671f396c4723d016dbddb72fe8e0397082b0a77a4fab8028923bec050e8 \
+ --hash=sha256:28b16024becceed8c6dfbc75629e27788d8a3f9030691a1dbf9821a128b22c36 \
+ --hash=sha256:2bb1a08b8008b281856e5971307cc386a8e9c5b625ac297e853d36da6efe9c17 \
+ --hash=sha256:30c5e0cb5ae493c04c8b42916e52ca38079f1b235c2f8ae5f4527b963c401caf \
+ --hash=sha256:31000ec67d4221a71bd3f67df918b1f88f676f1c3b535a7eb473255fdc0b83fc \
+ --hash=sha256:386c8bf53c502fff58903061338ce4f4950cbdcb23e2902d86c0f722b786bbe3 \
+ --hash=sha256:3edc8d958eb099c634dace3c7e16560ae474aa3803a5df240542b305d14e14ed \
+ --hash=sha256:45398b671ac6d70e67da8e4224a065cec6a93541bb7aebe1b198a61b58c7b702 \
+ --hash=sha256:46bf43160c1a35f7ec506d254e5c890f3c03648a4dbac12d624e4490a7046cd1 \
+ --hash=sha256:4ceb10419a9adf4460ea14cfd6bc43d08701f0835e979bf821052f1805850fe8 \
+ --hash=sha256:51...
[truncated]
|
✅ With the latest revision this PR passed the Python code formatter. |
You can test this locally with the following command:git diff -U0 origin/main..HEAD -- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp |
python3 clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py \
-path build -p1 -quiet View the output from clang-tidy here.
|
7a7e158
to
f922198
Compare
a2e6e1b
to
6fe9a55
Compare
6fe9a55
to
084eee2
Compare
You can test this locally with the following command:git diff -U0 origin/main...HEAD -- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp |
python3 clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py \
-path build -p1 -quiet View the output from clang-tidy here.
|
5296562
to
084eee2
Compare
e29c073
to
b6f3720
Compare
b6f3720
to
b81d4d2
Compare
3167674
to
f0cfcfa
Compare
55fd001
to
6e0c5fb
Compare
For test purposes, do not review!