[TEST] Add clang tidy premerge CI [WIP] #154223
Closed
Conversation
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
| @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 -quietView the output from clang-tidy here. |
vbvictor force-pushed the add-clang-tidy-ci branch from 7a7e158 to f922198 Compare vbvictor force-pushed the add-clang-tidy-ci branch from a2e6e1b to 6fe9a55 Compare vbvictor force-pushed the add-clang-tidy-ci branch from 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 -quietView the output from clang-tidy here. |
vbvictor force-pushed the add-clang-tidy-ci branch from 5296562 to 084eee2 Compare vbvictor changed the title vbvictor force-pushed the add-clang-tidy-ci branch 5 times, most recently from e29c073 to b6f3720 Compare vbvictor force-pushed the add-clang-tidy-ci branch from b6f3720 to b81d4d2 Compare vbvictor force-pushed the add-clang-tidy-ci branch 4 times, most recently from 3167674 to f0cfcfa Compare vbvictor force-pushed the add-clang-tidy-ci branch from 55fd001 to 6e0c5fb Compare | Original PR was merged |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
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.
For test purposes, do not review!