WTF: Make hash collection iterators more like normal C++ bidirectional iterators.

This makes them more usable with STL and base/ranges/ algorithms.

Change-Id: Iea3c30eabedf683685ec4fd24c3099099f0fe03f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2618604
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842569}

third_party/blink/renderer/platform/wtf/hash_iterators.h

@@ -54,10 +54,10 @@ struct HashTableConstIteratorAdapter<HashTableType,
       ValuesIterator;
 
   using iterator_category = std::bidirectional_iterator_tag;
-  using value_type = HashTableType;
+  using value_type = ValueType;
   using difference_type = ptrdiff_t;
-  using pointer = value_type*;
-  using reference = value_type&;
+  using pointer = const ValueType*;
+  using reference = const ValueType&;
 
   HashTableConstIteratorAdapter() = default;
   HashTableConstIteratorAdapter(
@@ -72,13 +72,21 @@ struct HashTableConstIteratorAdapter<HashTableType,
     ++impl_;
     return *this;
   }
-  // postfix ++ intentionally omitted
+  HashTableConstIteratorAdapter operator++(int) {
+    HashTableConstIteratorAdapter copy(*this);
+    ++*this;
+    return copy;
+  }
 
   HashTableConstIteratorAdapter& operator--() {
     --impl_;
     return *this;
   }
-  // postfix -- intentionally omitted
+  HashTableConstIteratorAdapter operator--(int) {
+    HashTableConstIteratorAdapter copy(*this);
+    --*this;
+    return copy;
+  }
 
   KeysIterator Keys() { return KeysIterator(*this); }
   ValuesIterator Values() { return ValuesIterator(*this); }
@@ -101,10 +109,10 @@ struct HashTableIteratorAdapter<HashTableType,
       ValuesIterator;
 
   using iterator_category = std::bidirectional_iterator_tag;
-  using value_type = HashTableType;
+  using value_type = ValueType;
   using difference_type = ptrdiff_t;
-  using pointer = value_type*;
-  using reference = value_type&;
+  using pointer = ValueType*;
+  using reference = ValueType&;
 
   HashTableIteratorAdapter() = default;
   HashTableIteratorAdapter(const typename HashTableType::iterator& impl)
@@ -118,13 +126,21 @@ struct HashTableIteratorAdapter<HashTableType,
     ++impl_;
     return *this;
   }
-  // postfix ++ intentionally omitted
+  HashTableIteratorAdapter operator++(int) {
+    HashTableIteratorAdapter copy(*this);
+    ++*this;
+    return copy;
+  }
 
   HashTableIteratorAdapter& operator--() {
     --impl_;
     return *this;
   }
-  // postfix -- intentionally omitted
+  HashTableIteratorAdapter operator--(int) {
+    HashTableIteratorAdapter copy(*this);
+    --*this;
+    return copy;
+  }
 
   operator HashTableConstIteratorAdapter<HashTableType, ValueType>() {
     typename HashTableType::const_iterator i = impl_;
@@ -147,6 +163,12 @@ struct HashTableConstKeysIterator {
       ConstIterator;
 
  public:
+  using iterator_category = typename ConstIterator::iterator_category;
+  using value_type = KeyType;
+  using difference_type = typename ConstIterator::difference_type;
+  using pointer = const KeyType*;
+  using reference = const KeyType&;
+
   HashTableConstKeysIterator(const ConstIterator& impl) : impl_(impl) {}
 
   const KeyType* Get() const { return &(impl_.Get()->key); }
@@ -157,13 +179,21 @@ struct HashTableConstKeysIterator {
     ++impl_;
     return *this;
   }
-  // postfix ++ intentionally omitted
+  HashTableConstKeysIterator operator++(int) {
+    HashTableConstKeysIterator copy(*this);
+    ++*this;
+    return copy;
+  }
 
   HashTableConstKeysIterator& operator--() {
     --impl_;
     return *this;
   }
-  // postfix -- intentionally omitted
+  HashTableConstKeysIterator operator--(int) {
+    HashTableConstKeysIterator copy(*this);
+    --*this;
+    return copy;
+  }
 
   ConstIterator impl_;
 };
@@ -178,6 +208,12 @@ struct HashTableConstValuesIterator {
       ConstIterator;
 
  public:
+  using iterator_category = typename ConstIterator::iterator_category;
+  using value_type = MappedType;
+  using difference_type = typename ConstIterator::difference_type;
+  using pointer = const MappedType*;
+  using reference = const MappedType&;
+
   HashTableConstValuesIterator(const ConstIterator& impl) : impl_(impl) {}
 
   const MappedType* Get() const { return &(impl_.Get()->value); }
@@ -188,13 +224,21 @@ struct HashTableConstValuesIterator {
     ++impl_;
     return *this;
   }
-  // postfix ++ intentionally omitted
+  HashTableConstValuesIterator operator++(int) {
+    HashTableConstValuesIterator copy(*this);
+    ++*this;
+    return copy;
+  }
 
   HashTableConstValuesIterator& operator--() {
     --impl_;
     return *this;
   }
-  // postfix -- intentionally omitted
+  HashTableConstValuesIterator operator--(int) {
+    HashTableConstValuesIterator copy(*this);
+    --*this;
+    return copy;
+  }
 
   ConstIterator impl_;
 };
@@ -212,6 +256,12 @@ struct HashTableKeysIterator {
       ConstIterator;
 
  public:
+  using iterator_category = typename Iterator::iterator_category;
+  using value_type = KeyType;
+  using difference_type = typename Iterator::difference_type;
+  using pointer = KeyType*;
+  using reference = KeyType&;
+
   HashTableKeysIterator(const Iterator& impl) : impl_(impl) {}
 
   KeyType* Get() const { return &(impl_.Get()->key); }
@@ -222,13 +272,21 @@ struct HashTableKeysIterator {
     ++impl_;
     return *this;
   }
-  // postfix ++ intentionally omitted
+  HashTableKeysIterator operator++(int) {
+    HashTableKeysIterator copy(*this);
+    ++*this;
+    return copy;
+  }
 
   HashTableKeysIterator& operator--() {
     --impl_;
     return *this;
   }
-  // postfix -- intentionally omitted
+  HashTableKeysIterator operator--(int) {
+    HashTableKeysIterator copy(*this);
+    --*this;
+    return copy;
+  }
 
   operator HashTableConstKeysIterator<HashTableType, KeyType, MappedType>() {
     ConstIterator i = impl_;
@@ -251,6 +309,12 @@ struct HashTableValuesIterator {
       ConstIterator;
 
  public:
+  using iterator_category = typename Iterator::iterator_category;
+  using value_type = MappedType;
+  using difference_type = typename Iterator::difference_type;
+  using pointer = MappedType*;
+  using reference = MappedType&;
+
   HashTableValuesIterator(const Iterator& impl) : impl_(impl) {}
 
   MappedType* Get() const { return &(impl_.Get()->value); }
@@ -261,13 +325,21 @@ struct HashTableValuesIterator {
     ++impl_;
     return *this;
   }
-  // postfix ++ intentionally omitted
+  HashTableValuesIterator operator++(int) {
+    HashTableValuesIterator copy(*this);
+    ++*this;
+    return copy;
+  }
 
   HashTableValuesIterator& operator--() {
     --impl_;
     return *this;
   }
-  // postfix -- intentionally omitted
+  HashTableValuesIterator operator--(int) {
+    HashTableValuesIterator copy(*this);
+    --*this;
+    return copy;
+  }
 
   operator HashTableConstValuesIterator<HashTableType, KeyType, MappedType>() {
     ConstIterator i = impl_;

third_party/blink/renderer/platform/wtf/hash_map_test.cc

@@ -25,6 +25,7 @@
 
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
 
+#include <iterator>
 #include <memory>
 
 #include "base/memory/ptr_util.h"
@@ -640,6 +641,77 @@ TEST(HashMapTest, IsValidKey) {
 static_assert(!IsTraceable<HashMap<int, int>>::value,
               "HashMap<int, int> must not be traceable.");
 
+static_assert(
+    std::is_convertible<
+        std::iterator_traits<HashMap<int, int>::iterator>::iterator_category,
+        std::bidirectional_iterator_tag>(),
+    "hash map iterators should be bidirectional");
+static_assert(
+    std::is_same<std::iterator_traits<HashMap<int, int>::iterator>::value_type,
+                 KeyValuePair<int, int>>(),
+    "hash map iterators should be over key-value pairs");
+
+static_assert(std::is_convertible<
+                  std::iterator_traits<
+                      HashMap<int, int>::const_iterator>::iterator_category,
+                  std::bidirectional_iterator_tag>(),
+              "hash map const iterators should be bidirectional");
+static_assert(
+    std::is_same<
+        std::iterator_traits<HashMap<int, int>::const_iterator>::value_type,
+        KeyValuePair<int, int>>(),
+    "hash map const iterators should be over key-value pairs");
+
+static_assert(
+    std::is_convertible<
+        std::iterator_traits<
+            HashMap<int, unsigned>::iterator::KeysIterator>::iterator_category,
+        std::bidirectional_iterator_tag>(),
+    "hash map key iterators should be bidirectional");
+static_assert(
+    std::is_same<
+        std::iterator_traits<
+            HashMap<int, unsigned>::iterator::KeysIterator>::value_type,
+        int>(),
+    "hash map key iterators should be over keys");
+
+static_assert(std::is_convertible<
+                  std::iterator_traits<HashMap<int, unsigned>::const_iterator::
+                                           KeysIterator>::iterator_category,
+                  std::bidirectional_iterator_tag>(),
+              "hash map const key iterators should be bidirectional");
+static_assert(
+    std::is_same<
+        std::iterator_traits<
+            HashMap<int, unsigned>::const_iterator::KeysIterator>::value_type,
+        int>(),
+    "hash map const key iterators should be over keys");
+
+static_assert(
+    std::is_convertible<
+        std::iterator_traits<HashMap<int, unsigned>::iterator::ValuesIterator>::
+            iterator_category,
+        std::bidirectional_iterator_tag>(),
+    "hash map value iterators should be bidirectional");
+static_assert(
+    std::is_same<
+        std::iterator_traits<
+            HashMap<int, unsigned>::iterator::ValuesIterator>::value_type,
+        unsigned>(),
+    "hash map value iterators should be over values");
+
+static_assert(std::is_convertible<
+                  std::iterator_traits<HashMap<int, unsigned>::const_iterator::
+                                           ValuesIterator>::iterator_category,
+                  std::bidirectional_iterator_tag>(),
+              "hash map const value iterators should be bidirectional");
+static_assert(
+    std::is_same<
+        std::iterator_traits<
+            HashMap<int, unsigned>::const_iterator::ValuesIterator>::value_type,
+        unsigned>(),
+    "hash map const value iterators should be over values");
+
 }  // anonymous namespace
 
 }  // namespace WTF