Greatly simplify the logic in ExplodedGraphImpl::TrimGraph. Now we just do a

vanilla reverse-BFS followed by a forward-DFS instead of resulting to strange
histrionics (whose purpose I can no longer remember) in the reverse-BFS stage.
This fixes an assertion failure in BugReporter due to edge cases where no root
was being hit in the reverse-BFS phase.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@65160 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/ExplodedGraph.cpp b/lib/Analysis/ExplodedGraph.cpp
index fe9cecadf3d929956c8c458e50896ce0999273b3..0e72af91e136e6f4a5c639949677a6eeac21bd3e 100644 (file)
--- a/lib/Analysis/ExplodedGraph.cpp
+++ b/lib/Analysis/ExplodedGraph.cpp
@@ -127,142 +127,80 @@ ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources,
                         llvm::DenseMap<const void*, const void*> *InverseMap) 
 const {
   
-  typedef llvm::DenseMap<const ExplodedNodeImpl*,
-                         const ExplodedNodeImpl*> Pass1Ty;  
+  typedef llvm::DenseSet<const ExplodedNodeImpl*> Pass1Ty;
   Pass1Ty Pass1;
   
-  typedef llvm::DenseMap<const ExplodedNodeImpl*,
-                         ExplodedNodeImpl*> Pass2Ty;
-
+  typedef llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*> Pass2Ty;
   Pass2Ty& Pass2 = M->M;
   
+  std::list<const ExplodedNodeImpl*> WL1;
   llvm::SmallVector<const ExplodedNodeImpl*, 10> WL2;
 
-  { // ===- Pass 1 (reverse BFS) -===
-    
-    // Enqueue the source nodes to the first worklist. 
+  // ===- Pass 1 (reverse BFS) -===
+  for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources; ++I) {
+    assert(*I);
+    WL1.push_back(*I);
+  }
     
-    std::list<std::pair<const ExplodedNodeImpl*,
-                        const ExplodedNodeImpl*> > WL1, WL1_Loops;
-  
-    for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources; ++I){
-      assert(*I);
-      WL1.push_back(std::make_pair(*I, *I));
-    }
+  // Process the first worklist until it is empty.  Because it is a std::list
+  // it acts like a FIFO queue.
+  while (!WL1.empty()) {
+    const ExplodedNodeImpl *N = WL1.back();
+    WL1.pop_back();
     
-    // Process the worklist.
-
-    while (!WL1.empty() || !WL1_Loops.empty()) {
-      // Only dequeue from the "loops" worklist if WL1 has no items.
-      // Thus we prioritize for paths that don't span loop boundaries.
-      const ExplodedNodeImpl *N = 0, *Src = 0;
+    // Have we already visited this node?  If so, continue to the next one.
+    if (Pass1.count(N))
+      continue;
 
-      if (WL1.empty()) {
-        assert(!WL1_Loops.empty());
-        N = WL1_Loops.back().first;
-        assert(N);
-        Src = WL1_Loops.back().second;
-        WL1_Loops.pop_back();
-      }
-      else {
-        N = WL1.back().first;
-        assert(N);
-        Src = WL1.back().second;
-        WL1.pop_back();
-      }      
-      
-      if (Pass1.find(N) != Pass1.end())
-        continue;
-      
-      bool PredHasSameSource = false;
-      bool VisitPreds = true;
-            
-      for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end();
-            I!=E; ++I) {
-        
-        Pass1Ty::iterator pi = Pass1.find(*I);
-        
-        if (pi == Pass1.end())
-          continue;
-        
-        VisitPreds = false;
-        
-        if (pi->second == Src) {
-          PredHasSameSource = true;
-          break;
-        }
-      }
-      
-      if (VisitPreds || !PredHasSameSource) {
-        
-        Pass1[N] = Src;
-      
-        if (N->Preds.empty()) {
-          WL2.push_back(N);
-          continue;      
-        }
-      }
-      else
-        Pass1[N] = NULL;
-      
-      if (VisitPreds)
-        for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end();
-             I!=E; ++I) {
-          
-          ProgramPoint P = Src->getLocation();
-          
-          if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P))
-            if (Stmt* T = BE->getSrc()->getTerminator())
-              switch (T->getStmtClass()) {
-                default: break;
-                case Stmt::ForStmtClass:
-                case Stmt::WhileStmtClass:
-                case Stmt::DoStmtClass:
-                  assert(*I);
-                  WL1_Loops.push_front(std::make_pair(*I, Src));
-                  continue;
-                  
-              }
-          
-          assert(*I);
-          WL1.push_front(std::make_pair(*I, Src));
-        }
+    // Otherwise, mark this node as visited.
+    Pass1.insert(N);
+    
+    // If this is a root enqueue it to the second worklist.
+    if (N->Preds.empty()) {
+      WL2.push_back(N);
+      continue;
     }
+      
+    // Visit our predecessors and enqueue them.
+    for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end(); I!=E; ++I)
+      WL1.push_front(*I);
   }
   
+  // We didn't hit a root? Return with a null pointer for the new graph.
   if (WL2.empty())
     return 0;
 
+  // Create an empty graph.
   ExplodedGraphImpl* G = MakeEmptyGraph();
   
-  // ===- Pass 2 (forward DFS to construct the new graph) -===
-  
+  // ===- Pass 2 (forward DFS to construct the new graph) -===  
   while (!WL2.empty()) {
-    
     const ExplodedNodeImpl* N = WL2.back();
     WL2.pop_back();
     
     // Skip this node if we have already processed it.
-    
     if (Pass2.find(N) != Pass2.end())
       continue;
     
-    // Create the corresponding node in the new graph.
-    
+    // Create the corresponding node in the new graph and record the mapping
+    // from the old node to the new node.
     ExplodedNodeImpl* NewN = G->getNodeImpl(N->getLocation(), N->State, NULL);
     Pass2[N] = NewN;
+    
+    // Also record the reverse mapping from the new node to the old node.
     if (InverseMap) (*InverseMap)[NewN] = N;
     
+    // If this node is a root, designate it as such in the graph.
     if (N->Preds.empty())
       G->addRoot(NewN);
     
     // In the case that some of the intended predecessors of NewN have already
     // been created, we should hook them up as predecessors.
-    
+
+    // Walk through the predecessors of 'N' and hook up their corresponding
+    // nodes in the new graph (if any) to the freshly created node.
     for (ExplodedNodeImpl **I=N->Preds.begin(), **E=N->Preds.end(); I!=E; ++I) {
-        
       Pass2Ty::iterator PI = Pass2.find(*I);
-
       if (PI == Pass2.end())
         continue;
       
@@ -273,26 +211,19 @@ const {
     // been created, we should hook them up as successors.  Otherwise, enqueue
     // the new nodes from the original graph that should have nodes created
     // in the new graph.
-   
     for (ExplodedNodeImpl **I=N->Succs.begin(), **E=N->Succs.end(); I!=E; ++I) {
-      
-      Pass2Ty::iterator PI = Pass2.find(*I);
-      
+      Pass2Ty::iterator PI = Pass2.find(*I);      
       if (PI != Pass2.end()) {
         PI->second->addPredecessor(NewN);
         continue;
       }
 
       // Enqueue nodes to the worklist that were marked during pass 1.
-      
-      Pass1Ty::iterator pi = Pass1.find(*I);
-      
-      if (pi == Pass1.end() || pi->second == NULL)
-        continue;
-            
-      WL2.push_back(*I);
+      if (Pass1.count(*I))
+        WL2.push_back(*I);
     }
     
+    // Finally, explictly mark all nodes without any successors as sinks.
     if (N->isSink())
       NewN->markAsSink();
   }