Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

Commit

Permalink
Update progress and error reporting in clang-tidy (pytorch#61672)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: pytorch#61672

This PR adds a progress bar to clang-tidy, and updates how it threads error codes (when run in parallel). The progress bar is disabled on GHA because backspace escape codes are not supported.

It also adds a `--quiet` flag to the script.

Screenshot of progress bar:
<img width="955" alt="Screen Shot 2021-07-14 at 3 17 11 PM" src="https://user-images.githubusercontent.com/40111357/125686114-a8a7c154-3e65-43a8-aa8f-c1fb14d51d27.png">

Test Plan: Imported from OSS

Reviewed By: malfet

Differential Revision: D29763848

Pulled By: 1ntEgr8

fbshipit-source-id: cbd352593b279f279911bc3bb8d5ed54abd5f1d5
  • Loading branch information
1ntEgr8 authored and facebook-github-bot committed Jul 19, 2021
1 parent 24a6eb3 commit 66c8d21
Show file tree
Hide file tree
Showing 5 changed files with 400 additions and 203 deletions.
42 changes: 42 additions & 0 deletions .clang-tidy-oss
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
# NOTE there must be no spaces before the '-', so put the comma last.
InheritParentConfig: true
Checks: '
bugprone-*,
-bugprone-forward-declaration-namespace,
-bugprone-macro-parentheses,
-bugprone-lambda-function-name,
-bugprone-reserved-identifier,
cppcoreguidelines-*,
-cppcoreguidelines-avoid-magic-numbers,
-cppcoreguidelines-interfaces-global-init,
-cppcoreguidelines-macro-usage,
-cppcoreguidelines-owning-memory,
-cppcoreguidelines-pro-bounds-array-to-pointer-decay,
-cppcoreguidelines-pro-bounds-constant-array-index,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-cppcoreguidelines-pro-type-cstyle-cast,
-cppcoreguidelines-pro-type-reinterpret-cast,
-cppcoreguidelines-pro-type-static-cast-downcast,
-cppcoreguidelines-pro-type-union-access,
-cppcoreguidelines-pro-type-vararg,
-cppcoreguidelines-special-member-functions,
-facebook-hte-RelativeInclude,
hicpp-exception-baseclass,
hicpp-avoid-goto,
modernize-*,
-modernize-concat-nested-namespaces,
-modernize-return-braced-init-list,
-modernize-use-auto,
-modernize-use-default-member-init,
-modernize-use-using,
-modernize-use-trailing-return-type,
performance-*,
-performance-noexcept-move-constructor,
-performance-unnecessary-value-param,
'
HeaderFilterRegex: 'torch/csrc/.*'
AnalyzeTemporaryDtors: false
WarningsAsErrors: '*'
CheckOptions:
...
17 changes: 16 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,19 @@ jobs:
run: |
cd "${GITHUB_WORKSPACE}"
wget -O pr.diff "https://patch-diff.githubusercontent.com/raw/pytorch/pytorch/pull/$PR_NUMBER.diff"
- name: Generate build files
run: |
cd "${GITHUB_WORKSPACE}"
python3 -m tools.linter.clang_tidy.generate_build_files
- name: Run clang-tidy
run: |
cd "${GITHUB_WORKSPACE}"
# The Docker image has our custom build, so we don't need to install it
python3 -m tools.linter.clang_tidy \
--clang-tidy-exe "$(which clang-tidy)" \
--diff-file pr.diff 2>&1 | tee "${GITHUB_WORKSPACE}"/clang-tidy-output.txt
--diff-file pr.diff \
--disable-progress-bar 2>&1 | tee "${GITHUB_WORKSPACE}"/clang-tidy-output.txt
- name: Annotate output
env:
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
Expand All @@ -354,6 +360,15 @@ jobs:
with:
name: clang-tidy
path: clang-tidy-output/
- name: Check for warnings
run: |
cd "${GITHUB_WORKSPACE}"
set -eu
cat "${GITHUB_WORKSPACE}"/clang-tidy-output.txt
if grep -Fq "Warnings detected!" "${GITHUB_WORKSPACE}"/clang-tidy-output.txt; then
echo 'Please fix the above clang-tidy warnings.'
false
fi
cmakelint:
runs-on: ubuntu-18.04
Expand Down
6 changes: 3 additions & 3 deletions tools/actions_local_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,22 +295,22 @@ class ClangTidy(Check):
common_options = [
"--clang-tidy-exe",
".clang-tidy-bin/clang-tidy",
"--parallel",
]

def filter_files(self, files: List[str]) -> List[str]:
return self.filter_ext(files, {".c", ".cc", ".cpp"})

async def quick(self, files: List[str]) -> CommandResult:
return await shell_cmd(
[sys.executable, "tools/linter/clang_tidy", "--paths"]
[sys.executable, "-m", "tools.linter.clang_tidy", "--paths"]
+ [os.path.join(REPO_ROOT, f) for f in files]
+ self.common_options,
)

async def full(self) -> None:
await shell_cmd(
[sys.executable, "tools/linter/clang_tidy"] + self.common_options
[sys.executable, "-m", "tools.linter.clang_tidy"] + self.common_options,
redirect=False,
)


Expand Down
37 changes: 16 additions & 21 deletions tools/linter/clang_tidy/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import shutil
import subprocess
import re
import sys
from typing import List


Expand Down Expand Up @@ -77,9 +78,9 @@ def clang_search_dirs() -> List[str]:
"paths": ["torch/csrc/"],
"include-dir": ["/usr/lib/llvm-11/include/openmp"] + clang_search_dirs(),
"clang-tidy-exe": INSTALLATION_PATH,
"parallel": True,
"compile-commands-dir": "build",
"config-file": ".clang-tidy",
"config-file": ".clang-tidy-oss",
"disable-progress-bar": False,
}


Expand Down Expand Up @@ -132,24 +133,12 @@ def parse_args() -> argparse.Namespace:
help="Only show the command to be executed, without running it",
)
parser.add_argument("-v", "--verbose", action="store_true", help="Verbose output")
parser.add_argument("-q", "--quiet", action="store_true", help="Don't print output")
parser.add_argument(
"--config-file",
default=DEFAULTS["config-file"],
help="Path to a clang-tidy config file. Defaults to '.clang-tidy'.",
)
parser.add_argument(
"-k",
"--keep-going",
action="store_true",
help="Don't error on compiler errors (clang-diagnostic-error)",
)
parser.add_argument(
"-j",
"--parallel",
action="store_true",
default=DEFAULTS["parallel"],
help="Run clang tidy in parallel per-file (requires ninja to be installed).",
)
parser.add_argument(
"--print-include-paths",
action="store_true",
Expand All @@ -168,6 +157,12 @@ def parse_args() -> argparse.Namespace:
action="store_true",
help="Add NOLINT to suppress clang-tidy violations",
)
parser.add_argument(
"--disable-progress-bar",
action="store_true",
default=DEFAULTS["disable-progress-bar"],
help="Disable the progress bar",
)
parser.add_argument(
"extra_args", nargs="*", help="Extra arguments to forward to clang-tidy"
)
Expand All @@ -185,15 +180,15 @@ def main() -> None:

if not exists:
msg = (
"Could not find '.clang-tidy-bin/clang-tidy'\n"
"You can install it by running:\n"
" python3 tools/linter/install/clang_tidy.py"
f"Could not find '{options.clang_tidy_exe}'\n"
+ "We provide a custom build of clang-tidy that has additional checks.\n"
+ "You can install it by running:\n"
+ "$ python3 tools/linter/install/clang_tidy.py"
)
raise RuntimeError(msg)

return_code = run(options)
if return_code != 0:
raise RuntimeError("Warnings found in clang-tidy output!")
result, _ = run(options)
sys.exit(result.returncode)


if __name__ == "__main__":
Expand Down
Loading

0 comments on commit 66c8d21

Please sign in to comment.