mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-12-05 01:10:24 +00:00
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:
0edcd19615
This commit is contained in:
committed by
Jelle Raaijmakers
parent
195a67ed80
commit
911ecf1450
Notes:
github-actions[bot]
2025-11-18 12:33:08 +00:00
Author: https://github.com/trflynn89 Commit: https://github.com/LadybirdBrowser/ladybird/commit/911ecf14505 Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/6852 Reviewed-by: https://github.com/gmta ✅
@@ -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;
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user