test pair-copyability on C++17 upwards (#1886)

* test pair-copyability on C++17 upwards

The stdlib falsely detects containers like M=std::map<T, U>
as copyable, even when one of T and U is not copyable.
Therefore we cannot rely on the stdlib dismissing std::pair<T, M>
by itself, even on C++17.

* fix is_copy_assignable

bind_map used std::is_copy_assignable which suffers from the same problems
as std::is_copy_constructible, therefore the same fix has been applied.

* created tests for copyability
This commit is contained in:
Sebastian Gsänger 2019-10-31 12:38:24 +01:00 committed by Wenzel Jakob
parent bdf6a5e870
commit a83d69e78f
4 changed files with 74 additions and 6 deletions

View File

@ -797,12 +797,20 @@ template <typename Container> struct is_copy_constructible<Container, enable_if_
negation<std::is_same<Container, typename Container::value_type>> negation<std::is_same<Container, typename Container::value_type>>
>::value>> : is_copy_constructible<typename Container::value_type> {}; >::value>> : is_copy_constructible<typename Container::value_type> {};
#if !defined(PYBIND11_CPP17) // Likewise for std::pair
// Likewise for std::pair before C++17 (which mandates that the copy constructor not exist when the // (after C++17 it is mandatory that the copy constructor not exist when the two types aren't themselves
// two types aren't themselves copy constructible). // copy constructible, but this can not be relied upon when T1 or T2 are themselves containers).
template <typename T1, typename T2> struct is_copy_constructible<std::pair<T1, T2>> template <typename T1, typename T2> struct is_copy_constructible<std::pair<T1, T2>>
: all_of<is_copy_constructible<T1>, is_copy_constructible<T2>> {}; : all_of<is_copy_constructible<T1>, is_copy_constructible<T2>> {};
#endif
// The same problems arise with std::is_copy_assignable, so we use the same workaround.
template <typename T, typename SFINAE = void> struct is_copy_assignable : std::is_copy_assignable<T> {};
template <typename Container> struct is_copy_assignable<Container, enable_if_t<all_of<
std::is_copy_assignable<Container>,
std::is_same<typename Container::value_type &, typename Container::reference>
>::value>> : is_copy_assignable<typename Container::value_type> {};
template <typename T1, typename T2> struct is_copy_assignable<std::pair<T1, T2>>
: all_of<is_copy_assignable<T1>, is_copy_assignable<T2>> {};
NAMESPACE_END(detail) NAMESPACE_END(detail)

View File

@ -512,7 +512,7 @@ template <typename, typename, typename... Args> void map_assignment(const Args &
// Map assignment when copy-assignable: just copy the value // Map assignment when copy-assignable: just copy the value
template <typename Map, typename Class_> template <typename Map, typename Class_>
void map_assignment(enable_if_t<std::is_copy_assignable<typename Map::mapped_type>::value, Class_> &cl) { void map_assignment(enable_if_t<is_copy_assignable<typename Map::mapped_type>::value, Class_> &cl) {
using KeyType = typename Map::key_type; using KeyType = typename Map::key_type;
using MappedType = typename Map::mapped_type; using MappedType = typename Map::mapped_type;
@ -528,7 +528,7 @@ void map_assignment(enable_if_t<std::is_copy_assignable<typename Map::mapped_typ
// Not copy-assignable, but still copy-constructible: we can update the value by erasing and reinserting // Not copy-assignable, but still copy-constructible: we can update the value by erasing and reinserting
template<typename Map, typename Class_> template<typename Map, typename Class_>
void map_assignment(enable_if_t< void map_assignment(enable_if_t<
!std::is_copy_assignable<typename Map::mapped_type>::value && !is_copy_assignable<typename Map::mapped_type>::value &&
is_copy_constructible<typename Map::mapped_type>::value, is_copy_constructible<typename Map::mapped_type>::value,
Class_> &cl) { Class_> &cl) {
using KeyType = typename Map::key_type; using KeyType = typename Map::key_type;

View File

@ -54,6 +54,14 @@ template <class Map> Map *times_ten(int n) {
return m; return m;
} }
template <class NestMap> NestMap *times_hundred(int n) {
auto m = new NestMap();
for (int i = 1; i <= n; i++)
for (int j = 1; j <= n; j++)
(*m)[i].emplace(int(j*10), E_nc(100*j));
return m;
}
TEST_SUBMODULE(stl_binders, m) { TEST_SUBMODULE(stl_binders, m) {
// test_vector_int // test_vector_int
py::bind_vector<std::vector<unsigned int>>(m, "VectorInt", py::buffer_protocol()); py::bind_vector<std::vector<unsigned int>>(m, "VectorInt", py::buffer_protocol());
@ -85,6 +93,20 @@ TEST_SUBMODULE(stl_binders, m) {
m.def("get_mnc", &times_ten<std::map<int, E_nc>>, py::return_value_policy::reference); m.def("get_mnc", &times_ten<std::map<int, E_nc>>, py::return_value_policy::reference);
py::bind_map<std::unordered_map<int, E_nc>>(m, "UmapENC"); py::bind_map<std::unordered_map<int, E_nc>>(m, "UmapENC");
m.def("get_umnc", &times_ten<std::unordered_map<int, E_nc>>, py::return_value_policy::reference); m.def("get_umnc", &times_ten<std::unordered_map<int, E_nc>>, py::return_value_policy::reference);
// Issue #1885: binding nested std::map<X, Container<E>> with E non-copyable
py::bind_map<std::map<int, std::vector<E_nc>>>(m, "MapVecENC");
m.def("get_nvnc", [](int n)
{
auto m = new std::map<int, std::vector<E_nc>>();
for (int i = 1; i <= n; i++)
for (int j = 1; j <= n; j++)
(*m)[i].emplace_back(j);
return m;
}, py::return_value_policy::reference);
py::bind_map<std::map<int, std::map<int, E_nc>>>(m, "MapMapENC");
m.def("get_nmnc", &times_hundred<std::map<int, std::map<int, E_nc>>>, py::return_value_policy::reference);
py::bind_map<std::unordered_map<int, std::unordered_map<int, E_nc>>>(m, "UmapUmapENC");
m.def("get_numnc", &times_hundred<std::unordered_map<int, std::unordered_map<int, E_nc>>>, py::return_value_policy::reference);
// test_vector_buffer // test_vector_buffer
py::bind_vector<std::vector<unsigned char>>(m, "VectorUChar", py::buffer_protocol()); py::bind_vector<std::vector<unsigned char>>(m, "VectorUChar", py::buffer_protocol());

View File

@ -212,6 +212,44 @@ def test_noncopyable_containers():
assert vsum == 150 assert vsum == 150
# nested std::map<std::vector>
nvnc = m.get_nvnc(5)
for i in range(1, 6):
for j in range(0, 5):
assert nvnc[i][j].value == j + 1
for k, v in nvnc.items():
for i, j in enumerate(v, start=1):
assert j.value == i
# nested std::map<std::map>
nmnc = m.get_nmnc(5)
for i in range(1, 6):
for j in range(10, 60, 10):
assert nmnc[i][j].value == 10 * j
vsum = 0
for k_o, v_o in nmnc.items():
for k_i, v_i in v_o.items():
assert v_i.value == 10 * k_i
vsum += v_i.value
assert vsum == 7500
# nested std::unordered_map<std::unordered_map>
numnc = m.get_numnc(5)
for i in range(1, 6):
for j in range(10, 60, 10):
assert numnc[i][j].value == 10 * j
vsum = 0
for k_o, v_o in numnc.items():
for k_i, v_i in v_o.items():
assert v_i.value == 10 * k_i
vsum += v_i.value
assert vsum == 7500
def test_map_delitem(): def test_map_delitem():
mm = m.MapStringDouble() mm = m.MapStringDouble()