Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clang-tidy to libcudf #9860

Merged
merged 50 commits into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
9d9cf87
Add .clang-tidy file
codereport Dec 7, 2021
79076d3
Clang-tiday single file
codereport Dec 7, 2021
1f710f7
CMake change
codereport Dec 7, 2021
a79f8e8
Initial script
codereport Dec 8, 2021
77c60d8
Temporary script hacks
codereport Dec 8, 2021
c213e09
Update script from cuml to cudf
codereport Dec 8, 2021
3a9fbed
Merge branch 'branch-22.02' into clang-tidy
codereport Dec 13, 2021
a9b43e0
Temporary incremental changes
codereport Dec 14, 2021
c7285e0
Clang-tidy cpp-coreguideline changes
codereport Dec 14, 2021
dff04d2
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 6, 2022
e772fc3
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 7, 2022
7af0249
modernize- fixes
codereport Jan 10, 2022
9379001
Revert "modernize- fixes"
codereport Jan 10, 2022
49aa922
modernize- fixes
codereport Jan 10, 2022
e557639
Clang-format fix
codereport Jan 10, 2022
c09e357
Pre-cache null count
codereport Jan 11, 2022
b546dc1
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 11, 2022
077ee81
Missing merge change
codereport Jan 11, 2022
d0f5d89
More modernize- changes
codereport Jan 11, 2022
8dad976
Clang-format
codereport Jan 11, 2022
97e1861
Reverst cxxopts changes
codereport Jan 11, 2022
669e828
Fix
codereport Jan 11, 2022
757dbba
Parquet test fix
codereport Jan 11, 2022
2bfa25d
More modernize- changes
codereport Jan 11, 2022
abf7fc9
Clang format
codereport Jan 11, 2022
1e85795
Updated/cleaned up .clang-tidy
codereport Jan 12, 2022
5c094ff
More modernize- changes
codereport Jan 12, 2022
d89566e
Reverse python script commit to isolate
codereport Jan 12, 2022
e2f248f
Clang-format
codereport Jan 12, 2022
b8a7744
Clean up python script
codereport Jan 12, 2022
b4a35c7
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 12, 2022
0cad418
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 12, 2022
6d3fd75
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 13, 2022
8b05ed9
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 14, 2022
c538fc4
Remove script and Cmake changes
codereport Jan 17, 2022
23c6e3a
Fix
codereport Jan 17, 2022
eccdb9e
Fix double [[nodiscard]]
codereport Jan 18, 2022
7b7aa57
Addressing PR comments
codereport Jan 18, 2022
780a799
Reverst nvt3.hpp
codereport Jan 18, 2022
45fc11e
Addressing PR comments
codereport Jan 18, 2022
5599c85
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 18, 2022
346d81f
Remove CamelCase on modernize loop
codereport Jan 18, 2022
8b92e54
Reverted trailing return type
codereport Jan 19, 2022
3b1666a
Revert trailing return type #2
codereport Jan 19, 2022
1726496
Update .clang-tidy file
codereport Jan 19, 2022
b06c7a4
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 19, 2022
5fc4fd7
Modernize changes
codereport Jan 19, 2022
7f8c0d7
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 19, 2022
4724ce6
Addres PR comments
codereport Jan 19, 2022
5bf662a
Merge branch 'branch-22.02' into clang-tidy
codereport Jan 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
25 changes: 25 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
Checks:
'modernize-*,
-modernize-use-equals-default,
-modernize-concat-nested-namespaces'
harrism marked this conversation as resolved.
Show resolved Hide resolved

# -modernize-use-equals-default # auto-fix is broken (doesn't insert =default correctly)
# -modernize-concat-nested-namespaces # auto-fix is broken (can delete code)
harrism marked this conversation as resolved.
Show resolved Hide resolved

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'
...
14 changes: 7 additions & 7 deletions cpp/benchmarks/common/generate_benchmark_input.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ auto deterministic_engine(unsigned seed) { return std::mt19937{seed}; }
* Computes the mean value for a distribution of given type and value bounds.
*/
template <typename T>
T get_distribution_mean(distribution_params<T> const& dist)
auto get_distribution_mean(distribution_params<T> const& dist) -> T
codereport marked this conversation as resolved.
Show resolved Hide resolved
{
switch (dist.id) {
case distribution_id::NORMAL:
Expand Down Expand Up @@ -77,14 +77,14 @@ std::enable_if_t<!cudf::is_fixed_width<T>(), size_t> avg_element_size(data_profi
}

template <>
size_t avg_element_size<cudf::string_view>(data_profile const& profile)
auto avg_element_size<cudf::string_view>(data_profile const& profile) -> size_t
{
auto const dist = profile.get_distribution_params<cudf::string_view>().length_params;
return get_distribution_mean(dist);
}

template <>
size_t avg_element_size<cudf::list_view>(data_profile const& profile)
auto avg_element_size<cudf::list_view>(data_profile const& profile) -> size_t
{
auto const dist_params = profile.get_distribution_params<cudf::list_view>();
auto const single_level_mean = get_distribution_mean(dist_params.length_params);
Expand All @@ -94,13 +94,13 @@ size_t avg_element_size<cudf::list_view>(data_profile const& profile)

struct avg_element_size_fn {
template <typename T>
size_t operator()(data_profile const& profile)
auto operator()(data_profile const& profile) -> size_t
{
return avg_element_size<T>(profile);
}
};

size_t avg_element_bytes(data_profile const& profile, cudf::type_id tid)
auto avg_element_bytes(data_profile const& profile, cudf::type_id tid) -> size_t
{
return cudf::type_dispatcher(cudf::data_type(tid), avg_element_size_fn{}, profile);
}
Expand Down Expand Up @@ -222,7 +222,7 @@ struct random_value_fn<T, typename std::enable_if_t<std::is_same_v<T, bool>>> {
bool operator()(std::mt19937& engine) { return b_dist(engine); }
};

size_t null_mask_size(cudf::size_type num_rows)
auto null_mask_size(cudf::size_type num_rows) -> size_t
{
constexpr size_t bitmask_bits = cudf::detail::size_in_bits<cudf::bitmask_type>();
return (num_rows + bitmask_bits - 1) / bitmask_bits;
Expand Down Expand Up @@ -428,7 +428,7 @@ std::unique_ptr<cudf::column> create_random_column<cudf::string_view>(data_profi
auto const avg_run_len = profile.get_avg_run_length();
auto run_len_dist = create_run_length_dist(avg_run_len);

string_column_data out_col(num_rows, num_rows * avg_string_len);
auto out_col = string_column_data(num_rows, num_rows * avg_string_len);
std::uniform_int_distribution<cudf::size_type> sample_dist{0, cardinality - 1};
for (cudf::size_type row = 0; row < num_rows; ++row) {
if (cardinality == 0) {
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/common/generate_benchmark_input.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ class data_profile {

auto get_bool_probability() const { return bool_probability; }
auto get_null_frequency() const { return null_frequency; };
codereport marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
8 changes: 5 additions & 3 deletions cpp/benchmarks/copying/contiguous_split_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -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();
codereport marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down Expand Up @@ -105,7 +107,7 @@ void BM_contiguous_split(benchmark::State& state)
class ContiguousSplitStrings : public cudf::benchmark {
};

int rand_range(int r)
auto rand_range(int r) -> int
{
return static_cast<int>((static_cast<float>(rand()) / static_cast<float>(RAND_MAX)) *
(float)(r - 1));
Expand Down
2 changes: 1 addition & 1 deletion cpp/benchmarks/copying/gather_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -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; });
Expand Down
2 changes: 1 addition & 1 deletion cpp/benchmarks/copying/scatter_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -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; });
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/copying/shift_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ struct value_func {
T* data;
cudf::size_type offset;

__device__ T operator()(int idx) { return data[idx - offset]; }
__device__ auto operator()(int idx) -> T { return data[idx - offset]; }
};

struct validity_func {
cudf::size_type size;
cudf::size_type offset;

__device__ bool operator()(int idx)
__device__ auto operator()(int idx) -> bool
{
auto source_idx = idx - offset;
return source_idx < 0 || source_idx >= size;
Expand Down
13 changes: 8 additions & 5 deletions cpp/benchmarks/fixture/benchmark_fixture.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/io/csv/csv_reader_benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/io/csv/csv_writer_benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/io/cuio_benchmark_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ cuio_source_sink_pair::cuio_source_sink_pair(io_type type)
{
}

cudf_io::source_info cuio_source_sink_pair::make_source_info()
auto cuio_source_sink_pair::make_source_info() -> cudf_io::source_info
{
switch (type) {
case io_type::FILEPATH: return cudf_io::source_info(file_name);
Expand All @@ -50,7 +50,7 @@ cudf_io::source_info cuio_source_sink_pair::make_source_info()
}
}

cudf_io::sink_info cuio_source_sink_pair::make_sink_info()
auto cuio_source_sink_pair::make_sink_info() -> cudf_io::sink_info
{
switch (type) {
case io_type::VOID: return cudf_io::sink_info();
Expand Down
8 changes: 4 additions & 4 deletions cpp/benchmarks/io/text/multibyte_split_benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ enum data_chunk_source_type {
host,
};

static cudf::string_scalar create_random_input(int32_t num_chars,
double delim_factor,
double deviation,
std::string delim)
static auto create_random_input(int32_t num_chars,
double delim_factor,
double deviation,
std::string delim) -> cudf::string_scalar
{
auto const num_delims = static_cast<int32_t>((num_chars * delim_factor) / delim.size());
auto const num_delim_chars = num_delims * delim.size();
Expand Down
5 changes: 3 additions & 2 deletions cpp/benchmarks/iterator/iterator_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
#include <random>

template <typename T>
T random_int(T min, T max)
auto random_int(T min, T max) -> T
{
static unsigned seed = 13377331;
static std::mt19937 engine{seed};
Expand Down Expand Up @@ -158,7 +158,8 @@ void BM_iterator(benchmark::State& state)

// operator+ defined for pair iterator reduction
template <typename T>
__device__ thrust::pair<T, bool> operator+(thrust::pair<T, bool> lhs, thrust::pair<T, bool> rhs)
__device__ auto operator+(thrust::pair<T, bool> lhs, thrust::pair<T, bool> rhs)
-> thrust::pair<T, bool>
{
return thrust::pair<T, bool>{lhs.first * lhs.second + rhs.first * rhs.second,
lhs.second + rhs.second};
Expand Down
2 changes: 1 addition & 1 deletion cpp/benchmarks/lists/copying/scatter_lists_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void size_range(benchmark::internal::Benchmark* b)
}

template <typename T>
T random_int(T min, T max)
auto random_int(T min, T max) -> T
{
static unsigned const seed = 13377331;
static std::mt19937 engine{seed};
Expand Down
2 changes: 1 addition & 1 deletion cpp/benchmarks/string/factory_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
namespace {
using string_pair = thrust::pair<char const*, cudf::size_type>;
struct string_view_to_pair {
__device__ string_pair operator()(thrust::pair<cudf::string_view, bool> const& p)
__device__ auto operator()(thrust::pair<cudf::string_view, bool> const& p) -> string_pair
{
return (p.second) ? string_pair{p.first.data(), p.first.size_bytes()} : string_pair{nullptr, 0};
}
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/string/json_benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
class JsonPath : public cudf::benchmark {
};

float frand() { return static_cast<float>(rand()) / static_cast<float>(RAND_MAX); }
auto frand() -> float { return static_cast<float>(rand()) / static_cast<float>(RAND_MAX); }

int rand_range(int min, int max) { return min + static_cast<int>(frand() * (max - min)); }
auto rand_range(int min, int max) -> int { return min + static_cast<int>(frand() * (max - min)); }

std::vector<std::string> Books{
"{\n\"category\": \"reference\",\n\"author\": \"Nigel Rees\",\n\"title\": \"Sayings of the "
Expand Down
5 changes: 2 additions & 3 deletions cpp/benchmarks/string/url_decode_benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,8 @@ struct url_string_generator {
}
};

cudf::test::strings_column_wrapper generate_column(cudf::size_type num_rows,
cudf::size_type chars_per_row,
double esc_seq_chance)
auto generate_column(cudf::size_type num_rows, cudf::size_type chars_per_row, double esc_seq_chance)
-> cudf::test::strings_column_wrapper
{
std::mt19937 engine(1);
url_string_generator url_gen(chars_per_row, esc_seq_chance);
Expand Down
8 changes: 4 additions & 4 deletions cpp/benchmarks/type_dispatcher/type_dispatcher_benchmark.cu
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ enum FunctorType { BANDWIDTH_BOUND, COMPUTE_BOUND };

template <class NotFloat, FunctorType ft, class DisableNotFloat = void>
struct Functor {
static __device__ NotFloat f(NotFloat x) { return x; }
static __device__ auto f(NotFloat x) -> NotFloat { return x; }
};

template <class Float, FunctorType ft>
Expand Down Expand Up @@ -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; });

Expand Down
12 changes: 6 additions & 6 deletions cpp/include/cudf/aggregation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -118,7 +118,7 @@ class aggregation {
*/
class rolling_aggregation : public virtual aggregation {
public:
~rolling_aggregation() = default;
~rolling_aggregation() override = default;

protected:
rolling_aggregation() {}
Expand All @@ -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() {}
Expand All @@ -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() {}
Expand Down
Loading