Skip to content

Fix stl_bind to support movable, non-copyable value types #490

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

Merged
merged 6 commits into from
Nov 15, 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
17 changes: 15 additions & 2 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,18 @@ using cast_op_type = typename std::conditional<std::is_pointer<typename std::rem
typename std::add_pointer<intrinsic_t<T>>::type,
typename std::add_lvalue_reference<intrinsic_t<T>>::type>::type;

// std::is_copy_constructible isn't quite enough: it lets std::vector<T> (and similar) through when
// T is non-copyable, but code containing such a copy constructor fails to actually compile.
template <typename T, typename SFINAE = void> struct is_copy_constructible : std::is_copy_constructible<T> {};

// Specialization for types that appear to be copy constructible but also look like stl containers
// (we specifically check for: has `value_type` and `reference` with `reference = value_type&`): if
// so, copy constructability depends on whether the value_type is copy constructible.
template <typename Container> struct is_copy_constructible<Container, enable_if_t<
std::is_copy_constructible<Container>::value &&
std::is_same<typename Container::value_type &, typename Container::reference>::value
>> : std::is_copy_constructible<typename Container::value_type> {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nifty :)


/// Generic type caster for objects stored on the heap
template <typename type> class type_caster_base : public type_caster_generic {
using itype = intrinsic_t<type>;
Expand Down Expand Up @@ -366,20 +378,21 @@ template <typename type> class type_caster_base : public type_caster_generic {
#if !defined(_MSC_VER)
/* Only enabled when the types are {copy,move}-constructible *and* when the type
does not have a private operator new implementaton. */
template <typename T = type> static auto make_copy_constructor(const T *value) -> decltype(new T(*value), Constructor(nullptr)) {
template <typename T = type, typename = enable_if_t<is_copy_constructible<T>::value>> static auto make_copy_constructor(const T *value) -> decltype(new T(*value), Constructor(nullptr)) {
return [](const void *arg) -> void * { return new T(*((const T *) arg)); }; }
template <typename T = type> static auto make_move_constructor(const T *value) -> decltype(new T(std::move(*((T *) value))), Constructor(nullptr)) {
return [](const void *arg) -> void * { return (void *) new T(std::move(*((T *) arg))); }; }
#else
/* Visual Studio 2015's SFINAE implementation doesn't yet handle the above robustly in all situations.
Use a workaround that only tests for constructibility for now. */
template <typename T = type, typename = enable_if_t<std::is_copy_constructible<T>::value>>
template <typename T = type, typename = enable_if_t<is_copy_constructible<T>::value>>
static Constructor make_copy_constructor(const T *value) {
return [](const void *arg) -> void * { return new T(*((const T *)arg)); }; }
template <typename T = type, typename = enable_if_t<std::is_move_constructible<T>::value>>
static Constructor make_move_constructor(const T *value) {
return [](const void *arg) -> void * { return (void *) new T(std::move(*((T *)arg))); }; }
#endif

static Constructor make_copy_constructor(...) { return nullptr; }
static Constructor make_move_constructor(...) { return nullptr; }
};
Expand Down
Loading