Add clang-tidy to libcudf

.clang-tidy

@@ -0,0 +1,27 @@
+---
+Checks:    
+      'modernize-*,
+       -modernize-use-equals-default,
+       -modernize-concat-nested-namespaces,
+       -modernize-use-trailing-return-type'
+
+      # -modernize-use-equals-default        # auto-fix is broken (doesn't insert =default correctly)
+      # -modernize-concat-nested-namespaces  # auto-fix is broken (can delete code)
+      # -modernize-use-trailing-return-type  # just a preference
+
+WarningsAsErrors: ''
+HeaderFilterRegex: ''
+AnalyzeTemporaryDtors: false
+FormatStyle:     none
+CheckOptions:
+ - key:             modernize-loop-convert.MaxCopySize
+   value:           '16'
+ - key:             modernize-loop-convert.MinConfidence
+   value:           reasonable
+ - key:             modernize-pass-by-value.IncludeStyle
+   value:           llvm
+ - key:             modernize-replace-auto-ptr.IncludeStyle
+   value:           llvm
+ - key:             modernize-use-nullptr.NullMacros
+   value:           'NULL'
+...

cpp/benchmarks/common/generate_benchmark_input.hpp

@@ -300,8 +300,8 @@ class data_profile {
 
   auto get_bool_probability() const { return bool_probability; }
   auto get_null_frequency() const { return null_frequency; };
-  auto get_cardinality() const { return cardinality; };
-  auto get_avg_run_length() const { return avg_run_length; };
+  [[nodiscard]] auto get_cardinality() const { return cardinality; };
+  [[nodiscard]] auto get_avg_run_length() const { return avg_run_length; };
 
   // Users should pass integral values for bounds when setting the parameters for types that have
   // discrete distributions (integers, strings, lists). Otherwise the call with have no effect.

cpp/benchmarks/copying/contiguous_split_benchmark.cu

@@ -51,10 +51,12 @@ void BM_contiguous_split_common(benchmark::State& state,
   std::vector<std::unique_ptr<cudf::column>> columns(src_cols.size());
   std::transform(src_cols.begin(), src_cols.end(), columns.begin(), [](T& in) {
     auto ret = in.release();
-    ret->null_count();
+    // computing the null count is not a part of the benchmark's target code path, and we want the
+    // property to be pre-computed so that we measure the performance of only the intended code path
+    [[maybe_unused]] auto const nulls = ret->null_count();
     return ret;
   });
-  cudf::table src_table(std::move(columns));
+  auto const src_table = cudf::table(std::move(columns));
 
   for (auto _ : state) {
     cuda_event_timer raii(state, true);  // flush_l2_cache = true, stream = 0

cpp/benchmarks/copying/gather_benchmark.cu

@@ -39,7 +39,7 @@ template <class TypeParam, bool coalesce>
 void BM_gather(benchmark::State& state)
 {
   const cudf::size_type source_size{(cudf::size_type)state.range(0)};
-  const cudf::size_type n_cols = (cudf::size_type)state.range(1);
+  const auto n_cols = (cudf::size_type)state.range(1);
 
   // Every element is valid
   auto data = cudf::detail::make_counting_transform_iterator(0, [](auto i) { return i; });

cpp/benchmarks/copying/scatter_benchmark.cu

@@ -40,7 +40,7 @@ template <class TypeParam, bool coalesce>
 void BM_scatter(benchmark::State& state)
 {
   const cudf::size_type source_size{(cudf::size_type)state.range(0)};
-  const cudf::size_type n_cols = (cudf::size_type)state.range(1);
+  const auto n_cols = (cudf::size_type)state.range(1);
 
   // Every element is valid
   auto data = cudf::detail::make_counting_transform_iterator(0, [](auto i) { return i; });

cpp/benchmarks/fixture/benchmark_fixture.hpp

@@ -68,22 +68,22 @@ inline auto make_pool()
  */
 class benchmark : public ::benchmark::Fixture {
  public:
-  virtual void SetUp(const ::benchmark::State& state)
+  void SetUp(const ::benchmark::State& state) override
   {
     mr = make_pool();
     rmm::mr::set_current_device_resource(mr.get());  // set default resource to pool
   }
 
-  virtual void TearDown(const ::benchmark::State& state)
+  void TearDown(const ::benchmark::State& state) override
   {
     // reset default resource to the initial resource
     rmm::mr::set_current_device_resource(nullptr);
     mr.reset();
   }
 
   // eliminate partial override warnings (see benchmark/benchmark.h)
-  virtual void SetUp(::benchmark::State& st) { SetUp(const_cast<const ::benchmark::State&>(st)); }
-  virtual void TearDown(::benchmark::State& st)
+  void SetUp(::benchmark::State& st) override { SetUp(const_cast<const ::benchmark::State&>(st)); }
+  void TearDown(::benchmark::State& st) override
   {
     TearDown(const_cast<const ::benchmark::State&>(st));
   }
@@ -102,7 +102,10 @@ class memory_stats_logger {
 
   ~memory_stats_logger() { rmm::mr::set_current_device_resource(existing_mr); }
 
-  size_t peak_memory_usage() const noexcept { return statistics_mr.get_bytes_counter().peak; }
+  [[nodiscard]] size_t peak_memory_usage() const noexcept
+  {
+    return statistics_mr.get_bytes_counter().peak;
+  }
 
  private:
   rmm::mr::device_memory_resource* existing_mr;

cpp/benchmarks/io/csv/csv_reader_benchmark.cpp

@@ -35,8 +35,8 @@ class CsvRead : public cudf::benchmark {
 
 void BM_csv_read_varying_input(benchmark::State& state)
 {
-  auto const data_types     = get_type_or_group(state.range(0));
-  io_type const source_type = static_cast<io_type>(state.range(1));
+  auto const data_types  = get_type_or_group(state.range(0));
+  auto const source_type = static_cast<io_type>(state.range(1));
 
   auto const tbl  = create_random_table(data_types, num_cols, table_size_bytes{data_size});
   auto const view = tbl->view();

cpp/benchmarks/io/csv/csv_writer_benchmark.cpp

@@ -35,8 +35,8 @@ class CsvWrite : public cudf::benchmark {
 
 void BM_csv_write_varying_inout(benchmark::State& state)
 {
-  auto const data_types   = get_type_or_group(state.range(0));
-  io_type const sink_type = static_cast<io_type>(state.range(1));
+  auto const data_types = get_type_or_group(state.range(0));
+  auto const sink_type  = static_cast<io_type>(state.range(1));
 
   auto const tbl  = create_random_table(data_types, num_cols, table_size_bytes{data_size});
   auto const view = tbl->view();

cpp/benchmarks/io/orc/orc_reader_benchmark.cpp

@@ -40,7 +40,7 @@ void BM_orc_read_varying_input(benchmark::State& state)
   cudf::size_type const run_length  = state.range(2);
   cudf_io::compression_type const compression =
     state.range(3) ? cudf_io::compression_type::SNAPPY : cudf_io::compression_type::NONE;
-  io_type const source_type = static_cast<io_type>(state.range(4));
+  auto const source_type = static_cast<io_type>(state.range(4));
 
   data_profile table_data_profile;
   table_data_profile.set_cardinality(cardinality);

cpp/benchmarks/io/orc/orc_writer_benchmark.cpp

@@ -40,7 +40,7 @@ void BM_orc_write_varying_inout(benchmark::State& state)
   cudf::size_type const run_length  = state.range(2);
   cudf_io::compression_type const compression =
     state.range(3) ? cudf_io::compression_type::SNAPPY : cudf_io::compression_type::NONE;
-  io_type const sink_type = static_cast<io_type>(state.range(4));
+  auto const sink_type = static_cast<io_type>(state.range(4));
 
   data_profile table_data_profile;
   table_data_profile.set_cardinality(cardinality);

cpp/benchmarks/io/parquet/parquet_reader_benchmark.cpp

@@ -40,7 +40,7 @@ void BM_parq_read_varying_input(benchmark::State& state)
   cudf::size_type const run_length  = state.range(2);
   cudf_io::compression_type const compression =
     state.range(3) ? cudf_io::compression_type::SNAPPY : cudf_io::compression_type::NONE;
-  io_type const source_type = static_cast<io_type>(state.range(4));
+  auto const source_type = static_cast<io_type>(state.range(4));
 
   data_profile table_data_profile;
   table_data_profile.set_cardinality(cardinality);

cpp/benchmarks/io/parquet/parquet_writer_benchmark.cpp

@@ -40,7 +40,7 @@ void BM_parq_write_varying_inout(benchmark::State& state)
   cudf::size_type const run_length  = state.range(2);
   cudf_io::compression_type const compression =
     state.range(3) ? cudf_io::compression_type::SNAPPY : cudf_io::compression_type::NONE;
-  io_type const sink_type = static_cast<io_type>(state.range(4));
+  auto const sink_type = static_cast<io_type>(state.range(4));
 
   data_profile table_data_profile;
   table_data_profile.set_cardinality(cardinality);

cpp/benchmarks/lists/copying/scatter_lists_benchmark.cu

@@ -45,7 +45,7 @@ void BM_lists_scatter(::benchmark::State& state)
 
   const size_type base_size{(size_type)state.range(0)};
   const size_type num_elements_per_row{(size_type)state.range(1)};
-  const size_type num_rows = (size_type)ceil(double(base_size) / num_elements_per_row);
+  const auto num_rows = (size_type)ceil(double(base_size) / num_elements_per_row);
 
   auto source_base_col = make_fixed_width_column(
     data_type{type_to_id<TypeParam>()}, base_size, mask_state::UNALLOCATED, stream, mr);

cpp/benchmarks/type_dispatcher/type_dispatcher_benchmark.cu

@@ -170,11 +170,11 @@ void launch_kernel(cudf::mutable_table_view input, T** d_ptr, int work_per_threa
 template <class TypeParam, FunctorType functor_type, DispatchingType dispatching_type>
 void type_dispatcher_benchmark(::benchmark::State& state)
 {
-  const cudf::size_type source_size = static_cast<cudf::size_type>(state.range(1));
+  const auto source_size = static_cast<cudf::size_type>(state.range(1));
 
-  const cudf::size_type n_cols = static_cast<cudf::size_type>(state.range(0));
+  const auto n_cols = static_cast<cudf::size_type>(state.range(0));
 
-  const cudf::size_type work_per_thread = static_cast<cudf::size_type>(state.range(2));
+  const auto work_per_thread = static_cast<cudf::size_type>(state.range(2));
 
   auto data = cudf::detail::make_counting_transform_iterator(0, [](auto i) { return i; });
 

cpp/include/cudf/aggregation.hpp

@@ -99,9 +99,9 @@ class aggregation {
   Kind kind;  ///< The aggregation to perform
   virtual ~aggregation() = default;
 
-  virtual bool is_equal(aggregation const& other) const { return kind == other.kind; }
-  virtual size_t do_hash() const { return std::hash<int>{}(kind); }
-  virtual std::unique_ptr<aggregation> clone() const = 0;
+  [[nodiscard]] virtual bool is_equal(aggregation const& other) const { return kind == other.kind; }
+  [[nodiscard]] virtual size_t do_hash() const { return std::hash<int>{}(kind); }
+  [[nodiscard]] virtual std::unique_ptr<aggregation> clone() const = 0;
 
   // override functions for compound aggregations
   virtual std::vector<std::unique_ptr<aggregation>> get_simple_aggregations(
@@ -118,7 +118,7 @@ class aggregation {
  */
 class rolling_aggregation : public virtual aggregation {
  public:
-  ~rolling_aggregation() = default;
+  ~rolling_aggregation() override = default;
 
  protected:
   rolling_aggregation() {}
@@ -130,7 +130,7 @@ class rolling_aggregation : public virtual aggregation {
  */
 class groupby_aggregation : public virtual aggregation {
  public:
-  ~groupby_aggregation() = default;
+  ~groupby_aggregation() override = default;
 
  protected:
   groupby_aggregation() {}
@@ -141,7 +141,7 @@ class groupby_aggregation : public virtual aggregation {
  */
 class groupby_scan_aggregation : public virtual aggregation {
  public:
-  ~groupby_scan_aggregation() = default;
+  ~groupby_scan_aggregation() override = default;
 
  protected:
   groupby_scan_aggregation() {}

cpp/include/cudf/ast/detail/expression_evaluator.cuh

@@ -76,7 +76,7 @@ struct expression_result {
     subclass().template set_value<Element>(index, result);
   }
 
-  __device__ inline bool is_valid() const { return subclass().is_valid(); }
+  [[nodiscard]] __device__ inline bool is_valid() const { return subclass().is_valid(); }
 
   __device__ inline T value() const { return subclass().value(); }
 };
@@ -110,7 +110,7 @@ struct value_expression_result
   /**
    * @brief Returns true if the underlying data is valid and false otherwise.
    */
-  __device__ inline bool is_valid() const
+  [[nodiscard]] __device__ inline bool is_valid() const
   {
     if constexpr (has_nulls) { return _obj.has_value(); }
     return true;
@@ -174,7 +174,7 @@ struct mutable_column_expression_result
   /**
    * @brief Not implemented for this specialization.
    */
-  __device__ inline bool is_valid() const
+  [[nodiscard]] __device__ inline bool is_valid() const
   {
     // Not implemented since it would require modifying the API in the parent class to accept an
     // index.
@@ -186,7 +186,7 @@ struct mutable_column_expression_result
   /**
    * @brief Not implemented for this specialization.
    */
-  __device__ inline mutable_column_device_view value() const
+  [[nodiscard]] __device__ inline mutable_column_device_view value() const
   {
     // Not implemented since it would require modifying the API in the parent class to accept an
     // index.

cpp/include/cudf/ast/detail/expression_parser.hpp

@@ -166,7 +166,7 @@ class expression_parser {
    *
    * @return cudf::data_type
    */
-  cudf::data_type output_type() const;
+  [[nodiscard]] cudf::data_type output_type() const;
 
   /**
    * @brief Visit a literal expression.
@@ -206,10 +206,10 @@ class expression_parser {
    */
   class intermediate_counter {
    public:
-    intermediate_counter() : used_values(), max_used(0) {}
+    intermediate_counter() : used_values() {}
     cudf::size_type take();
     void give(cudf::size_type value);
-    cudf::size_type get_max_used() const { return max_used; }
+    [[nodiscard]] cudf::size_type get_max_used() const { return max_used; }
 
    private:
     /**
@@ -221,10 +221,10 @@ class expression_parser {
      *
      * @return cudf::size_type Smallest value not already in the container.
      */
-    cudf::size_type find_first_missing() const;
+    [[nodiscard]] cudf::size_type find_first_missing() const;
 
     std::vector<cudf::size_type> used_values;
-    cudf::size_type max_used;
+    cudf::size_type max_used{0};
   };
 
   expression_device_view device_expression_data;  ///< The collection of data required to evaluate