diff --git a/cpp/REVIEW_GUIDELINES.md b/cpp/REVIEW_GUIDELINES.md index 8cdc790765f0..15d401c3fd6c 100644 --- a/cpp/REVIEW_GUIDELINES.md +++ b/cpp/REVIEW_GUIDELINES.md @@ -42,6 +42,7 @@ - CUDA stream/event leaks or improper cleanup - Missing RAII or proper cleanup, including in exception paths - Raw owning pointers instead of `std::unique_ptr`, `std::shared_ptr`, `std::reference_wrapper` +- Moving from the raw pointer returned by `std::unique_ptr::release()` (e.g. `std::move(*ptr.release())`, `ptr.release()[0]`, `auto p = contents.data.release();`) leaks the host object; use `std::move(*ptr)` instead. (`table::release()` and `device_uvector::release()` return owning values and are fine.) ### API Breaking Changes - C++ API changes without proper deprecation warnings diff --git a/cpp/benchmarks/common/generate_input.cu b/cpp/benchmarks/common/generate_input.cu index eb57a85a13ac..25419cdb34a5 100644 --- a/cpp/benchmarks/common/generate_input.cu +++ b/cpp/benchmarks/common/generate_input.cu @@ -523,11 +523,8 @@ std::unique_ptr create_random_utf8_string_column(data_profile cons cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED)), 0}; - return cudf::make_strings_column(num_rows, - std::move(offsets), - chars.release(), - null_count, - std::move(*result_bitmask.release())); + return cudf::make_strings_column( + num_rows, std::move(offsets), chars.release(), null_count, std::move(*result_bitmask)); } // Forward declarations for create_rand_col_fn @@ -632,7 +629,7 @@ std::unique_ptr create_random_column(data_profile const& profile, 0}; return std::make_unique( - dtype, num_rows, data.release(), std::move(*result_bitmask.release()), null_count); + dtype, num_rows, data.release(), std::move(*result_bitmask), null_count); } /** @@ -724,7 +721,7 @@ std::unique_ptr create_random_column(data_profi current_child += children_to_adopt.size(); *current_parent = cudf::make_structs_column( - num_rows, std::move(children_to_adopt), null_count, std::move(*null_mask.release())); + num_rows, std::move(children_to_adopt), null_count, std::move(*null_mask)); } if (lvl == 1) { @@ -812,7 +809,7 @@ std::unique_ptr create_random_column(data_profile std::move(offsets_column), std::move(current_child_column), null_count, - std::move(*null_mask.release())); + std::move(*null_mask)); if (auto const cv = list_column->view(); cudf::has_nonempty_nulls(cv, cudf::get_default_stream())) { @@ -844,7 +841,7 @@ std::unique_ptr create_distinct_rows_column(data_profile const& pr auto null_mask = valid_dist(engine, num_rows); auto [result_bitmask, null_count] = cudf::bools_to_mask(cudf::device_span(null_mask), cudf::get_default_stream()); - col->set_null_mask(std::move(*result_bitmask.release()), null_count); + col->set_null_mask(std::move(*result_bitmask), null_count); } return col; diff --git a/cpp/benchmarks/io/text/multibyte_split.cpp b/cpp/benchmarks/io/text/multibyte_split.cpp index 845c6c7edf2e..53dcb21c86c6 100644 --- a/cpp/benchmarks/io/text/multibyte_split.cpp +++ b/cpp/benchmarks/io/text/multibyte_split.cpp @@ -74,7 +74,7 @@ static cudf::string_scalar create_random_input(int32_t num_chars, // extract the chars from the returned strings column. auto input_column_contents = input_column->release(); - auto chars_buffer = input_column_contents.data.release(); + auto chars_buffer = std::move(input_column_contents.data); // turn the chars in to a string scalar. return cudf::string_scalar(std::move(*chars_buffer)); diff --git a/cpp/src/copying/copy_range.cu b/cpp/src/copying/copy_range.cu index 7ae0cfcd5c7e..ab41bcf9f2c5 100644 --- a/cpp/src/copying/copy_range.cu +++ b/cpp/src/copying/copy_range.cu @@ -184,7 +184,7 @@ std::unique_ptr out_of_place_copy_range_dispatch::operator()( new_indices.type(), new_indices.size(), - std::move(*(target_indices->release().data.release())), + std::move(*(target_indices->release().data)), cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED, stream, mr), 0); @@ -194,7 +194,7 @@ std::unique_ptr out_of_place_copy_range_dispatch::operator() concatenate(host_span columns, // finally, frankenstein that dictionary column together return make_dictionary_column(std::move(keys_column), std::move(indices_column), - std::move(*(contents.null_mask.release())), + std::move(*(contents.null_mask)), null_count); } diff --git a/cpp/src/dictionary/dictionary_factories.cu b/cpp/src/dictionary/dictionary_factories.cu index 63085759b44c..8c7b502a75cf 100644 --- a/cpp/src/dictionary/dictionary_factories.cu +++ b/cpp/src/dictionary/dictionary_factories.cu @@ -141,7 +141,7 @@ std::unique_ptr make_dictionary_column(std::unique_ptr keys, if (new_type == indices_type || cudf::size_of(new_type) == cudf::size_of(indices_type)) { return std::make_unique(new_type, indices_size, - std::move(*(contents.data.release())), + std::move(*(contents.data)), cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED), 0); } @@ -150,10 +150,8 @@ std::unique_ptr make_dictionary_column(std::unique_ptr keys, return cudf::detail::cast(cast_view, new_type, stream, mr); }(); - return make_dictionary_column(std::move(keys), - std::move(indices_column), - std::move(*(contents.null_mask.release())), - null_count); + return make_dictionary_column( + std::move(keys), std::move(indices_column), std::move(*(contents.null_mask)), null_count); } } // namespace cudf diff --git a/cpp/src/interop/from_arrow_host_strings.cu b/cpp/src/interop/from_arrow_host_strings.cu index bd5c058a0c4c..c8d2363b74f8 100644 --- a/cpp/src/interop/from_arrow_host_strings.cu +++ b/cpp/src/interop/from_arrow_host_strings.cu @@ -62,7 +62,7 @@ std::unique_ptr from_arrow_string( std::move(offsets_column), std::move(chars), null_count, - std::move(*mask.release())); + std::move(*mask)); } constexpr int stringview_vector_idx = 1; diff --git a/cpp/src/io/json/write_json.cu b/cpp/src/io/json/write_json.cu index 9c95c976eca1..3701ff5d3994 100644 --- a/cpp/src/io/json/write_json.cu +++ b/cpp/src/io/json/write_json.cu @@ -383,7 +383,7 @@ std::unique_ptr struct_to_strings(table_view const& strings_columns, num_rows, std::make_unique( std::move(row_string_offsets), cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED), 0), - std::move(chars_data.release()[0]), + std::move(*chars_data), 0, cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED)); } @@ -546,7 +546,7 @@ std::unique_ptr join_list_of_strings(lists_column_view const& lists_stri num_lists, std::make_unique( std::move(row_string_offsets), cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED), 0), - std::move(chars_data.release()[0]), + std::move(*chars_data), lists_strings.null_count(), cudf::detail::copy_bitmask(lists_strings.parent(), stream, mr)); } diff --git a/cpp/src/io/utilities/column_buffer.cpp b/cpp/src/io/utilities/column_buffer.cpp index ad67cc1e1e6f..93794bcb8e55 100644 --- a/cpp/src/io/utilities/column_buffer.cpp +++ b/cpp/src/io/utilities/column_buffer.cpp @@ -210,7 +210,7 @@ std::unique_ptr make_column(column_buffer_base& buffer, auto col_content = string_col->release(); // convert to uint8 column, strings are currently stored as int8 - auto data = col_content.data.release(); + auto data = std::move(col_content.data); auto char_size = data->size(); CUDF_EXPECTS(char_size < static_cast(std::numeric_limits::max()), diff --git a/cpp/src/lists/contains.cu b/cpp/src/lists/contains.cu index f5044020012d..38a2ec067292 100644 --- a/cpp/src/lists/contains.cu +++ b/cpp/src/lists/contains.cu @@ -256,7 +256,7 @@ std::unique_ptr to_contains(std::unique_ptr&& key_positions, auto const null_count = key_positions->null_count(); [[maybe_unused]] auto [data, null_mask, children] = key_positions->release(); - result->set_null_mask(std::move(*null_mask.release()), null_count); + result->set_null_mask(std::move(*null_mask), null_count); return result; } diff --git a/cpp/src/reductions/collect_ops.cu b/cpp/src/reductions/collect_ops.cu index f02f5cb14bd0..0d686b4430a6 100644 --- a/cpp/src/reductions/collect_ops.cu +++ b/cpp/src/reductions/collect_ops.cu @@ -38,10 +38,10 @@ std::unique_ptr collect_list(column_view const& col, rmm::device_async_resource_ref mr) { if (need_handle_nulls(col, null_handling)) { - auto d_view = column_device_view::create(col, stream); - auto filter = cudf::detail::validity_accessor(*d_view); - auto null_purged_table = cudf::detail::copy_if(table_view{{col}}, filter, stream, mr); - column* null_purged_col = null_purged_table->release().front().release(); + auto d_view = column_device_view::create(col, stream); + auto filter = cudf::detail::validity_accessor(*d_view); + auto null_purged_table = cudf::detail::copy_if(table_view{{col}}, filter, stream, mr); + auto null_purged_col = std::move(null_purged_table->release().front()); null_purged_col->set_null_mask( cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED, stream, mr), 0); return std::make_unique(std::move(*null_purged_col), true, stream, mr); diff --git a/cpp/src/reductions/histogram.cu b/cpp/src/reductions/histogram.cu index 0cb2ea023dd7..d59ee290e15d 100644 --- a/cpp/src/reductions/histogram.cu +++ b/cpp/src/reductions/histogram.cu @@ -85,8 +85,7 @@ auto gather_histogram(table_view const& input, stream, mr); - return std::make_unique( - std::move(*output_structs.release()), true, stream, mr); + return std::make_unique(std::move(*output_structs), true, stream, mr); } } // namespace diff --git a/cpp/src/reductions/segmented/simple.cuh b/cpp/src/reductions/segmented/simple.cuh index 90e3e043daf5..a63bd1061683 100644 --- a/cpp/src/reductions/segmented/simple.cuh +++ b/cpp/src/reductions/segmented/simple.cuh @@ -241,8 +241,8 @@ std::unique_ptr fixed_point_segmented_reduction( return std::make_unique(data_type{type_to_id(), scale}, size, - std::move(*(contents.data.release())), - std::move(*(contents.null_mask.release())), + std::move(*(contents.data)), + std::move(*(contents.null_mask)), null_count); } diff --git a/cpp/src/strings/replace/multi.cu b/cpp/src/strings/replace/multi.cu index 794f563b9831..47935bc2b8d3 100644 --- a/cpp/src/strings/replace/multi.cu +++ b/cpp/src/strings/replace/multi.cu @@ -417,7 +417,7 @@ std::unique_ptr replace_character_parallel(strings_column_view const& in // build the strings columns from the chars and offsets return make_strings_column(strings_count, std::move(offsets), - std::move(chars_data.release()[0]), + std::move(*chars_data), input.null_count(), cudf::detail::copy_bitmask(input.parent(), stream, mr)); } diff --git a/cpp/src/strings/replace/replace.cu b/cpp/src/strings/replace/replace.cu index b8f0d9cc6ca7..af31d9396844 100644 --- a/cpp/src/strings/replace/replace.cu +++ b/cpp/src/strings/replace/replace.cu @@ -358,7 +358,7 @@ std::unique_ptr replace_character_parallel(strings_column_view const& in // build the strings columns from the chars and offsets return make_strings_column(strings_count, std::move(offsets), - std::move(chars_data.release()[0]), + std::move(*chars_data), input.null_count(), cudf::detail::copy_bitmask(input.parent(), stream, mr)); } diff --git a/cpp/src/transform/nans_to_nulls.cu b/cpp/src/transform/nans_to_nulls.cu index f0e28ab5e38d..d8bb6d255606 100644 --- a/cpp/src/transform/nans_to_nulls.cu +++ b/cpp/src/transform/nans_to_nulls.cu @@ -88,11 +88,8 @@ std::unique_ptr column_nans_to_nulls(column_view const& input, cudf::type_dispatcher(input.type(), dispatch_nan_to_null{}, input, stream, mr); auto data = cudf::type_dispatcher(input.type(), copy_float_data_fn{input, stream, mr}); - return std::make_unique(input.type(), - input.size(), - std::move(*data.release()), - std::move(*null_mask.release()), - null_count); + return std::make_unique( + input.type(), input.size(), std::move(*data), std::move(*null_mask), null_count); } } // namespace detail diff --git a/java/src/main/native/src/ColumnViewJni.cu b/java/src/main/native/src/ColumnViewJni.cu index 80168c041577..ebbb43081f7e 100644 --- a/java/src/main/native/src/ColumnViewJni.cu +++ b/java/src/main/native/src/ColumnViewJni.cu @@ -136,7 +136,7 @@ void post_process_list_overlap(cudf::column_view const& lhs, overlap_result->set_null_mask(std::move(null_mask), null_count); } else { // Just set the output nullmask as the new nullmask. - overlap_result->set_null_mask(std::move(*new_null_mask.release()), new_null_count); + overlap_result->set_null_mask(std::move(*new_null_mask), new_null_count); } } }