diff --git a/docs/changelog.rst b/docs/changelog.rst index f03d4f4c..dcab3b11 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -17,6 +17,11 @@ v2.2.1 (Not yet released) * Fixed compilation with Clang on host GCC < 5 (old libstdc++ which isn't fully C++11 compliant). `#1062 `_. +* Fixed a regression where the automatic ``std::vector`` caster would + fail to compile. The same fix also applies to any container which returns + element proxies instead of references. + `#1053 `_. + * Fixed a regression where the ``py::keep_alive`` policy could not be applied to constructors. `#1065 `_. diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index ac212bfe..db900e67 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -83,7 +83,7 @@ template struct set_caster { template 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(key_conv::cast(forward_like(value), policy, parent)); if (!value_ || !s.add(value_)) return handle(); @@ -117,7 +117,7 @@ template struct map_caster { template 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(key_conv::cast(forward_like(kv.first), policy, parent)); auto value = reinterpret_steal(value_conv::cast(forward_like(kv.second), policy, parent)); if (!key || !value) @@ -159,7 +159,7 @@ public: 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(value_conv::cast(forward_like(value), policy, parent)); if (!value_) return handle(); @@ -213,7 +213,7 @@ public: 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(value_conv::cast(forward_like(value), policy, parent)); if (!value_) return handle(); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 52326297..d8c53c21 100644 --- a/tests/CMakeLists.txt +++ b/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 diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp index 20916244..7dd70e0a 100644 --- a/tests/pybind11_cross_module_tests.cpp +++ b/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_(m, "MixGL2", py::module_local()).def(py::init()); + + // test_vector_bool + // We can't test both stl.h and stl_bind.h conversions of `std::vector` 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>(m, "VectorBool"); } diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 0746fb4d..7d53e9c1 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -48,6 +48,11 @@ TEST_SUBMODULE(stl, m) { // test_vector m.def("cast_vector", []() { return std::vector{1}; }); m.def("load_vector", [](const std::vector &v) { return v.at(0) == 1 && v.at(1) == 2; }); + // `std::vector` is special because it returns proxy objects instead of references + m.def("cast_bool_vector", []() { return std::vector{true, false}; }); + m.def("load_bool_vector", [](const std::vector &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 lvv{2}; m.def("cast_ptr_vector", []() { return &lvv; }); diff --git a/tests/test_stl.py b/tests/test_stl.py index c2fa7db1..f04eaeb8 100644 --- a/tests/test_stl.py +++ b/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" diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index 2df6ca03..a88b589e 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -55,13 +55,9 @@ template Map *times_ten(int n) { } TEST_SUBMODULE(stl_binders, m) { - // test_vector_int py::bind_vector>(m, "VectorInt", py::buffer_protocol()); - // test_vector_bool - py::bind_vector>(m, "VectorBool"); - // test_vector_custom py::class_(m, "El") .def(py::init()); diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index 7496d056..bf1aa674 100644 --- a/tests/test_stl_binders.py +++ b/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):