Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix STL caster regression for containers with proxies #1053

Merged
merged 1 commit into from Sep 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog.rst
Expand Up @@ -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 <https://github.com/pybind/pybind11/pull/1062>`_.

* 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>`_.

Expand Down
8 changes: 4 additions & 4 deletions include/pybind11/stl.h
Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions tests/pybind11_cross_module_tests.cpp
Expand Up @@ -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");
}
5 changes: 5 additions & 0 deletions tests/test_stl.cpp
Expand Up @@ -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; });
Expand Down
3 changes: 3 additions & 0 deletions tests/test_stl.py
Expand Up @@ -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"

Expand Down
4 changes: 0 additions & 4 deletions tests/test_stl_binders.cpp
Expand Up @@ -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>());
Expand Down
4 changes: 3 additions & 1 deletion tests/test_stl_binders.py
Expand Up @@ -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):
Expand Down