Skip to content

Commit 43755c3

Browse files
committed
Don't provide make_copy_constructor for non-copyable container
make_copy_constructor currently fails for various stl containers (e.g. std::vector, std::unordered_map, std::deque, etc.) when the container's value type (e.g. the "T" or the std::pair<K,T> for a map) is non-copyable. This adds an override that, for types that look like containers, also requires that the value_type be copyable.
1 parent cba08f1 commit 43755c3

File tree

1 file changed

+15
-2
lines changed

1 file changed

+15
-2
lines changed

include/pybind11/cast.h

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,18 @@ using cast_op_type = typename std::conditional<std::is_pointer<typename std::rem
329329
typename std::add_pointer<intrinsic_t<T>>::type,
330330
typename std::add_lvalue_reference<intrinsic_t<T>>::type>::type;
331331

332+
// std::is_copy_constructible isn't quite enough: it lets std::vector<T> (and similar) through when
333+
// T is non-copyable, but code containing such a copy constructor fails to actually compile.
334+
template <typename T, typename SFINAE = void> struct is_copy_constructible : std::is_copy_constructible<T> {};
335+
336+
// Specialization for types that appear to be copy constructible but also look like stl containers
337+
// (we specifically check for: has `value_type` and `reference` with `reference = value_type&`): if
338+
// so, copy constructability depends on whether the value_type is copy constructible.
339+
template <typename Container> struct is_copy_constructible<Container, enable_if_t<
340+
std::is_copy_constructible<Container>::value &&
341+
std::is_same<typename Container::value_type &, typename Container::reference>::value
342+
>> : std::is_copy_constructible<typename Container::value_type> {};
343+
332344
/// Generic type caster for objects stored on the heap
333345
template <typename type> class type_caster_base : public type_caster_generic {
334346
using itype = intrinsic_t<type>;
@@ -366,20 +378,21 @@ template <typename type> class type_caster_base : public type_caster_generic {
366378
#if !defined(_MSC_VER)
367379
/* Only enabled when the types are {copy,move}-constructible *and* when the type
368380
does not have a private operator new implementaton. */
369-
template <typename T = type> static auto make_copy_constructor(const T *value) -> decltype(new T(*value), Constructor(nullptr)) {
381+
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)) {
370382
return [](const void *arg) -> void * { return new T(*((const T *) arg)); }; }
371383
template <typename T = type> static auto make_move_constructor(const T *value) -> decltype(new T(std::move(*((T *) value))), Constructor(nullptr)) {
372384
return [](const void *arg) -> void * { return (void *) new T(std::move(*((T *) arg))); }; }
373385
#else
374386
/* Visual Studio 2015's SFINAE implementation doesn't yet handle the above robustly in all situations.
375387
Use a workaround that only tests for constructibility for now. */
376-
template <typename T = type, typename = enable_if_t<std::is_copy_constructible<T>::value>>
388+
template <typename T = type, typename = enable_if_t<is_copy_constructible<T>::value>>
377389
static Constructor make_copy_constructor(const T *value) {
378390
return [](const void *arg) -> void * { return new T(*((const T *)arg)); }; }
379391
template <typename T = type, typename = enable_if_t<std::is_move_constructible<T>::value>>
380392
static Constructor make_move_constructor(const T *value) {
381393
return [](const void *arg) -> void * { return (void *) new T(std::move(*((T *)arg))); }; }
382394
#endif
395+
383396
static Constructor make_copy_constructor(...) { return nullptr; }
384397
static Constructor make_move_constructor(...) { return nullptr; }
385398
};

0 commit comments

Comments
 (0)