diff --git a/.github/workflows/hipobject-clang-tidy.yml b/.github/workflows/hipobject-clang-tidy.yml index 0c4bac28..0d6da76c 100644 --- a/.github/workflows/hipobject-clang-tidy.yml +++ b/.github/workflows/hipobject-clang-tidy.yml @@ -121,8 +121,9 @@ jobs: "clang-tidy-${LLVM_VERSION}" "clang-tidy-${LLVM_VERSION}" --version - # CLANG_TIDY_EXE is the variable HIPOBJClangTidy.cmake searches - # for clang-tidy with, so setting it skips the search. + # Setting HIPOBJ_CLANG_TIDY_EXE makes the build use the clang-tidy + # installed above, rather than whatever HIPOBJClangTidy.cmake's + # search finds first. # # This builds the MinIO bridge, which the Build workflow's Clang # job (code coverage) doesn't, so HIPOBJ_WERROR is on to make this @@ -140,7 +141,7 @@ jobs: -DHIPOBJ_IONIC=ON \ -DHIPOBJ_MINIO_CLIENT=ON \ -DHIPOBJ_USE_CLANG_TIDY=ON \ - -DCLANG_TIDY_EXE="/usr/bin/clang-tidy-${LLVM_VERSION}" + -DHIPOBJ_CLANG_TIDY_EXE="/usr/bin/clang-tidy-${LLVM_VERSION}" # -k 0 keeps building after a file fails, so one run reports every # finding, not just those in the first files to fail. pipefail diff --git a/AGENTS.md b/AGENTS.md index d4b9a331..c79a6a08 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -538,6 +538,8 @@ configuration files. does, `readability-magic-numbers`, since a unit test's numbers are its data. Other code names its constants, the test server included. ROCm's clang-tidy lacks the `clang-analyzer-*` checks, so prefer an upstream one. + Configuring finds one, and only falls back to ROCm's when there's no + other clang-tidy 19 or later; set `HIPOBJ_CLANG_TIDY_EXE` to choose. CI: [hipobject-clang-tidy.yml](.github/workflows/hipobject-clang-tidy.yml), with upstream clang-tidy 23. diff --git a/CHANGELOG.md b/CHANGELOG.md index 35222fdc..663ef69a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,13 @@ - The `HIPOBJ_USE_CLANG_TIDY` CMake option now requires Clang as the C++ compiler, and configuring fails with another one. clang-tidy rejected GCC's compiler flags, so it couldn't check a GCC build. +- The `HIPOBJ_USE_CLANG_TIDY` CMake option now needs clang-tidy 19 or + later, and configuring fails if it doesn't find one, instead of + warning and building without clang-tidy. It looks on `PATH` for + `clang-tidy`, then for versioned names such as `clang-tidy-23`, newest + first, and then in ROCm's LLVM, and passes over any older than 19. + Set the new `HIPOBJ_CLANG_TIDY_EXE` CMake variable to choose one; it + replaces `CLANG_TIDY_EXE`. - hipObject now requires ROCm 7.14 or later, and configuring fails if `ROCM_VERSION` is earlier. The documentation used to list ROCm 6.x as the minimum. diff --git a/cmake/HIPOBJClangTidy.cmake b/cmake/HIPOBJClangTidy.cmake index 2f8f4ecc..ad7843f9 100644 --- a/cmake/HIPOBJClangTidy.cmake +++ b/cmake/HIPOBJClangTidy.cmake @@ -9,6 +9,32 @@ include_guard(GLOBAL) option(HIPOBJ_USE_CLANG_TIDY "Run clang-tidy during compilation" OFF) +# .clang-tidy uses ExcludeHeaderFilterRegex, which clang-tidy 19 added +set(HIPOBJ_CLANG_TIDY_MIN_VERSION 19) + +# Get the LLVM major version of a clang-tidy, or an empty string if it +# can't be run or its version can't be read +function(hipobj_get_clang_tidy_version outvar exe) + execute_process( + COMMAND "${exe}" --version + RESULT_VARIABLE result + OUTPUT_VARIABLE output + ERROR_QUIET) + set(version "") + if(result EQUAL 0 AND output MATCHES "LLVM version ([0-9]+)") + set(version "${CMAKE_MATCH_1}") + endif() + set(${outvar} "${version}" PARENT_SCOPE) +endfunction() + +# find_program() validator that rejects a clang-tidy that's too old +function(hipobj_validate_clang_tidy result_var exe) + hipobj_get_clang_tidy_version(version "${exe}") + if(NOT version OR version VERSION_LESS HIPOBJ_CLANG_TIDY_MIN_VERSION) + set(${result_var} FALSE PARENT_SCOPE) + endif() +endfunction() + if(HIPOBJ_USE_CLANG_TIDY) # clang-tidy parses each file with the compiler's flags, and rejects # the GCC-only ones @@ -18,22 +44,57 @@ if(HIPOBJ_USE_CLANG_TIDY) "(CMAKE_CXX_COMPILER)") endif() - # Prefer the clang-tidy on PATH, and fall back to ROCm's. CMake + # Look on PATH for clang-tidy, then for the versioned names that + # Linux distributions and apt.llvm.org use, newest first. CMake # searches CMAKE_PREFIX_PATH, as a CMake or environment variable, # before PATH, and it may have ROCm's LLVM in it, so it's skipped. - find_program(CLANG_TIDY_EXE - NAMES clang-tidy - PATHS ${ROCM_PATH}/llvm/bin + set(versioned_names "") + foreach(version RANGE ${HIPOBJ_CLANG_TIDY_MIN_VERSION} 30) + list(PREPEND versioned_names clang-tidy-${version}) + endforeach() + find_program(HIPOBJ_CLANG_TIDY_EXE + NAMES clang-tidy ${versioned_names} NO_CMAKE_PATH - NO_CMAKE_ENVIRONMENT_PATH) - - if(CLANG_TIDY_EXE) - message(STATUS - "clang-tidy enabled: ${CLANG_TIDY_EXE}") - else() - message(WARNING - "clang-tidy requested but not found") + NO_CMAKE_ENVIRONMENT_PATH + VALIDATOR hipobj_validate_clang_tidy + DOC "clang-tidy ${HIPOBJ_CLANG_TIDY_MIN_VERSION} or later") + unset(versioned_names) + + # Fall back to ROCm's, which is built without the clang-analyzer-* + # checks. find_program() doesn't search again if the first one found + # a clang-tidy. + find_program(HIPOBJ_CLANG_TIDY_EXE + NAMES clang-tidy + PATHS "${ROCM_PATH}/llvm/bin" + NO_DEFAULT_PATH + VALIDATOR hipobj_validate_clang_tidy + DOC "clang-tidy ${HIPOBJ_CLANG_TIDY_MIN_VERSION} or later") + + if(NOT HIPOBJ_CLANG_TIDY_EXE) + message(FATAL_ERROR + "HIPOBJ_USE_CLANG_TIDY needs clang-tidy " + "${HIPOBJ_CLANG_TIDY_MIN_VERSION} or later, which wasn't found. " + "Set HIPOBJ_CLANG_TIDY_EXE to its path.") + endif() + + # find_program() doesn't validate a path that was set, or found by an + # earlier configuration, so check it here + hipobj_get_clang_tidy_version(HIPOBJ_CLANG_TIDY_VERSION + "${HIPOBJ_CLANG_TIDY_EXE}") + if(NOT HIPOBJ_CLANG_TIDY_VERSION) + message(FATAL_ERROR + "Can't get the version of clang-tidy from " + "HIPOBJ_CLANG_TIDY_EXE (${HIPOBJ_CLANG_TIDY_EXE})") + elseif(HIPOBJ_CLANG_TIDY_VERSION VERSION_LESS HIPOBJ_CLANG_TIDY_MIN_VERSION) + message(FATAL_ERROR + "HIPOBJ_CLANG_TIDY_EXE (${HIPOBJ_CLANG_TIDY_EXE}) is clang-tidy " + "${HIPOBJ_CLANG_TIDY_VERSION}, but hipObject needs " + "${HIPOBJ_CLANG_TIDY_MIN_VERSION} or later") endif() + + message(STATUS + "clang-tidy ${HIPOBJ_CLANG_TIDY_VERSION} enabled: " + "${HIPOBJ_CLANG_TIDY_EXE}") endif() # Run clang-tidy on a target's C++ sources @@ -41,8 +102,9 @@ endif() # This is set per target instead of with CMAKE_CXX_CLANG_TIDY so # that third-party code pulled in with FetchContent isn't checked. function(hipobj_set_clang_tidy target) - if(HIPOBJ_USE_CLANG_TIDY AND CLANG_TIDY_EXE) + if(HIPOBJ_USE_CLANG_TIDY) set_target_properties(${target} PROPERTIES - CXX_CLANG_TIDY "${CLANG_TIDY_EXE};--extra-arg=-Wno-unknown-warning-option") + CXX_CLANG_TIDY + "${HIPOBJ_CLANG_TIDY_EXE};--extra-arg=-Wno-unknown-warning-option") endif() endfunction() diff --git a/docs/building.rst b/docs/building.rst index 87b7764f..f48008ed 100644 --- a/docs/building.rst +++ b/docs/building.rst @@ -262,8 +262,13 @@ uses more bits than ThreadSanitizer supports. Reduce it with ``HIPOBJ_USE_CLANG_TIDY`` runs clang-tidy (19 or later) on hipObject's own code as it compiles. It needs Clang as the C++ -compiler. ``HIPOBJ_USE_IWYU`` runs include-what-you-use the same -way; its suggestions don't fail the build. +compiler. Configuring looks on ``PATH`` for ``clang-tidy``, then for +versioned names such as ``clang-tidy-23``, newest first, and then in +``ROCM_PATH``/llvm/bin, and passes over any older than 19. ROCm's +clang-tidy comes last because it's built without the +``clang-analyzer-*`` checks. Set ``HIPOBJ_CLANG_TIDY_EXE`` to the +path of the one to use. ``HIPOBJ_USE_IWYU`` runs include-what-you-use +the same way; its suggestions don't fail the build. ``ROCM_PATH`` and ``ROCM_VERSION`` can also be set in the environment. If ``ROCM_PATH`` isn't set but ``ROCM_VERSION`` is,