[llvm] r207743 - [LCG] Fix a bad bug in the new fancy iterator scheme I added to support

[Chandler Carruth]

Author: chandlerc
Date: Thu May  1 05:41:51 2014
New Revision: 207743

URL: http://llvm.org/viewvc/llvm-project?rev=207743&view=rev
Log:
[LCG] Fix a bad bug in the new fancy iterator scheme I added to support
removal. We can't just blindly increment (or decrement) the adapted
iterator when the value is null because doing so can walk past the end
(or beginning) and keep inspecting the value. The fix I've implemented
is to restrict this further to a forward iterator and add an end
iterator to the members (replacing a member that had become dead when
I switched to the adaptor base!) and using that to stop the iteration.

I'm not entirely pleased with this solution. I feel like forward
iteration is too restrictive. I wasn't even happy about bidirectional
iteration. It also makes the iterator objects larger and the iteration
loops more complex. However, I also don't really like the other
alternative that seems obvious: a sentinel node. I'm still hoping to
come up with a more elegant solution here, but this at least fixes the
MSan and Valgrind errors on this code.

Modified:
    llvm/trunk/include/llvm/Analysis/LazyCallGraph.h

Modified: llvm/trunk/include/llvm/Analysis/LazyCallGraph.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/LazyCallGraph.h?rev=207743&r1=207742&r2=207743&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/LazyCallGraph.h (original)
+++ llvm/trunk/include/llvm/Analysis/LazyCallGraph.h Thu May  1 05:41:51 2014
@@ -115,17 +115,18 @@ public:
   /// the graph.
   class iterator
       : public iterator_adaptor_base<iterator, NodeVectorImplT::iterator,
-                                     std::bidirectional_iterator_tag, Node> {
+                                     std::forward_iterator_tag, Node> {
     friend class LazyCallGraph;
     friend class LazyCallGraph::Node;
 
     LazyCallGraph *G;
-    NodeVectorImplT::iterator NI;
+    NodeVectorImplT::iterator E;
 
     // Build the iterator for a specific position in a node list.
-    iterator(LazyCallGraph &G, NodeVectorImplT::iterator NI)
-        : iterator_adaptor_base(NI), G(&G) {
-      while (I->isNull())
+    iterator(LazyCallGraph &G, NodeVectorImplT::iterator NI,
+             NodeVectorImplT::iterator E)
+        : iterator_adaptor_base(NI), G(&G), E(E) {
+      while (I != E && I->isNull())
         ++I;
     }
 
@@ -136,15 +137,7 @@ public:
     iterator &operator++() {
       do {
         ++I;
-      } while (I->isNull());
-      return *this;
-    }
-
-    using iterator_adaptor_base::operator--;
-    iterator &operator--() {
-      do {
-        --I;
-      } while (I->isNull());
+      } while (I != E && I->isNull());
       return *this;
     }
 
@@ -199,8 +192,10 @@ public:
       return F;
     };
 
-    iterator begin() const { return iterator(*G, Callees.begin()); }
-    iterator end() const { return iterator(*G, Callees.end()); }
+    iterator begin() const {
+      return iterator(*G, Callees.begin(), Callees.end());
+    }
+    iterator end() const { return iterator(*G, Callees.end(), Callees.end()); }
 
     /// Equality is defined as address equality.
     bool operator==(const Node &N) const { return this == &N; }
@@ -358,8 +353,10 @@ public:
   LazyCallGraph(LazyCallGraph &&G);
   LazyCallGraph &operator=(LazyCallGraph &&RHS);
 
-  iterator begin() { return iterator(*this, EntryNodes.begin()); }
-  iterator end() { return iterator(*this, EntryNodes.end()); }
+  iterator begin() {
+    return iterator(*this, EntryNodes.begin(), EntryNodes.end());
+  }
+  iterator end() { return iterator(*this, EntryNodes.end(), EntryNodes.end()); }
 
   postorder_scc_iterator postorder_scc_begin() {
     return postorder_scc_iterator(*this);