Skip to content

Commit

Permalink
Merge branch 'branch-25.02' into ast-precompute-arity
Browse files Browse the repository at this point in the history
  • Loading branch information
lamarrr authored Nov 26, 2024
2 parents f7dfb54 + 165d756 commit 4d58e9f
Show file tree
Hide file tree
Showing 105 changed files with 3,186 additions and 1,318 deletions.
8 changes: 8 additions & 0 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
- changed-files
- checks
- conda-cpp-build
- cpp-linters
- conda-cpp-checks
- conda-cpp-tests
- conda-python-build
Expand Down Expand Up @@ -113,6 +114,13 @@ jobs:
uses: rapidsai/shared-workflows/.github/workflows/[email protected]
with:
build_type: pull-request
cpp-linters:
secrets: inherit
needs: checks
uses: rapidsai/shared-workflows/.github/workflows/[email protected]
with:
build_type: pull-request
run_script: "ci/cpp_linters.sh"
conda-cpp-checks:
needs: conda-cpp-build
secrets: inherit
Expand Down
7 changes: 6 additions & 1 deletion ci/cpp_linters.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions ci/run_cudf_polars_pytests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ set -euo pipefail
# Support invoking run_cudf_polars_pytests.sh outside the script directory
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cudf_polars/

# Test the default "cudf" executor
python -m pytest --cache-clear "$@" tests

# Test the "dask-experimental" executor
python -m pytest --cache-clear "$@" tests --executor dask-experimental
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).*'
ExcludeHeaderFilterRegex: '.*(Message_generated.h|Schema_generated.h|brotli_dict.hpp|unbz2.hpp|cxxopts.hpp).*'
FormatStyle: none
Expand Down
82 changes: 45 additions & 37 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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()

Expand All @@ -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()

# ##################################################################################################
Expand Down Expand Up @@ -813,9 +812,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 "$<$<COMPILE_LANGUAGE:CXX>:${CUDF_CXX_FLAGS}>"
"$<$<COMPILE_LANGUAGE:CUDA>:${CUDF_CUDA_FLAGS}>"
Expand Down
6 changes: 3 additions & 3 deletions cpp/include/cudf/ast/expressions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions cpp/include/cudf/detail/utilities/cuda.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#pragma once

#include <cudf/detail/nvtx/ranges.hpp>
#include <cudf/detail/utilities/integer_utils.hpp>
#include <cudf/types.hpp>
#include <cudf/utilities/default_stream.hpp>
Expand All @@ -25,8 +24,7 @@
#include <rmm/cuda_stream_view.hpp>

#include <cub/cub.cuh>

#include <type_traits>
#include <cuda/std/type_traits>

namespace cudf {
namespace detail {
Expand Down Expand Up @@ -164,7 +162,7 @@ template <int32_t block_size, int32_t leader_lane = 0, typename T>
__device__ T single_lane_block_sum_reduce(T lane_value)
{
static_assert(block_size <= 1024, "Invalid block size.");
static_assert(std::is_arithmetic_v<T>, "Invalid non-arithmetic type.");
static_assert(cuda::std::is_arithmetic_v<T>, "Invalid non-arithmetic type.");
constexpr auto warps_per_block{block_size / warp_size};
auto const lane_id{threadIdx.x % warp_size};
auto const warp_id{threadIdx.x / warp_size};
Expand Down
175 changes: 0 additions & 175 deletions cpp/include/cudf/detail/utilities/int_fastdiv.h

This file was deleted.

Loading

0 comments on commit 4d58e9f

Please sign in to comment.