tests: add or delete copy/move ctors where needed to make type traits match reality#5833
Merged
rwgk merged 1 commit intopybind:masterfrom Sep 8, 2025
Merged
Conversation
Collaborator
|
I don't remember seeing this flake before: CI / 🐍 (macos-latest, 3.14t, -DCMAKE_CXX_STANDARD=20) / 🧪 (pull_request) Failing after 7m I'll try a rerun. |
rwgk
approved these changes
Sep 8, 2025
Collaborator
|
Question for my understanding: After #5800 is merged, will general users have to make similar changes? (I think that's fine, but if yes, we shouldn't forget to mention that in the release notes and maybe upgrade guide.) |
Collaborator
Author
Yes. There's some discussion of that in the PR description. |
Collaborator
|
Ugh CI / 🐍 (macos-latest, 3.14t, -DCMAKE_CXX_STANDARD=20) / 🧪 (pull_request) Cancelled after 29m Now it was stuck here, which I've seen at least twice before: Trying again... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#5800 will attempt to instantiate copy/move constructors for every bound type that appears copy/move constructible. (Currently, only types that are returned from a bound function will try to use these constructors, so if a type is used only in argument position, it's possible that it satisfies
is_copy_constructiblebut actually odr-using the copy constructor would fail. Ditto for moves.) See that PR for more on the rationale.There are several classes used in pybind11's test suite that satisfy
is_copy_constructible, but produce warnings or errors if you try to actually odr-use their (implicitly-defined) copy constructor:DataFieldsHolderintest_class_sh_property_non_owning.cpp, which contains a std::vector of move-only typeDataField.This PR adds defaulted or deleted copy constructors for these cases, as semantically appropriate.
(No user-visible impact, so no changelog entry.)