Skip to content

Commit

Permalink
Detect std::pair non-copyability
Browse files Browse the repository at this point in the history
Pre-C++17, std::pair can technically have an copy constructor even
though it can't actually be invoked without a compilation failure (due
to the underlying types being non-copyable).  Most stls, including
libc++ since ~3.4, use the C++17 behaviour of not exposing an uncallable
copy constructor, but FreeBSD deliberately broke their libc++ to
preserve the nonsensical behaviour
(https://svnweb.freebsd.org/base?view=revision&revision=261801).

This updates pybind's internal `is_copy_constructible` to also detect
the std::pair case under pre-C++17.

This also everything (except for a couple cases in the internal version)
to use the internal `is_copy_constructible` rather than
`std::is_copy_constructible`.
  • Loading branch information
jagerman committed Jul 29, 2017
1 parent e07f758 commit 7937260
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
19 changes: 13 additions & 6 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -758,10 +758,17 @@ template <typename T, typename SFINAE = void> struct is_copy_constructible : std
// 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> {};
template <typename Container> struct is_copy_constructible<Container, enable_if_t<all_of<
std::is_copy_constructible<Container>,
std::is_same<typename Container::value_type &, typename Container::reference>
>::value>> : is_copy_constructible<typename Container::value_type> {};

#if !defined(PYBIND11_CPP17)
// Likewise for std::pair before C++17 (which mandates that the copy constructor not exist when the
// two types aren't themselves copy constructible).
template <typename T1, typename T2> struct is_copy_constructible<std::pair<T1, T2>>
: all_of<is_copy_constructible<T1>, is_copy_constructible<T2>> {};
#endif

/// Generic type caster for objects stored on the heap
template <typename type> class type_caster_base : public type_caster_generic {
Expand Down Expand Up @@ -1460,7 +1467,7 @@ class type_caster<std::unique_ptr<type, deleter>>
: public move_only_holder_caster<type, std::unique_ptr<type, deleter>> { };

template <typename type, typename holder_type>
using type_caster_holder = conditional_t<std::is_copy_constructible<holder_type>::value,
using type_caster_holder = conditional_t<is_copy_constructible<holder_type>::value,
copyable_holder_caster<type, holder_type>,
move_only_holder_caster<type, holder_type>>;

Expand Down Expand Up @@ -1525,7 +1532,7 @@ template <typename T> using move_is_plain_type = satisfies_none_of<T,
template <typename T, typename SFINAE = void> struct move_always : std::false_type {};
template <typename T> struct move_always<T, enable_if_t<all_of<
move_is_plain_type<T>,
negation<std::is_copy_constructible<T>>,
negation<is_copy_constructible<T>>,
std::is_move_constructible<T>,
std::is_same<decltype(std::declval<make_caster<T>>().operator T&()), T&>
>::value>> : std::true_type {};
Expand Down
9 changes: 3 additions & 6 deletions include/pybind11/stl_bind.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,7 @@ template <typename, typename, typename... Args> void vector_if_insertion_operato
template <typename, typename, typename... Args> void vector_modifiers(const Args &...) { }

template<typename Vector, typename Class_>
void vector_if_copy_constructible(enable_if_t<
std::is_copy_constructible<Vector>::value &&
std::is_copy_constructible<typename Vector::value_type>::value, Class_> &cl) {

void vector_if_copy_constructible(enable_if_t<is_copy_constructible<Vector>::value, Class_> &cl) {
cl.def(init<const Vector &>(), "Copy constructor");
}

Expand Down Expand Up @@ -113,7 +110,7 @@ void vector_if_equal_operator(enable_if_t<is_comparable<Vector>::value, Class_>
// (Technically, some of these (pop and __delitem__) don't actually require copyability, but it seems
// silly to allow deletion but not insertion, so include them here too.)
template <typename Vector, typename Class_>
void vector_modifiers(enable_if_t<std::is_copy_constructible<typename Vector::value_type>::value, Class_> &cl) {
void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_type>::value, Class_> &cl) {
using T = typename Vector::value_type;
using SizeType = typename Vector::size_type;
using DiffType = typename Vector::difference_type;
Expand Down Expand Up @@ -487,7 +484,7 @@ void map_assignment(enable_if_t<std::is_copy_assignable<typename Map::mapped_typ
template<typename Map, typename Class_>
void map_assignment(enable_if_t<
!std::is_copy_assignable<typename Map::mapped_type>::value &&
std::is_copy_constructible<typename Map::mapped_type>::value,
is_copy_constructible<typename Map::mapped_type>::value,
Class_> &cl) {
using KeyType = typename Map::key_type;
using MappedType = typename Map::mapped_type;
Expand Down

0 comments on commit 7937260

Please sign in to comment.