From 911ecf14505e3dad181c48225b7f543fe9ea150d 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.

Cherry-picked from:
https://github.com/SerenityOS/serenity/commit/0edcd19615bc554ce4bb8d72a6bef1118a375a4e
---
 AK/Enumerate.h             | 42 +++++++++++++++-----------------------
 Tests/AK/TestEnumerate.cpp | 40 ++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/AK/Enumerate.h b/AK/Enumerate.h
index eadb30f677..841e3fc94e 100644
--- a/AK/Enumerate.h
+++ b/AK/Enumerate.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, Tim Flynn <trflynn89@serenityos.org>
+ * Copyright (c) 2024-2025, Tim Flynn <trflynn89@ladybird.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -13,7 +13,7 @@ namespace AK {
 namespace Detail {
 
 template<typename Iterable>
-class Enumerator {
+struct Enumerator {
     using IteratorType = decltype(declval<Iterable>().begin());
     using ValueType = decltype(*declval<IteratorType>());
 
@@ -22,34 +22,26 @@ 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;
 };
 
 }
diff --git a/Tests/AK/TestEnumerate.cpp b/Tests/AK/TestEnumerate.cpp
index dffc2869b6..728962aeab 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);
+    }
+}