Fix STL casters for containers with proxies (regression)

To avoid an ODR violation in the test suite while testing
both `stl.h` and `std_bind.h` with `std::vector<bool>`,
the `py::bind_vector<std::vector<bool>>` test is moved to
the secondary module (which does not include `stl.h`).

docs/changelog.rst

@@ -14,6 +14,11 @@ v2.3.0 (Not yet released)
 v2.2.1 (Not yet released)
 -----------------------------------------------------
 
+* Fixed a regression where the automatic ``std::vector<bool>`` caster would
+  fail to compile. The same fix also applies to any container which returns
+  element proxies instead of references.
+  `#1053 <https://github.com/pybind/pybind11/pull/1053>`_.
+
 * Fixed a regression where the ``py::keep_alive`` policy could not be applied
   to constructors. `#1065 <https://github.com/pybind/pybind11/pull/1065>`_.
 

include/pybind11/stl.h

@@ -83,7 +83,7 @@ template <typename Type, typename Key> struct set_caster {
     template <typename T>
     static handle cast(T &&src, return_value_policy policy, handle parent) {
         pybind11::set s;
-        for (auto &value: src) {
+        for (auto &&value : src) {
             auto value_ = reinterpret_steal<object>(key_conv::cast(forward_like<T>(value), policy, parent));
             if (!value_ || !s.add(value_))
                 return handle();
@@ -117,7 +117,7 @@ template <typename Type, typename Key, typename Value> struct map_caster {
     template <typename T>
     static handle cast(T &&src, return_value_policy policy, handle parent) {
         dict d;
-        for (auto &kv: src) {
+        for (auto &&kv : src) {
             auto key = reinterpret_steal<object>(key_conv::cast(forward_like<T>(kv.first), policy, parent));
             auto value = reinterpret_steal<object>(value_conv::cast(forward_like<T>(kv.second), policy, parent));
             if (!key || !value)
@@ -159,7 +159,7 @@ template <typename Type, typename Value> struct list_caster {
     static handle cast(T &&src, return_value_policy policy, handle parent) {
         list l(src.size());
         size_t index = 0;
-        for (auto &value: src) {
+        for (auto &&value : src) {
             auto value_ = reinterpret_steal<object>(value_conv::cast(forward_like<T>(value), policy, parent));
             if (!value_)
                 return handle();
@@ -213,7 +213,7 @@ template <typename ArrayType, typename Value, bool Resizable, size_t Size = 0> s
     static handle cast(T &&src, return_value_policy policy, handle parent) {
         list l(src.size());
         size_t index = 0;
-        for (auto &value: src) {
+        for (auto &&value : src) {
             auto value_ = reinterpret_steal<object>(value_conv::cast(forward_like<T>(value), policy, parent));
             if (!value_)
                 return handle();

tests/CMakeLists.txt

@@ -76,6 +76,7 @@ string(REPLACE ".cpp" ".py" PYBIND11_PYTEST_FILES "${PYBIND11_TEST_FILES}")
 set(PYBIND11_CROSS_MODULE_TESTS
   test_exceptions.py
   test_local_bindings.py
+  test_stl_binders.py
 )
 
 # Check if Eigen is available; if not, remove from PYBIND11_TEST_FILES (but

tests/pybind11_cross_module_tests.cpp

@@ -104,4 +104,10 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
     m.def("get_gl_value", [](MixGL &o) { return o.i + 100; });
 
     py::class_<MixGL2>(m, "MixGL2", py::module_local()).def(py::init<int>());
+
+    // test_vector_bool
+    // We can't test both stl.h and stl_bind.h conversions of `std::vector<bool>` within
+    // the same module (it would be an ODR violation). Therefore `bind_vector` of `bool`
+    // is defined here and tested in `test_stl_binders.py`.
+    py::bind_vector<std::vector<bool>>(m, "VectorBool");
 }

tests/test_stl.cpp

@@ -48,6 +48,11 @@ TEST_SUBMODULE(stl, m) {
     // test_vector
     m.def("cast_vector", []() { return std::vector<int>{1}; });
     m.def("load_vector", [](const std::vector<int> &v) { return v.at(0) == 1 && v.at(1) == 2; });
+    // `std::vector<bool>` is special because it returns proxy objects instead of references
+    m.def("cast_bool_vector", []() { return std::vector<bool>{true, false}; });
+    m.def("load_bool_vector", [](const std::vector<bool> &v) {
+        return v.at(0) == true && v.at(1) == false;
+    });
     // Unnumbered regression (caused by #936): pointers to stl containers aren't castable
     static std::vector<RValueCaster> lvv{2};
     m.def("cast_ptr_vector", []() { return &lvv; });

tests/test_stl.py

@@ -12,6 +12,9 @@ def test_vector(doc):
     assert m.load_vector(l)
     assert m.load_vector(tuple(l))
 
+    assert m.cast_bool_vector() == [True, False]
+    assert m.load_bool_vector([True, False])
+
     assert doc(m.cast_vector) == "cast_vector() -> List[int]"
     assert doc(m.load_vector) == "load_vector(arg0: List[int]) -> bool"
 

tests/test_stl_binders.cpp

@@ -55,13 +55,9 @@ template <class Map> Map *times_ten(int n) {
 }
 
 TEST_SUBMODULE(stl_binders, m) {
-
     // test_vector_int
     py::bind_vector<std::vector<unsigned int>>(m, "VectorInt", py::buffer_protocol());
 
-    // test_vector_bool
-    py::bind_vector<std::vector<bool>>(m, "VectorBool");
-
     // test_vector_custom
     py::class_<El>(m, "El")
         .def(py::init<int>());

tests/test_stl_binders.py

@@ -86,7 +86,9 @@ def test_vector_buffer_numpy():
 
 
 def test_vector_bool():
-    vv_c = m.VectorBool()
+    import pybind11_cross_module_tests as cm
+
+    vv_c = cm.VectorBool()
     for i in range(10):
         vv_c.append(i % 2 == 0)
     for i in range(10):