Repository: pybind/pybind11 (language: python)

Issue:
[BUG]: iterator increment operator does not skip first item
### Required prerequisites

- [X] Make sure you've read the [documentation](https://pybind11.readthedocs.io). Your issue may be addressed there.
- [X] Search the [issue tracker](https://github.com/pybind/pybind11/issues) and [Discussions](https:/pybind/pybind11/discussions) to verify that this hasn't already been reported. +1 or comment there if it has.
- [X] Consider asking first in the [Gitter chat room](https://gitter.im/pybind/Lobby) or in a [Discussion](https:/pybind/pybind11/discussions/new).

### What version (or hash if on master) of pybind11 are you using?

master (c4a05f)

### Problem description

The bug is that as currently implemented, the first call to `iterator::operator++` is semantically a no-op, since it calls `iterator::advance()`, but `operator*` also automatically calls `advance()` if it has never been advanced yet (that is, when `iterator::value` is null). So the two following snippets are equivalent:

```
// Snippet 1
py::iterator it = my_iterable.begin();
return *it; // Returns the first item, as expected

// Snippet 2
py::iterator it = my_iterable.begin();
++it;
return *it; // BUG: also returns the first item
```

I have locally added unit tests that reproduce the problem (see below), and I'm working on a fix.

### Reproducible example code

Adding the following to test_pytypes.cpp:

```
    m.def("get_second_item_from_iterable", [](const py::iterable &iter) {
        py::iterator it = iter.begin();
        ++it;
        return *it;
    });
```

and to test_pytypes.py:

```
def test_get_second_item_from_iterable():
    lins = [1, 2]
    i = m.get_second_item_from_iterable(lins)
    assert i == 2
```

Fails with:

```
___ test_get_second_item_from_iterable ___

    def test_get_second_item_from_iterable():
        lins = [1, 2]
        i = m.get_second_item_from_iterable(lins)
>       assert i == 2
E       assert 1 == 2

i          = 1
lins       = [1, 2]
```

### Is this a regression? Put the last known working version here if it is.

Not a regression
[BUG]: iterator increment operator does not skip first item
### Required prerequisites

- [X] Make sure you've read the [documentation](https://pybind11.readthedocs.io). Your issue may be addressed there.
- [X] Search the [issue tracker](https://github.com/pybind/pybind11/issues) and [Discussions](https:/pybind/pybind11/discussions) to verify that this hasn't already been reported. +1 or comment there if it has.
- [X] Consider asking first in the [Gitter chat room](https://gitter.im/pybind/Lobby) or in a [Discussion](https:/pybind/pybind11/discussions/new).

### What version (or hash if on master) of pybind11 are you using?

master (c4a05f)

### Problem description

The bug is that as currently implemented, the first call to `iterator::operator++` is semantically a no-op, since it calls `iterator::advance()`, but `operator*` also automatically calls `advance()` if it has never been advanced yet (that is, when `iterator::value` is null). So the two following snippets are equivalent:

```
// Snippet 1
py::iterator it = my_iterable.begin();
return *it; // Returns the first item, as expected

// Snippet 2
py::iterator it = my_iterable.begin();
++it;
return *it; // BUG: also returns the first item
```

I have locally added unit tests that reproduce the problem (see below), and I'm working on a fix.

### Reproducible example code

Adding the following to test_pytypes.cpp:

```
    m.def("get_second_item_from_iterable", [](const py::iterable &iter) {
        py::iterator it = iter.begin();
        ++it;
        return *it;
    });
```

and to test_pytypes.py:

```
def test_get_second_item_from_iterable():
    lins = [1, 2]
    i = m.get_second_item_from_iterable(lins)
    assert i == 2
```

Fails with:

```
___ test_get_second_item_from_iterable ___

    def test_get_second_item_from_iterable():
        lins = [1, 2]
        i = m.get_second_item_from_iterable(lins)
>       assert i == 2
E       assert 1 == 2

i          = 1
lins       = [1, 2]
```

### Is this a regression? Put the last known working version here if it is.

Not a regression


Discussion / hints:

Fix patch:
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 7aafab6dcc..027e36098b 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -1470,11 +1470,17 @@ class iterator : public object {
     PYBIND11_OBJECT_DEFAULT(iterator, object, PyIter_Check)
 
     iterator &operator++() {
+        init();
         advance();
         return *this;
     }
 
     iterator operator++(int) {
+        // Note: We must call init() first so that rv.value is
+        // the same as this->value just before calling advance().
+        // Otherwise, dereferencing the returned iterator may call
+        // advance() again and return the 3rd item instead of the 1st.
+        init();
         auto rv = *this;
         advance();
         return rv;
@@ -1482,15 +1488,12 @@ class iterator : public object {
 
     // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     reference operator*() const {
-        if (m_ptr && !value.ptr()) {
-            auto &self = const_cast<iterator &>(*this);
-            self.advance();
-        }
+        init();
         return value;
     }
 
     pointer operator->() const {
-        operator*();
+        init();
         return &value;
     }
 
@@ -1513,6 +1516,13 @@ class iterator : public object {
     friend bool operator!=(const iterator &a, const iterator &b) { return a->ptr() != b->ptr(); }
 
 private:
+    void init() const {
+        if (m_ptr && !value.ptr()) {
+            auto &self = const_cast<iterator &>(*this);
+            self.advance();
+        }
+    }
+
     void advance() {
         value = reinterpret_steal<object>(PyIter_Next(m_ptr));
         if (value.ptr() == nullptr && PyErr_Occurred()) {


Test patch:
diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
index 19f65ce7eb..8df4cdd3f6 100644
--- a/tests/test_pytypes.cpp
+++ b/tests/test_pytypes.cpp
@@ -150,6 +150,18 @@ TEST_SUBMODULE(pytypes, m) {
     m.def("get_iterator", [] { return py::iterator(); });
     // test_iterable
     m.def("get_iterable", [] { return py::iterable(); });
+    m.def("get_first_item_from_iterable", [](const py::iterable &iter) {
+        // This tests the postfix increment operator
+        py::iterator it = iter.begin();
+        py::iterator it2 = it++;
+        return *it2;
+    });
+    m.def("get_second_item_from_iterable", [](const py::iterable &iter) {
+        // This tests the prefix increment operator
+        py::iterator it = iter.begin();
+        ++it;
+        return *it;
+    });
     m.def("get_frozenset_from_iterable",
           [](const py::iterable &iter) { return py::frozenset(iter); });
     m.def("get_list_from_iterable", [](const py::iterable &iter) { return py::list(iter); });
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index 39d0b619b8..9fd24b34f1 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -52,6 +52,11 @@ def test_from_iterable(pytype, from_iter_func):
 
 def test_iterable(doc):
     assert doc(m.get_iterable) == "get_iterable() -> Iterable"
+    lins = [1, 2, 3]
+    i = m.get_first_item_from_iterable(lins)
+    assert i == 1
+    i = m.get_second_item_from_iterable(lins)
+    assert i == 2
 
 
 def test_float(doc):