Skip to content
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

Merged
merged 1 commit into from
Aug 9, 2016
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
61 changes: 61 additions & 0 deletions example/example-virtual-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,53 @@ class PyExampleVirt : public ExampleVirt {
}
};

class NonCopyable {
public:
NonCopyable(int a, int b) : value{new int(a*b)} {}
NonCopyable(NonCopyable &&) = default;
NonCopyable(const NonCopyable &) = delete;
NonCopyable() = delete;
void operator=(const NonCopyable &) = delete;
void operator=(NonCopyable &&) = delete;
std::string get_value() const {
if (value) return std::to_string(*value); else return "(null)";
}
~NonCopyable() { std::cout << "NonCopyable destructor @ " << this << "; value = " << get_value() << std::endl; }

private:
std::unique_ptr<int> value;
};

// This is like the above, but is both copy and movable. In effect this means it should get moved
// when it is not referenced elsewhere, but copied if it is still referenced.
class Movable {
public:
Movable(int a, int b) : value{a+b} {}
Movable(const Movable &m) { value = m.value; std::cout << "Movable @ " << this << " copy constructor" << std::endl; }
Movable(Movable &&m) { value = std::move(m.value); std::cout << "Movable @ " << this << " move constructor" << std::endl; }
int get_value() const { return value; }
~Movable() { std::cout << "Movable destructor @ " << this << "; value = " << get_value() << std::endl; }
private:
int value;
};

class NCVirt {
public:
virtual NonCopyable get_noncopyable(int a, int b) { return NonCopyable(a, b); }
virtual Movable get_movable(int a, int b) = 0;

void print_nc(int a, int b) { std::cout << get_noncopyable(a, b).get_value() << std::endl; }
void print_movable(int a, int b) { std::cout << get_movable(a, b).get_value() << std::endl; }
};
class NCVirtTrampoline : public NCVirt {
virtual NonCopyable get_noncopyable(int a, int b) {
PYBIND11_OVERLOAD(NonCopyable, NCVirt, get_noncopyable, a, b);
}
virtual Movable get_movable(int a, int b) {
PYBIND11_OVERLOAD_PURE(Movable, NCVirt, get_movable, a, b);
}
};

int runExampleVirt(ExampleVirt *ex, int value) {
return ex->run(value);
}
Expand Down Expand Up @@ -240,6 +287,20 @@ void init_ex_virtual_functions(py::module &m) {
.def("run_bool", &ExampleVirt::run_bool)
.def("pure_virtual", &ExampleVirt::pure_virtual);

py::class_<NonCopyable>(m, "NonCopyable")
.def(py::init<int, int>())
;
py::class_<Movable>(m, "Movable")
.def(py::init<int, int>())
;
py::class_<NCVirt, std::unique_ptr<NCVirt>, NCVirtTrampoline>(m, "NCVirt")
.def(py::init<>())
.def("get_noncopyable", &NCVirt::get_noncopyable)
.def("get_movable", &NCVirt::get_movable)
.def("print_nc", &NCVirt::print_nc)
.def("print_movable", &NCVirt::print_movable)
;

m.def("runExampleVirt", &runExampleVirt);
m.def("runExampleVirtBool", &runExampleVirtBool);
m.def("runExampleVirtVirtual", &runExampleVirtVirtual);
Expand Down
35 changes: 35 additions & 0 deletions example/example-virtual-functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

from example import ExampleVirt, runExampleVirt, runExampleVirtVirtual, runExampleVirtBool
from example import A_Repeat, B_Repeat, C_Repeat, D_Repeat, A_Tpl, B_Tpl, C_Tpl, D_Tpl
from example import NCVirt, NonCopyable, Movable


class ExtendedExampleVirt(ExampleVirt):
def __init__(self, state):
Expand Down Expand Up @@ -87,3 +89,36 @@ def lucky_number(self):
if hasattr(obj, "lucky_number"):
print("Lucky = %.2f" % obj.lucky_number())

class NCVirtExt(NCVirt):
def get_noncopyable(self, a, b):
# Constructs and returns a new instance:
nc = NonCopyable(a*a, b*b)
return nc
def get_movable(self, a, b):
# Return a referenced copy
self.movable = Movable(a, b)
return self.movable

class NCVirtExt2(NCVirt):
def get_noncopyable(self, a, b):
# Keep a reference: this is going to throw an exception
self.nc = NonCopyable(a, b)
return self.nc
def get_movable(self, a, b):
# Return a new instance without storing it
return Movable(a, b)

ncv1 = NCVirtExt()
print("2^2 * 3^2 =")
ncv1.print_nc(2, 3)
print("4 + 5 =")
ncv1.print_movable(4, 5)
ncv2 = NCVirtExt2()
print("7 + 7 =")
ncv2.print_movable(7, 7)
try:
ncv2.print_nc(9, 9)
print("Something's wrong: exception not raised!")
except RuntimeError as e:
# Don't print the exception message here because it differs under debug/non-debug mode
print("Caught expected exception")
16 changes: 16 additions & 0 deletions example/example-virtual-functions.ref
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,21 @@ VI_DT:
VI_DT says: quack quack quack
Unlucky = 1234
Lucky = -4.25
2^2 * 3^2 =
NonCopyable destructor @ 0x1a6c3f0; value = (null)
36
NonCopyable destructor @ 0x7ffc6d1fbaa8; value = 36
4 + 5 =
Movable @ 0x7ffc6d1fbacc copy constructor
9
Movable destructor @ 0x7ffc6d1fbacc; value = 9
7 + 7 =
Movable @ 0x7ffc6d1fbacc move constructor
Movable destructor @ 0x1a6c4d0; value = 14
14
Movable destructor @ 0x7ffc6d1fbacc; value = 14
Caught expected exception
NonCopyable destructor @ 0x29a64b0; value = 81
Movable destructor @ 0x1a6c410; value = 9
Destructing ExampleVirt..
Destructing ExampleVirt..
80 changes: 79 additions & 1 deletion include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -809,9 +809,36 @@ struct type_caster<type, typename std::enable_if<std::is_base_of<handle, type>::
PYBIND11_TYPE_CASTER(type, handle_type_name<type>::name());
};

// Our conditions for enabling moving are quite restrictive:
// At compile time:
// - T needs to be a non-const, non-pointer, non-reference type
// - type_caster<T>::operator T&() must exist
// - the type must be move constructible (obviously)
// At run-time:
// - if the type is non-copy-constructible, the object must be the sole owner of the type (i.e. it
// must have ref_count() == 1)h
// If any of the above are not satisfied, we fall back to copying.
template <typename T, typename SFINAE = void> struct move_is_plain_type : std::false_type {};
template <typename T> struct move_is_plain_type<T, typename std::enable_if<
!std::is_void<T>::value && !std::is_pointer<T>::value && !std::is_reference<T>::value && !std::is_const<T>::value
>::type> : std::true_type {};
template <typename T, typename SFINAE = void> struct move_always : std::false_type {};
template <typename T> struct move_always<T, typename std::enable_if<
move_is_plain_type<T>::value &&
!std::is_copy_constructible<T>::value && std::is_move_constructible<T>::value &&
std::is_same<decltype(std::declval<type_caster<T>>().operator T&()), T&>::value
>::type> : std::true_type {};
template <typename T, typename SFINAE = void> struct move_if_unreferenced : std::false_type {};
template <typename T> struct move_if_unreferenced<T, typename std::enable_if<
move_is_plain_type<T>::value &&
!move_always<T>::value && std::is_move_constructible<T>::value &&
std::is_same<decltype(std::declval<type_caster<T>>().operator T&()), T&>::value
>::type> : std::true_type {};
template <typename T> using move_never = std::integral_constant<bool, !move_always<T>::value && !move_if_unreferenced<T>::value>;

NAMESPACE_END(detail)

template <typename T> T cast(handle handle) {
template <typename T> T cast(const handle &handle) {
typedef detail::type_caster<typename detail::intrinsic_type<T>::type> type_caster;
type_caster conv;
if (!conv.load(handle, true)) {
Expand All @@ -838,6 +865,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) {
Copy link
Member

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.

Copy link
Member Author

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)).

if (obj.ref_count() > 1)
#if defined(NDEBUG)
throw cast_error("Unable to cast Python instance to C++ rvalue: instance has multiple references"
" (compile in debug mode for details)");
#else
throw cast_error("Unable to move from Python " + (std::string) obj.get_type().str() +
" instance to C++ " + type_id<T>() + " instance: instance has multiple references");
#endif

typedef detail::type_caster<T> type_caster;
type_caster conv;
if (!conv.load(obj, true))
#if defined(NDEBUG)
throw cast_error("Unable to cast Python instance to C++ type (compile in debug mode for details)");
#else
throw cast_error("Unable to cast Python instance of type " +
(std::string) obj.get_type().str() + " to C++ type '" + type_id<T>() + "''");
#endif

// Move into a temporary and return that, because the reference may be a local value of `conv`
T ret = std::move(conv.operator T&());
return ret;
}

// Calling cast() on an rvalue calls pybind::cast with the object rvalue, which does:
// - If we have to move (because T has no copy constructor), do it. This will fail if the moved
// object has multiple references, but trying to copy will fail to compile.
// - If both movable and copyable, check ref count: if 1, move; otherwise copy
// - Otherwise (not movable), copy.
template <typename T> typename std::enable_if<detail::move_always<T>::value, T>::type cast(object &&object) {
return move<T>(std::move(object));
}
template <typename T> typename std::enable_if<detail::move_if_unreferenced<T>::value, T>::type cast(object &&object) {
if (object.ref_count() > 1)
return cast<T>(object);
else
return move<T>(std::move(object));
}
template <typename T> typename std::enable_if<detail::move_never<T>::value, T>::type cast(object &&object) {
return cast<T>(object);
}

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; }
Copy link
Member

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?

Copy link
Member Author

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).

template <> inline void object::cast() && { return; }



template <return_value_policy policy = return_value_policy::automatic_reference,
typename... Args> tuple make_tuple(Args&&... args_) {
const size_t size = sizeof...(Args);
Expand Down
5 changes: 5 additions & 0 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ class object : public handle {
}
return *this;
}

// Calling cast() on an object lvalue just copies (via handle::cast)
template <typename T> T cast() const &;
// Calling on an object rvalue does a move, if needed and/or possible
template <typename T> T cast() &&;
};

NAMESPACE_BEGIN(detail)
Expand Down