Add support for range-based for loops to SkTHashSet/Map.

This allows loops over SkTHashes to break in the middle, and also
removes the need to use lambda captures to bring variables inside the
loop's scope.

Change-Id: Ief55d776b2c57a44b24cfe1c94493a5d514791c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346496
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>

include/private/SkTHash.h

@@ -60,6 +60,10 @@ public:
     // How many entries are in the table?
     int count() const { return fCount; }
 
+    // How many slots does the table contain? (Note that unlike an array, hash tables can grow
+    // before reaching 100% capacity.)
+    int capacity() const { return fCapacity; }
+
     // Approximately how many bytes of memory do we use beyond sizeof(*this)?
     size_t approxBytesUsed() const { return fCapacity * sizeof(Slot); }
 
@@ -149,7 +153,84 @@ public:
         }
     }
 
+    // A basic iterator-like class which disallows mutation; sufficient for range-based for loops.
+    // Intended for use by SkTHashMap and SkTHashSet via begin() and end().
+    // Adding or removing elements may invalidate all iterators.
+    class Iter {
+    public:
+        using TTable = SkTHashTable<T, K, Traits>;
+
+        Iter(const TTable* table, int slot) : fTable(table), fSlot(slot) {}
+
+        static Iter MakeBegin(const TTable* table) {
+            return Iter{table, table->firstPopulatedSlot()};
+        }
+
+        static Iter MakeEnd(const TTable* table) {
+            return Iter{table, table->capacity()};
+        }
+
+        const T& operator*() const {
+            return *fTable->slot(fSlot);
+        }
+
+        const T* operator->() const {
+            return fTable->slot(fSlot);
+        }
+
+        bool operator==(const Iter& that) const {
+            // Iterators from different tables shouldn't be compared against each other.
+            SkASSERT(fTable == that.fTable);
+            return fSlot == that.fSlot;
+        }
+
+        bool operator!=(const Iter& that) const {
+            return !(*this == that);
+        }
+
+        Iter& operator++() {
+            fSlot = fTable->nextPopulatedSlot(fSlot);
+            return *this;
+        }
+
+        Iter operator++(int) {
+            Iter old = *this;
+            this->operator++();
+            return old;
+        }
+
+    protected:
+        const TTable* fTable;
+        int fSlot;
+    };
+
 private:
+    // Finds the first non-empty slot for an iterator.
+    int firstPopulatedSlot() const {
+        for (int i = 0; i < fCapacity; i++) {
+            if (!fSlots[i].empty()) {
+                return i;
+            }
+        }
+        return fCapacity;
+    }
+
+    // Increments an iterator's slot.
+    int nextPopulatedSlot(int currentSlot) const {
+        for (int i = currentSlot + 1; i < fCapacity; i++) {
+            if (!fSlots[i].empty()) {
+                return i;
+            }
+        }
+        return fCapacity;
+    }
+
+    // Reads from an iterator's slot.
+    const T* slot(int i) const {
+        SkASSERT(!fSlots[i].empty());
+        return &fSlots[i].val;
+    }
+
     T* uncheckedSet(T&& val) {
         const K& key = Traits::GetKey(val);
         uint32_t hash = Hash(key);
@@ -309,7 +390,7 @@ public:
         fTable.foreach([&fn](const Pair& p){ fn(p.key, p.val); });
     }
 
-private:
+    // Dereferencing an iterator gives back a key-value pair, suitable for structured binding.
     struct Pair {
         K key;
         V val;
@@ -317,6 +398,17 @@ private:
         static auto Hash(const K& key) { return HashK()(key); }
     };
 
+    using Iter = typename SkTHashTable<Pair, K>::Iter;
+
+    Iter begin() const {
+        return Iter::MakeBegin(&fTable);
+    }
+
+    Iter end() const {
+        return Iter::MakeEnd(&fTable);
+    }
+
+private:
     SkTHashTable<Pair, K> fTable;
 };
 
@@ -363,6 +455,19 @@ private:
         static const T& GetKey(const T& item) { return item; }
         static auto Hash(const T& item) { return HashT()(item); }
     };
+
+public:
+    using Iter = typename SkTHashTable<T, T, Traits>::Iter;
+
+    Iter begin() const {
+        return Iter::MakeBegin(&fTable);
+    }
+
+    Iter end() const {
+        return Iter::MakeEnd(&fTable);
+    }
+
+private:
     SkTHashTable<T, T, Traits> fTable;
 };
 

src/gpu/GrMemoryPool.cpp

@@ -44,15 +44,16 @@ void GrMemoryPool::reportLeaks() const {
 #ifdef SK_DEBUG
     int i = 0;
     int n = fAllocatedIDs.count();
-    fAllocatedIDs.foreach([&i, n] (int id) {
+    for (int id : fAllocatedIDs) {
         if (++i == 1) {
             SkDebugf("Leaked %d IDs (in no particular order): %d%s", n, id, (n == i) ? "\n" : "");
         } else if (i < 11) {
             SkDebugf(", %d%s", id, (n == i ? "\n" : ""));
         } else if (i == 11) {
             SkDebugf(", ...\n");
+            break;
         }
-    });
+    }
 #endif
 }
 

src/sksl/SkSLCFGGenerator.cpp

@@ -77,12 +77,12 @@ void CFG::dump() const {
 void BasicBlock::dump() const {
     printf("Before: [");
     const char* separator = "";
-    fBefore.foreach([&](const Variable* var, std::unique_ptr<Expression>* expr) {
+    for (const auto& [var, expr] : fBefore) {
         printf("%s%s = %s", separator,
                             var->description().c_str(),
                             expr ? (*expr)->description().c_str() : "<undefined>");
         separator = ", ";
-    });
+    }
     printf("]\nIs Reachable: [%s]\n", fIsReachable ? "yes" : "no");
     for (size_t j = 0; j < fNodes.size(); j++) {
         const BasicBlock::Node& n = fNodes[j];

tests/HashTest.cpp

@@ -45,6 +45,32 @@ DEF_TEST(HashMap, r) {
         map.set(i, 2.0*i);
     }
 
+    // Test walking the map with iterators, using preincrement (++iter).
+    for (SkTHashMap<int, double>::Iter iter = map.begin(); iter != map.end(); ++iter) {
+        REPORTER_ASSERT(r, iter->key * 2 == (*iter).val);
+    }
+
+    // Test walking the map with range-based for.
+    for (auto& entry : map) {
+        REPORTER_ASSERT(r, entry.key * 2 == entry.val);
+    }
+
+    // Ensure that iteration works equally well on a const map, using postincrement (iter++).
+    const auto& cmap = map;
+    for (SkTHashMap<int, double>::Iter iter = cmap.begin(); iter != cmap.end(); iter++) {
+        REPORTER_ASSERT(r, iter->key * 2 == (*iter).val);
+    }
+
+    // Ensure that range-based for works equally well on a const map.
+    for (const auto& entry : cmap) {
+        REPORTER_ASSERT(r, entry.key * 2 == entry.val);
+    }
+
+    // Ensure that structured bindings work.
+    for (const auto& [number, timesTwo] : cmap) {
+        REPORTER_ASSERT(r, number * 2 == timesTwo);
+    }
+
     SkTHashMap<int, double> clone = map;
 
     for (int i = 0; i < N; i++) {
@@ -112,6 +138,32 @@ DEF_TEST(HashSet, r) {
     REPORTER_ASSERT(r, set.find(SkString("Hello")));
     REPORTER_ASSERT(r, *set.find(SkString("Hello")) == SkString("Hello"));
 
+    // Test walking the set with iterators, using preincrement (++iter).
+    for (SkTHashSet<SkString>::Iter iter = set.begin(); iter != set.end(); ++iter) {
+        REPORTER_ASSERT(r, iter->equals("Hello") || (*iter).equals("World"));
+    }
+
+    // Test walking the set with iterators, using postincrement (iter++).
+    for (SkTHashSet<SkString>::Iter iter = set.begin(); iter != set.end(); iter++) {
+        REPORTER_ASSERT(r, iter->equals("Hello") || (*iter).equals("World"));
+    }
+
+    // Test walking the set with range-based for.
+    for (auto& entry : set) {
+        REPORTER_ASSERT(r, entry.equals("Hello") || entry.equals("World"));
+    }
+
+    // Ensure that iteration works equally well on a const set.
+    const auto& cset = set;
+    for (SkTHashSet<SkString>::Iter iter = cset.begin(); iter != cset.end(); iter++) {
+        REPORTER_ASSERT(r, iter->equals("Hello") || (*iter).equals("World"));
+    }
+
+    // Ensure that range-based for works equally well on a const set.
+    for (auto& entry : cset) {
+        REPORTER_ASSERT(r, entry.equals("Hello") || entry.equals("World"));
+    }
+
     SkTHashSet<SkString> clone = set;
     REPORTER_ASSERT(r, clone.count() == 2);
     REPORTER_ASSERT(r, clone.contains(SkString("Hello")));