Simplify ownership of ExplodedGraph in the CoreEngine by removing unique_ptr indirection.

[dblaikie]

Summary

I was going to fix the use of raw pointer ownership in "takeGraph" when I realized that function was unused and the whole ExplodedGraph could just be owned by value without the std::unique_ptr indirection at all.

[jordan_rose]

That seems entirely reasonable. If we ever need to have ExplodedGraphs persist past the lifetime of the CoreEngine, we can put back something like this. Or make the copy/move constructor efficient!

[dblaikie]

Closed by commit rL215257 (authored by @dblaikie).

include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h

public:
   typedef std::vector<std::pair<const CFGBlock*, const ExplodedNode*> >
             BlocksAborted;
 
 private:
 
   SubEngine& SubEng;
 
   /// G - The simulation graph.  Each node is a (location,state) pair.
-  std::unique_ptr<ExplodedGraph> G;
+  mutable ExplodedGraph G;
 
   /// WList - A set of queued nodes that need to be processed by the
   ///  worklist algorithm.  It is up to the implementation of WList to decide
   ///  the order that nodes are processed.
   std::unique_ptr<WorkList> WList;
 
   /// BCounterFactory - A factory object for created BlockCounter objects.
   ///   These are used to record for key nodes in the ExplodedGraph the

 private:
   CoreEngine(const CoreEngine &) LLVM_DELETED_FUNCTION;
   void operator=(const CoreEngine &) LLVM_DELETED_FUNCTION;
 
   ExplodedNode *generateCallExitBeginNode(ExplodedNode *N);
 
 public:
   /// Construct a CoreEngine object to analyze the provided CFG.
-  CoreEngine(SubEngine& subengine,
-             FunctionSummariesTy *FS)
-    : SubEng(subengine), G(new ExplodedGraph()),
-      WList(WorkList::makeDFS()),
-      BCounterFactory(G->getAllocator()),
-      FunctionSummaries(FS){}
+  CoreEngine(SubEngine &subengine, FunctionSummariesTy *FS)
+      : SubEng(subengine), WList(WorkList::makeDFS()),
+        BCounterFactory(G.getAllocator()), FunctionSummaries(FS) {}
 
   /// getGraph - Returns the exploded graph.
-  ExplodedGraph& getGraph() { return *G.get(); }
-
-  /// takeGraph - Returns the exploded graph.  Ownership of the graph is
-  ///  transferred to the caller.
-  ExplodedGraph *takeGraph() { return G.release(); }
+  ExplodedGraph &getGraph() { return G; }
 
   /// ExecuteWorkList - Run the worklist algorithm for a maximum number of
   ///  steps.  Returns true if there is still simulation state on the worklist.
   bool ExecuteWorkList(const LocationContext *L, unsigned Steps,
                        ProgramStateRef InitState);
   /// Returns true if there is still simulation state on the worklist.
   bool ExecuteWorkListWithInitialState(const LocationContext *L,
                                        unsigned Steps,

lib/StaticAnalyzer/Core/CoreEngine.cpp

 //===----------------------------------------------------------------------===//
 // Core analysis engine.
 //===----------------------------------------------------------------------===//
 
 /// ExecuteWorkList - Run the worklist algorithm for a maximum number of steps.
 bool CoreEngine::ExecuteWorkList(const LocationContext *L, unsigned Steps,
                                    ProgramStateRef InitState) {
 
-  if (G->num_roots() == 0) { // Initialize the analysis by constructing
+  if (G.num_roots() == 0) { // Initialize the analysis by constructing
     // the root if none exists.
 
     const CFGBlock *Entry = &(L->getCFG()->getEntry());
 
     assert (Entry->empty() &&
             "Entry block must be empty.");
 
     assert (Entry->succ_size() == 1 &&
void CoreEngine::dispatchWorkItem(ExplodedNode* Pred, ProgramPoint Loc,
   }
 }
 
 bool CoreEngine::ExecuteWorkListWithInitialState(const LocationContext *L,
                                                  unsigned Steps,
                                                  ProgramStateRef InitState, 
                                                  ExplodedNodeSet &Dst) {
   bool DidNotFinish = ExecuteWorkList(L, Steps, InitState);
-  for (ExplodedGraph::eop_iterator I = G->eop_begin(), 
-                                   E = G->eop_end(); I != E; ++I) {
+  for (ExplodedGraph::eop_iterator I = G.eop_begin(), E = G.eop_end(); I != E;
+       ++I) {
     Dst.Add(*I);
   }
   return DidNotFinish;
 }
 
 void CoreEngine::HandleBlockEdge(const BlockEdge &L, ExplodedNode *Pred) {
 
   const CFGBlock *Blk = L.getDst();

 
 /// generateNode - Utility method to generate nodes, hook up successors,
 ///  and add nodes to the worklist.
 void CoreEngine::generateNode(const ProgramPoint &Loc,
                               ProgramStateRef State,
                               ExplodedNode *Pred) {
 
   bool IsNew;
-  ExplodedNode *Node = G->getNode(Loc, State, false, &IsNew);
+  ExplodedNode *Node = G.getNode(Loc, State, false, &IsNew);
 
   if (Pred)
-    Node->addPredecessor(Pred, *G);  // Link 'Node' with its predecessor.
+    Node->addPredecessor(Pred, G); // Link 'Node' with its predecessor.
   else {
     assert (IsNew);
-    G->addRoot(Node);  // 'Node' has no predecessor.  Make it a root.
+    G.addRoot(Node); // 'Node' has no predecessor.  Make it a root.
   }
 
   // Only add 'Node' to the worklist if it was freshly generated.
   if (IsNew) WList->enqueue(Node);
 }
 
 void CoreEngine::enqueueStmtNode(ExplodedNode *N,
                                  const CFGBlock *Block, unsigned Idx) {
void CoreEngine::enqueueStmtNode(ExplodedNode *N,
   if (Loc == N->getLocation().withTag(nullptr)) {
     // Note: 'N' should be a fresh node because otherwise it shouldn't be
     // a member of Deferred.
     WList->enqueue(N, Block, Idx+1);
     return;
   }
 
   bool IsNew;
-  ExplodedNode *Succ = G->getNode(Loc, N->getState(), false, &IsNew);
-  Succ->addPredecessor(N, *G);
+  ExplodedNode *Succ = G.getNode(Loc, N->getState(), false, &IsNew);
+  Succ->addPredecessor(N, G);
 
   if (IsNew)
     WList->enqueue(Succ, Block, Idx+1);
 }
 
 ExplodedNode *CoreEngine::generateCallExitBeginNode(ExplodedNode *N) {
   // Create a CallExitBegin node and enqueue it.
   const StackFrameContext *LocCtx
                          = cast<StackFrameContext>(N->getLocationContext());
 
   // Use the callee location context.
   CallExitBegin Loc(LocCtx);
 
   bool isNew;
-  ExplodedNode *Node = G->getNode(Loc, N->getState(), false, &isNew);
-  Node->addPredecessor(N, *G);
+  ExplodedNode *Node = G.getNode(Loc, N->getState(), false, &isNew);
+  Node->addPredecessor(N, G);
   return isNew ? Node : nullptr;
 }
 
 
 void CoreEngine::enqueue(ExplodedNodeSet &Set) {
   for (ExplodedNodeSet::iterator I = Set.begin(),
                                  E = Set.end(); I != E; ++I) {
     WList->enqueue(*I);
for (ExplodedNodeSet::iterator I = Set.begin(), E = Set.end(); I != E; ++I) {
     ExplodedNode *N = *I;
     // If we are in an inlined call, generate CallExitBegin node.
     if (N->getLocationContext()->getParent()) {
       N = generateCallExitBeginNode(N);
       if (N)
         WList->enqueue(N);
     } else {
       // TODO: We should run remove dead bindings here.
-      G->addEndOfPath(N);
+      G.addEndOfPath(N);
       NumPathsExplored++;
     }
   }
 }
 
 
 void NodeBuilder::anchor() { }
 
 ExplodedNode* NodeBuilder::generateNodeImpl(const ProgramPoint &Loc,
                                             ProgramStateRef State,
                                             ExplodedNode *FromN,
                                             bool MarkAsSink) {
   HasGeneratedNodes = true;
   bool IsNew;
-  ExplodedNode *N = C.Eng.G->getNode(Loc, State, MarkAsSink, &IsNew);
-  N->addPredecessor(FromN, *C.Eng.G);
+  ExplodedNode *N = C.Eng.G.getNode(Loc, State, MarkAsSink, &IsNew);
+  N->addPredecessor(FromN, C.Eng.G);
   Frontier.erase(FromN);
 
   if (!IsNew)
     return nullptr;
 
   if (!MarkAsSink)
     Frontier.Add(N);
 
ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State,
   return Succ;
 }
 
 ExplodedNode*
 IndirectGotoNodeBuilder::generateNode(const iterator &I,
                                       ProgramStateRef St,
                                       bool IsSink) {
   bool IsNew;
-  ExplodedNode *Succ = Eng.G->getNode(BlockEdge(Src, I.getBlock(),
-                                      Pred->getLocationContext()), St,
-                                      IsSink, &IsNew);
-  Succ->addPredecessor(Pred, *Eng.G);
+  ExplodedNode *Succ =
+      Eng.G.getNode(BlockEdge(Src, I.getBlock(), Pred->getLocationContext()),
+                    St, IsSink, &IsNew);
+  Succ->addPredecessor(Pred, Eng.G);
 
   if (!IsNew)
     return nullptr;
 
   if (!IsSink)
     Eng.WList->enqueue(Succ);
 
   return Succ;
 }
 
 
 ExplodedNode*
 SwitchNodeBuilder::generateCaseStmtNode(const iterator &I,
                                         ProgramStateRef St) {
 
   bool IsNew;
-  ExplodedNode *Succ = Eng.G->getNode(BlockEdge(Src, I.getBlock(),
-                                      Pred->getLocationContext()), St,
-                                      false, &IsNew);
-  Succ->addPredecessor(Pred, *Eng.G);
+  ExplodedNode *Succ =
+      Eng.G.getNode(BlockEdge(Src, I.getBlock(), Pred->getLocationContext()),
+                    St, false, &IsNew);
+  Succ->addPredecessor(Pred, Eng.G);
   if (!IsNew)
     return nullptr;
 
   Eng.WList->enqueue(Succ);
   return Succ;
 }
 
 
 ExplodedNode*
 SwitchNodeBuilder::generateDefaultCaseNode(ProgramStateRef St,
                                            bool IsSink) {
   // Get the block for the default case.
   assert(Src->succ_rbegin() != Src->succ_rend());
   CFGBlock *DefaultBlock = *Src->succ_rbegin();
 
   // Sanity check for default blocks that are unreachable and not caught
   // by earlier stages.
   if (!DefaultBlock)
     return nullptr;
 
   bool IsNew;
-  ExplodedNode *Succ = Eng.G->getNode(BlockEdge(Src, DefaultBlock,
-                                      Pred->getLocationContext()), St,
-                                      IsSink, &IsNew);
-  Succ->addPredecessor(Pred, *Eng.G);
+  ExplodedNode *Succ =
+      Eng.G.getNode(BlockEdge(Src, DefaultBlock, Pred->getLocationContext()),
+                    St, IsSink, &IsNew);
+  Succ->addPredecessor(Pred, Eng.G);
 
   if (!IsNew)
     return nullptr;
 
   if (!IsSink)
     Eng.WList->enqueue(Succ);
 
   return Succ;
 }