fix: returning a copyable type with a deleted move constructor (#6143)
* fix: returning a copyable type with a deleted move constructor (#6142)
detail::function_ref's constructor SFINAE required
is_convertible<Ret, Ret>, which is false for a copyable type whose move
constructor is explicitly deleted (the trait tests conversion from an
xvalue). In C++17, such a prvalue return is legal via guaranteed copy
elision, so also accept an exact type match. Regression introduced in
3.1.0 by the call_impl outlining (#5887).
Fixes #6142
Assisted-by: ClaudeCode:claude-fable-5
* refactor: guard with __cpp_guaranteed_copy_elision
Per review: the feature-test macro expresses the intent more precisely
than PYBIND11_CPP17.
Assisted-by: ClaudeCode:claude-opus-5
diff --git a/include/pybind11/detail/function_ref.h b/include/pybind11/detail/function_ref.h
index a81bdfe..f5f9d7a 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 guaranteed copy elision
+// (issue #6142)
// torch::executor: modified from llvm::function_ref
// - renamed to FunctionRef
@@ -55,6 +57,17 @@
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;
@@ -81,8 +94,8 @@
// 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)) {}
diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp
index b163406..ed1bf61 100644
--- a/tests/test_copy_move.cpp
+++ b/tests/test_copy_move.cpp
@@ -107,6 +107,22 @@
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 <>
@@ -176,6 +192,13 @@
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) {
diff --git a/tests/test_copy_move.py b/tests/test_copy_move.py
index d843793..1c3094f 100644
--- a/tests/test_copy_move.py
+++ b/tests/test_copy_move.py
@@ -142,3 +142,15 @@
# 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