Skip to content

Commit

Permalink
Add clang-tidy to CI (#16958)
Browse files Browse the repository at this point in the history
This PR adds clang-tidy checks to our CI. clang-tidy will be run in nightly CI via CMake. For now, only the parts of the code base that were already made compliant in the PRs leading up to this have been enabled, namely cudf source and test cpp files. Over time we can add more files like benchmarks and examples, add or subtract more rules, or enable linting of cu files (see https://gitlab.kitware.com/cmake/cmake/-/issues/25399). This PR is intended to be the starting point enabling systematic linting, at which point everything else should be significantly easier.

Resolves #584

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)
  - Bradley Dice (https://github.com/bdice)

URL: #16958
  • Loading branch information
vyasr authored Oct 14, 2024
1 parent 768fbaa commit 44afc51
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 257 deletions.
14 changes: 13 additions & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,23 @@ jobs:
secrets: inherit
uses: rapidsai/shared-workflows/.github/workflows/[email protected]
with:
build_type: pull-request
build_type: nightly
branch: ${{ inputs.branch }}
date: ${{ inputs.date }}
sha: ${{ inputs.sha }}
# Use the wheel container so we can skip conda solves and since our
# primary static consumers (Spark) are not in conda anyway.
container_image: "rapidsai/ci-wheel:latest"
run_script: "ci/configure_cpp_static.sh"
clang-tidy:
secrets: inherit
uses: rapidsai/shared-workflows/.github/workflows/[email protected]
with:
build_type: nightly
branch: ${{ inputs.branch }}
date: ${{ inputs.date }}
sha: ${{ inputs.sha }}
run_script: "ci/clang_tidy.sh"
conda-python-cudf-tests:
secrets: inherit
uses: rapidsai/shared-workflows/.github/workflows/[email protected]
Expand Down
29 changes: 29 additions & 0 deletions ci/clang_tidy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/bin/bash
# Copyright (c) 2024, NVIDIA CORPORATION.

set -euo pipefail

rapids-logger "Create clang-tidy conda environment"
. /opt/conda/etc/profile.d/conda.sh

ENV_YAML_DIR="$(mktemp -d)"

rapids-dependency-file-generator \
--output conda \
--file-key clang_tidy \
--matrix "cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION}" | tee "${ENV_YAML_DIR}/env.yaml"

rapids-mamba-retry env create --yes -f "${ENV_YAML_DIR}/env.yaml" -n clang_tidy

# Temporarily allow unbound variables for conda activation.
set +u
conda activate clang_tidy
set -u

RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)"

source rapids-configure-sccache

# Run the build via CMake, which will run clang-tidy when CUDF_CLANG_TIDY is enabled.
cmake -S cpp -B cpp/build -DCMAKE_BUILD_TYPE=Release -DCUDF_CLANG_TIDY=ON -GNinja
cmake --build cpp/build
2 changes: 1 addition & 1 deletion cpp/.clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Checks:
-clang-analyzer-optin.core.EnumCastOutOfRange,
-clang-analyzer-optin.cplusplus.UninitializedObject'

WarningsAsErrors: ''
WarningsAsErrors: '*'
HeaderFilterRegex: '.*cudf/cpp/(src|include|tests).*'
ExcludeHeaderFilterRegex: '.*(Message_generated.h|Schema_generated.h|brotli_dict.hpp|unbz2.hpp|cxxopts.hpp).*'
FormatStyle: none
Expand Down
54 changes: 54 additions & 0 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ option(
${DEFAULT_CUDF_BUILD_STREAMS_TEST_UTIL}
)
mark_as_advanced(CUDF_BUILD_STREAMS_TEST_UTIL)
option(CUDF_CLANG_TIDY "Enable clang-tidy checking" OFF)

message(VERBOSE "CUDF: Build with NVTX support: ${USE_NVTX}")
message(VERBOSE "CUDF: Configure CMake to build tests: ${BUILD_TESTS}")
Expand Down Expand Up @@ -144,6 +145,58 @@ if(NOT CUDF_GENERATED_INCLUDE_DIR)
set(CUDF_GENERATED_INCLUDE_DIR ${CUDF_BINARY_DIR})
endif()

# ##################################################################################################
# * clang-tidy configuration ----------------------------------------------------------------------
if(CUDF_CLANG_TIDY)
find_program(
CLANG_TIDY_EXE
NAMES "clang-tidy"
DOC "Path to clang-tidy executable" REQUIRED
)

execute_process(
COMMAND ${CLANG_TIDY_EXE} --version
OUTPUT_VARIABLE CLANG_TIDY_OUTPUT
OUTPUT_STRIP_TRAILING_WHITESPACE
)
string(REGEX MATCH "LLVM version ([0-9]+\\.[0-9]+)\\.[0-9]+" LLVM_VERSION_MATCH
"${CLANG_TIDY_OUTPUT}"
)
# Discard the patch version and allow it to float. Empirically, results between patch versions are
# mostly stable, and looking at available packages on some package managers sometimes patch
# versions are skipped so we don't want to constrain to a patch version that the user can't
# install.
set(LLVM_VERSION "${CMAKE_MATCH_1}")
set(expected_clang_tidy_version 19.1)
if(NOT expected_clang_tidy_version VERSION_EQUAL LLVM_VERSION)
message(
FATAL_ERROR
"clang-tidy version ${expected_clang_tidy_version} is required, but found ${LLVM_VERSION}"
)
endif()
endif()

# Turn on the clang-tidy property for a target excluding the files specified in SKIPPED_FILES.
function(enable_clang_tidy target)
set(_tidy_options)
set(_tidy_one_value)
set(_tidy_multi_value SKIPPED_FILES)
cmake_parse_arguments(
_TIDY "${_tidy_options}" "${_tidy_one_value}" "${_tidy_multi_value}" ${ARGN}
)

if(CUDF_CLANG_TIDY)
# clang will complain about unused link libraries on the compile line unless we specify
# -Qunused-arguments.
set_target_properties(
${target} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};--extra-arg=-Qunused-arguments"
)
foreach(file IN LISTS _TIDY_SKIPPED_FILES)
set_source_files_properties(${file} PROPERTIES SKIP_LINTING ON)
endforeach()
endif()
endfunction()

# ##################################################################################################
# * conda environment -----------------------------------------------------------------------------
rapids_cmake_support_conda_env(conda_env MODIFY_PREFIX_PATH)
Expand Down Expand Up @@ -714,6 +767,7 @@ target_compile_options(
cudf PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${CUDF_CXX_FLAGS}>"
"$<$<COMPILE_LANGUAGE:CUDA>:${CUDF_CUDA_FLAGS}>"
)
enable_clang_tidy(cudf SKIPPED_FILES src/io/comp/cpu_unbz2.cpp src/io/comp/brotli_dict.cpp)

if(CUDF_BUILD_STACKTRACE_DEBUG)
# Remove any optimization level to avoid nvcc warning "incompatible redefinition for option
Expand Down
253 changes: 0 additions & 253 deletions cpp/scripts/run-clang-tidy.py

This file was deleted.

Loading

0 comments on commit 44afc51

Please sign in to comment.