Apply clang-tidy autofixes (#15894)

This changeset is large, but it's not very substantial. It's all the automated fixes produced by clang-tidy using our script. The bulk of the changes are either adding `[[nodiscard]]` to many functions or changing const ref args to pass by value and then move in cases where the parameter is only used to set a value. There are also some places where clang-tidy preferred either more or less namespacing of objects depending on the current namespace. The goal is to enable clang-tidy in CI, which we made progress towards in #9860 but stalled in #10064. This PR contains the first set of changes that will required for such a check to pass. I've marked this PR as breaking because some of the functions now marked as `[[nodiscard]]` are public APIs, so if consumers were ignoring the return values they will now see warnings, and if they are compiling with warnings as errors then the builds will break. Contributes to #584 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) URL: #15894
rapidsai · Jun 12, 2024 · 2b10299 · 2b10299
1 parent f7ba6ab
commit 2b10299
Show file tree

Hide file tree

Showing 261 changed files with 2,911 additions and 2,151 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -56,12 +56,20 @@ repos:
       - id: clang-format
         types_or: [c, c++, cuda]
         args: ["-fallback-style=none", "-style=file", "-i"]
+        exclude: |
+          (?x)^(
+            ^cpp/src/io/parquet/ipc/Schema_generated.h|
+            ^cpp/src/io/parquet/ipc/Message_generated.h|
+            ^cpp/include/cudf_test/cxxopts.hpp|
+          )
   - repo: https://github.com/sirosen/texthooks
     rev: 0.6.6
     hooks:
       - id: fix-smartquotes
         exclude: |
           (?x)^(
+            ^cpp/src/io/parquet/ipc/Schema_generated.h|
+            ^cpp/src/io/parquet/ipc/Message_generated.h|
             ^cpp/include/cudf_test/cxxopts.hpp|
             ^python/cudf/cudf/tests/data/subword_tokenizer_data/.*|
             ^python/cudf/cudf/tests/text/test_text_methods.py

diff --git a/cpp/include/cudf/ast/expressions.hpp b/cpp/include/cudf/ast/expressions.hpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020-2023, NVIDIA CORPORATION.
+ * Copyright (c) 2020-2024, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -478,7 +478,10 @@ class operation : public expression {
    *
    * @return Vector of operands
    */
-  std::vector<std::reference_wrapper<expression const>> get_operands() const { return operands; }
+  [[nodiscard]] std::vector<std::reference_wrapper<expression const>> get_operands() const
+  {
+    return operands;
+  }
 
   /**
    * @copydoc expression::accept

diff --git a/cpp/include/cudf/column/column_device_view.cuh b/cpp/include/cudf/column/column_device_view.cuh
@@ -442,7 +442,7 @@ class alignas(16) column_device_view : public detail::column_device_view_base {
    * @return string_view instance representing this element at this index
    */
   template <typename T, CUDF_ENABLE_IF(std::is_same_v<T, string_view>)>
-  __device__ T element(size_type element_index) const noexcept
+  __device__ [[nodiscard]] T element(size_type element_index) const noexcept
   {
     size_type index       = element_index + offset();  // account for this view's _offset
     char const* d_strings = static_cast<char const*>(_data);
@@ -501,7 +501,7 @@ class alignas(16) column_device_view : public detail::column_device_view_base {
    * @return dictionary32 instance representing this element at this index
    */
   template <typename T, CUDF_ENABLE_IF(std::is_same_v<T, dictionary32>)>
-  __device__ T element(size_type element_index) const noexcept
+  __device__ [[nodiscard]] T element(size_type element_index) const noexcept
   {
     size_type index    = element_index + offset();  // account for this view's _offset
     auto const indices = d_children[0];
@@ -519,7 +519,7 @@ class alignas(16) column_device_view : public detail::column_device_view_base {
    * @return numeric::fixed_point representing the element at this index
    */
   template <typename T, CUDF_ENABLE_IF(cudf::is_fixed_point<T>())>
-  __device__ T element(size_type element_index) const noexcept
+  __device__ [[nodiscard]] T element(size_type element_index) const noexcept
   {
     using namespace numeric;
     using rep        = typename T::rep;
@@ -858,7 +858,7 @@ class alignas(16) column_device_view : public detail::column_device_view_base {
    */
   [[nodiscard]] __device__ device_span<column_device_view const> children() const noexcept
   {
-    return device_span<column_device_view const>(d_children, _num_children);
+    return {d_children, static_cast<std::size_t>(_num_children)};
   }
 
   /**
@@ -1032,7 +1032,7 @@ class alignas(16) mutable_column_device_view : public detail::column_device_view
    * @return Reference to the element at the specified index
    */
   template <typename T, CUDF_ENABLE_IF(is_rep_layout_compatible<T>())>
-  __device__ T& element(size_type element_index) const noexcept
+  __device__ [[nodiscard]] T& element(size_type element_index) const noexcept
   {
     return data<T>()[element_index];
   }

diff --git a/cpp/include/cudf/detail/aggregation/aggregation.hpp b/cpp/include/cudf/detail/aggregation/aggregation.hpp
@@ -24,6 +24,7 @@
 
 #include <functional>
 #include <numeric>
+#include <utility>
 
 namespace cudf {
 namespace detail {
@@ -510,7 +511,7 @@ class quantile_aggregation final : public groupby_aggregation, public reduce_agg
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  private:
-  size_t hash_impl() const
+  [[nodiscard]] size_t hash_impl() const
   {
     return std::hash<int>{}(static_cast<int>(_interpolation)) ^
            std::accumulate(
@@ -596,7 +597,10 @@ class nunique_aggregation final : public groupby_aggregation,
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  private:
-  size_t hash_impl() const { return std::hash<int>{}(static_cast<int>(_null_handling)); }
+  [[nodiscard]] size_t hash_impl() const
+  {
+    return std::hash<int>{}(static_cast<int>(_null_handling));
+  }
 };
 
 /**
@@ -638,7 +642,7 @@ class nth_element_aggregation final : public groupby_aggregation,
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  private:
-  size_t hash_impl() const
+  [[nodiscard]] size_t hash_impl() const
   {
     return std::hash<size_type>{}(_n) ^ std::hash<int>{}(static_cast<int>(_null_handling));
   }
@@ -763,7 +767,10 @@ class collect_list_aggregation final : public rolling_aggregation,
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  private:
-  size_t hash_impl() const { return std::hash<int>{}(static_cast<int>(_null_handling)); }
+  [[nodiscard]] size_t hash_impl() const
+  {
+    return std::hash<int>{}(static_cast<int>(_null_handling));
+  }
 };
 
 /**
@@ -813,7 +820,7 @@ class collect_set_aggregation final : public rolling_aggregation,
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  protected:
-  size_t hash_impl() const
+  [[nodiscard]] size_t hash_impl() const
   {
     return std::hash<int>{}(static_cast<int>(_null_handling) ^ static_cast<int>(_nulls_equal) ^
                             static_cast<int>(_nans_equal));
@@ -866,10 +873,10 @@ class lead_lag_aggregation final : public rolling_aggregation {
 class udf_aggregation final : public rolling_aggregation {
  public:
   udf_aggregation(aggregation::Kind type,
-                  std::string const& user_defined_aggregator,
+                  std::string user_defined_aggregator,
                   data_type output_type)
     : aggregation{type},
-      _source{user_defined_aggregator},
+      _source{std::move(user_defined_aggregator)},
       _operator_name{(type == aggregation::PTX) ? "rolling_udf_ptx" : "rolling_udf_cuda"},
       _function_name{"rolling_udf"},
       _output_type{output_type}
@@ -973,7 +980,7 @@ class merge_sets_aggregation final : public groupby_aggregation, public reduce_a
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  protected:
-  size_t hash_impl() const
+  [[nodiscard]] size_t hash_impl() const
   {
     return std::hash<int>{}(static_cast<int>(_nulls_equal) ^ static_cast<int>(_nans_equal));
   }
@@ -1046,7 +1053,7 @@ class covariance_aggregation final : public groupby_aggregation {
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  protected:
-  size_t hash_impl() const
+  [[nodiscard]] size_t hash_impl() const
   {
     return std::hash<size_type>{}(_min_periods) ^ std::hash<size_type>{}(_ddof);
   }
@@ -1088,7 +1095,7 @@ class correlation_aggregation final : public groupby_aggregation {
   void finalize(aggregation_finalizer& finalizer) const override { finalizer.visit(*this); }
 
  protected:
-  size_t hash_impl() const
+  [[nodiscard]] size_t hash_impl() const
   {
     return std::hash<int>{}(static_cast<int>(_type)) ^ std::hash<size_type>{}(_min_periods);
   }

diff --git a/cpp/include/cudf/detail/contiguous_split.hpp b/cpp/include/cudf/detail/contiguous_split.hpp
@@ -104,7 +104,7 @@ class metadata_builder {
    *
    * @returns A vector containing the serialized column metadata
    */
-  std::vector<uint8_t> build() const;
+  [[nodiscard]] std::vector<uint8_t> build() const;
 
   /**
    * @brief Clear the internal buffer containing all added metadata.

diff --git a/cpp/include/cudf/detail/normalizing_iterator.cuh b/cpp/include/cudf/detail/normalizing_iterator.cuh
@@ -51,7 +51,7 @@ struct alignas(16) base_normalator {
    */
   CUDF_HOST_DEVICE inline Derived& operator++()
   {
-    Derived& derived = static_cast<Derived&>(*this);
+    auto& derived = static_cast<Derived&>(*this);
     derived.p_ += width_;
     return derived;
   }
@@ -71,7 +71,7 @@ struct alignas(16) base_normalator {
    */
   CUDF_HOST_DEVICE inline Derived& operator--()
   {
-    Derived& derived = static_cast<Derived&>(*this);
+    auto& derived = static_cast<Derived&>(*this);
     derived.p_ -= width_;
     return derived;
   }
@@ -91,7 +91,7 @@ struct alignas(16) base_normalator {
    */
   CUDF_HOST_DEVICE inline Derived& operator+=(difference_type offset)
   {
-    Derived& derived = static_cast<Derived&>(*this);
+    auto& derived = static_cast<Derived&>(*this);
     derived.p_ += offset * width_;
     return derived;
   }
@@ -121,7 +121,7 @@ struct alignas(16) base_normalator {
    */
   CUDF_HOST_DEVICE inline Derived& operator-=(difference_type offset)
   {
-    Derived& derived = static_cast<Derived&>(*this);
+    auto& derived = static_cast<Derived&>(*this);
     derived.p_ -= offset * width_;
     return derived;
   }

diff --git a/cpp/include/cudf/detail/structs/utilities.hpp b/cpp/include/cudf/detail/structs/utilities.hpp
@@ -25,6 +25,8 @@
 #include <rmm/device_buffer.hpp>
 #include <rmm/resource_ref.hpp>
 
+#include <utility>
+
 namespace cudf::structs::detail {
 
 enum class column_nullability {
@@ -112,12 +114,12 @@ class flattened_table {
    * @param columns_ Newly allocated columns to back the table_view
    * @param nullable_data_ Newly generated temporary data that needs to be kept alive
    */
-  flattened_table(table_view const& flattened_columns_,
+  flattened_table(table_view flattened_columns_,
                   std::vector<order> const& orders_,
                   std::vector<null_order> const& null_orders_,
                   std::vector<std::unique_ptr<column>>&& columns_,
                   temporary_nullable_data&& nullable_data_)
-    : _flattened_columns{flattened_columns_},
+    : _flattened_columns{std::move(flattened_columns_)},
       _orders{orders_},
       _null_orders{null_orders_},
       _columns{std::move(columns_)},
@@ -170,11 +172,11 @@ class flattened_table {
  *         orders, flattened null precedence, alongside the supporting columns and device_buffers
  *         for the flattened table.
  */
-[[nodiscard]] std::unique_ptr<flattened_table> flatten_nested_columns(
+[[nodiscard]] std::unique_ptr<cudf::structs::detail::flattened_table> flatten_nested_columns(
   table_view const& input,
-  std::vector<order> const& column_order,
-  std::vector<null_order> const& null_precedence,
-  column_nullability nullability,
+  std::vector<cudf::order> const& column_order,
+  std::vector<cudf::null_order> const& null_precedence,
+  cudf::structs::detail::column_nullability nullability,
   rmm::cuda_stream_view stream,
   rmm::device_async_resource_ref mr);
 
@@ -194,11 +196,11 @@ class flattened_table {
  * @param mr Device memory resource used to allocate new device memory
  * @return A new column with potentially new null mask
  */
-[[nodiscard]] std::unique_ptr<column> superimpose_nulls(bitmask_type const* null_mask,
-                                                        size_type null_count,
-                                                        std::unique_ptr<column>&& input,
-                                                        rmm::cuda_stream_view stream,
-                                                        rmm::device_async_resource_ref mr);
+[[nodiscard]] std::unique_ptr<cudf::column> superimpose_nulls(bitmask_type const* null_mask,
+                                                              cudf::size_type null_count,
+                                                              std::unique_ptr<cudf::column>&& input,
+                                                              rmm::cuda_stream_view stream,
+                                                              rmm::device_async_resource_ref mr);
 
 /**
  * @brief Push down nulls from the given input column into its children columns, using bitwise AND.

diff --git a/cpp/include/cudf/detail/utilities/host_vector.hpp b/cpp/include/cudf/detail/utilities/host_vector.hpp
@@ -82,7 +82,7 @@ class rmm_host_allocator {
   using size_type       = std::size_t;     ///< The type used for the size of the allocation
   using difference_type = std::ptrdiff_t;  ///< The type of the distance between two pointers
 
-  typedef cuda::std::true_type propagate_on_container_move_assignment;
+  using propagate_on_container_move_assignment = cuda::std::true_type;
 
   /**
    * @brief converts a `rmm_host_allocator<T>` to `rmm_host_allocator<U>`
@@ -147,7 +147,7 @@ class rmm_host_allocator {
    *  @return The maximum number of objects that may be allocated
    *          by a single call to \p allocate().
    */
-  constexpr inline size_type max_size() const
+  [[nodiscard]] constexpr inline size_type max_size() const
   {
     return (std::numeric_limits<size_type>::max)() / sizeof(T);
   }

diff --git a/cpp/include/cudf/detail/utilities/stream_pool.hpp b/cpp/include/cudf/detail/utilities/stream_pool.hpp
@@ -73,7 +73,7 @@ class cuda_stream_pool {
    *
    * @return the number of stream objects in the pool
    */
-  virtual std::size_t get_stream_pool_size() const = 0;
+  [[nodiscard]] virtual std::size_t get_stream_pool_size() const = 0;
 };
 
 /**

diff --git a/cpp/include/cudf/fixed_point/fixed_point.hpp b/cpp/include/cudf/fixed_point/fixed_point.hpp
@@ -291,14 +291,14 @@ class fixed_point {
    *
    * @return The underlying value of the `fixed_point` number
    */
-  CUDF_HOST_DEVICE inline rep value() const { return _value; }
+  CUDF_HOST_DEVICE [[nodiscard]] inline rep value() const { return _value; }
 
   /**
    * @brief Method that returns the scale of the `fixed_point` number
    *
    * @return The scale of the `fixed_point` number
    */
-  CUDF_HOST_DEVICE inline scale_type scale() const { return _scale; }
+  CUDF_HOST_DEVICE [[nodiscard]] inline scale_type scale() const { return _scale; }
 
   /**
    * @brief Explicit conversion operator to `bool`
@@ -573,7 +573,7 @@ class fixed_point {
    * @param scale The `scale` of the returned `fixed_point` number
    * @return `fixed_point` number with a new `scale`
    */
-  CUDF_HOST_DEVICE inline fixed_point<Rep, Rad> rescaled(scale_type scale) const
+  CUDF_HOST_DEVICE [[nodiscard]] inline fixed_point<Rep, Rad> rescaled(scale_type scale) const
   {
     if (scale == _scale) { return *this; }
     Rep const value = detail::shift<Rep, Rad>(_value, scale_type{scale - _scale});

diff --git a/cpp/include/cudf/interop.hpp b/cpp/include/cudf/interop.hpp
@@ -40,6 +40,8 @@
 
 #include <rmm/mr/device/per_device_resource.hpp>
 
+#include <utility>
+
 struct DLManagedTensor;
 
 struct ArrowDeviceArray;
@@ -121,7 +123,7 @@ struct column_metadata {
    *
    * @param _name Name of the column
    */
-  column_metadata(std::string const& _name) : name(_name) {}
+  column_metadata(std::string _name) : name(std::move(_name)) {}
   column_metadata() = default;
 };
 

diff --git a/cpp/include/cudf/interop/detail/arrow.hpp b/cpp/include/cudf/interop/detail/arrow.hpp
@@ -24,8 +24,12 @@
 #define ARROW_C_DEVICE_DATA_INTERFACE
 
 // Device type for the allocated memory
-typedef int32_t ArrowDeviceType;
+using ArrowDeviceType = int32_t;
 
+// The Arrow spec specifies using macros rather than enums here to avoid being
+// susceptible to changes in the underlying type chosen by the compiler, but
+// clang-tidy doesn't like this.
+// NOLINTBEGIN
 // CPU device, same as using ArrowArray directly
 #define ARROW_DEVICE_CPU 1
 // CUDA GPU Device
@@ -34,6 +38,7 @@ typedef int32_t ArrowDeviceType;
 #define ARROW_DEVICE_CUDA_HOST 3
 // CUDA managed/unified memory allocated by cudaMallocManaged
 #define ARROW_DEVICE_CUDA_MANAGED 13
+// NOLINTEND
 
 struct ArrowDeviceArray {
   struct ArrowArray array;