Skip to content

Commit f290765

Browse files
authored
Fix #5399: iterator increment operator does not skip first item (#5400)
* Fix #5399: iterator increment operator does not skip first item * Fix postfix increment operator: init() must be called before copying *this
1 parent af67e87 commit f290765

File tree

3 files changed

+32
-5
lines changed

3 files changed

+32
-5
lines changed

include/pybind11/pytypes.h

+15-5
Original file line numberDiff line numberDiff line change
@@ -1470,27 +1470,30 @@ class iterator : public object {
14701470
PYBIND11_OBJECT_DEFAULT(iterator, object, PyIter_Check)
14711471

14721472
iterator &operator++() {
1473+
init();
14731474
advance();
14741475
return *this;
14751476
}
14761477

14771478
iterator operator++(int) {
1479+
// Note: We must call init() first so that rv.value is
1480+
// the same as this->value just before calling advance().
1481+
// Otherwise, dereferencing the returned iterator may call
1482+
// advance() again and return the 3rd item instead of the 1st.
1483+
init();
14781484
auto rv = *this;
14791485
advance();
14801486
return rv;
14811487
}
14821488

14831489
// NOLINTNEXTLINE(readability-const-return-type) // PR #3263
14841490
reference operator*() const {
1485-
if (m_ptr && !value.ptr()) {
1486-
auto &self = const_cast<iterator &>(*this);
1487-
self.advance();
1488-
}
1491+
init();
14891492
return value;
14901493
}
14911494

14921495
pointer operator->() const {
1493-
operator*();
1496+
init();
14941497
return &value;
14951498
}
14961499

@@ -1513,6 +1516,13 @@ class iterator : public object {
15131516
friend bool operator!=(const iterator &a, const iterator &b) { return a->ptr() != b->ptr(); }
15141517

15151518
private:
1519+
void init() const {
1520+
if (m_ptr && !value.ptr()) {
1521+
auto &self = const_cast<iterator &>(*this);
1522+
self.advance();
1523+
}
1524+
}
1525+
15161526
void advance() {
15171527
value = reinterpret_steal<object>(PyIter_Next(m_ptr));
15181528
if (value.ptr() == nullptr && PyErr_Occurred()) {

tests/test_pytypes.cpp

+12
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,18 @@ TEST_SUBMODULE(pytypes, m) {
150150
m.def("get_iterator", [] { return py::iterator(); });
151151
// test_iterable
152152
m.def("get_iterable", [] { return py::iterable(); });
153+
m.def("get_first_item_from_iterable", [](const py::iterable &iter) {
154+
// This tests the postfix increment operator
155+
py::iterator it = iter.begin();
156+
py::iterator it2 = it++;
157+
return *it2;
158+
});
159+
m.def("get_second_item_from_iterable", [](const py::iterable &iter) {
160+
// This tests the prefix increment operator
161+
py::iterator it = iter.begin();
162+
++it;
163+
return *it;
164+
});
153165
m.def("get_frozenset_from_iterable",
154166
[](const py::iterable &iter) { return py::frozenset(iter); });
155167
m.def("get_list_from_iterable", [](const py::iterable &iter) { return py::list(iter); });

tests/test_pytypes.py

+5
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ def test_from_iterable(pytype, from_iter_func):
5252

5353
def test_iterable(doc):
5454
assert doc(m.get_iterable) == "get_iterable() -> Iterable"
55+
lins = [1, 2, 3]
56+
i = m.get_first_item_from_iterable(lins)
57+
assert i == 1
58+
i = m.get_second_item_from_iterable(lins)
59+
assert i == 2
5560

5661

5762
def test_float(doc):

0 commit comments

Comments
 (0)