Skip to content

Commit

Permalink
clang-tidy fixes part 3 (#16939)
Browse files Browse the repository at this point in the history 
Subset of improvements to the code base proposed by the latest version of clang-tidy.

**Note to reviewers**: The changeset looks deceptively large. Almost all of the change are really just switching from raw C-style arrays to C++ std::arrays.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Basit Ayantunde (https://github.com/lamarrr)

URL: #16939
  • Loading branch information
@vyasr @lamarrr
vyasr authored and lamarrr committed Sep 28, 2024
1 parent b09037c commit 08e2e67
Show file tree
Hide file tree
Showing 22 changed files with 483 additions and 512 deletions.
174 changes: 81 additions & 93 deletions cpp/tests/copying/copy_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,44 +73,45 @@ TYPED_TEST(CopyTest, CopyIfElseTestLong)
using T = TypeParam;

// make sure we span at least 2 warps
int num_els = 64;

bool mask[] = {true, false, true, false, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, false, false, false, false, true, true, true,
true, true, true, true, true, true, false, false, false, false, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + num_els);

bool lhs_v[] = {true, true, true, true, false, false, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true};
wrapper<T, int32_t> lhs_w({5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5},
lhs_v);

bool rhs_v[] = {true, true, true, true, true, true, false, false, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true};
wrapper<T, int32_t> rhs_w({6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6},
rhs_v);

bool exp_v[] = {true, true, true, true, false, false, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true};
wrapper<T, int32_t> expected_w({5, 6, 5, 6, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 6, 6, 6,
6, 5, 5, 5, 5, 5, 5, 5, 5, 5, 6, 6, 6, 6, 5, 5, 5, 5, 5, 5, 5, 5,
5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5},
exp_v);
constexpr int num_els = 64;

std::array<bool, num_els> mask{
true, false, true, false, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, false, false, false, false, true, true, true,
true, true, true, true, true, true, false, false, false, false, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

wrapper<T, int32_t> lhs_w(
{5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5},
{true, true, true, true, false, false, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true});

wrapper<T, int32_t> rhs_w(
{6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6},
{true, true, true, true, true, true, false, false, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true});

wrapper<T, int32_t> expected_w(
{5, 6, 5, 6, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 6, 6, 6,
6, 5, 5, 5, 5, 5, 5, 5, 5, 5, 6, 6, 6, 6, 5, 5, 5, 5, 5, 5, 5, 5,
5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5},
{true, true, true, true, false, false, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true, true,
true, true, true, true, true, true, true, true, true, true, true, true});

auto out = cudf::copy_if_else(lhs_w, rhs_w, mask_w);
CUDF_TEST_EXPECT_COLUMNS_EQUAL(out->view(), expected_w);
Expand Down Expand Up @@ -318,19 +319,17 @@ TYPED_TEST(CopyTestNumeric, CopyIfElseTestScalarColumn)
{
using T = TypeParam;

int num_els = 4;

bool mask[] = {true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + num_els);
std::array mask{true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

cudf::numeric_scalar<T> lhs_w(5);

auto const rhs = cudf::test::make_type_param_vector<T>({6, 6, 6, 6});
bool rhs_v[] = {true, false, true, true};
wrapper<T> rhs_w(rhs.begin(), rhs.end(), rhs_v);
std::array rhs_v{true, false, true, true};
wrapper<T> rhs_w(rhs.begin(), rhs.end(), rhs_v.begin());

auto const expected = cudf::test::make_type_param_vector<T>({5, 6, 6, 5});
wrapper<T> expected_w(expected.begin(), expected.end(), rhs_v);
wrapper<T> expected_w(expected.begin(), expected.end(), rhs_v.begin());

auto out = cudf::copy_if_else(lhs_w, rhs_w, mask_w);
CUDF_TEST_EXPECT_COLUMNS_EQUAL(out->view(), expected_w);
Expand All @@ -340,20 +339,18 @@ TYPED_TEST(CopyTestNumeric, CopyIfElseTestColumnScalar)
{
using T = TypeParam;

int num_els = 4;

bool mask[] = {true, false, false, true};
bool mask_v[] = {true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + num_els, mask_v);
std::array mask{true, false, false, true};
std::array mask_v{true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end(), mask_v.begin());

auto const lhs = cudf::test::make_type_param_vector<T>({5, 5, 5, 5});
bool lhs_v[] = {false, true, true, true};
wrapper<T> lhs_w(lhs.begin(), lhs.end(), lhs_v);
std::array lhs_v{false, true, true, true};
wrapper<T> lhs_w(lhs.begin(), lhs.end(), lhs_v.begin());

cudf::numeric_scalar<T> rhs_w(6);

auto const expected = cudf::test::make_type_param_vector<T>({5, 6, 6, 6});
wrapper<T> expected_w(expected.begin(), expected.end(), lhs_v);
wrapper<T> expected_w(expected.begin(), expected.end(), lhs_v.begin());

auto out = cudf::copy_if_else(lhs_w, rhs_w, mask_w);
CUDF_TEST_EXPECT_COLUMNS_EQUAL(out->view(), expected_w);
Expand All @@ -363,16 +360,14 @@ TYPED_TEST(CopyTestNumeric, CopyIfElseTestScalarScalar)
{
using T = TypeParam;

int num_els = 4;

bool mask[] = {true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + num_els);
std::array mask{true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

cudf::numeric_scalar<T> lhs_w(5);
cudf::numeric_scalar<T> rhs_w(6, false);

auto const expected = cudf::test::make_type_param_vector<T>({5, 6, 6, 5});
wrapper<T> expected_w(expected.begin(), expected.end(), mask);
wrapper<T> expected_w(expected.begin(), expected.end(), mask.begin());

auto out = cudf::copy_if_else(lhs_w, rhs_w, mask_w);
CUDF_TEST_EXPECT_COLUMNS_EQUAL(out->view(), expected_w);
Expand Down Expand Up @@ -405,17 +400,15 @@ TYPED_TEST(CopyTestChrono, CopyIfElseTestScalarColumn)
{
using T = TypeParam;

int num_els = 4;

bool mask[] = {true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + num_els);
std::array mask{true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

auto lhs_w = create_chrono_scalar<T>{}(cudf::test::make_type_param_scalar<T>(5), true);

bool rhs_v[] = {true, false, true, true};
wrapper<T, int32_t> rhs_w({6, 6, 6, 6}, rhs_v);
std::array rhs_v{true, false, true, true};
wrapper<T, int32_t> rhs_w({6, 6, 6, 6}, rhs_v.begin());

wrapper<T, int32_t> expected_w({5, 6, 6, 5}, rhs_v);
wrapper<T, int32_t> expected_w({5, 6, 6, 5}, rhs_v.begin());

auto out = cudf::copy_if_else(lhs_w, rhs_w, mask_w);
CUDF_TEST_EXPECT_COLUMNS_EQUAL(out->view(), expected_w);
Expand All @@ -425,17 +418,15 @@ TYPED_TEST(CopyTestChrono, CopyIfElseTestColumnScalar)
{
using T = TypeParam;

int num_els = 4;

bool mask[] = {true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + num_els);
std::array mask{true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

bool lhs_v[] = {false, true, true, true};
wrapper<T, int32_t> lhs_w({5, 5, 5, 5}, lhs_v);
std::array lhs_v{false, true, true, true};
wrapper<T, int32_t> lhs_w({5, 5, 5, 5}, lhs_v.begin());

auto rhs_w = create_chrono_scalar<T>{}(cudf::test::make_type_param_scalar<T>(6), true);

wrapper<T, int32_t> expected_w({5, 6, 6, 5}, lhs_v);
wrapper<T, int32_t> expected_w({5, 6, 6, 5}, lhs_v.begin());

auto out = cudf::copy_if_else(lhs_w, rhs_w, mask_w);
CUDF_TEST_EXPECT_COLUMNS_EQUAL(out->view(), expected_w);
Expand All @@ -445,15 +436,13 @@ TYPED_TEST(CopyTestChrono, CopyIfElseTestScalarScalar)
{
using T = TypeParam;

int num_els = 4;

bool mask[] = {true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + num_els);
std::array mask{true, false, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

auto lhs_w = create_chrono_scalar<T>{}(cudf::test::make_type_param_scalar<T>(5), true);
auto rhs_w = create_chrono_scalar<T>{}(cudf::test::make_type_param_scalar<T>(6), false);

wrapper<T, int32_t> expected_w({5, 6, 6, 5}, mask);
wrapper<T, int32_t> expected_w({5, 6, 6, 5}, mask.begin());

auto out = cudf::copy_if_else(lhs_w, rhs_w, mask_w);
CUDF_TEST_EXPECT_COLUMNS_EQUAL(out->view(), expected_w);
Expand Down Expand Up @@ -483,9 +472,9 @@ TEST_F(StringsCopyIfElseTest, CopyIfElse)
std::vector<char const*> h_strings2{"zz", "", "yyy", "w", "ééé", "ooo"};
cudf::test::strings_column_wrapper strings2(h_strings2.begin(), h_strings2.end(), valids);

bool mask[] = {true, true, false, true, false, true};
bool mask_v[] = {true, true, true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + 6, mask_v);
std::array mask{true, true, false, true, false, true};
std::array mask_v{true, true, true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end(), mask_v.begin());

auto results = cudf::copy_if_else(strings1, strings2, mask_w);

Expand All @@ -510,9 +499,9 @@ TEST_F(StringsCopyIfElseTest, CopyIfElseScalarColumn)
std::vector<char const*> h_strings2{"zz", "", "yyy", "w", "ééé", "ooo"};
cudf::test::strings_column_wrapper strings2(h_strings2.begin(), h_strings2.end(), valids);

bool mask[] = {true, false, true, false, true, false};
bool mask_v[] = {true, true, true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + 6, mask_v);
std::array mask{true, false, true, false, true, false};
std::array mask_v{true, true, true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end(), mask_v.begin());

auto results = cudf::copy_if_else(strings1, strings2, mask_w);

Expand All @@ -538,8 +527,8 @@ TEST_F(StringsCopyIfElseTest, CopyIfElseColumnScalar)
std::vector<char const*> h_strings2{"zz", "", "yyy", "w", "ééé", "ooo"};
cudf::test::strings_column_wrapper strings2(h_strings2.begin(), h_strings2.end(), valids);

bool mask[] = {false, true, true, true, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + 6);
std::array mask{false, true, true, true, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

auto results = cudf::copy_if_else(strings2, strings1, mask_w);

Expand All @@ -565,9 +554,8 @@ TEST_F(StringsCopyIfElseTest, CopyIfElseScalarScalar)
std::vector<char const*> h_string2{"aaa"};
cudf::string_scalar string2{h_string2[0], false};

constexpr cudf::size_type mask_size = 6;
bool mask[] = {true, false, true, false, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + mask_size);
std::array mask{true, false, true, false, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

auto results = cudf::copy_if_else(string1, string2, mask_w);

Expand Down Expand Up @@ -652,9 +640,9 @@ TEST_F(DictionaryCopyIfElseTest, ColumnColumn)
cudf::test::dictionary_column_wrapper<std::string> input2(
h_strings2.begin(), h_strings2.end(), valids);

bool mask[] = {true, true, false, true, false, true};
bool mask_v[] = {true, true, true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + 6, mask_v);
std::array mask{true, true, false, true, false, true};
std::array mask_v{true, true, true, true, true, false};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end(), mask_v.begin());

auto results = cudf::copy_if_else(input1, input2, mask_w);
auto decoded = cudf::dictionary::decode(cudf::dictionary_column_view(results->view()));
Expand All @@ -679,8 +667,8 @@ TEST_F(DictionaryCopyIfElseTest, ColumnScalar)
cudf::test::dictionary_column_wrapper<std::string> input2(
h_strings.begin(), h_strings.end(), valids);

bool mask[] = {false, true, true, true, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask, mask + 6);
std::array mask{false, true, true, true, false, true};
cudf::test::fixed_width_column_wrapper<bool> mask_w(mask.begin(), mask.end());

auto results = cudf::copy_if_else(input2, input1, mask_w);
auto decoded = cudf::dictionary::decode(cudf::dictionary_column_view(results->view()));
Expand Down
13 changes: 5 additions & 8 deletions cpp/tests/filling/sequence_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ TYPED_TEST(SequenceTypedTestFixture, Incrementing)

cudf::size_type num_els = 10;

T expected[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
cudf::test::fixed_width_column_wrapper<T> expected_w(expected, expected + num_els);
cudf::test::fixed_width_column_wrapper<T> expected_w({0, 1, 2, 3, 4, 5, 6, 7, 8, 9});

auto result = cudf::sequence(num_els, init, step);

Expand All @@ -58,8 +57,8 @@ TYPED_TEST(SequenceTypedTestFixture, Decrementing)

cudf::size_type num_els = 10;

T expected[] = {0, -5, -10, -15, -20, -25, -30, -35, -40, -45};
cudf::test::fixed_width_column_wrapper<T> expected_w(expected, expected + num_els);
cudf::test::fixed_width_column_wrapper<T> expected_w(
{0, -5, -10, -15, -20, -25, -30, -35, -40, -45});

auto result = cudf::sequence(num_els, init, step);

Expand All @@ -75,8 +74,7 @@ TYPED_TEST(SequenceTypedTestFixture, EmptyOutput)

cudf::size_type num_els = 0;

T expected[] = {};
cudf::test::fixed_width_column_wrapper<T> expected_w(expected, expected + num_els);
cudf::test::fixed_width_column_wrapper<T> expected_w({});

auto result = cudf::sequence(num_els, init, step);

Expand Down Expand Up @@ -121,8 +119,7 @@ TYPED_TEST(SequenceTypedTestFixture, DefaultStep)

cudf::size_type num_els = 10;

T expected[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
cudf::test::fixed_width_column_wrapper<T> expected_w(expected, expected + num_els);
cudf::test::fixed_width_column_wrapper<T> expected_w({0, 1, 2, 3, 4, 5, 6, 7, 8, 9});

auto result = cudf::sequence(num_els, init);

Expand Down
7 changes: 4 additions & 3 deletions cpp/tests/groupby/collect_list_tests.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2023, NVIDIA CORPORATION.
* Copyright (c) 2021-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.
Expand Down Expand Up @@ -127,8 +127,9 @@ TYPED_TEST(groupby_collect_list_test, CollectListsWithNullExclusion)
using LCW = cudf::test::lists_column_wrapper<V, int32_t>;

cudf::test::fixed_width_column_wrapper<K, int32_t> keys{1, 1, 2, 2, 3, 3, 4, 4};
bool const validity_mask[] = {true, false, false, true, true, true, false, false};
LCW values{{{1, 2}, {3, 4}, {5, 6, 7}, LCW{}, {9, 10}, {11}, {20, 30, 40}, LCW{}}, validity_mask};
std::array const validity_mask{true, false, false, true, true, true, false, false};
LCW values{{{1, 2}, {3, 4}, {5, 6, 7}, LCW{}, {9, 10}, {11}, {20, 30, 40}, LCW{}},
validity_mask.data()};

cudf::test::fixed_width_column_wrapper<K, int32_t> expect_keys{1, 2, 3, 4};

Expand Down
Loading

0 comments on commit 08e2e67

Please sign in to comment.