Merge pull request #407 from wjakob/fix_iterator

parameterize iterators by return value policy (fixes #388)

include/pybind11/pybind11.h

@@ -1175,7 +1175,7 @@ PYBIND11_NOINLINE inline void keep_alive_impl(int Nurse, int Patient, handle arg
     keep_alive_impl(nurse, patient);
     keep_alive_impl(nurse, patient);
 }
 }
 
 
-template <typename Iterator, typename Sentinel, bool KeyIterator, typename... Extra>
+template <typename Iterator, typename Sentinel, bool KeyIterator, return_value_policy Policy>
 struct iterator_state {
 struct iterator_state {
     Iterator it;
     Iterator it;
     Sentinel end;
     Sentinel end;
@@ -1187,12 +1187,13 @@ NAMESPACE_END(detail)
 template <typename... Args> detail::init<Args...> init() { return detail::init<Args...>(); }
 template <typename... Args> detail::init<Args...> init() { return detail::init<Args...>(); }
 template <typename... Args> detail::init_alias<Args...> init_alias() { return detail::init_alias<Args...>(); }
 template <typename... Args> detail::init_alias<Args...> init_alias() { return detail::init_alias<Args...>(); }
 
 
-template <typename Iterator,
+template <return_value_policy Policy = return_value_policy::reference_internal,
+          typename Iterator,
           typename Sentinel,
           typename Sentinel,
           typename ValueType = decltype(*std::declval<Iterator>()),
           typename ValueType = decltype(*std::declval<Iterator>()),
           typename... Extra>
           typename... Extra>
 iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
 iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
-    typedef detail::iterator_state<Iterator, Sentinel, false, Extra...> state;
+    typedef detail::iterator_state<Iterator, Sentinel, false, Policy> state;
 
 
     if (!detail::get_type_info(typeid(state))) {
     if (!detail::get_type_info(typeid(state))) {
         class_<state>(handle(), "iterator")
         class_<state>(handle(), "iterator")
@@ -1205,18 +1206,19 @@ iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
                 if (s.it == s.end)
                 if (s.it == s.end)
                     throw stop_iteration();
                     throw stop_iteration();
                 return *s.it;
                 return *s.it;
-            }, return_value_policy::reference_internal, std::forward<Extra>(extra)...);
+            }, std::forward<Extra>(extra)..., Policy);
     }
     }
 
 
     return (iterator) cast(state { first, last, true });
     return (iterator) cast(state { first, last, true });
 }
 }
 
 
-template <typename Iterator,
+template <return_value_policy Policy = return_value_policy::reference_internal,
+          typename Iterator,
           typename Sentinel,
           typename Sentinel,
           typename KeyType = decltype((*std::declval<Iterator>()).first),
           typename KeyType = decltype((*std::declval<Iterator>()).first),
           typename... Extra>
           typename... Extra>
 iterator make_key_iterator(Iterator first, Sentinel last, Extra &&... extra) {
 iterator make_key_iterator(Iterator first, Sentinel last, Extra &&... extra) {
-    typedef detail::iterator_state<Iterator, Sentinel, true, Extra...> state;
+    typedef detail::iterator_state<Iterator, Sentinel, true, Policy> state;
 
 
     if (!detail::get_type_info(typeid(state))) {
     if (!detail::get_type_info(typeid(state))) {
         class_<state>(handle(), "iterator")
         class_<state>(handle(), "iterator")
@@ -1229,18 +1231,20 @@ iterator make_key_iterator(Iterator first, Sentinel last, Extra &&... extra) {
                 if (s.it == s.end)
                 if (s.it == s.end)
                     throw stop_iteration();
                     throw stop_iteration();
                 return (*s.it).first;
                 return (*s.it).first;
-            }, return_value_policy::reference_internal, std::forward<Extra>(extra)...);
+            }, std::forward<Extra>(extra)..., Policy);
     }
     }
 
 
     return (iterator) cast(state { first, last, true });
     return (iterator) cast(state { first, last, true });
 }
 }
 
 
-template <typename Type, typename... Extra> iterator make_iterator(Type &value, Extra&&... extra) {
-    return make_iterator(std::begin(value), std::end(value), extra...);
+template <return_value_policy Policy = return_value_policy::reference_internal,
+          typename Type, typename... Extra> iterator make_iterator(Type &value, Extra&&... extra) {
+    return make_iterator<Policy>(std::begin(value), std::end(value), extra...);
 }
 }
 
 
-template <typename Type, typename... Extra> iterator make_key_iterator(Type &value, Extra&&... extra) {
-    return make_key_iterator(std::begin(value), std::end(value), extra...);
+template <return_value_policy Policy = return_value_policy::reference_internal,
+          typename Type, typename... Extra> iterator make_key_iterator(Type &value, Extra&&... extra) {
+    return make_key_iterator<Policy>(std::begin(value), std::end(value), extra...);
 }
 }
 
 
 template <typename InputType, typename OutputType> void implicitly_convertible() {
 template <typename InputType, typename OutputType> void implicitly_convertible() {

include/pybind11/stl_bind.h

@@ -247,7 +247,9 @@ pybind11::class_<Vector, holder_type> bind_vector(pybind11::module &m, std::stri
 
 
     cl.def("__iter__",
     cl.def("__iter__",
            [](Vector &v) {
            [](Vector &v) {
-            return pybind11::make_iterator<ItType, ItType, T>(v.begin(), v.end());
+               return pybind11::make_iterator<
+                   return_value_policy::reference_internal, ItType, ItType, T>(
+                   v.begin(), v.end());
            },
            },
            pybind11::keep_alive<0, 1>() /* Essential: keep list alive while iterator exists */
            pybind11::keep_alive<0, 1>() /* Essential: keep list alive while iterator exists */
     );
     );

tests/test_issues.cpp

@@ -233,6 +233,11 @@ void init_issues(py::module &m) {
         .def(py::self + py::self)
         .def(py::self + py::self)
         .def("__add__", [](const OpTest2& c2, const OpTest1& c1) { return c2 + c1; })
         .def("__add__", [](const OpTest2& c2, const OpTest1& c1) { return c2 + c1; })
         .def("__radd__", [](const OpTest2& c2, const OpTest1& c1) { return c2 + c1; });
         .def("__radd__", [](const OpTest2& c2, const OpTest1& c1) { return c2 + c1; });
+
+    // Issue 388: Can't make iterators via make_iterator() with different r/v policies
+    static std::vector<int> list = { 1, 2, 3 };
+    m2.def("make_iterator_1", []() { return py::make_iterator<py::return_value_policy::copy>(list); });
+    m2.def("make_iterator_2", []() { return py::make_iterator<py::return_value_policy::automatic>(list); });
 }
 }
 
 
 // MSVC workaround: trying to use a lambda here crashes MSCV
 // MSVC workaround: trying to use a lambda here crashes MSCV

tests/test_issues.py

@@ -170,3 +170,12 @@ def test_operators_notimplemented(capture):
 Add OpTest2 with OpTest2
 Add OpTest2 with OpTest2
 Add OpTest2 with OpTest1
 Add OpTest2 with OpTest1
 Add OpTest2 with OpTest1"""
 Add OpTest2 with OpTest1"""
+
+def test_iterator_rvpolicy():
+    """ Issue 388: Can't make iterators via make_iterator() with different r/v policies """
+    from pybind11_tests.issues import make_iterator_1
+    from pybind11_tests.issues import make_iterator_2
+
+    assert list(make_iterator_1()) == [1, 2, 3]
+    assert list(make_iterator_2()) == [1, 2, 3]
+    assert(type(make_iterator_1()) != type(make_iterator_2()))