-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move support for return values of called Python functions #297
Conversation
@@ -837,6 +864,57 @@ template <typename T> object cast(const T &value, | |||
template <typename T> T handle::cast() const { return pybind11::cast<T>(*this); } | |||
template <> inline void handle::cast() const { return; } | |||
|
|||
template <typename T> | |||
typename std::enable_if<detail::move_always<T>::value || detail::move_if_unreferenced<T>::value, T>::type move(object &&obj) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the enable_if can be removed here. The function will only ever be instantiated if move_always or move_if_unreferenced are true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I threw them on there to protect external callers from accessing it, because move()
isn't in detail
, i.e. a pybind11-caller can explicitly ask for the instance to be moved by calling py::move<T>(object)
as a shortcut for py::cast<T>(std::move(object))
.
This looks great -- I added one comment. |
|
||
template <typename T> T object::cast() const & { return pybind11::cast<T>(*this); } | ||
template <typename T> T object::cast() && { return pybind11::cast<T>(std::move(*this)); } | ||
template <> inline void object::cast() const & { return; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very much like a function partial template overload to me. Is that even legal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a full template specialization, rather than partial, which is legal. (That's also why it needs the explicit "inline" to avoid a multiple definition link error).
FYI: The PR will need to be rebased onto master to be merged. |
Currently pybind11 always translates values returned by Python functions invoked from C++ code by copying, even when moving is feasible--and, more importantly, even when moving is required. The first, and relatively minor, concern is that moving may be considerably more efficient for some types. The second problem, however, is more serious: there's currently no way python code can return a non-copyable type to C++ code. I ran into this while trying to add a PYBIND11_OVERLOAD of a virtual method that returns just such a type: it simply fails to compile because this: overload = ... overload(args).template cast<ret_type>(); involves a copy: overload(args) returns an object instance, and the invoked object::cast() loads the returned value, then returns a copy of the loaded value. We can, however, safely move that returned value *if* the object has the only reference to it (i.e. if ref_count() == 1) and the object is itself temporary (i.e. if it's an rvalue). This commit does that by adding an rvalue-qualified object::cast() method that allows the returned value to be move-constructed out of the stored instance when feasible. This basically comes down to three cases: - For objects that are movable but not copyable, we always try the move, with a runtime exception raised if this would involve moving a value with multiple references. - When the type is both movable and non-trivially copyable, the move happens only if the invoked object has a ref_count of 1, otherwise the object is copied. (Trivially copyable types are excluded from this case because they are typically just collections of primitive types, which can be copied just as easily as they can be moved.) - Non-movable and trivially copy constructible objects are simply copied. This also adds examples to example-virtual-functions that shows both a non-copyable object and a movable/copyable object in action: the former raises an exception if returned while holding a reference, the latter invokes a move constructor if unreferenced, or a copy constructor if referenced. Basically this allows code such as: class MyClass(Pybind11Class): def somemethod(self, whatever): mt = MovableType(whatever) # ... return mt which allows the MovableType instance to be returned to the C++ code via its move constructor. Of course if you attempt to violate this by doing something like: self.value = MovableType(whatever) return self.value you get an exception--but right now, the pybind11-side of that code won't compile at all.
81f8425
to
ed14879
Compare
Squashed and rebased. |
LGTM, thanks! |
Currently pybind11 always translates values returned by Python functions invoked from C++ code by copying, even when moving is feasible--and, more importantly, even when copying is impossible.
The first, and relatively minor, concern is that moving may be considerably more efficient for some types. The second problem, however, is more serious: there's currently no way python code can return a non-copyable type to C++ code.
I ran into this while trying to add a PYBIND11_OVERLOAD of a virtual method that returns just such a type: it simply fails to compile because this:
overload = ... overload(args).template cast<ret_type>();
involves a copy:
overload(args)
returns anobject
instance, and the invokedobject::cast()
loads the returned value, then returns a copy of the loaded value.We can, however, safely move that returned value if the object has the only reference to it (i.e. if
ref_count() == 1
) and the object is itself temporary (i.e. if it's an rvalue).This commit does that by adding an rvalue-qualified
object::cast()
method that allows the returned value to be move-constructed out of the stored instance when feasible.This basically comes down to three cases:
case because they are typically just collections of primitive types, for which copying is typically as efficient as moving).
This also adds examples to example-virtual-functions that shows both a non-copyable object and a movable/copyable object in action: the former raises an exception if returned while holding a reference, the latter invokes a move constructor if unreferenced, or a copy constructor if referenced.
Basically this allows code such as:
which allows the MovableType instance to be returned to the C++ code via its move constructor.
Of course if you attempt to violate this by doing something like:
you get an exception (but right now, the pybind11-side of that code won't compile at all.)