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
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index e1bbf03..90407eb 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -797,12 +797,20 @@
negation<std::is_same<Container, typename Container::value_type>>
>::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).
+// Likewise for std::pair
+// (after C++17 it is mandatory that the copy constructor not exist when the two types aren't themselves
+// 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>>
: 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)
diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h
index d3adaed..62bd908 100644
--- a/include/pybind11/stl_bind.h
+++ b/include/pybind11/stl_bind.h
@@ -512,7 +512,7 @@
// Map assignment when copy-assignable: just copy the value
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 MappedType = typename Map::mapped_type;
@@ -528,7 +528,7 @@
// Not copy-assignable, but still copy-constructible: we can update the value by erasing and reinserting
template<typename Map, typename Class_>
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,
Class_> &cl) {
using KeyType = typename Map::key_type;
diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp
index a88b589..8688874 100644
--- a/tests/test_stl_binders.cpp
+++ b/tests/test_stl_binders.cpp
@@ -54,6 +54,14 @@
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_vector_int
py::bind_vector<std::vector<unsigned int>>(m, "VectorInt", py::buffer_protocol());
@@ -85,6 +93,20 @@
m.def("get_mnc", ×_ten<std::map<int, E_nc>>, py::return_value_policy::reference);
py::bind_map<std::unordered_map<int, E_nc>>(m, "UmapENC");
m.def("get_umnc", ×_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", ×_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", ×_hundred<std::unordered_map<int, std::unordered_map<int, E_nc>>>, py::return_value_policy::reference);
// test_vector_buffer
py::bind_vector<std::vector<unsigned char>>(m, "VectorUChar", py::buffer_protocol());
diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py
index 6d5a159..b83a587 100644
--- a/tests/test_stl_binders.py
+++ b/tests/test_stl_binders.py
@@ -212,6 +212,44 @@
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():
mm = m.MapStringDouble()