diff --git a/.github/workflows/sycl-clang-tidy.yml b/.github/workflows/sycl-clang-tidy.yml new file mode 100644 index 0000000000000..d258b5527e6f3 --- /dev/null +++ b/.github/workflows/sycl-clang-tidy.yml @@ -0,0 +1,80 @@ +name: clang-tidy + +on: + workflow_call: + +permissions: read-all + +jobs: + run-clang-tidy: + runs-on: [Linux, build] + container: + image: ghcr.io/intel/llvm/sycl_ubuntu2404_nightly:latest + options: -u 1001:1001 + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + sparse-checkout: | + devops/actions + devops/scripts + - name: Register cleanup after job is finished + uses: ./devops/actions/cleanup + + - name: Check diff + id: should_run_tidy + run: | + echo "Downloading the diff" + curl -L "${{ github.server_url }}/${{ github.repository }}/pull/${{ github.event.pull_request.number }}.diff" -o pr.diff + + if python3 devops/scripts/should_run_clang-tidy.py pr.diff; then + echo "run=true" >> "$GITHUB_OUTPUT" + else + echo "run=false" >> "$GITHUB_OUTPUT" + fi + + - if: steps.should_run_tidy.outputs.run == 'true' + uses: ./devops/actions/cached_checkout + with: + path: src + ref: ${{ github.sha }} + cache_path: "/__w/repo_cache/" + - name: Configure + if: steps.should_run_tidy.outputs.run == 'true' + env: + CC: gcc + CXX: g++ + CUDA_LIB_PATH: "/usr/local/cuda/lib64/stubs" + run: | + mkdir -p $GITHUB_WORKSPACE/build + cd $GITHUB_WORKSPACE/build + python3 $GITHUB_WORKSPACE/src/buildbot/configure.py -w $GITHUB_WORKSPACE \ + -s $GITHUB_WORKSPACE/src -o $GITHUB_WORKSPACE/build -t Release \ + --ci-defaults -DCMAKE_EXPORT_COMPILE_COMMANDS=ON + + - name: Preprocess compile_commands.json + if: steps.should_run_tidy.outputs.run == 'true' + run: | + # Remove commands containing "-D__INTEL_PREVIEW_BREAKING_CHANGES" to avoid running on the same file twice. + jq '[ .[] | select(.command | contains("-D__INTEL_PREVIEW_BREAKING_CHANGES") | not) ]' $GITHUB_WORKSPACE/build/compile_commands.json > $GITHUB_WORKSPACE/build/compile_commands.temp.json + mv $GITHUB_WORKSPACE/build/compile_commands.temp.json $GITHUB_WORKSPACE/build/compile_commands.json + + # Remove gcc-specific flags + perl -0777 -pe ' + s/(?:^|[ \t])\-Wno-class-memaccess(?=$|[ \t])//g; + s/(?:^|[ \t])\-Wno-dangling-reference(?=$|[ \t])//g; + s/(?:^|[ \t])\-Wno-stringop-overread(?=$|[ \t])//g; + s/[ \t]{2,}/ /g; + ' -i $GITHUB_WORKSPACE/build/compile_commands.json + + - name: Run clang-tidy on modified files + if: steps.should_run_tidy.outputs.run == 'true' + # Exeprimental workflow, it won't affect the pre-commit status in case of failure. + continue-on-error: true + run: | + cd "$GITHUB_WORKSPACE/src" + python3 "$GITHUB_WORKSPACE/src/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py" \ + -clang-tidy-binary "/opt/sycl/bin/clang-tidy" \ + -p 1 \ + -path "$GITHUB_WORKSPACE/build" \ + -checks "clang-analyzer-*,bugprone-*,performance-*,-bugprone-std-namespace-modification" \ + < "$GITHUB_WORKSPACE/pr.diff" diff --git a/.github/workflows/sycl-linux-precommit.yml b/.github/workflows/sycl-linux-precommit.yml index 70c7a5c4bee2c..7b871b36243a9 100644 --- a/.github/workflows/sycl-linux-precommit.yml +++ b/.github/workflows/sycl-linux-precommit.yml @@ -68,6 +68,10 @@ jobs: e2e_binaries_preview_artifact: e2e_bin_preview e2e_binaries_new_offload_model_artifact: e2e_bin_new_offload_model + clang-tidy: + if: ${{ !contains(github.event.pull_request.labels.*.name, 'disable-lint') }} + uses: ./.github/workflows/sycl-clang-tidy.yml + # Build and run native cpu e2e tests separately as cannot currently # build all the e2e tests build_run_native_cpu_e2e_tests: diff --git a/devops/scripts/should_run_clang-tidy.py b/devops/scripts/should_run_clang-tidy.py new file mode 100644 index 0000000000000..11f4da1f33669 --- /dev/null +++ b/devops/scripts/should_run_clang-tidy.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 + +# This script checks if a diff contains changes that should be inspected by +# clang-tidy. + +from __future__ import annotations +import re +import sys +from typing import Iterator + +EXCLUDE_PATH_RE = re.compile(r"(?:^|/)(test|test-e2e|unittests)(?:/|$)") + + +def is_relevant_path(path: str) -> bool: + return EXCLUDE_PATH_RE.search(path) is None + + +# This iterator splits a diff file into separate sections like: +# diff --git a/path-to/file b/path-to/file +# ... code changes ... +def iter_diff_sections(diff_content: str) -> Iterator[str]: + section_lines: list[str] = [] + started = False + + for line in diff_content.splitlines(True): # keep '\n' + if line.startswith("diff --git "): + if started and section_lines: + yield "".join(section_lines) + section_lines = [] + started = True + + if started: + section_lines.append(line) + + if started and section_lines: + yield "".join(section_lines) + + +def main() -> int: + diff_path = sys.argv[1] + with open(diff_path, "r", encoding="utf-8", errors="replace") as f: + diff_content = f.read() + + should_run = False + for section in iter_diff_sections(diff_content): + lines = section.splitlines() + # Skip removed files. + if lines[4] == "+++ /dev/null": + continue + result_file = lines[3] + # Skip non-c++ files. + if not result_file.endswith((".cpp", ".hpp", ".h")): + continue + # Skip tests etc. + if not is_relevant_path(result_file): + continue + # Check if any non-comment string was added. + for line in lines[4:]: + if line.startswith("+") and not line[1:].lstrip().startswith("//"): + should_run = True + break + if should_run == True: + break + + sys.exit(0 if should_run else 1) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/sycl/include/sycl/accessor.hpp b/sycl/include/sycl/accessor.hpp index d804c9ca7af29..366819e9bd2db 100644 --- a/sycl/include/sycl/accessor.hpp +++ b/sycl/include/sycl/accessor.hpp @@ -767,8 +767,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor : detail::InitializedVal::template get<0>()) {} #else - accessor(const detail::AccessorImplPtr &Impl) - : detail::AccessorBaseHost{Impl} {} + accessor(const detail::AccessorImplPtr &Impl) : detail::AccessorBaseHost{Impl} {} void *getPtr() { return AccessorBaseHost::getPtr(); } diff --git a/sycl/source/accessor.cpp b/sycl/source/accessor.cpp index dc6f9a1ec206f..5340be01c160d 100644 --- a/sycl/source/accessor.cpp +++ b/sycl/source/accessor.cpp @@ -102,8 +102,7 @@ LocalAccessorBaseHost::LocalAccessorBaseHost( sycl::range<3> Size, int Dims, int ElemSize, const property_list &PropertyList) { verifyAccessorProps(PropertyList); - impl = std::shared_ptr( - new LocalAccessorImplHost(Size, Dims, ElemSize, PropertyList)); + impl = std::shared_ptr(new LocalAccessorImplHost(Size, Dims, ElemSize, PropertyList)); } sycl::range<3> &LocalAccessorBaseHost::getSize() { return impl->MSize; } const sycl::range<3> &LocalAccessorBaseHost::getSize() const { diff --git a/sycl/source/device.cpp b/sycl/source/device.cpp index 0c99630effbc2..849aec2d1b164 100644 --- a/sycl/source/device.cpp +++ b/sycl/source/device.cpp @@ -59,8 +59,7 @@ device::device(const device_selector &deviceSelector) { std::vector device::get_devices(info::device_type deviceType) { std::vector devices; - detail::ods_target_list *OdsTargetList = - detail::SYCLConfig::get(); + detail::ods_target_list *OdsTargetList = detail::SYCLConfig::get(); auto thePlatforms = platform::get_platforms(); for (const auto &plt : thePlatforms) { diff --git a/sycl/source/image.cpp b/sycl/source/image.cpp index 511d816e5d31f..4c62a6d6b0950 100644 --- a/sycl/source/image.cpp +++ b/sycl/source/image.cpp @@ -16,8 +16,7 @@ image_plain::image_plain(image_channel_order Order, image_channel_type Type, const range<3> &Range, std::unique_ptr Allocator, uint8_t Dimensions, const property_list &PropList) { - impl = std::make_shared( - Order, Type, Range, std::move(Allocator), Dimensions, PropList); + impl = std::make_shared(Order, Type, Range, std::move(Allocator), Dimensions, PropList); } image_plain::image_plain(image_channel_order Order, image_channel_type Type,