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

@@ -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>

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);
+    }
+}