Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions include/pybind11/detail/function_ref.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 guaranteed copy elision
// (issue #6142)

// torch::executor: modified from llvm::function_ref
// - renamed to FunctionRef
Expand All @@ -55,6 +57,17 @@ PYBIND11_NAMESPACE_BEGIN(detail)
template <typename Fn>
class function_ref;

// pybind11: is_convertible<Ret, Ret> is false for a copyable but non-movable
// type (it tests conversion from an xvalue, which selects the deleted move
// constructor), but with guaranteed copy elision a same-type prvalue is still
// returnable. Accept that case explicitly. See issue #6142.
template <typename From, typename To>
using is_returnable_as = bool_constant<
#if defined(__cpp_guaranteed_copy_elision) && __cpp_guaranteed_copy_elision >= 201606L
std::is_same<From, To>::value ||
#endif
std::is_convertible<From, To>::value>;

template <typename Ret, typename... Params>
class function_ref<Ret(Params...)> {
Ret (*callback)(intptr_t callable, Params... params) = nullptr;
Expand All @@ -81,8 +94,8 @@ class function_ref<Ret(Params...)> {
// Functor must be callable and return a suitable type.
enable_if_t<
std::is_void<Ret>::value
|| std::is_convertible<decltype(std::declval<Callable>()(std::declval<Params>()...)),
Ret>::value> * = nullptr)
|| is_returnable_as<decltype(std::declval<Callable>()(std::declval<Params>()...)),
Ret>::value> * = nullptr)
: callback(callback_fn<remove_reference_t<Callable>>),
callable(reinterpret_cast<intptr_t>(&callable)) {}

Expand Down
23 changes: 23 additions & 0 deletions tests/test_copy_move.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 guaranteed copy elision.
#if defined(__cpp_guaranteed_copy_elision) && __cpp_guaranteed_copy_elision >= 201606L
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 <>
Expand Down Expand Up @@ -176,6 +192,13 @@ TEST_SUBMODULE(copy_move_policies, m) {
py::class_<lacking_move_ctor>(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(__cpp_guaranteed_copy_elision) && __cpp_guaranteed_copy_elision >= 201606L
py::class_<CopyOnlyDeletedMove>(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) {
Expand Down
12 changes: 12 additions & 0 deletions tests/test_copy_move.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,15 @@ 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 guaranteed 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
Loading