Use the correct data structures!

ExplodedGraph::TrimGraph:
- Just do a DFS both ways instead of BFS-DFS. We're just determining what subset
  of the nodes are reachable from the root and reverse-reachable from the bug
  nodes.  DFS is more efficient for this task.
  
BugReporter:
- MakeReportGraph: Do a reverse-BFS instead of a reverse-DFS to determine the
  approximate shortest path through the simulation graph. We were seeing some
  weird cases where too many loops were being reported for simple bugs. Possibly
  we will need to replace this with actually computing the shortest path in
  terms of line numbers.

llvm-svn: 66842

clang/lib/Analysis/BugReporter.cpp

@@ -24,6 +24,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
+#include <queue>
 
 using namespace clang;
 
@@ -298,18 +299,19 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
     new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(),
                                GTrim->getContext());
   
-  // Sometimes the trimmed graph can contain a cycle.  Perform a reverse DFS
+  // Sometimes the trimmed graph can contain a cycle.  Perform a reverse BFS
   // to the root node, and then construct a new graph that contains only
   // a single path.
   llvm::DenseMap<const void*,unsigned> Visited;
-  llvm::SmallVector<const ExplodedNode<GRState>*, 10> WS;
-  WS.push_back(N);
+  std::queue<const ExplodedNode<GRState>*> WS;
+  WS.push(N);
+  
   unsigned cnt = 0;
   const ExplodedNode<GRState>* Root = 0;
   
   while (!WS.empty()) {
-    const ExplodedNode<GRState>* Node = WS.back();
-    WS.pop_back();
+    const ExplodedNode<GRState>* Node = WS.front();
+    WS.pop();
     
     if (Visited.find(Node) != Visited.end())
       continue;
@@ -323,12 +325,12 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
     
     for (ExplodedNode<GRState>::const_pred_iterator I=Node->pred_begin(),
          E=Node->pred_end(); I!=E; ++I)
-      WS.push_back(*I);
+      WS.push(*I);
   }
   
   assert (Root);
   
-  // Now walk from the root down the DFS path, always taking the successor
+  // Now walk from the root down the BFS path, always taking the successor
   // with the lowest number.
   ExplodedNode<GRState> *Last = 0, *First = 0;  
   NodeBackMap *BM = new NodeBackMap();

clang/lib/Analysis/ExplodedGraph.cpp

@@ -18,7 +18,6 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include <vector>
-#include <list>
 
 using namespace clang;
 
@@ -133,10 +132,9 @@ const {
   typedef llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*> Pass2Ty;
   Pass2Ty& Pass2 = M->M;
   
-  std::list<const ExplodedNodeImpl*> WL1;
-  llvm::SmallVector<const ExplodedNodeImpl*, 10> WL2;
+  llvm::SmallVector<const ExplodedNodeImpl*, 10> WL1, WL2;
 
-  // ===- Pass 1 (reverse BFS) -===
+  // ===- Pass 1 (reverse DFS) -===
   for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources; ++I) {
     assert(*I);
     WL1.push_back(*I);
@@ -163,7 +161,7 @@ const {
       
     // Visit our predecessors and enqueue them.
     for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end(); I!=E; ++I)
-      WL1.push_front(*I);
+      WL1.push_back(*I);
   }
   
   // We didn't hit a root? Return with a null pointer for the new graph.