[PATCH] [ADT] Add a leader_iterator to EquivalenceClasses

[Adam Nemet]

Hi dblaikie, dexonsmith,

Recently we had a bug that was caught during the review where the
isLeader check was mistakenly omitted from the usual pattern of
traversing an EC:

  for (EquivalenceClasses<int>::iterator I = EC.begin(), E = EC.end();
       I != E; ++I) {
    if (!I->isLeader()) continue // <----------- this line
    for (EquivalenceClasses<int>::member_iterator MI = EC.member_begin(I);
         MI != EC.member_end(); ++MI)
      ...

I don't think that anything bad really happens if the check is omitted
other than some overhead but that made me think if we could have an
iterator in the outer loop that only traverses the leaders/classes:

  for (EquivalenceClasses<int>::leader_iterator I = EC.leader_begin(),
         E = EC.leader_end(); I != E; ++I) {
    for (EquivalenceClasses<int>::member_iterator MI = EC.member_begin(I);
         MI != EC.member_end(); ++MI)
      ...

EC internally uses a set, so leader_iterator is built on top of the
set::iterator.  In addition, it also needs the end iterator to avoid
over-running the set when looking for the leader.

Let me know if this seems like a good idea and then I will update the
patch with unit tests.

http://reviews.llvm.org/D10891

Files:
  include/llvm/ADT/EquivalenceClasses.h

Index: include/llvm/ADT/EquivalenceClasses.h
===================================================================
--- include/llvm/ADT/EquivalenceClasses.h
+++ include/llvm/ADT/EquivalenceClasses.h
@@ -15,6 +15,7 @@
 #ifndef LLVM_ADT_EQUIVALENCECLASSES_H
 #define LLVM_ADT_EQUIVALENCECLASSES_H
 
+#include "llvm/ADT/iterator.h"
 #include "llvm/Support/DataTypes.h"
 #include <cassert>
 #include <cstddef>
@@ -150,10 +151,23 @@
     // Only leaders provide anything to iterate over.
     return member_iterator(I->isLeader() ? &*I : nullptr);
   }
+  class leader_iterator;
+  member_iterator member_begin(leader_iterator I) const {
+    return member_iterator(&*I);
+  }
   member_iterator member_end() const {
     return member_iterator(nullptr);
   }
 
+  /// Iterate over the leaders (i.e. the equivalence classes).
+  leader_iterator leader_begin() const {
+    // Only leaders provide anything to iterate over.
+    return leader_iterator(TheMapping.begin(), TheMapping.end());
+  }
+  leader_iterator leader_end() const {
+    return leader_iterator(TheMapping.end(), TheMapping.end());
+  }
+
   /// findValue - Return an iterator to the specified value.  If it does not
   /// exist, end() is returned.
   iterator findValue(const ElemTy &V) const {
@@ -276,6 +290,51 @@
       return Node != RHS.Node;
     }
   };
+
+  class leader_iterator
+      : public iterator_facade_base<leader_iterator, std::forward_iterator_tag,
+                                    const ECValue> {
+    typedef iterator_facade_base<leader_iterator, std::forward_iterator_tag,
+                                 const ECValue> super;
+
+    iterator Iter;
+    iterator End;
+
+    void findLeader() {
+      for (; Iter != End; ++Iter)
+        if (Iter->isLeader())
+          break;
+    }
+
+  public:
+    typedef size_t size_type;
+    typedef typename super::pointer pointer;
+    typedef typename super::reference reference;
+
+    explicit leader_iterator() {}
+    explicit leader_iterator(iterator First, iterator Last)
+        : Iter(First), End(Last) {
+      // Ensure that Iter is pointing at the first leader.
+      findLeader();
+    }
+
+    leader_iterator &operator++() {
+      assert(Iter != End && "++'d off the end of the list!");
+      ++Iter;
+      // If this is not a leader, keep going.
+      findLeader();
+      return *this;
+    }
+
+    reference operator*() const {
+      assert(Iter != End && "Dereferencing end()!");
+      return *Iter;
+    }
+
+    bool operator==(const leader_iterator &RHS) const {
+      return Iter == RHS.Iter;
+    }
+  };
 };
 
 } // End llvm namespace

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D10891.28935.patch
Type: text/x-patch
Size: 2608 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150702/a4b0e98c/attachment.bin>