Fix enumerate to handle refs
The SkEnumerate was capturing elements by value instead of
by reference. This caused odd things to happen when the value
was used on the left side of an assignment.
Added a test to demonstrate this.
Change-Id: I14bbeee8041bcbaa4e5ca845d6510096558673b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341462
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
diff --git a/src/core/SkEnumerate.h b/src/core/SkEnumerate.h
index 030d0e7..6107328 100644
--- a/src/core/SkEnumerate.h
+++ b/src/core/SkEnumerate.h
@@ -5,8 +5,8 @@
* found in the LICENSE file.
*/
-#ifndef SkIota_DEFINED
-#define SkIota_DEFINED
+#ifndef SkEnumerate_DEFINED
+#define SkEnumerate_DEFINED
#include <cstddef>
#include <iterator>
@@ -19,14 +19,17 @@
using Captured = decltype(*std::declval<Iter>());
template <typename> struct is_tuple : std::false_type {};
template <typename... T> struct is_tuple<std::tuple<T...>> : std::true_type {};
+
+ // v must be a r-value to bind to temporary non-const references.
static constexpr auto MakeResult(size_t i, Captured&& v) {
if constexpr (is_tuple<Captured>::value) {
- return std::tuple_cat(std::tuple<size_t>{i}, std::forward<Captured>(v));
+ return std::tuple_cat(std::tuple<size_t>{i}, v);
} else {
- return std::tuple_cat(std::tuple<size_t>{i},
- std::make_tuple(std::forward<Captured>(v)));
+ // Capture v by reference instead of by value by using std::tie.
+ return std::tuple_cat(std::tuple<size_t>{i}, std::tie(v));
}
}
+
using Result = decltype(MakeResult(0, std::declval<Captured>()));
class Iterator {
@@ -109,4 +112,4 @@
inline constexpr SkEnumerate<Iter> SkMakeEnumerate(T (&a)[N]) {
return SkEnumerate<Iter>{std::begin(a), std::end(a)};
}
-#endif // SkIota_DEFINED
+#endif // SkEnumerate_DEFINED
diff --git a/tests/UtilsTest.cpp b/tests/UtilsTest.cpp
index 9d3ecfd..8f635d9 100644
--- a/tests/UtilsTest.cpp
+++ b/tests/UtilsTest.cpp
@@ -286,6 +286,63 @@
}
REPORTER_ASSERT(reporter, e.size() == 2);
}
+
+ {
+ struct I {
+ I() = default;
+ I(const I&) = default;
+ I(int v) : i{v} { }
+ ~I() {}
+ int i;
+ };
+
+ I is[10];
+ SkSpan s{is};
+ for (auto [i, v] : SkMakeEnumerate(s)) {
+ new (&v) I(i);
+ }
+
+ for (size_t i = 0; i < s.size(); i++) {
+ REPORTER_ASSERT(reporter, s[i].i == (int)i);
+ REPORTER_ASSERT(reporter, is[i].i == (int)i);
+ }
+ }
+
+ {
+ std::unique_ptr<int> is[10];
+ std::unique_ptr<int> os[10];
+ SkSpan s{is};
+ for (auto [i, v] : SkMakeEnumerate(s)) {
+ v = std::make_unique<int>(i);
+ }
+
+ for (auto [i, v] : SkMakeEnumerate(SkSpan(os))) {
+ v = std::move(s[i]);
+ }
+
+ for (size_t i = 0; i < s.size(); i++) {
+ REPORTER_ASSERT(reporter, *os[i] == (int)i);
+ REPORTER_ASSERT(reporter, is[i] == nullptr);
+ }
+ }
+
+ {
+ std::unique_ptr<int> is[10];
+ std::unique_ptr<int> os[10];
+ SkSpan s{is};
+ for (auto [i, v] : SkMakeEnumerate(s)) {
+ v = std::make_unique<int>(i);
+ }
+
+ for (auto [i, ov, iv] : SkMakeEnumerate(SkMakeZip(os, is))) {
+ ov = std::move(iv);
+ }
+
+ for (size_t i = 0; i < s.size(); i++) {
+ REPORTER_ASSERT(reporter, *os[i] == (int)i);
+ REPORTER_ASSERT(reporter, is[i] == nullptr);
+ }
+ }
}
DEF_TEST(SkZip, reporter) {
