Enable clang-tidy job by Silv3S · Pull Request #3062 · intel/torch-xpu-ops

Silv3S · 2026-03-16T09:50:31Z

This PR enables clang-tidy as one of checks in preci-lint-check. Some changes were required to make it working:

clone PyTorch in preci-lint-check to have headers added to include paths,
add SYCL include dir,
drop dependency on build dir, as in preci-lint-check nothing is built yet,
ignore clang-diagnostic error which are not configurable from .clang-tidy file,
bump clang-tidy binary in s3 config to match upstream version,
skip all known issues (and convert them into subtasks when this PR is merged)

This PR also fixes readability-duplicate-include:

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR enables clang-tidy as a lint check in the preci-lint-check CI job. It removes the dependency on a build directory (compile_commands.json), clones PyTorch headers for include paths, bumps the clang-tidy binary version to 19.1.4, and suppresses known issues in the .clang-tidy config. Several source files also have duplicate/unnecessary includes removed.

Changes:

Removed build-dir dependency from clang-tidy linter, using PYTORCH_ROOT env var and SYCL include discovery instead
Updated .clang-tidy config to disable many additional checks (known issues) and bumped the clang-tidy S3 binary to version 19.1.4 (Linux only, Darwin entries removed)
Cleaned up duplicate #include directives across multiple source files

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`.clang-tidy`	Expanded list of disabled checks; removed Darwin-specific LineFilter
`tools/linter/adapters/clangtidy_linter.py`	Replaced `scm_root()` with `PYTORCH_ROOT` env var; removed build-dir logic; added SYCL include dir; suppressed `clang-diagnostic-error`
`tools/linter/adapters/s3_init_config.json`	Bumped clang-tidy to 19.1.4; removed Darwin entries
`.lintrunner.toml`	Removed `--build_dir` argument
`.github/workflows/pull.yml`	Added PyTorch checkout step for headers; enabled CLANGTIDY in lint
`src/ATen/native/xpu/**` (multiple files)	Removed duplicate/unnecessary `#include` directives

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tools/linter/adapters/clangtidy_linter.py

.clang-tidy

tools/linter/adapters/clangtidy_linter.py

Copilot

Pull request overview

This PR enables the clang-tidy linter job in the preci-lint-check CI workflow. It removes the dependency on a build directory (compile_commands.json), clones PyTorch for headers, updates the clang-tidy binary version, cleans up duplicate/unused includes found by the linter, and expands the .clang-tidy suppression list to skip known issues.

Changes:

Rewires clangtidy_linter.py to use a PYTORCH_ROOT env variable instead of scanning for SCM root, removes build-dir dependency, adds system include paths, and silently skips clang-diagnostic-error diagnostics.
Updates .clang-tidy configuration with many additional check suppressions and bumps the clang-tidy binary in s3_init_config.json from 17.0.6 to 19.1.4 (Linux only, Darwin entries removed).
Cleans up duplicate/unused #include directives across ~10 source files and enables CLANGTIDY in the CI workflow alongside CLANGFORMAT.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`.clang-tidy`	Expanded suppression list for known issues, sorted checks, removed `LineFilter`
`tools/linter/adapters/clangtidy_linter.py`	Replaced SCM root detection with `PYTORCH_ROOT` env var, removed build-dir dependency, updated include paths
`tools/linter/adapters/s3_init_config.json`	Bumped clang-tidy to 19.1.4, removed Darwin entries
`.lintrunner.toml`	Removed `--build_dir` argument from clang-tidy linter config
`.github/workflows/pull.yml`	Added PyTorch checkout step, enabled `CLANGTIDY` in Clang lint step
`src/ATen/native/xpu/sycl/UpSampleNearest1dKernels.cpp`	Removed duplicate include
`src/ATen/native/xpu/sycl/UnaryComplexKernels.cpp`	Removed duplicate includes
`src/ATen/native/xpu/sycl/RepeatKernel.cpp`	Removed duplicate include
`src/ATen/native/xpu/sycl/MultinomialKernel.cpp`	Removed duplicate include
`src/ATen/native/xpu/sycl/LossNLLKernel.cpp`	Removed unused include
`src/ATen/native/xpu/sycl/DistributionTemplates.h`	Removed unused include
`src/ATen/native/xpu/sycl/AveragePool2dKernels.cpp`	Removed duplicate include
`src/ATen/native/xpu/Resize.cpp`	Removed duplicate include
`src/ATen/native/xpu/ForeachOpScalarList.cpp`	Removed unused includes
`src/ATen/native/xpu/Activation.cpp`	Removed duplicate include
`src/ATen/native/sparse/xpu/sycl/SparseSoftmaxKernels.cpp`	Removed unused include

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tools/linter/adapters/clangtidy_linter.py

.clang-tidy

tools/linter/adapters/clangtidy_linter.py

Silv3S · 2026-03-16T14:24:30Z

@guangyey, @chuanqi129 could you please take a look? I wonder if CLANGTIDY should be on-demand workflow or integrated directly in pull.yml. It takes ~20s to complete and list of currently failing checks is significant, therefore I skipped most of the failing and will create subtasks for each failure

chuanqi129 · 2026-03-17T01:18:47Z

Hi @Silv3S, let's add it into current lint check job by default, but we need to make the test pass, whatever we skip some failures as post fix or fix them directly. And please rebase the PR to trigger the CI test

tools/linter/adapters/clangtidy_linter.py

guangyey · 2026-03-17T03:02:04Z

.clang-tidy

 InheritParentConfig: true
 Checks: '
 bugprone-*,
+-bugprone-branch-clone,


This file seems different from https://github.com/pytorch/pytorch/blob/main/.clang-tidy. Could you help me understand the difference.

Sure. In upstream config there are groups of checks enabled, for example all checks from bugprone- category, but some are failing, so individual checks can be skipped with - prefix. It totally disables the check. Also, one can use in-line comment with NOLINT(check-name) to skip particular occurrence of failed check.

Some checks are debatable and implementing them is not worth it, so explicit skip allows us to ignore one check while enabling all other checks from this category. One example is modernize-concat-nested-namespaces which enforces squashing namespaces, that is purely formatting that depends mostly on personal preferences.

If we used upstream config without modifications, CI would fail on preci-lint-check as we have more issues in codebase than upstream. That's why I extended config with few explicitly skipped checks. Now it will pass CI and guard from further regressions. I plan to create tasks to enable each check one-by-one to not introduce them in one PR.

Thanks for your details.

guangyey · 2026-03-17T04:13:22Z

.github/workflows/pull.yml

+          repository: pytorch/pytorch
+          path: pytorch-headers
+          fetch-depth: 1
+          submodules: false


There are some code gen files that are handled in

torch-xpu-ops/.github/scripts/lintrunner.sh

Lines 37 to 38 in e3bd5f9

if [[ -e "third_party/torch-xpu-ops/tools/linter/clang_tidy/generate_build_files.py" ]];then

python3 third_party/torch-xpu-ops/tools/linter/clang_tidy/generate_build_files.py

Please ensure these paths are matched.

I think it would require extracting clang-tidy to separate CI job that builds PyTorch+XPU and pass compile_commands.json to linter. Currently clang-tidy only clones PyTorch (for headers) and torch-xpu-ops to lint. It doesn't build nor generate any files, and both conditions

if [[ "${CLANG}" == "1" ]]; then if [[ -e "third_party/torch-xpu-ops/tools/linter/clang_tidy/generate_build_files.py" ]];then

are not met.

This lightweight version lets us quickly fix issues that reproduce without build dependency. It's fast so can guard any regressions in preci-lint-check stage. I agree that it should be extended to cover autogen files, but maybe it could be done in next iteration?

guangyey

Sounds good at this stage!

Copilot

Pull request overview

This PR enables clang-tidy as part of the preci-lint-check CI workflow and updates repository/configuration to support running it without a build directory, while also cleaning up several duplicate includes to satisfy readability-duplicate-include.

Changes:

Enable CLANGTIDY in the GitHub Actions preci-lint-check job and check out a PyTorch repo for headers.
Update the clang-tidy adapter to remove -p/build-dir dependency, adjust include args, and ignore clang-diagnostic-error.
Remove a set of duplicate includes across multiple XPU/SYCL source files and expand .clang-tidy check configuration.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tools/linter/adapters/s3_init_config.json	Bumps clang-tidy binary version and updates S3 download config.
tools/linter/adapters/clangtidy_linter.py	Refactors clang-tidy invocation to run without a build directory and adds env-based header root configuration.
.lintrunner.toml	Removes the now-unused `--build_dir` argument from the clang-tidy command.
.github/workflows/pull.yml	Runs clang-tidy in CI and checks out PyTorch headers to support parsing.
.clang-tidy	Expands the enabled/disabled checks list and updates some options.
src/ATen/native/xpu/sycl/UpSampleNearest1dKernels.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/sycl/UnaryComplexKernels.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/sycl/RepeatKernel.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/sycl/MultinomialKernel.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/sycl/LossNLLKernel.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/sycl/DistributionTemplates.h	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/sycl/AveragePool2dKernels.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/Resize.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/ForeachOpScalarList.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/xpu/Activation.cpp	Removes redundant includes to satisfy duplicate-include checks.
src/ATen/native/sparse/xpu/sycl/SparseSoftmaxKernels.cpp	Removes redundant includes to satisfy duplicate-include checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tools/linter/adapters/clangtidy_linter.py

tools/linter/adapters/s3_init_config.json

tools/linter/adapters/clangtidy_linter.py

guangyey · 2026-03-19T08:16:12Z

@EikanWang any idea?

Silv3S and others added 30 commits March 5, 2026 12:18

Hardcode build path to enable lints

61604ed

Exclude sycl

7d3f8eb

Temporary skip known issues

026d2d1

list todos

c29ab79

readability-duplicate-include

d5d7f65

bugprone-argument-comment

96e7b3e

bugprone-branch-clone

1358822

modernize-use-bool-literals

ffaa7dc

misc-header-include-cycle

a1c75a2

readability-container-size-empty

ec4779b

modernize-use-equals-default

f96c03f

Update reducer_xpu.cpp

8c0900a

helper scripts

81f2e28

use-default-member-init

702dc6c

modernize-type-traits

641fa1a

modernize-use-transparent-functors

5434b63

cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays

c53d0d9

Remove tools for local repro

1a96ac4

remove todo

3c05940

Merge branch 'main' into clangtidy

109b931

Fix condition

3a8b317

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Skip known failures

bfe5ec1

Bump clang-tidy binary and adjust include paths

5aca925

readability-duplicate-include

d088b90

modernize-type-traits

0058c41

modernize-deprecated-headers

ea3a972

modernize-type-traits

935d151

Copy upstream config and add CI job

1e6daa1

temp skip CI for clang-tidy stage enablement

43f160b

remove preci

d0b8b0b

Silv3S added 2 commits March 16, 2026 12:14

Fail readability-duplicate-include

92fcb68

Remove duplicate includes

93395e9

Silv3S marked this pull request as ready for review March 16, 2026 12:31

Copilot AI review requested due to automatic review settings March 16, 2026 12:31

Copilot started reviewing on behalf of Silv3S March 16, 2026 12:31 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

tools/linter/adapters/clangtidy_linter.py Show resolved Hide resolved

.clang-tidy Show resolved Hide resolved

.clang-tidy Show resolved Hide resolved

tools/linter/adapters/clangtidy_linter.py Show resolved Hide resolved

Silv3S added 3 commits March 16, 2026 12:44

Add asserts for envs

db35537

Reduce duplications in config

f239e53

hardcode CMPRL_ROOT

49e9a73

Copilot AI review requested due to automatic review settings March 16, 2026 12:58

Copilot started reviewing on behalf of Silv3S March 16, 2026 12:58 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

tools/linter/adapters/clangtidy_linter.py Show resolved Hide resolved

.clang-tidy Show resolved Hide resolved

tools/linter/adapters/clangtidy_linter.py Show resolved Hide resolved

tools/linter/adapters/clangtidy_linter.py Outdated Show resolved Hide resolved

Silv3S requested review from chuanqi129 and guangyey March 16, 2026 14:22

Silv3S added disable_e2e Disable all e2e test jobs for the PR disable_distributed Disable distributed UT test jobs for the PR disable_accelerate Disable accelerate test job in PR CI testing disable_transformers Disable transformers UT test in PR CI labels Mar 16, 2026

guangyey reviewed Mar 17, 2026

View reviewed changes

tools/linter/adapters/clangtidy_linter.py Outdated Show resolved Hide resolved

guangyey reviewed Mar 17, 2026

View reviewed changes

use CMPLR_ROOT

9ca3265

guangyey approved these changes Mar 19, 2026

View reviewed changes

Merge branch 'main' into clangtidy_actions

bf0addc

Copilot AI review requested due to automatic review settings March 19, 2026 07:46

Copilot AI reviewed Mar 19, 2026

View reviewed changes

guangyey requested a review from EikanWang March 19, 2026 08:15

	if [[ -e "third_party/torch-xpu-ops/tools/linter/clang_tidy/generate_build_files.py" ]];then
	python3 third_party/torch-xpu-ops/tools/linter/clang_tidy/generate_build_files.py

Conversation

Silv3S commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Silv3S commented Mar 16, 2026

Uh oh!

chuanqi129 commented Mar 17, 2026

Uh oh!

Uh oh!

guangyey Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Silv3S Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

guangyey Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

guangyey Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Silv3S Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guangyey left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guangyey commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Silv3S commented Mar 16, 2026 •

edited

Loading

Silv3S Mar 17, 2026 •

edited

Loading