From 07725c28c0b1d68e044ccbc7820920c15c1f21e3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 24 Apr 2023 00:19:21 -0700 Subject: [PATCH] Introduce `pybind11::detail::is_move_constructible` (#4631) To support the use case captured in the new test_vector_unique_ptr_member.cpp --- include/pybind11/cast.h | 4 +- include/pybind11/detail/init.h | 4 +- include/pybind11/detail/type_caster_base.h | 5 +- tests/CMakeLists.txt | 1 + tests/test_vector_unique_ptr_member.cpp | 56 ++++++++++++++++++++++ tests/test_vector_unique_ptr_member.py | 14 ++++++ 6 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 tests/test_vector_unique_ptr_member.cpp create mode 100644 tests/test_vector_unique_ptr_member.py diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 9b013bc39..9d1ce15d4 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -964,7 +964,7 @@ struct move_always< enable_if_t< all_of, negation>, - std::is_move_constructible, + is_move_constructible, std::is_same>().operator T &()), T &>>::value>> : std::true_type {}; template @@ -975,7 +975,7 @@ struct move_if_unreferenced< enable_if_t< all_of, negation>, - std::is_move_constructible, + is_move_constructible, std::is_same>().operator T &()), T &>>::value>> : std::true_type {}; template diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index 9f71278c2..e21171688 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -175,7 +175,7 @@ void construct(value_and_holder &v_h, Holder holder, bool need_alias) { template void construct(value_and_holder &v_h, Cpp &&result, bool need_alias) { PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias); - static_assert(std::is_move_constructible>::value, + static_assert(is_move_constructible>::value, "pybind11::init() return-by-value factory function requires a movable class"); if (Class::has_alias && need_alias) { construct_alias_from_cpp(is_alias_constructible{}, v_h, std::move(result)); @@ -190,7 +190,7 @@ void construct(value_and_holder &v_h, Cpp &&result, bool need_alias) { template void construct(value_and_holder &v_h, Alias &&result, bool) { static_assert( - std::is_move_constructible>::value, + is_move_constructible>::value, "pybind11::init() return-by-alias-value factory function requires a movable alias class"); v_h.value_ptr() = new Alias(std::move(result)); } diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 0b710d7e4..34dcd26ce 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -827,6 +827,9 @@ using movable_cast_op_type template struct is_copy_constructible : std::is_copy_constructible {}; +template +struct is_move_constructible : std::is_move_constructible {}; + // Specialization for types that appear to be copy constructible but also look like stl containers // (we specifically check for: has `value_type` and `reference` with `reference = value_type&`): if // so, copy constructability depends on whether the value_type is copy constructible. @@ -994,7 +997,7 @@ protected: return [](const void *arg) -> void * { return new T(*reinterpret_cast(arg)); }; } - template ::value>> + template ::value>> static auto make_move_constructor(const T *) -> decltype(new T(std::declval()), Constructor{}) { return [](const void *arg) -> void * { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b1cb222b4..1d0eb27c0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -155,6 +155,7 @@ set(PYBIND11_TEST_FILES test_tagbased_polymorphic test_thread test_union + test_vector_unique_ptr_member test_virtual_functions) # Invoking cmake with something like: diff --git a/tests/test_vector_unique_ptr_member.cpp b/tests/test_vector_unique_ptr_member.cpp new file mode 100644 index 000000000..657743e4d --- /dev/null +++ b/tests/test_vector_unique_ptr_member.cpp @@ -0,0 +1,56 @@ +#include "pybind11_tests.h" + +#include +#include +#include + +namespace pybind11_tests { +namespace vector_unique_ptr_member { + +struct DataType {}; + +// Reduced from a use case in the wild. +struct VectorOwner { + static std::unique_ptr Create(std::size_t num_elems) { + return std::unique_ptr( + new VectorOwner(std::vector>(num_elems))); + } + + std::size_t data_size() const { return data_.size(); } + +private: + explicit VectorOwner(std::vector> data) : data_(std::move(data)) {} + + const std::vector> data_; +}; + +} // namespace vector_unique_ptr_member +} // namespace pybind11_tests + +namespace pybind11 { +namespace detail { + +template <> +struct is_copy_constructible + : std::false_type {}; + +template <> +struct is_move_constructible + : std::false_type {}; + +} // namespace detail +} // namespace pybind11 + +using namespace pybind11_tests::vector_unique_ptr_member; + +py::object py_cast_VectorOwner_ptr(VectorOwner *ptr) { return py::cast(ptr); } + +// PYBIND11_SMART_HOLDER_TYPE_CASTERS(VectorOwner) + +TEST_SUBMODULE(vector_unique_ptr_member, m) { + py::class_(m, "VectorOwner") + .def_static("Create", &VectorOwner::Create) + .def("data_size", &VectorOwner::data_size); + + m.def("py_cast_VectorOwner_ptr", py_cast_VectorOwner_ptr); +} diff --git a/tests/test_vector_unique_ptr_member.py b/tests/test_vector_unique_ptr_member.py new file mode 100644 index 000000000..2da3d97c3 --- /dev/null +++ b/tests/test_vector_unique_ptr_member.py @@ -0,0 +1,14 @@ +import pytest + +from pybind11_tests import vector_unique_ptr_member as m + + +@pytest.mark.parametrize("num_elems", range(3)) +def test_create(num_elems): + vo = m.VectorOwner.Create(num_elems) + assert vo.data_size() == num_elems + + +def test_cast(): + vo = m.VectorOwner.Create(0) + assert m.py_cast_VectorOwner_ptr(vo) is vo