From c83605936b5e09ae806c40fe798d1e0b1143e272 Mon Sep 17 00:00:00 2001
From: Sergei Izmailov <sergei.a.izmailov@gmail.com>
Date: Thu, 7 Sep 2023 21:57:39 +0900
Subject: [PATCH] feature: Support move-only iterators in `py::make_*iterator`
 (#4834)

* feature: Support move-only iterators in `py::make_*iterator`

* fix: Missing static assertion message

* fixup: Missing `explicit` in single argument constructors

* fix: Simplify tests: make existing iterator move-only

* fix: Missing `noexcept`
---
 include/pybind11/pybind11.h            | 14 ++++++++++----
 tests/test_sequences_and_iterators.cpp | 19 +++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 0a82f8dd75..5e3c5c6577 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -2434,7 +2434,7 @@ iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&...extra) {
                 Policy);
     }
 
-    return cast(state{first, last, true});
+    return cast(state{std::forward<Iterator>(first), std::forward<Sentinel>(last), true});
 }
 
 PYBIND11_NAMESPACE_END(detail)
@@ -2451,7 +2451,9 @@ iterator make_iterator(Iterator first, Sentinel last, Extra &&...extra) {
                                       Iterator,
                                       Sentinel,
                                       ValueType,
-                                      Extra...>(first, last, std::forward<Extra>(extra)...);
+                                      Extra...>(std::forward<Iterator>(first),
+                                                std::forward<Sentinel>(last),
+                                                std::forward<Extra>(extra)...);
 }
 
 /// Makes a python iterator over the keys (`.first`) of a iterator over pairs from a
@@ -2467,7 +2469,9 @@ iterator make_key_iterator(Iterator first, Sentinel last, Extra &&...extra) {
                                       Iterator,
                                       Sentinel,
                                       KeyType,
-                                      Extra...>(first, last, std::forward<Extra>(extra)...);
+                                      Extra...>(std::forward<Iterator>(first),
+                                                std::forward<Sentinel>(last),
+                                                std::forward<Extra>(extra)...);
 }
 
 /// Makes a python iterator over the values (`.second`) of a iterator over pairs from a
@@ -2483,7 +2487,9 @@ iterator make_value_iterator(Iterator first, Sentinel last, Extra &&...extra) {
                                       Iterator,
                                       Sentinel,
                                       ValueType,
-                                      Extra...>(first, last, std::forward<Extra>(extra)...);
+                                      Extra...>(std::forward<Iterator>(first),
+                                                std::forward<Sentinel>(last),
+                                                std::forward<Extra>(extra)...);
 }
 
 /// Makes an iterator over values of an stl container or other container supporting
diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp
index 1de65edbf2..4a1d37f4de 100644
--- a/tests/test_sequences_and_iterators.cpp
+++ b/tests/test_sequences_and_iterators.cpp
@@ -28,6 +28,13 @@ class NonZeroIterator {
 
 public:
     explicit NonZeroIterator(const T *ptr) : ptr_(ptr) {}
+
+    // Make the iterator non-copyable and movable
+    NonZeroIterator(const NonZeroIterator &) = delete;
+    NonZeroIterator(NonZeroIterator &&) noexcept = default;
+    NonZeroIterator &operator=(const NonZeroIterator &) = delete;
+    NonZeroIterator &operator=(NonZeroIterator &&) noexcept = default;
+
     const T &operator*() const { return *ptr_; }
     NonZeroIterator &operator++() {
         ++ptr_;
@@ -78,6 +85,7 @@ class NonCopyableInt {
     int value_;
 };
 using NonCopyableIntPair = std::pair<NonCopyableInt, NonCopyableInt>;
+
 PYBIND11_MAKE_OPAQUE(std::vector<NonCopyableInt>);
 PYBIND11_MAKE_OPAQUE(std::vector<NonCopyableIntPair>);
 
@@ -375,6 +383,17 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
     private:
         std::vector<std::pair<int, int>> data_;
     };
+
+    {
+        // #4383 : Make sure `py::make_*iterator` functions work with move-only iterators
+        using iterator_t = NonZeroIterator<std::pair<int, int>>;
+
+        static_assert(std::is_move_assignable<iterator_t>::value, "");
+        static_assert(std::is_move_constructible<iterator_t>::value, "");
+        static_assert(!std::is_copy_assignable<iterator_t>::value, "");
+        static_assert(!std::is_copy_constructible<iterator_t>::value, "");
+    }
+
     py::class_<IntPairs>(m, "IntPairs")
         .def(py::init<std::vector<std::pair<int, int>>>())
         .def(