[PATCH] D33419: BitVector: use iterator_facade

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg created this revision.

As pointed out by @dblaikie, the BitVector iterator introduced in r303227 <https://reviews.llvm.org/rL303227> could use `iterator_facade_base` to provide some default behaviors for iterators, like `It++` and `!=`.

For that, we need to update the constraints on `iterator_facade_base` to support iterators with an `operator->` returning a value, and not a `const T&`.


https://reviews.llvm.org/D33419

Files:
  include/llvm/ADT/BitVector.h
  include/llvm/ADT/iterator.h


Index: include/llvm/ADT/iterator.h
===================================================================
--- include/llvm/ADT/iterator.h
+++ include/llvm/ADT/iterator.h
@@ -42,8 +42,11 @@
 ///   (All of the following methods)
 ///   - DerivedT &operator=(const DerivedT &R);
 ///   - bool operator==(const DerivedT &R) const;
-///   - const T &operator*() const;
-///   - T &operator*();
+///   - either
+///            - const T &operator*() const;
+///            - T &operator*();
+///   - or
+///            - T operator*() const;
 ///   - DerivedT &operator++();
 ///
 /// Bidirectional Iterators:
@@ -164,9 +167,18 @@
         "Relational operators are only defined for random access iterators.");
     return !static_cast<const DerivedT *>(this)->operator<(RHS);
   }
-
-  PointerT operator->() { return &static_cast<DerivedT *>(this)->operator*(); }
+  PointerT operator->() {
+    static_assert(
+        std::is_reference<decltype(
+            static_cast<DerivedT *>(this)->operator*())>::value,
+        "operator-> is defined only on iterators returning a const T&");
+    return &static_cast<DerivedT *>(this)->operator*();
+  }
   PointerT operator->() const {
+    static_assert(
+        std::is_reference<decltype(
+            static_cast<const DerivedT *>(this)->operator*())>::value,
+        "operator-> is defined only on iterators returning a const T&");
     return &static_cast<const DerivedT *>(this)->operator*();
   }
   ReferenceProxy operator[](DifferenceTypeT n) {
Index: include/llvm/ADT/BitVector.h
===================================================================
--- include/llvm/ADT/BitVector.h
+++ include/llvm/ADT/BitVector.h
@@ -15,6 +15,7 @@
 #define LLVM_ADT_BITVECTOR_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/MathExtras.h"
 #include <algorithm>
@@ -29,7 +30,10 @@
 
 /// ForwardIterator for the bits that are set.
 /// Iterators get invalidated when resize / reserve is called.
-template <typename BitVectorT> class const_set_bits_iterator_impl {
+template <typename BitVectorT>
+class const_set_bits_iterator_impl
+    : public iterator_facade_base<const_set_bits_iterator_impl<BitVectorT>,
+                                  std::forward_iterator_tag, unsigned> {
   const BitVectorT &Parent;
   int Current = 0;
 
@@ -44,12 +48,8 @@
   explicit const_set_bits_iterator_impl(const BitVectorT &Parent)
       : const_set_bits_iterator_impl(Parent, Parent.find_first()) {}
   const_set_bits_iterator_impl(const const_set_bits_iterator_impl &) = default;
-
-  const_set_bits_iterator_impl operator++(int) {
-    auto Prev = *this;
-    advance();
-    return Prev;
-  }
+  const_set_bits_iterator_impl &
+  operator=(const const_set_bits_iterator_impl &other) = default;
 
   const_set_bits_iterator_impl &operator++() {
     advance();
@@ -63,12 +63,6 @@
            "Comparing iterators from different BitVectors");
     return Current == Other.Current;
   }
-
-  bool operator!=(const const_set_bits_iterator_impl &Other) const {
-    assert(&Parent == &Other.Parent &&
-           "Comparing iterators from different BitVectors");
-    return Current != Other.Current;
-  }
 };
 
 class BitVector {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33419.99800.patch
Type: text/x-patch
Size: 3235 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170522/f99c5a39/attachment.bin>