diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index c4e5ea6e1..07d2d0436 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1987,6 +1987,7 @@ iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) { throw stop_iteration(); } return *s.it; + // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 }, std::forward(extra)..., Policy); } diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a1cf6bef0..d1d3dcb05 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -733,7 +733,9 @@ public: generic_iterator() = default; generic_iterator(handle seq, ssize_t index) : Policy(seq, index) { } + // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 reference operator*() const { return Policy::dereference(); } + // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 reference operator[](difference_type n) const { return *(*this + n); } pointer operator->() const { return **this; } @@ -773,11 +775,12 @@ class sequence_fast_readonly { protected: using iterator_category = std::random_access_iterator_tag; using value_type = handle; - using reference = handle; + using reference = const handle; // PR #3263 using pointer = arrow_proxy; sequence_fast_readonly(handle obj, ssize_t n) : ptr(PySequence_Fast_ITEMS(obj.ptr()) + n) { } + // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 reference dereference() const { return *ptr; } void increment() { ++ptr; } void decrement() { --ptr; } @@ -816,12 +819,13 @@ class dict_readonly { protected: using iterator_category = std::forward_iterator_tag; using value_type = std::pair; - using reference = value_type; + using reference = const value_type; // PR #3263 using pointer = arrow_proxy; dict_readonly() = default; dict_readonly(handle obj, ssize_t pos) : obj(obj), pos(pos) { increment(); } + // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 reference dereference() const { return {key, value}; } void increment() { if (PyDict_Next(obj.ptr(), &pos, &key, &value) == 0) { @@ -966,7 +970,7 @@ public: using iterator_category = std::input_iterator_tag; using difference_type = ssize_t; using value_type = handle; - using reference = handle; + using reference = const handle; // PR #3263 using pointer = const handle *; PYBIND11_OBJECT_DEFAULT(iterator, object, PyIter_Check) @@ -982,6 +986,7 @@ public: return rv; } + // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 reference operator*() const { if (m_ptr && !value.ptr()) { auto& self = const_cast(*this); diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 96c97351e..2157dc097 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -462,6 +462,49 @@ TEST_SUBMODULE(pytypes, m) { m.def("weakref_from_object_and_function", [](py::object o, py::function f) { return py::weakref(std::move(o), std::move(f)); }); +// See PR #3263 for background (https://github.com/pybind/pybind11/pull/3263): +// pytypes.h could be changed to enforce the "most correct" user code below, by removing +// `const` from iterator `reference` using type aliases, but that will break existing +// user code. +#if (defined(__APPLE__) && defined(__clang__)) || defined(PYPY_VERSION) +// This is "most correct" and enforced on these platforms. +# define PYBIND11_AUTO_IT auto it +#else +// This works on many platforms and is (unfortunately) reflective of existing user code. +// NOLINTNEXTLINE(bugprone-macro-parentheses) +# define PYBIND11_AUTO_IT auto &it +#endif + + m.def("tuple_iterator", []() { + auto tup = py::make_tuple(5, 7); + int tup_sum = 0; + for (PYBIND11_AUTO_IT : tup) { + tup_sum += it.cast(); + } + return tup_sum; + }); + + m.def("dict_iterator", []() { + py::dict dct; + dct[py::int_(3)] = 5; + dct[py::int_(7)] = 11; + int kv_sum = 0; + for (PYBIND11_AUTO_IT : dct) { + kv_sum += it.first.cast() * 100 + it.second.cast(); + } + return kv_sum; + }); + + m.def("passed_iterator", [](const py::iterator &py_it) { + int elem_sum = 0; + for (PYBIND11_AUTO_IT : py_it) { + elem_sum += it.cast(); + } + return elem_sum; + }); + +#undef PYBIND11_AUTO_IT + // Tests below this line are for pybind11 IMPLEMENTATION DETAILS: m.def("sequence_item_get_ssize_t", [](const py::object &o) { diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 18847550f..070538cc9 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -623,6 +623,12 @@ def test_weakref(create_weakref, create_weakref_with_callback): assert callback.called +def test_cpp_iterators(): + assert m.tuple_iterator() == 12 + assert m.dict_iterator() == 305 + 711 + assert m.passed_iterator(iter((-7, 3))) == -4 + + def test_implementation_details(): lst = [39, 43, 92, 49, 22, 29, 93, 98, 26, 57, 8] tup = tuple(lst)