Skip to content

Commit

Permalink
Support gcc 12 as the C++ compiler (#13316)
Browse files Browse the repository at this point in the history
Corrects compilation issues found by gcc 12 and the new warnings it provides.

Three major classes of issues was found:

- incorrect lambda captures of the wrong container for the iterators being used
- incorrect implicit conversions
- warnings generated by using std::transform and std::initializer_lists

Authors:
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - David Wendt (https://github.com/davidwendt)
  - Nghia Truong (https://github.com/ttnghia)
  - Mark Harris (https://github.com/harrism)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #13316
  • Loading branch information
robertmaynard authored May 11, 2023
1 parent deaba97 commit 98122d3
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 18 deletions.
5 changes: 5 additions & 0 deletions cpp/cmake/thirdparty/patches/nvbench_override.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
"file" : "nvbench/use_existing_fmt.diff",
"issue" : "Fix add support for using an existing fmt [https://github.com/NVIDIA/nvbench/pull/125]",
"fixed_in" : ""
},
{
"file" : "nvbench/public_fmt_dep_in_conda.diff",
"issue" : "Propagate fmt requirement in conda envs [https://github.com/NVIDIA/nvbench/pull/127]",
"fixed_in" : ""
}
]
}
Expand Down
8 changes: 5 additions & 3 deletions cpp/include/cudf_test/iterator_utilities.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
* Copyright (c) 2021-2023, 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 @@ -121,15 +121,17 @@ template <typename Iter>
* The returned iterator yields `false` (to mark `null`) at the indices corresponding to the
* pointers having `nullptr` values and `true` for the remaining indices.
*
* @note The input vector is referenced by the transform iterator, so the
* lifespan must be just as long as the iterator.
*
* @tparam T the data type
* @param ptrs The data pointers for which the validity iterator is computed
* @return auto Validity iterator
*/
template <class T>
[[maybe_unused]] static auto nulls_from_nullptrs(std::vector<T const*> const& ptrs)
{
// The vector `indices` is copied into the lambda as it can be destroyed at the caller site.
return thrust::make_transform_iterator(ptrs.begin(), [ptrs](auto ptr) { return ptr != nullptr; });
return thrust::make_transform_iterator(ptrs.begin(), [](auto ptr) { return ptr != nullptr; });
}

} // namespace iterators
Expand Down
14 changes: 8 additions & 6 deletions cpp/include/cudf_test/type_lists.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
* Copyright (c) 2019-2023, 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 @@ -84,11 +84,13 @@ std::enable_if_t<cudf::is_fixed_width<TypeParam>() && !cudf::is_timestamp_t<Type
thrust::host_vector<TypeParam>>
make_type_param_vector(std::initializer_list<T> const& init_list)
{
thrust::host_vector<TypeParam> vec(init_list.size());
std::transform(std::cbegin(init_list), std::cend(init_list), std::begin(vec), [](auto const& e) {
if constexpr (std::is_unsigned_v<TypeParam>) { return static_cast<TypeParam>(std::abs(e)); }
return static_cast<TypeParam>(e);
});
std::vector<T> input{init_list};
std::vector<TypeParam> vec(init_list.size());
std::transform(
std::cbegin(input), std::cend(input), std::begin(vec), [](auto const& e) -> TypeParam {
if constexpr (std::is_unsigned_v<TypeParam>) { return static_cast<TypeParam>(std::abs(e)); }
return static_cast<TypeParam>(e);
});
return vec;
}

Expand Down
6 changes: 3 additions & 3 deletions cpp/src/io/json/json_column.cu
Original file line number Diff line number Diff line change
Expand Up @@ -813,10 +813,10 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> device_json_co

// For string columns return ["offsets", "char"] schema
if (target_type.id() == type_id::STRING) {
return {std::move(col), {{"offsets"}, {"chars"}}};
return {std::move(col), std::vector<column_name_info>{{"offsets"}, {"chars"}}};
}
// Non-string leaf-columns (e.g., numeric) do not have child columns in the schema
return {std::move(col), {}};
return {std::move(col), std::vector<column_name_info>{}};
}
case json_col_t::StructColumn: {
std::vector<std::unique_ptr<column>> child_columns;
Expand Down Expand Up @@ -860,7 +860,7 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> device_json_co
data_type{type_id::INT8},
0,
rmm::device_buffer{0, stream, mr}),
{}}
std::vector<column_name_info>{}}
: device_json_column_to_cudf_column(
json_col.child_columns.begin()->second,
d_input,
Expand Down
7 changes: 4 additions & 3 deletions cpp/src/io/json/nested_json_gpu.cu
Original file line number Diff line number Diff line change
Expand Up @@ -1678,11 +1678,11 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> json_column_to

// For string columns return ["offsets", "char"] schema
if (target_type.id() == type_id::STRING) {
return {std::move(col), {{"offsets"}, {"chars"}}};
return {std::move(col), std::vector<column_name_info>{{"offsets"}, {"chars"}}};
}
// Non-string leaf-columns (e.g., numeric) do not have child columns in the schema
else {
return {std::move(col), {}};
return {std::move(col), std::vector<column_name_info>{}};
}
break;
}
Expand Down Expand Up @@ -1724,7 +1724,8 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> json_column_to
auto [child_column, names] =
json_col.child_columns.empty()
? std::pair<std::unique_ptr<column>,
std::vector<column_name_info>>{std::make_unique<column>(), {}}
std::vector<column_name_info>>{std::make_unique<column>(),
std::vector<column_name_info>{}}
: json_column_to_cudf_column(json_col.child_columns.begin()->second,
d_input,
options,
Expand Down
2 changes: 1 addition & 1 deletion cpp/tests/io/json_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1568,7 +1568,7 @@ TEST_P(JsonReaderParamTest, JsonDtypeParsing)

auto make_validity = [](std::vector<int> const& validity) {
return cudf::detail::make_counting_transform_iterator(
0, [=](auto i) -> bool { return static_cast<bool>(validity[i]); });
0, [&](auto i) -> bool { return static_cast<bool>(validity[i]); });
};

constexpr int int_ignore{};
Expand Down
4 changes: 2 additions & 2 deletions cpp/tests/scalar/scalar_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ TYPED_TEST_SUITE(TypedScalarTestWithoutFixedPoint, cudf::test::FixedWidthTypesWi
TYPED_TEST(TypedScalarTest, DefaultValidity)
{
using Type = cudf::device_storage_type_t<TypeParam>;
Type value = cudf::test::make_type_param_scalar<TypeParam>(7);
Type value = static_cast<Type>(cudf::test::make_type_param_scalar<TypeParam>(7));
cudf::scalar_type_t<TypeParam> s(value);

EXPECT_TRUE(s.is_valid());
Expand Down Expand Up @@ -71,7 +71,7 @@ TYPED_TEST(TypedScalarTestWithoutFixedPoint, SetNull)
TYPED_TEST(TypedScalarTest, CopyConstructor)
{
using Type = cudf::device_storage_type_t<TypeParam>;
Type value = cudf::test::make_type_param_scalar<TypeParam>(8);
Type value = static_cast<Type>(cudf::test::make_type_param_scalar<TypeParam>(8));
cudf::scalar_type_t<TypeParam> s(value);
auto s2 = s;

Expand Down

0 comments on commit 98122d3

Please sign in to comment.