[CPU] Introduce clang-tidy

openvinotoolkit · Dec 12, 2024 · a4204f2 · a4204f2
1 parent dba4ef1
commit a4204f2
Show file tree

Hide file tree

Showing 7 changed files with 250 additions and 2 deletions.
diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml
@@ -38,6 +38,27 @@ jobs:
           level: warning
           fail_on_error: true
 
+  clang-tidy:
+    runs-on: ubuntu-22.04
+    if: ${{ github.repository_owner == 'openvinotoolkit' }}
+    permissions:
+      pull-requests: write
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          submodules: 'true'
+
+      - name: Install clang-tidy-15
+        run: |
+          sudo apt update
+          sudo apt --assume-yes install clang-tidy-15
+
+      - name: CMake configure
+        run: cmake -DENABLE_PYTHON=ON -DENABLE_TESTS=ON -DENABLE_PROFILING_ITT=ON -DSELECTIVE_BUILD=COLLECT -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -B build
+
+      - name: Run clang-tidy check
+        run: cmake --build build --target clang_tidy_check_all -j8
+
   ShellCheck:
     runs-on: ubuntu-22.04
     if: ${{ github.repository_owner == 'openvinotoolkit' }}

diff --git a/cmake/developer_package/clang_tidy/clang_tidy.cmake b/cmake/developer_package/clang_tidy/clang_tidy.cmake
@@ -0,0 +1,116 @@
+# Copyright (C) 2018-2024 Intel Corporation
+# SPDX-License-Identifier: Apache-2.0
+#
+
+if(ENABLE_CLANG_TIDY)
+    set(CLANG_TIDY_REQUIRED_VERSION 15 CACHE STRING "clang-tidy version to use")
+    set(CLANG_TIDY_FILENAME clang-tidy-${CLANG_TIDY_REQUIRED_VERSION} clang-tidy)
+    find_host_program(CLANG_TIDY NAMES ${CLANG_TIDY_FILENAME} PATHS ENV PATH)
+    if(CLANG_TIDY)
+        execute_process(COMMAND ${CLANG_TIDY} ${CMAKE_CURRENT_SOURCE_DIR} ARGS --version OUTPUT_VARIABLE CLANG_VERSION)
+        if(NOT CLANG_VERSION)
+            message(WARNING "Supported clang-tidy version is ${CLANG_TIDY_REQUIRED_VERSION}!")
+            set(ENABLE_CLANG_TIDY OFF)
+        else()
+            string(REGEX REPLACE "[^0-9]+([0-9]+)\\..*" "\\1" CLANG_TIDY_MAJOR_VERSION ${CLANG_VERSION})
+            if(NOT CLANG_TIDY_MAJOR_VERSION EQUAL CLANG_TIDY_REQUIRED_VERSION)
+                message(WARNING "Supported clang-tidy version is ${CLANG_TIDY_REQUIRED_VERSION}! Provided version ${CLANG_TIDY_MAJOR_VERSION}")
+                set(ENABLE_CLANG_TIDY OFF)
+            endif()
+        endif()
+    else()
+        message(WARNING "Supported clang-tidy-${CLANG_TIDY_REQUIRED_VERSION} is not found!")
+        set(ENABLE_CLANG_TIDY OFF)
+    endif()
+endif()
+
+if(ENABLE_CLANG_TIDY AND NOT TARGET clang_tidy_check_all)
+    add_custom_target(clang_tidy_check_all)
+    set_target_properties(clang_tidy_check_all
+                          PROPERTIES FOLDER clang_tidy)
+endif()
+
+#
+# ov_add_clang_tidy_target(FOR_TARGETS <target1 target2 ...> | FOR_SOURCES <source1 source2 ...>
+#                            [EXCLUDE_PATTERNS <pattern1 pattern2 ...>])
+#
+function(ov_add_clang_tidy_target TARGET_NAME)
+    if(NOT ENABLE_CLANG_TIDY)
+        return()
+    endif()
+
+    set(options ALL)
+    set(oneValueArgs "")
+    set(multiValueArgs "FOR_TARGETS" "FOR_SOURCES" "EXCLUDE_PATTERNS")
+    cmake_parse_arguments(CLANG_TIDY "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+
+    foreach(target IN LISTS CLANG_TIDY_FOR_TARGETS)
+        get_target_property(target_sources "${target}" SOURCES)
+        list(APPEND CLANG_TIDY_FOR_SOURCES ${target_sources})
+    endforeach()
+    list(REMOVE_DUPLICATES CLANG_TIDY_FOR_SOURCES)
+
+    set(all_output_files "")
+    foreach(source_file IN LISTS CLANG_TIDY_FOR_SOURCES)
+        set(exclude FALSE)
+        foreach(pattern IN LISTS CLANG_TIDY_EXCLUDE_PATTERNS)
+            if(source_file MATCHES "${pattern}")
+                set(exclude ON)
+                break()
+            endif()
+        endforeach()
+
+        if(exclude)
+            continue()
+        endif()
+
+        # ignore object libraries
+        if(NOT EXISTS "${source_file}")
+            continue()
+        endif()
+
+        if(IS_DIRECTORY "${source_file}")
+            message(FATAL_ERROR "Directory ${source_file} cannot be passed to clang-tidy")
+        endif()
+
+        file(RELATIVE_PATH source_file_relative "${CMAKE_CURRENT_SOURCE_DIR}" "${source_file}")
+        set(output_file "${CMAKE_CURRENT_BINARY_DIR}/clang_tidy/${source_file_relative}.clang")
+        string(REPLACE ".." "__" output_file "${output_file}")
+        get_filename_component(output_dir "${output_file}" DIRECTORY)
+        file(MAKE_DIRECTORY "${output_dir}")
+
+        add_custom_command(
+            OUTPUT
+            "${output_file}"
+            COMMAND
+            "${CMAKE_COMMAND}"
+            -D "CLANG_TIDY=${CLANG_TIDY}"
+            -D "INPUT_FILE=${source_file}"
+            -D "OUTPUT_FILE=${output_file}"
+            -P "${OpenVINODeveloperScripts_DIR}/clang_tidy/clang_tidy_check.cmake"
+            DEPENDS
+            "${source_file}"
+            "${OpenVINODeveloperScripts_DIR}/clang_tidy/clang_tidy_check.cmake"
+            COMMENT
+            "[clang-tidy] ${source_file}"
+            VERBATIM)
+
+        list(APPEND all_input_sources "${source_file}")
+        list(APPEND all_output_files "${output_file}")
+    endforeach()
+
+    add_custom_target(${TARGET_NAME}_check
+        DEPENDS ${all_output_files}
+        COMMENT "[clang-tidy] ${TARGET_NAME}_check")
+
+    set_target_properties(${TARGET_NAME}_check
+                          PROPERTIES FOLDER clang_tidy)
+
+    # if(CLANG_TIDY_FOR_TARGETS)
+    #     foreach(target IN LISTS CLANG_TIDY_FOR_TARGETS)
+    #         add_dependencies(${target} ${TARGET_NAME})
+    #     endforeach()
+    # endif()
+
+    add_dependencies(clang_tidy_check_all ${TARGET_NAME}_check)
+endfunction()
diff --git a/cmake/developer_package/clang_tidy/clang_tidy_check.cmake b/cmake/developer_package/clang_tidy/clang_tidy_check.cmake
@@ -0,0 +1,21 @@
+# Copyright (C) 2018-2024 Intel Corporation
+# SPDX-License-Identifier: Apache-2.0
+#
+
+file(REMOVE "${OUTPUT_FILE}")
+
+execute_process(COMMAND ${CLANG_TIDY} -p=${CMAKE_BINARY_DIR} ${INPUT_FILE}
+    OUTPUT_VARIABLE STYLE_CHECK_RESULT RESULT_VARIABLE STYLE_CHECK_RETURN_CODE
+)
+
+if(NOT STYLE_CHECK_RETURN_CODE EQUAL 0)
+    message(SEND_ERROR "Clang-tidy check failed for ${INPUT_FILE}")
+endif()
+
+file(WRITE "${OUTPUT_FILE}" "${STYLE_CHECK_RESULT}")
+
+if(NOT SKIP_RETURN_CODE)
+    if("${STYLE_CHECK_RESULT}" MATCHES ".*<replacement .*")
+        message(FATAL_ERROR "[clang-tidy] Code style check failed for: ${INPUT_FILE}")
+    endif()
+endif()
diff --git a/cmake/developer_package/features.cmake b/cmake/developer_package/features.cmake
@@ -73,6 +73,8 @@ ov_dependent_option (ENABLE_CPPLINT_REPORT "Build cpplint report instead of fail
 
 ov_option (ENABLE_CLANG_FORMAT "Enable clang-format checks during the build" ${STYLE_CHECKS_DEFAULT})
 
+ov_option (ENABLE_CLANG_TIDY "Enable clang-tidy checks during the build" ${STYLE_CHECKS_DEFAULT})
+
 ov_option (ENABLE_NCC_STYLE "Enable ncc style check" ${STYLE_CHECKS_DEFAULT})
 
 ov_option (ENABLE_UNSAFE_LOCATIONS "skip check for MD5 for dependency" OFF)

diff --git a/cmake/developer_package/plugins/plugins.cmake b/cmake/developer_package/plugins/plugins.cmake
@@ -3,6 +3,7 @@
 #
 
 include(CMakeParseArguments)
+include(${CMAKE_SOURCE_DIR}/cmake/developer_package/clang_tidy/clang_tidy.cmake)
 
 set(PLUGIN_FILES "" CACHE INTERNAL "")
 
@@ -34,10 +35,11 @@ endif()
 #               [SKIP_INSTALL]
 #               [SKIP_REGISTRATION] Skip creation of <device>.xml
 #               [ADD_CLANG_FORMAT]
+#               [ADD_CLANG_TIDY]
 #               )
 #
 function(ov_add_plugin)
-    set(options SKIP_INSTALL PSEUDO_DEVICE ADD_CLANG_FORMAT AS_EXTENSION SKIP_REGISTRATION)
+    set(options SKIP_INSTALL PSEUDO_DEVICE ADD_CLANG_FORMAT ADD_CLANG_TIDY AS_EXTENSION SKIP_REGISTRATION)
     set(oneValueArgs NAME DEVICE_NAME VERSION_DEFINES_FOR PSEUDO_PLUGIN_FOR)
     set(multiValueArgs DEFAULT_CONFIG SOURCES OBJECT_LIBRARIES CPPLINT_FILTERS)
     cmake_parse_arguments(OV_PLUGIN "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
@@ -111,6 +113,10 @@ function(ov_add_plugin)
             add_cpplint_target(${OV_PLUGIN_NAME}_cpplint FOR_TARGETS ${OV_PLUGIN_NAME} CUSTOM_FILTERS ${custom_filter})
         endif()
 
+        if (OV_PLUGIN_ADD_CLANG_TIDY)
+            ov_add_clang_tidy_target(${OV_PLUGIN_NAME}_clang_tidy FOR_SOURCES ${OV_PLUGIN_SOURCES})
+        endif()
+
         # plugins does not have to be CXX ABI free, because nobody links with plugins,
         # but let's add this mark to see how it goes
         ov_abi_free_target(${OV_PLUGIN_NAME})

diff --git a/src/plugins/intel_cpu/CMakeLists.txt b/src/plugins/intel_cpu/CMakeLists.txt
@@ -243,7 +243,8 @@ ov_add_plugin(NAME ${TARGET_NAME}
               AS_EXTENSION
               VERSION_DEFINES_FOR src/plugin.cpp
               SOURCES ${SOURCES} ${HEADERS}
-              ADD_CLANG_FORMAT)
+              ADD_CLANG_FORMAT
+              ADD_CLANG_TIDY)
 
 # give a different file name depending on target platform architecture
 if(ARM OR AARCH64)

diff --git a/src/plugins/intel_cpu/src/.clang-tidy b/src/plugins/intel_cpu/src/.clang-tidy
@@ -0,0 +1,81 @@
+---
+
+### NOTE:
+# The 'Checks: >' is a multiline string here. Comment must not be moved into the string.
+#
+### Scopes to be enabled:
+#
+# cppcoreguidelines-*,
+# google-*,
+# readability-*,
+# modernize-*,
+# bugprone-*,
+# misc-*,
+#
+### Checks that are turned off for a reason:
+#
+# -cppcoreguidelines-pro-bounds-pointer-arithmetic
+# -google-readability-todo. No big reason to enforce
+# -modernize-use-trailing-return-type. Just stylistic preference
+# -readability-identifier-length. A lot of code use short names for readability, i.e. 'B' for batch
+# -readability-uppercase-literal-suffix. 
+#
+### Checks that are turned off but better be enabled:
+# -bugprone-narrowing-conversions
+# -bugprone-easily-swappable-parameters
+# -bugprone-fold-init-type
+# -bugprone-implicit-widening-of-multiplication-result
+# -cppcoreguidelines-narrowing-conversions
+# -google-readability-braces-around-statements
+# -readability-implicit-bool-conversion,
+# -readability-magic-numbers, cppcoreguidelines-avoid-magic-numbers
+# -readability-function-cognitive-complexity. Reasonable way to enforce splitting complex code into simple functions
+# -modernize-concat-nested-namespaces. More compact way when C++17 is available
+
+Checks: >
+  -*,
+  performance-*,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-fold-init-type,
+  -bugprone-implicit-widening-of-multiplication-result,
+  -bugprone-narrowing-conversions,
+  -cppcoreguidelines-narrowing-conversions,
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+  -google-build-using-namespace,
+  -google-readability-todo,
+  -readability-braces-around-statements,
+  -google-readability-braces-around-statements,
+  -modernize-use-trailing-return-type,
+  -readability-identifier-length,
+  -readability-implicit-bool-conversion,
+  -readability-magic-numbers,
+  -cppcoreguidelines-avoid-magic-numbers,
+  -readability-uppercase-literal-suffix,
+  -readability-function-cognitive-complexity,
+  -modernize-concat-nested-namespaces,
+# Treat warnings as errors
+WarningsAsErrors: '*'
+# Use clang-format for applied fixes
+FormatStyle: file
+HeaderFilterRegex: ''
+CheckOptions:
+  - key: cppcoreguidelines-avoid-do-while.IgnoreMacros
+    value: true
+  # matches with corresponding cpplink check  
+  - key: google-readability-namespace-comments.ShortNamespaceLines
+    value: "10"
+  # matches with corresponding cpplink check  
+  - key: google-readability-namespace-comments.SpacesBeforeComments
+    value: "2"
+  - key: modernize-loop-convert.MinConfidence
+    value: reasonable
+  - key: modernize-pass-by-value.IncludeStyle
+    value: google
+### To be considered to enable:
+#  # Unifies the usage of the statements
+#  - key: readability-braces-around-statements.ShortStatementLines
+#    value: "1"
+#  Reasonable way to enforce splitting complex code into simple functions
+#  - key: google-readability-function-size.StatementThreshold
+#    value: "800"
+---