diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index a8c4e481683..a8afede4821 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -16,6 +16,7 @@ jobs: - changed-files - checks - conda-cpp-build + - cpp-linters - conda-cpp-checks - conda-cpp-tests - conda-python-build @@ -113,6 +114,13 @@ jobs: uses: rapidsai/shared-workflows/.github/workflows/conda-cpp-build.yaml@branch-25.02 with: build_type: pull-request + cpp-linters: + secrets: inherit + needs: checks + uses: rapidsai/shared-workflows/.github/workflows/custom-job.yaml@branch-25.02 + with: + build_type: pull-request + run_script: "ci/cpp_linters.sh" conda-cpp-checks: needs: conda-cpp-build secrets: inherit diff --git a/ci/cpp_linters.sh b/ci/cpp_linters.sh index 286c7bfbc66..4d5b62ba280 100755 --- a/ci/cpp_linters.sh +++ b/ci/cpp_linters.sh @@ -25,7 +25,12 @@ RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)" source rapids-configure-sccache # Run the build via CMake, which will run clang-tidy when CUDF_STATIC_LINTERS is enabled. -cmake -S cpp -B cpp/build -DCMAKE_BUILD_TYPE=Release -DCUDF_STATIC_LINTERS=ON -GNinja + +iwyu_flag="" +if [[ "${RAPIDS_BUILD_TYPE}" == "nightly" ]]; then + iwyu_flag="-DCUDF_IWYU=ON" +fi +cmake -S cpp -B cpp/build -DCMAKE_BUILD_TYPE=Release -DCUDF_CLANG_TIDY=ON ${iwyu_flag} -DBUILD_TESTS=OFF -GNinja cmake --build cpp/build 2>&1 | python cpp/scripts/parse_iwyu_output.py # Remove invalid components of the path for local usage. The path below is diff --git a/cpp/.clang-tidy b/cpp/.clang-tidy index 0e5699876fc..60c0b5d3ba7 100644 --- a/cpp/.clang-tidy +++ b/cpp/.clang-tidy @@ -39,7 +39,7 @@ Checks: -clang-analyzer-optin.core.EnumCastOutOfRange, -clang-analyzer-optin.cplusplus.UninitializedObject' -WarningsAsErrors: '' +WarningsAsErrors: '*' HeaderFilterRegex: '.*cudf/cpp/(src|include).*' ExcludeHeaderFilterRegex: '.*(Message_generated.h|Schema_generated.h|brotli_dict.hpp|unbz2.hpp|cxxopts.hpp).*' FormatStyle: none diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 506f6c185f5..e4fa3b28383 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -91,7 +91,8 @@ option( ${DEFAULT_CUDF_BUILD_STREAMS_TEST_UTIL} ) mark_as_advanced(CUDF_BUILD_STREAMS_TEST_UTIL) -option(CUDF_STATIC_LINTERS "Enable static linters during compilation" OFF) +option(CUDF_CLANG_TIDY "Enable clang-tidy during compilation" OFF) +option(CUDF_IWYU "Enable IWYU during compilation" OFF) option( CUDF_KVIKIO_REMOTE_IO @@ -159,9 +160,7 @@ endif() # ################################################################################################## # * linter configuration --------------------------------------------------------------------------- -if(CUDF_STATIC_LINTERS) - # For simplicity, for now we assume that all linters can be installed into an environment where - # any linter is being run. We could relax this requirement if desired. +if(CUDF_CLANG_TIDY) find_program( CLANG_TIDY_EXE NAMES "clang-tidy" @@ -188,7 +187,9 @@ if(CUDF_STATIC_LINTERS) "clang-tidy version ${expected_clang_tidy_version} is required, but found ${LLVM_VERSION}" ) endif() +endif() +if(CUDF_IWYU) find_program(IWYU_EXE NAMES include-what-you-use iwyu REQUIRED) endif() @@ -201,38 +202,36 @@ function(enable_static_checkers target) _LINT "${_tidy_options}" "${_tidy_one_value}" "${_tidy_multi_value}" ${ARGN} ) - if(CUDF_STATIC_LINTERS) - if(_LINT_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" - ) - endif() - if(_LINT_IWYU) - # A few extra warnings pop up when building with IWYU. I'm not sure why, but they are not - # relevant since they don't show up in any other build so it's better to suppress them until - # we can figure out the cause. Setting this as part of CXX_INCLUDE_WHAT_YOU_USE does not - # appear to be sufficient, we must also ensure that it is set to the underlying target's CXX - # compile flags. To do this completely cleanly we should modify the flags on the target rather - # than the global CUDF_CXX_FLAGS, but this solution is good enough for now since we never run - # the linters on real builds. - foreach(_flag -Wno-missing-braces -Wno-unneeded-internal-declaration) - list(FIND CUDF_CXX_FLAGS "${_flag}" _flag_index) - if(_flag_index EQUAL -1) - list(APPEND CUDF_CXX_FLAGS ${_flag}) - endif() - endforeach() - set(CUDF_CXX_FLAGS - "${CUDF_CXX_FLAGS}" - PARENT_SCOPE - ) - set_target_properties(${target} PROPERTIES CXX_INCLUDE_WHAT_YOU_USE "${IWYU_EXE}") - endif() - foreach(file IN LISTS _LINT_SKIPPED_FILES) - set_source_files_properties(${file} PROPERTIES SKIP_LINTING ON) + if(_LINT_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" + ) + endif() + if(_LINT_IWYU) + # A few extra warnings pop up when building with IWYU. I'm not sure why, but they are not + # relevant since they don't show up in any other build so it's better to suppress them until we + # can figure out the cause. Setting this as part of CXX_INCLUDE_WHAT_YOU_USE does not appear to + # be sufficient, we must also ensure that it is set to the underlying target's CXX compile + # flags. To do this completely cleanly we should modify the flags on the target rather than the + # global CUDF_CXX_FLAGS, but this solution is good enough for now since we never run the linters + # on real builds. + foreach(_flag -Wno-missing-braces -Wno-unneeded-internal-declaration) + list(FIND CUDF_CXX_FLAGS "${_flag}" _flag_index) + if(_flag_index EQUAL -1) + list(APPEND CUDF_CXX_FLAGS ${_flag}) + endif() endforeach() + set(CUDF_CXX_FLAGS + "${CUDF_CXX_FLAGS}" + PARENT_SCOPE + ) + set_target_properties(${target} PROPERTIES CXX_INCLUDE_WHAT_YOU_USE "${IWYU_EXE}") endif() + foreach(file IN LISTS _LINT_SKIPPED_FILES) + set_source_files_properties(${file} PROPERTIES SKIP_LINTING ON) + endforeach() endfunction() # ################################################################################################## @@ -812,9 +811,18 @@ set_target_properties( # Note: This must come before the target_compile_options below so that the function can modify the # flags if necessary. -enable_static_checkers( - cudf SKIPPED_FILES src/io/comp/cpu_unbz2.cpp src/io/comp/brotli_dict.cpp CLANG_TIDY IWYU -) +if(CUDF_CLANG_TIDY OR CUDF_IWYU) + set(linters) + if(CUDF_CLANG_TIDY) + list(APPEND linters CLANG_TIDY) + endif() + if(CUDF_IWYU) + list(APPEND linters IWYU) + endif() + enable_static_checkers( + cudf SKIPPED_FILES src/io/comp/cpu_unbz2.cpp src/io/comp/brotli_dict.cpp ${linters} + ) +endif() target_compile_options( cudf PRIVATE "$<$:${CUDF_CXX_FLAGS}>" "$<$:${CUDF_CUDA_FLAGS}>" diff --git a/cpp/include/cudf/ast/expressions.hpp b/cpp/include/cudf/ast/expressions.hpp index bcc9ad1b391..85289a52831 100644 --- a/cpp/include/cudf/ast/expressions.hpp +++ b/cpp/include/cudf/ast/expressions.hpp @@ -612,19 +612,19 @@ class tree { * @brief get the first expression in the tree * @returns the first inserted expression into the tree */ - expression const& front() const { return *expressions.front(); } + [[nodiscard]] expression const& front() const { return *expressions.front(); } /** * @brief get the last expression in the tree * @returns the last inserted expression into the tree */ - expression const& back() const { return *expressions.back(); } + [[nodiscard]] expression const& back() const { return *expressions.back(); } /** * @brief get the number of expressions added to the tree * @returns the number of expressions added to the tree */ - size_t size() const { return expressions.size(); } + [[nodiscard]] size_t size() const { return expressions.size(); } /** * @brief get the expression at an index in the tree. Index is checked. diff --git a/cpp/src/io/json/parser_features.cpp b/cpp/src/io/json/parser_features.cpp index 401a6e992de..e795e8e09d8 100644 --- a/cpp/src/io/json/parser_features.cpp +++ b/cpp/src/io/json/parser_features.cpp @@ -109,7 +109,7 @@ struct allnull_column_functor { rmm::device_async_resource_ref mr; private: - auto make_zeroed_offsets(size_type size) const + [[nodiscard]] auto make_zeroed_offsets(size_type size) const { auto offsets_buff = cudf::detail::make_zeroed_device_uvector_async(size + 1, stream, mr);