<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Mar 15, 2015 at 6:26 AM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@googlemail.com" target="_blank">benny.kra@googlemail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: d0k<br>
Date: Sun Mar 15 08:26:03 2015<br>
New Revision: 232319<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=232319&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=232319&view=rev</a><br>
Log:<br>
Factor the iterators of ImmutableSet/ImmutableMap into a common base class<br>
<br>
This simplifies code quite a bit and brings the iterators closer to<br>
C++'s iterator concept. No functional change intended.<br></blockquote><div><br>Nice - these half-pointer/half-value iterators always annoy me :/<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
    llvm/trunk/include/llvm/ADT/ImmutableMap.h<br>
    llvm/trunk/include/llvm/ADT/ImmutableSet.h<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/ImmutableMap.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ImmutableMap.h?rev=232319&r1=232318&r2=232319&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ImmutableMap.h?rev=232319&r1=232318&r2=232319&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/ImmutableMap.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/ImmutableMap.h Sun Mar 15 08:26:03 2015<br>
@@ -203,33 +203,14 @@ public:<br>
   // Iterators.<br>
   //===--------------------------------------------------===//<br>
<br>
-  class iterator {<br>
-    typename TreeTy::iterator itr;<br>
-<br>
-    iterator() {}<br>
-    iterator(TreeTy* t) : itr(t) {}<br>
+  class iterator : public ImutAVLValueIterator<ImmutableMap> {<br>
+    iterator() = default;<br>
+    explicit iterator(TreeTy *Tree) : iterator::ImutAVLValueIterator(Tree) {}<br>
     friend class ImmutableMap;<br>
<br>
   public:<br>
-    typedef ptrdiff_t difference_type;<br>
-    typedef typename ImmutableMap<KeyT,ValT,ValInfo>::value_type value_type;<br>
-    typedef typename ImmutableMap<KeyT,ValT,ValInfo>::value_type_ref reference;<br>
-    typedef typename iterator::value_type *pointer;<br>
-    typedef std::bidirectional_iterator_tag iterator_category;<br>
-<br>
-    typename iterator::reference operator*() const { return itr->getValue(); }<br>
-    typename iterator::pointer   operator->() const { return &itr->getValue(); }<br>
-<br>
-    key_type_ref getKey() const { return itr->getValue().first; }<br>
-    data_type_ref getData() const { return itr->getValue().second; }<br>
-<br>
-    iterator& operator++() { ++itr; return *this; }<br>
-    iterator  operator++(int) { iterator tmp(*this); ++itr; return tmp; }<br>
-    iterator& operator--() { --itr; return *this; }<br>
-    iterator  operator--(int) { iterator tmp(*this); --itr; return tmp; }<br>
-<br>
-    bool operator==(const iterator& RHS) const { return RHS.itr == itr; }<br>
-    bool operator!=(const iterator& RHS) const { return RHS.itr != itr; }<br>
+    key_type_ref getKey() const { return (*this)->first; }<br>
+    data_type_ref getData() const { return (*this)->second; }<br>
   };<br>
<br>
   iterator begin() const { return iterator(Root); }<br>
@@ -376,30 +357,17 @@ public:<br>
   //===--------------------------------------------------===//<br>
   // Iterators.<br>
   //===--------------------------------------------------===//<br>
-<br>
-  class iterator {<br>
-    typename TreeTy::iterator itr;<br>
-<br>
-    iterator() {}<br>
-    iterator(TreeTy* t) : itr(t) {}<br>
+<br>
+  class iterator : public ImutAVLValueIterator<ImmutableMapRef> {<br>
+    iterator() = default;<br>
+    explicit iterator(TreeTy *Tree) : iterator::ImutAVLValueIterator(Tree) {}<br>
     friend class ImmutableMapRef;<br>
-<br>
+<br>
   public:<br>
-    value_type_ref operator*() const { return itr->getValue(); }<br>
-    value_type*    operator->() const { return &itr->getValue(); }<br>
-<br>
-    key_type_ref getKey() const { return itr->getValue().first; }<br>
-    data_type_ref getData() const { return itr->getValue().second; }<br>
-<br>
-<br>
-    iterator& operator++() { ++itr; return *this; }<br>
-    iterator  operator++(int) { iterator tmp(*this); ++itr; return tmp; }<br>
-    iterator& operator--() { --itr; return *this; }<br>
-    iterator  operator--(int) { iterator tmp(*this); --itr; return tmp; }<br>
-    bool operator==(const iterator& RHS) const { return RHS.itr == itr; }<br>
-    bool operator!=(const iterator& RHS) const { return RHS.itr != itr; }<br>
+    key_type_ref getKey() const { return (*this)->first; }<br>
+    data_type_ref getData() const { return (*this)->second; }<br>
   };<br>
-<br>
+<br>
   iterator begin() const { return iterator(Root); }<br>
   iterator end() const { return iterator(); }<br>
<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/ImmutableSet.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ImmutableSet.h?rev=232319&r1=232318&r2=232319&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ImmutableSet.h?rev=232319&r1=232318&r2=232319&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/ImmutableSet.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/ImmutableSet.h Sun Mar 15 08:26:03 2015<br>
@@ -142,13 +142,13 @@ public:<br>
     iterator RItr = RHS.begin(), REnd = RHS.end();<br>
<br>
     while (LItr != LEnd && RItr != REnd) {<br>
-      if (*LItr == *RItr) {<br>
+      if (&*LItr == &*RItr) {<br>
         LItr.skipSubTree();<br>
         RItr.skipSubTree();<br>
         continue;<br>
       }<br>
<br>
-      if (!LItr->isElementEqual(*RItr))<br>
+      if (!LItr->isElementEqual(&*RItr))<br>
         return false;<br>
<br>
       ++LItr;<br>
@@ -442,7 +442,7 @@ protected:<br>
                                      typename TreeTy::iterator& TE) {<br>
     typename TreeTy::iterator I = T->begin(), E = T->end();<br>
     for ( ; I!=E ; ++I, ++TI) {<br>
-      if (TI == TE || !I->isElementEqual(*TI))<br>
+      if (TI == TE || !I->isElementEqual(&*TI))<br>
         return false;<br>
     }<br>
     return true;<br>
@@ -645,7 +645,9 @@ public:<br>
 //===----------------------------------------------------------------------===//<br>
<br>
 template <typename ImutInfo><br>
-class ImutAVLTreeGenericIterator {<br>
+class ImutAVLTreeGenericIterator<br>
+    : public std::iterator<std::bidirectional_iterator_tag,<br>
+                           ImutAVLTree<ImutInfo>> {<br>
   SmallVector<uintptr_t,20> stack;<br>
 public:<br>
   enum VisitFlag { VisitedNone=0x0, VisitedLeft=0x1, VisitedRight=0x3,<br>
@@ -659,10 +661,11 @@ public:<br>
     if (Root) stack.push_back(reinterpret_cast<uintptr_t>(Root));<br>
   }<br>
<br>
-  TreeTy* operator*() const {<br>
+  TreeTy &operator*() const {<br>
     assert(!stack.empty());<br>
-    return reinterpret_cast<TreeTy*>(stack.back() & ~Flags);<br>
+    return *reinterpret_cast<TreeTy *>(stack.back() & ~Flags);<br>
   }<br>
+  TreeTy *operator->() const { return &*this; }<br>
<br>
   uintptr_t getVisitState() const {<br>
     assert(!stack.empty());<br>
@@ -750,7 +753,9 @@ public:<br>
 };<br>
<br>
 template <typename ImutInfo><br>
-class ImutAVLTreeInOrderIterator {<br>
+class ImutAVLTreeInOrderIterator<br>
+    : public std::iterator<std::bidirectional_iterator_tag,<br>
+                           ImutAVLTree<ImutInfo>> {<br>
   typedef ImutAVLTreeGenericIterator<ImutInfo> InternalIteratorTy;<br>
   InternalIteratorTy InternalItr;<br>
<br>
@@ -771,8 +776,8 @@ public:<br>
<br>
   bool operator!=(const SelfTy &x) const { return !(*this == x); }<br>
<br>
-  TreeTy *operator*() const { return *InternalItr; }<br>
-  TreeTy *operator->() const { return *InternalItr; }<br>
+  TreeTy &operator*() const { return *InternalItr; }<br>
+  TreeTy *operator->() const { return &*InternalItr; }<br>
<br>
   SelfTy &operator++() {<br>
     do ++InternalItr;<br>
@@ -799,6 +804,24 @@ public:<br>
   }<br>
 };<br>
<br>
+/// Generic iterator that wraps a T::TreeTy::iterator and exposes<br>
+/// iterator::getValue() on dereference.<br>
+template <typename T><br>
+struct ImutAVLValueIterator<br>
+    : iterator_adaptor_base<<br>
+          ImutAVLValueIterator<T>, typename T::TreeTy::iterator,<br>
+          typename std::iterator_traits<<br>
+              typename T::TreeTy::iterator>::iterator_category,<br>
+          const typename T::value_type> {<br>
+  ImutAVLValueIterator() = default;<br>
+  explicit ImutAVLValueIterator(typename T::TreeTy *Tree)<br>
+      : ImutAVLValueIterator::iterator_adaptor_base(Tree) {}<br>
+<br>
+  typename ImutAVLValueIterator::reference operator*() const {<br>
+    return this->I->getValue();<br>
+  }<br>
+};<br>
+<br>
 //===----------------------------------------------------------------------===//<br>
 // Trait classes for Profile information.<br>
 //===----------------------------------------------------------------------===//<br>
@@ -1052,31 +1075,7 @@ public:<br>
   // Iterators.<br>
   //===--------------------------------------------------===//<br>
<br>
-  class iterator {<br>
-    typename TreeTy::iterator itr;<br>
-<br>
-    iterator() {}<br>
-    iterator(TreeTy* t) : itr(t) {}<br>
-    friend class ImmutableSet<ValT,ValInfo>;<br>
-<br>
-  public:<br>
-    typedef ptrdiff_t difference_type;<br>
-    typedef typename ImmutableSet<ValT,ValInfo>::value_type value_type;<br>
-    typedef typename ImmutableSet<ValT,ValInfo>::value_type_ref reference;<br>
-    typedef typename iterator::value_type *pointer;<br>
-    typedef std::bidirectional_iterator_tag iterator_category;<br>
-<br>
-    typename iterator::reference operator*() const { return itr->getValue(); }<br>
-    typename iterator::pointer operator->() const { return &**this; }<br>
-<br>
-    iterator& operator++() { ++itr; return *this; }<br>
-    iterator  operator++(int) { iterator tmp(*this); ++itr; return tmp; }<br>
-    iterator& operator--() { --itr; return *this; }<br>
-    iterator  operator--(int) { iterator tmp(*this); --itr; return tmp; }<br>
-<br>
-    bool operator==(const iterator& RHS) const { return RHS.itr == itr; }<br>
-    bool operator!=(const iterator& RHS) const { return RHS.itr != itr; }<br>
-  };<br>
+  typedef ImutAVLValueIterator<ImmutableSet> iterator;<br>
<br>
   iterator begin() const { return iterator(Root); }<br>
   iterator end() const { return iterator(); }<br>
@@ -1186,35 +1185,7 @@ public:<br>
   // Iterators.<br>
   //===--------------------------------------------------===//<br>
<br>
-  class iterator {<br>
-    typename TreeTy::iterator itr;<br>
-    iterator(TreeTy* t) : itr(t) {}<br>
-    friend class ImmutableSetRef<ValT,ValInfo>;<br>
-  public:<br>
-    iterator() {}<br>
-    value_type_ref operator*() const { return itr->getValue(); }<br>
-    iterator &operator++() {<br>
-      ++itr;<br>
-      return *this;<br>
-    }<br>
-    iterator operator++(int) {<br>
-      iterator tmp(*this);<br>
-      ++itr;<br>
-      return tmp;<br>
-    }<br>
-    iterator &operator--() {<br>
-      --itr;<br>
-      return *this;<br>
-    }<br>
-    iterator operator--(int) {<br>
-      iterator tmp(*this);<br>
-      --itr;<br>
-      return tmp;<br>
-    }<br>
-    bool operator==(const iterator &RHS) const { return RHS.itr == itr; }<br>
-    bool operator!=(const iterator &RHS) const { return RHS.itr != itr; }<br>
-    value_type *operator->() const { return &**this; }<br>
-  };<br>
+  typedef ImutAVLValueIterator<ImmutableSetRef> iterator;<br>
<br>
   iterator begin() const { return iterator(Root); }<br>
   iterator end() const { return iterator(); }<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>