diff --git a/include/pybind11/detail/function_ref.h b/include/pybind11/detail/function_ref.h index a81bdfe13f..c70e4ae15f 100644 --- a/include/pybind11/detail/function_ref.h +++ b/include/pybind11/detail/function_ref.h @@ -31,6 +31,8 @@ // - renamed back to function_ref // - use pybind11 enable_if_t, remove_cvref_t, and remove_reference_t // - lint suppressions +// - accept same-type non-movable returns under C++17 guaranteed copy elision +// (issue #6142) // torch::executor: modified from llvm::function_ref // - renamed to FunctionRef @@ -55,6 +57,17 @@ PYBIND11_NAMESPACE_BEGIN(detail) template class function_ref; +// pybind11: is_convertible is false for a copyable but non-movable +// type (it tests conversion from an xvalue, which selects the deleted move +// constructor), but in C++17 a same-type prvalue is returnable via guaranteed +// copy elision. Accept that case explicitly. See issue #6142. +template +using is_returnable_as = bool_constant< +#if defined(PYBIND11_CPP17) + std::is_same::value || +#endif + std::is_convertible::value>; + template class function_ref { Ret (*callback)(intptr_t callable, Params... params) = nullptr; @@ -81,8 +94,8 @@ class function_ref { // Functor must be callable and return a suitable type. enable_if_t< std::is_void::value - || std::is_convertible()(std::declval()...)), - Ret>::value> * = nullptr) + || is_returnable_as()(std::declval()...)), + Ret>::value> * = nullptr) : callback(callback_fn>), callable(reinterpret_cast(&callable)) {} diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index b163406a57..cd1c27215c 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -107,6 +107,22 @@ class CopyOnlyInt { int value; }; +// #6142: returning a copy-constructible type with an explicitly deleted move +// constructor failed to compile inside detail::function_ref (3.1.0 regression). +// Returning such a type by value requires C++17 guaranteed copy elision. +#if defined(PYBIND11_CPP17) +class CopyOnlyDeletedMove { +public: + explicit CopyOnlyDeletedMove(int v) : value{v} {} + CopyOnlyDeletedMove(const CopyOnlyDeletedMove &) = default; + CopyOnlyDeletedMove &operator=(const CopyOnlyDeletedMove &) = delete; + CopyOnlyDeletedMove(CopyOnlyDeletedMove &&) = delete; + CopyOnlyDeletedMove &operator=(CopyOnlyDeletedMove &&) = delete; + + int value; +}; +#endif + PYBIND11_NAMESPACE_BEGIN(pybind11) PYBIND11_NAMESPACE_BEGIN(detail) template <> @@ -176,6 +192,13 @@ TEST_SUBMODULE(copy_move_policies, m) { py::class_(m, "lacking_move_ctor") .def_static("get_one", &lacking_move_ctor::get_one, py::return_value_policy::move); + // test_copy_only_deleted_move (#6142) +#if defined(PYBIND11_CPP17) + py::class_(m, "CopyOnlyDeletedMove") + .def_readonly("value", &CopyOnlyDeletedMove::value); + m.def("get_copy_only_deleted_move", []() { return CopyOnlyDeletedMove(42); }); +#endif + // test_move_and_copy_casts // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("move_and_copy_casts", [](const py::object &o) { diff --git a/tests/test_copy_move.py b/tests/test_copy_move.py index d843793350..c032797b1b 100644 --- a/tests/test_copy_move.py +++ b/tests/test_copy_move.py @@ -142,3 +142,14 @@ def test_unusual_op_ref(): # Merely to test that this still exists and built successfully. assert m.CallCastUnusualOpRefConstRef().__class__.__name__ == "UnusualOpRef" assert m.CallCastUnusualOpRefMovable().__class__.__name__ == "UnusualOpRef" + + +@pytest.mark.skipif( + not hasattr(m, "get_copy_only_deleted_move"), reason="requires C++17 copy elision" +) +def test_copy_only_deleted_move(): + """#6142: a copyable type with a deleted move constructor can be returned by value + + This is primarily a compile-time regression test. + """ + assert m.get_copy_only_deleted_move().value == 42