fix: regression in #3271

Author: henryiii

include/pybind11/pybind11.h

@@ -1966,27 +1966,29 @@ struct iterator_state {
 };
 
 // Note: these helpers take the iterator by non-const reference because some
-// iterators in the wild can't be dereferenced when const.
-template <typename Iterator>
+// iterators in the wild can't be dereferenced when const. C++ needs the extra parens in decltype
+// to enforce an lvalue. The & after Iterator is required for MSVC < 16.9. ResultType cannot be
+// reused for result_type due to bugs in ICC, NVCC, and PGI. See #3293.
+template <typename Iterator, typename ResultType = decltype((*std::declval<Iterator &>()))>
 struct iterator_access {
-    using result_type = decltype((*std::declval<Iterator>()));
+    using result_type = decltype((*std::declval<Iterator &>()));
     // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     result_type operator()(Iterator &it) const {
         return *it;
     }
 };
 
-template <typename Iterator>
+template <typename Iterator, typename ResultType = decltype(((*std::declval<Iterator &>()).first)) >
 struct iterator_key_access {
-    using result_type = decltype(((*std::declval<Iterator>()).first));
+    using result_type = decltype(((*std::declval<Iterator &>()).first));
     result_type operator()(Iterator &it) const {
         return (*it).first;
     }
 };
 
-template <typename Iterator>
+template <typename Iterator, typename ResultType = decltype(((*std::declval<Iterator &>()).second))>
 struct iterator_value_access {
-    using result_type = decltype(((*std::declval<Iterator>()).second));
+    using result_type = decltype(((*std::declval<Iterator &>()).second));
     result_type operator()(Iterator &it) const {
         return (*it).second;
     }

tests/test_sequences_and_iterators.cpp

@@ -339,18 +339,23 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
         .def("nonzero_values", [](const IntPairs& s) {
             return py::make_value_iterator(NonZeroIterator<std::pair<int, int>>(s.begin()), NonZeroSentinel());
         }, py::keep_alive<0, 1>())
+
+        // test single-argument make_iterator
         .def("simple_iterator", [](IntPairs& self) {
             return py::make_iterator(self);
         }, py::keep_alive<0, 1>())
         .def("simple_keys", [](IntPairs& self) {
             return py::make_key_iterator(self);
         }, py::keep_alive<0, 1>())
+        .def("simple_values", [](IntPairs& self) {
+            return py::make_value_iterator(self);
+        }, py::keep_alive<0, 1>())
 
-        // test iterator with keep_alive (doesn't work so not used at runtime, but tests compile)
+        // test iterator with keep_alive (doesn't work on this class so not used
+        // at runtime, but tests need to be able to compile)
         .def("make_iterator_keep_alive", [](IntPairs& self) {
             return py::make_iterator(self, py::keep_alive<0, 1>());
         }, py::keep_alive<0, 1>())
-        ;
 
     // test_iterater_referencing
     py::class_<NonCopyableInt>(m, "NonCopyableInt")

tests/test_sequences_and_iterators.py

@@ -59,6 +59,7 @@ def test_generalized_iterators_simple():
         (0, 5),
     ]
     assert list(m.IntPairs([(1, 2), (3, 4), (0, 5)]).simple_keys()) == [1, 3, 0]
+    assert list(m.IntPairs([(1, 2), (3, 4), (0, 5)]).simple_values()) == [2, 4, 5]
 
 
 def test_iterator_referencing():