Enable clang-tidy job

.clang-tidy

@@ -7,35 +7,52 @@
 InheritParentConfig: true
 Checks: '
 bugprone-*,
+-bugprone-branch-clone,
 -bugprone-easily-swappable-parameters,
 -bugprone-forward-declaration-namespace,
--bugprone-macro-parentheses,
+-bugprone-implicit-widening-of-multiplication-result,
+-bugprone-infinite-loop,
 -bugprone-lambda-function-name,
+-bugprone-macro-parentheses,
+-bugprone-macro-repeated-side-effects,
+-bugprone-narrowing-conversions,
 -bugprone-reserved-identifier,
 -bugprone-return-const-ref-from-parameter,
+-bugprone-sizeof-expression,
 -bugprone-swapped-arguments,
+-bugprone-switch-missing-default-case,
+-bugprone-unchecked-optional-access,
+-bugprone-unused-local-non-trivial-variable,
 clang-analyzer-core.*,
 clang-analyzer-cplusplus.*,
 clang-analyzer-nullability.*,
 clang-analyzer-deadcode.*,
 clang-diagnostic-missing-prototypes,
 cppcoreguidelines-*,
+-cppcoreguidelines-avoid-c-arrays,
+-cppcoreguidelines-avoid-const-or-ref-data-members,
 -cppcoreguidelines-avoid-do-while,
 -cppcoreguidelines-avoid-magic-numbers,
 -cppcoreguidelines-avoid-non-const-global-variables,
+-cppcoreguidelines-init-variables,
 -cppcoreguidelines-interfaces-global-init,
--cppcoreguidelines-macro-usage,
 -cppcoreguidelines-macro-to-enum,
+-cppcoreguidelines-macro-usage,
+-cppcoreguidelines-narrowing-conversions,
+-cppcoreguidelines-non-private-member-variables-in-classes,
 -cppcoreguidelines-owning-memory,
+-cppcoreguidelines-prefer-member-initializer,
 -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
 -cppcoreguidelines-pro-bounds-constant-array-index,
 -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+-cppcoreguidelines-pro-type-const-cast,
 -cppcoreguidelines-pro-type-cstyle-cast,
+-cppcoreguidelines-pro-type-member-init,
 -cppcoreguidelines-pro-type-reinterpret-cast,
 -cppcoreguidelines-pro-type-static-cast-downcast,
 -cppcoreguidelines-pro-type-union-access,
 -cppcoreguidelines-pro-type-vararg,
--cppcoreguidelines-non-private-member-variables-in-classes,
+-cppcoreguidelines-special-member-functions,
 -facebook-hte-RelativeInclude,
 hicpp-exception-baseclass,
 hicpp-avoid-goto,
@@ -47,35 +64,48 @@ misc-*,
 -misc-unused-parameters,
 -misc-no-recursion,
 -misc-non-private-member-variables-in-classes,
+-misc-redundant-expression,
 -misc-unused-using-decls,
+-misc-use-internal-linkage,
 modernize-*,
+-modernize-avoid-c-arrays,
+-modernize-concat-nested-namespaces,
+-modernize-deprecated-headers,
+-modernize-loop-convert,
 -modernize-macro-to-enum,
 -modernize-return-braced-init-list,
+-modernize-type-traits,
+-modernize-unary-static-assert,
 -modernize-use-auto,
--modernize-use-using,
--modernize-use-trailing-return-type,
+-modernize-use-bool-literals,
+-modernize-use-equals-default,
 -modernize-use-nodiscard,
+-modernize-use-trailing-return-type,
+-modernize-use-using,
 performance-*,
 -performance-enum-size,
+-performance-for-range-copy,
+-performance-unnecessary-value-param,
 readability-container-size-empty,
 readability-delete-null-pointer,
 readability-duplicate-include,
-readability-named-parameter,
 readability-misplaced-array-index,
-readability-redundant*,
 readability-simplify-subscript-expr,
 readability-static-definition-in-anonymous-namespace,
 readability-string-compare,
+-readability-named-parameter,
 -readability-redundant-access-specifiers,
+-readability-redundant-casting,
 -readability-redundant-control-flow,
+-readability-redundant-declaration,
 -readability-redundant-inline-specifier,
+-readability-redundant-smartptr-get,
+-readability-redundant*,
 '
 HeaderFilterRegex: '^(src/).*$'
-WarningsAsErrors: '*'
-LineFilter:
-  - name: '/usr/include/.*'
+WarningsAsErrors: "*"
 CheckOptions:
   cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor: true
   cppcoreguidelines-special-member-functions.AllowImplicitlyDeletedCopyOrMove: true
-  misc-header-include-cycle.IgnoredFilesList: 'format.h;ivalue.h;custom_class.h;Dict.h;List.h;IListRef.h'
+  misc-header-include-cycle.IgnoredFilesList: 'library.h'
 ...

.github/workflows/pull.yml

@@ -34,10 +34,19 @@ jobs:
           export ADDITIONAL_LINTRUNNER_ARGS="--skip CLANGTIDY,CLANGFORMAT,MERGE_CONFLICTLESS_CSV --all-files"
           cd ./torch-xpu-ops
           bash .github/scripts/lintrunner.sh
+      - name: Checkout PyTorch (for headers)
+        uses: actions/checkout@v4
+        with:
+          repository: pytorch/pytorch
+          path: pytorch-headers
+          fetch-depth: 1
+          submodules: false
       - name: Run lint check with Clang
+        env:
+          PYTORCH_ROOT: ${{ github.workspace }}/pytorch-headers
         run: |
           cd ./torch-xpu-ops
-          export ADDITIONAL_LINTRUNNER_ARGS="--take CLANGFORMAT --all-files"
+          export ADDITIONAL_LINTRUNNER_ARGS="--take CLANGFORMAT,CLANGTIDY --all-files"
           bash .github/scripts/lintrunner.sh
 
   conditions-filter-win:

.lintrunner.toml

@@ -134,7 +134,6 @@ command = [
     'python3',
     'tools/linter/adapters/clangtidy_linter.py',
     '--binary=.lintbin/clang-tidy',
-    '--build_dir=./build',
     '--',
     '@{{PATHSFILE}}'
 ]

src/ATen/native/sparse/xpu/sycl/SparseSoftmaxKernels.cpp

@@ -66,7 +66,6 @@
 #include <comm/Memory.h>
 
 #include <ATen/native/sparse/xpu/sycl/SparseSoftmaxKernels.h>
-#include <ATen/native/xpu/sycl/Loops.h>
 #include <comm/SYCLContext.h>
 #include <comm/TensorInfo.h>
 

src/ATen/native/xpu/Activation.cpp

@@ -14,8 +14,6 @@
 #include <ATen/native/DispatchStub.h>
 #include <ATen/native/TensorIterator.h>
 
-#include <ATen/native/TensorIterator.h>
-
 #include <ATen/ops/empty.h>
 #include <ATen/ops/empty_like.h>
 #include <ATen/ops/gelu_backward_native.h>

src/ATen/native/xpu/ForeachOpScalarList.cpp

@@ -26,9 +26,6 @@
 #include <ATen/native/xpu/sycl/ForeachPointwiseOpScalarListKernels.h>
 #include <ATen/native/xpu/sycl/ForeachTernaryOpScalarListKernels.h>
 
-#include <ATen/ops/_foreach_add_native.h>
-#include <ATen/ops/_foreach_mul_native.h>
-
 namespace at {
 namespace native {
 

src/ATen/native/xpu/Resize.cpp

@@ -15,7 +15,6 @@
 #include <comm/xpu_aten.h>
 #include <torch/library.h>
 
-#include <ATen/native/Resize.h>
 #include <ATen/ops/copy.h>
 #include <ATen/ops/resize_native.h>
 #include <ATen/ops/set_native.h>

src/ATen/native/xpu/sycl/AveragePool2dKernels.cpp

@@ -24,8 +24,6 @@
 #include <comm/SYCLContext.h>
 #include <comm/SYCLHelpers.h>
 
-#include <ATen/native/xpu/sycl/AveragePool2dKernels.h>
-
 namespace at::native {
 namespace xpu {
 

src/ATen/native/xpu/sycl/DistributionTemplates.h

@@ -28,8 +28,6 @@
 #include <comm/DeviceProperties.h>
 #include <comm/Runtime.h>
 
-#include <ATen/ops/empty.h>
-
 namespace at {
 namespace native {
 namespace xpu {

src/ATen/native/xpu/sycl/LossNLLKernel.cpp

@@ -23,7 +23,6 @@
 #include <ATen/native/xpu/sycl/Loops.h>
 #include <ATen/native/xpu/sycl/LossNLLKernel.h>
 
-#include <comm/SYCLContext.h>
 #include <comm/TensorInfo.h>
 
 namespace at::native::xpu {

src/ATen/native/xpu/sycl/MultinomialKernel.cpp

@@ -26,8 +26,6 @@
 #include <comm/SYCLContext.h>
 #include <comm/SYCLHelpers.h>
 
-#include <ATen/native/xpu/sycl/MultinomialKernel.h>
-
 namespace at::native::xpu {
 
 template <typename scalar_t, typename item_t>

src/ATen/native/xpu/sycl/RepeatKernel.cpp

@@ -13,8 +13,6 @@
 #include <ATen/native/xpu/sycl/RepeatKernel.h>
 #include <comm/SYCLContext.h>
 
-#include <ATen/native/xpu/sycl/RepeatKernel.h>
-
 namespace at::native::xpu {
 template <typename index_t>
 struct RepeatInterleaveKernelFunctor {

src/ATen/native/xpu/sycl/UnaryComplexKernels.cpp

@@ -20,8 +20,6 @@
 #include <ATen/native/xpu/sycl/Loops.h>
 #include <comm/SYCLContext.h>
 
-#include <ATen/native/xpu/sycl/CopyKernel.h>
-#include <ATen/native/xpu/sycl/Loops.h>
 #include <ATen/native/xpu/sycl/UnaryComplexKernels.h>
 
 namespace at::native::xpu {

src/ATen/native/xpu/sycl/UpSampleNearest1dKernels.cpp

@@ -15,8 +15,6 @@
 #include <comm/SYCLContext.h>
 #include <comm/SYCLHelpers.h>
 
-#include <ATen/native/xpu/sycl/UpSampleNearest1dKernels.h>
-
 namespace at::native {
 namespace xpu {
 

tools/linter/adapters/clangtidy_linter.py

@@ -28,21 +28,15 @@
 from typing import Any, NamedTuple
 
 
-# PyTorch directory root
-def scm_root() -> str:
-    path = os.path.abspath(os.getcwd())
-    while True:
-        if os.path.exists(os.path.join(path, ".git")):
-            return path
-        if os.path.isdir(os.path.join(path, ".hg")):
-            return path
-        n = len(path)
-        path = os.path.dirname(path)
-        if len(path) == n:
-            raise RuntimeError("Unable to find SCM root")
-
-
-PYTORCH_ROOT = scm_root()
+PYTORCH_ROOT = os.environ.get("PYTORCH_ROOT")
+if PYTORCH_ROOT is None:
+     raise RuntimeError(
+         "Environment variable PYTORCH_ROOT is not set. "
+         "Please set PYTORCH_ROOT to the root directory of the PyTorch source "
+         "tree so that clang-tidy can locate the appropriate headers."
+     )
+
+CMPLR_ROOT = os.environ.get("CMPLR_ROOT", "/opt/intel/oneapi/compiler/latest")
 IS_WINDOWS: bool = os.name == "nt"
 
 
@@ -154,22 +148,20 @@ def clang_search_dirs() -> list[str]:
 
 include_args = []
 include_dir = [
-    "/usr/lib/llvm-11/include/openmp",
-    get_python_include_dir(),
-    os.path.join(PYTORCH_ROOT, "third_party/pybind11/include"),
+    os.path.join(PYTORCH_ROOT, "include"),
+    os.path.join(CMPLR_ROOT, "include")
 ] + clang_search_dirs()
 for dir in include_dir:
-    include_args += ["--extra-arg", f"-I{dir}"]
+    include_args += ["--extra-arg", f"-isystem{dir}"]
 
 
 def check_file(
     filename: str,
     binary: str,
-    build_dir: Path,
 ) -> list[LintMessage]:
     try:
         proc = run_command(
-            [binary, f"-p={build_dir}", *include_args, filename],
+            [binary, *include_args, filename],
         )
     except OSError as err:
         return [
@@ -186,31 +178,25 @@ def check_file(
             )
         ]
     lint_messages = []
-    try:
-        # Change the current working directory to the build directory, since
-        # clang-tidy will report files relative to the build directory.
-        saved_cwd = os.getcwd()
-        os.chdir(build_dir)
-
-        for match in RESULTS_RE.finditer(proc.stdout.decode()):
-            # Convert the reported path to an absolute path.
-            abs_path = str(Path(match["file"]).resolve())
-            message = LintMessage(
-                path=abs_path,
-                name=match["code"],
-                description=match["message"],
-                line=int(match["line"]),
-                char=int(match["column"])
-                if match["column"] is not None and not match["column"].startswith("-")
-                else None,
-                code="CLANGTIDY",
-                severity=severities.get(match["severity"], LintSeverity.ERROR),
-                original=None,
-                replacement=None,
-            )
-            lint_messages.append(message)
-    finally:
-        os.chdir(saved_cwd)
+    for match in RESULTS_RE.finditer(proc.stdout.decode()):
+        abs_path = str(Path(match["file"]).resolve())
+        # clang-diagnostic-error cannot be suppressed via .clang-tidy config.
+        if match["code"] == "[clang-diagnostic-error]":
+            continue
+        message = LintMessage(
+            path=abs_path,
+            name=match["code"],
+            description=match["message"],
+            line=int(match["line"]),
+            char=int(match["column"])
+            if match["column"] is not None and not match["column"].startswith("-")
+            else None,
+            code="CLANGTIDY",
+            severity=severities.get(match["severity"], LintSeverity.ERROR),
+            original=None,
+            replacement=None,
+        )
+        lint_messages.append(message)
 
     return lint_messages
 
@@ -225,15 +211,6 @@ def main() -> None:
         required=True,
         help="clang-tidy binary path",
     )
-    parser.add_argument(
-        "--build-dir",
-        "--build_dir",
-        required=True,
-        help=(
-            "Where the compile_commands.json file is located. "
-            "Gets passed to clang-tidy -p"
-        ),
-    )
     parser.add_argument(
         "--verbose",
         action="store_true",
@@ -274,8 +251,6 @@ def main() -> None:
         print(json.dumps(err_msg._asdict()), flush=True)
         sys.exit(0)
 
-    abs_build_dir = Path(args.build_dir).resolve()
-
     # Get the absolute path to clang-tidy and use this instead of the relative
     # path such as .lintbin/clang-tidy. The problem here is that os.chdir is
     # per process, and the linter uses it to move between the current directory
@@ -293,7 +268,6 @@ def main() -> None:
                 check_file,
                 filename,
                 binary_path,
-                abs_build_dir,
             ): filename
             for filename in args.filenames
         }