Skip to content

Conversation

@vuule
Copy link
Contributor

@vuule vuule commented Oct 10, 2025

Description

Deleted copy constructor of classes that are expensive to copy. Some of these classes have const data members, so they previously had the move constructors implicitly deleted, making accidental copies even more likely.
Also made the non-default move constructors noexcept to avoid implicit copies in container like std::vector.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Oct 10, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Oct 10, 2025
@github-actions github-actions bot added the Java Affects Java cuDF API. label Oct 10, 2025
@vuule vuule changed the title Delete copy constructors and assignment operators to prevent accidental copies Prevent accidental copies Oct 10, 2025
@vuule vuule added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 10, 2025
@vuule vuule changed the title Prevent accidental copies Prevent accidental copies of expensive-to-copy object types Oct 10, 2025
@vuule
Copy link
Contributor Author

vuule commented Oct 10, 2025

/ok to test c72f8ea

@vuule vuule marked this pull request as ready for review October 13, 2025 15:49
@vuule vuule requested review from a team as code owners October 13, 2025 15:49
@vuule vuule requested review from vyasr and wence- October 13, 2025 15:49
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me. How did you decide which of the move constructors to make noexcept? Should we be more aggressive in making more of those move constructors noexcept?

@vuule
Copy link
Contributor Author

vuule commented Oct 13, 2025

Seems fine to me. How did you decide which of the move constructors to make noexcept? Should we be more aggressive in making more of those move constructors noexcept?

Defaulted move constructor don't need the keyword, since the noexcept-ness is automatically determined based on the data members (which do have noexpect move ctors). So, we only need explicit noexcept for the custom move ctors, of which there are very few and should be covered in this PR.

@vuule
Copy link
Contributor Author

vuule commented Oct 13, 2025

/merge

@rapids-bot rapids-bot bot merged commit 572dbce into rapidsai:branch-25.12 Oct 13, 2025
132 checks passed
@vuule vuule deleted the impr-delete-copy-ctors branch October 13, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants