From 0edcd19615bc554ce4bb8d72a6bef1118a375a4e Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Mon, 17 Nov 2025 10:11:05 -0500
Subject: [PATCH] AK: Avoid copying the iterable container in AK::enumerate

There are actually a couple of issues here:

1. We are not properly perfect-forwarding the iterable to the Enumerator
   member. We are using the class template as the constructor type, but
   we would actually have to do something like this to achieve perfect
   forwarding:

   template <typname Iter = Iterable>
   Enumerator(Iter&&)

2. The begin / end methods on Enumerator (although they return by const-
   ref) are making copies during for-each loops. The compiler basically
   generates this when we call enumerate:

   for (auto it = Enumerator::begin(); it != Enumerator::end(); ++it)

   The creation of `it` above actually creates a copy of the returned
   Enumerator instance.

To avoid all of this, let's create an intermediate structure to act as
the enumerated iterator. This structure does not hold the iterable and
thus is fine to copy. We can then let the compiler handle forwarding
the iterable to the Enumerator.
---
 AK/Enumerate.h             | 42 ++++++++++++++++----------------------
 Tests/AK/TestEnumerate.cpp | 40 ++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/AK/Enumerate.h b/AK/Enumerate.h
index d824605e7dd..0203f9b3dcf 100644
--- a/AK/Enumerate.h
+++ b/AK/Enumerate.h
@@ -11,8 +11,9 @@
 namespace AK {
 
 namespace Detail {
+
 template<typename Iterable>
-class Enumerator {
+struct Enumerator {
     using IteratorType = decltype(declval<Iterable>().begin());
     using ValueType = decltype(*declval<IteratorType>());
 
@@ -21,35 +22,28 @@ class Enumerator {
         ValueType value;
     };
 
-public:
-    Enumerator(Iterable&& iterable)
-        : m_iterable(forward<Iterable>(iterable))
-        , m_iterator(m_iterable.begin())
-        , m_end(m_iterable.end())
-    {
-    }
+    struct Iterator {
+        Enumeration operator*() { return { index, *iterator }; }
+        Enumeration operator*() const { return { index, *iterator }; }
 
-    Enumerator const& begin() const { return *this; }
-    Enumerator const& end() const { return *this; }
+        bool operator!=(Iterator const& other) const { return iterator != other.iterator; }
 
-    Enumeration operator*() { return { m_index, *m_iterator }; }
-    Enumeration operator*() const { return { m_index, *m_iterator }; }
+        void operator++()
+        {
+            ++index;
+            ++iterator;
+        }
 
-    bool operator!=(Enumerator const&) const { return m_iterator != m_end; }
+        size_t index { 0 };
+        IteratorType iterator;
+    };
 
-    void operator++()
-    {
-        ++m_index;
-        ++m_iterator;
-    }
+    Iterator begin() { return { 0, iterable.begin() }; }
+    Iterator end() { return { 0, iterable.end() }; }
 
-private:
-    Iterable m_iterable;
-
-    size_t m_index { 0 };
-    IteratorType m_iterator;
-    IteratorType const m_end;
+    Iterable iterable;
 };
+
 }
 
 template<typename T>
diff --git a/Tests/AK/TestEnumerate.cpp b/Tests/AK/TestEnumerate.cpp
index dffc2869b62..728962aeab1 100644
--- a/Tests/AK/TestEnumerate.cpp
+++ b/Tests/AK/TestEnumerate.cpp
@@ -49,3 +49,43 @@ TEST_CASE(enumerate)
         EXPECT_EQ(result, (Vector<IndexAndValue> { { 0, 9 }, { 1, 8 }, { 2, 7 }, { 3, 6 } }));
     }
 }
+
+class CopyCounter {
+public:
+    static inline size_t copy_count = 0;
+
+    CopyCounter() = default;
+    CopyCounter(CopyCounter const&) { ++copy_count; }
+    CopyCounter(CopyCounter&&) { }
+
+    auto begin() const { return m_vec.begin(); }
+    auto end() const { return m_vec.end(); }
+
+private:
+    Vector<int> m_vec { 1, 2, 3, 4 };
+};
+
+TEST_CASE(do_not_copy)
+{
+    {
+        Vector<IndexAndValue> result;
+        CopyCounter::copy_count = 0;
+        CopyCounter counter {};
+
+        for (auto [i, value] : enumerate(counter))
+            result.append({ i, value });
+
+        EXPECT_EQ(result, (Vector<IndexAndValue> { { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 } }));
+        EXPECT_EQ(CopyCounter::copy_count, 0uz);
+    }
+    {
+        Vector<IndexAndValue> result;
+        CopyCounter::copy_count = 0;
+
+        for (auto [i, value] : enumerate(CopyCounter {}))
+            result.append({ i, value });
+
+        EXPECT_EQ(result, (Vector<IndexAndValue> { { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 } }));
+        EXPECT_EQ(CopyCounter::copy_count, 0uz);
+    }
+}