From: Ted Kremenek <kremenek@apple.com>
Date: Wed, 2 Jul 2008 21:24:01 +0000 (+0000)
Subject: Refactored some of the BugReporter interface so that data such as the ASTContext... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c095997b853270d8adb6fe55209a4dbc42803d16;p=clang

Refactored some of the BugReporter interface so that data such as the ASTContext&, PathDiagnosticClient*, can be provided by an external source.
Split BugReporter into BugReporter and GRBugReporter so checkers not based on GRExprEngine can still use the BugReporter mechanism.


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

diff --git a/Driver/AnalysisConsumer.cpp b/Driver/AnalysisConsumer.cpp
index 65a3dae507..db5c7e0b5e 100644
--- a/Driver/AnalysisConsumer.cpp
+++ b/Driver/AnalysisConsumer.cpp
@@ -25,6 +25,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/Analysis/PathSensitive/BugReporter.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/LocalCheckers.h"
 #include "clang/Analysis/PathSensitive/GRTransferFuncs.h"
@@ -98,7 +99,7 @@ namespace {
   };
     
   
-  class VISIBILITY_HIDDEN AnalysisManager {
+  class VISIBILITY_HIDDEN AnalysisManager : public BugReporterData {
     Decl* D;
     Stmt* Body;    
     AnalysisConsumer& C;
@@ -117,25 +118,25 @@ namespace {
     Decl* getCodeDecl() const { return D; }
     Stmt* getBody() const { return Body; }
     
-    CFG* getCFG() {
+    virtual CFG& getCFG() {
       if (!cfg) cfg.reset(CFG::buildCFG(getBody()));
-      return cfg.get();
+      return *cfg.get();
     }
     
-    ParentMap* getParentMap() {
+    virtual ParentMap& getParentMap() {
       if (!PM) PM.reset(new ParentMap(getBody()));
-      return PM.get();
+      return *PM.get();
     }
     
-    ASTContext& getContext() {
+    virtual ASTContext& getContext() {
       return *C.Ctx;
     }
     
-    SourceManager& getSourceManager() {
+    virtual SourceManager& getSourceManager() {
       return getContext().getSourceManager();
     }
     
-    Diagnostic& getDiagnostic() {
+    virtual Diagnostic& getDiagnostic() {
       return C.Diags;
     }
     
@@ -143,20 +144,21 @@ namespace {
       return C.LOpts;
     }
     
-    PathDiagnosticClient* getPathDiagnosticClient() {
+    virtual PathDiagnosticClient* getPathDiagnosticClient() {
       if (PD.get() == 0 && !C.HTMLDir.empty())
         PD.reset(CreateHTMLDiagnosticClient(C.HTMLDir, C.PP, C.PPF));
       
       return PD.get();      
     }
       
-    LiveVariables* getLiveVariables() {
+    virtual LiveVariables& getLiveVariables() {
       if (!liveness) {
-        liveness.reset(new LiveVariables(*getCFG()));
-        liveness->runOnCFG(*getCFG());
-        liveness->runOnAllBlocks(*getCFG(), 0, true);
+        liveness.reset(new LiveVariables(getCFG()));
+        liveness->runOnCFG(getCFG());
+        liveness->runOnAllBlocks(getCFG(), 0, true);
       }
-      return liveness.get();
+      
+      return *liveness.get();
     }
     
     bool shouldVisualize() const {
@@ -255,7 +257,7 @@ void AnalysisConsumer::HandleCode(Decl* D, Stmt* Body, Actions actions) {
   AnalysisManager mgr(*this, D, Body);
   
   // Dispatch on the actions.  
-  for (Actions::iterator I = actions.begin(), 
+  for (Actions::iterator I = actions.begin(),
                          E = actions.end(); I != E; ++I)
     ((*I).getHead())(mgr);  
 }
@@ -265,13 +267,13 @@ void AnalysisConsumer::HandleCode(Decl* D, Stmt* Body, Actions actions) {
 //===----------------------------------------------------------------------===//
 
 static void ActionDeadStores(AnalysisManager& mgr) {
-  CheckDeadStores(*mgr.getCFG(), mgr.getContext(),
-                  *mgr.getLiveVariables(), *mgr.getParentMap(),
+  CheckDeadStores(mgr.getCFG(), mgr.getContext(),
+                  mgr.getLiveVariables(), mgr.getParentMap(),
                   mgr.getDiagnostic());
 }
 
 static void ActionUninitVals(AnalysisManager& mgr) {
-  CheckUninitializedValues(*mgr.getCFG(), mgr.getContext(),
+  CheckUninitializedValues(mgr.getCFG(), mgr.getContext(),
                            mgr.getDiagnostic());
 }
 
@@ -285,15 +287,15 @@ static void ActionGRExprEngine(AnalysisManager& mgr, GRTransferFuncs* tf) {
     mgr.DisplayFunction();
   
   // Construct the analysis engine.
-  GRExprEngine Eng(*mgr.getCFG(), *mgr.getCodeDecl(), mgr.getContext(),
-                   *mgr.getLiveVariables());  
+  GRExprEngine Eng(mgr.getCFG(), *mgr.getCodeDecl(), mgr.getContext(),
+                   mgr.getLiveVariables());  
   Eng.setTransferFunctions(tf);
   
   // Execute the worklist algorithm.
   Eng.ExecuteWorkList();
   
   // Display warnings.
-  Eng.EmitWarnings(mgr.getDiagnostic(), mgr.getPathDiagnosticClient());
+  Eng.EmitWarnings(mgr);
   
   // Visualize the exploded graph.
   if (mgr.shouldVisualize())
@@ -337,17 +339,17 @@ static void ActionSimpleChecks(AnalysisManager& mgr) {
 
 static void ActionLiveness(AnalysisManager& mgr) {
   mgr.DisplayFunction();
-  mgr.getLiveVariables()->dumpBlockLiveness(mgr.getSourceManager());
+  mgr.getLiveVariables().dumpBlockLiveness(mgr.getSourceManager());
 }
 
 static void ActionCFGDump(AnalysisManager& mgr) {
   mgr.DisplayFunction();
-  mgr.getCFG()->dump();
+  mgr.getCFG().dump();
 }
 
 static void ActionCFGView(AnalysisManager& mgr) {
   mgr.DisplayFunction();
-  mgr.getCFG()->viewCFG();  
+  mgr.getCFG().viewCFG();  
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/include/clang/Analysis/PathSensitive/BugReporter.h b/include/clang/Analysis/PathSensitive/BugReporter.h
index cf9de720a2..398c9a9a59 100644
--- a/include/clang/Analysis/PathSensitive/BugReporter.h
+++ b/include/clang/Analysis/PathSensitive/BugReporter.h
@@ -127,44 +127,100 @@ public:
   }
 };
   
+class BugReporterData {
+public:
+  virtual ~BugReporterData();
+  virtual Diagnostic& getDiagnostic() = 0;  
+  virtual PathDiagnosticClient* getPathDiagnosticClient() = 0;  
+  virtual ASTContext& getContext() = 0;
+  virtual SourceManager& getSourceManager() = 0;
+  virtual CFG& getCFG() = 0;
+  virtual ParentMap& getParentMap() = 0;
+  virtual LiveVariables& getLiveVariables() = 0;
+};
+  
 class BugReporter {
-  Diagnostic& Diag;
-  PathDiagnosticClient* PD;
-  ASTContext& Ctx;
-  GRExprEngine& Eng;
-  llvm::SmallSet<SymbolID, 10> NotableSymbols;
+public:
+  enum Kind { BaseBRKind, GRBugReporterKind };
+
+protected:
+  Kind kind;  
+  BugReporterData& D;
+  
+  BugReporter(BugReporterData& d, Kind k) : kind(k), D(d) {}
   
 public:
-  BugReporter(Diagnostic& diag, PathDiagnosticClient* pd,
-              ASTContext& ctx, GRExprEngine& eng)
-  : Diag(diag), PD(pd), Ctx(ctx), Eng(eng) {}
+  BugReporter(BugReporterData& d) : kind(BaseBRKind), D(d) {}
+  virtual ~BugReporter();
   
-  ~BugReporter();
+  Kind getKind() const { return kind; }
   
-  Diagnostic& getDiagnostic() { return Diag; }
+  Diagnostic& getDiagnostic() {
+    return D.getDiagnostic();
+  }
   
-  PathDiagnosticClient* getDiagnosticClient() { return PD; }
+  PathDiagnosticClient* getPathDiagnosticClient() {
+    return D.getPathDiagnosticClient();
+  }
   
-  ASTContext& getContext() { return Ctx; }
+  ASTContext& getContext() {
+    return D.getContext();
+  }
   
-  SourceManager& getSourceManager() { return Ctx.getSourceManager(); }
+  SourceManager& getSourceManager() {
+    return getContext().getSourceManager();
+  }
   
-  ExplodedGraph<ValueState>& getGraph();
+  CFG& getCFG() {
+    return D.getCFG();
+  }
+  
+  ParentMap& getParentMap() {
+    return D.getParentMap();  
+  }
+  
+  LiveVariables& getLiveVariables() {
+    return D.getLiveVariables();
+  }
+  
+  virtual void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R) {}
 
-  GRExprEngine& getEngine() { return Eng; }
+  void EmitWarning(BugReport& R);
   
-  ValueStateManager& getStateManager();
+  static bool classof(const BugReporter* R) { return true; }
+};
   
-  CFG& getCFG() { return getGraph().getCFG(); }
+class GRBugReporter : public BugReporter {
+  GRExprEngine& Eng;
+  llvm::SmallSet<SymbolID, 10> NotableSymbols;
+public:
   
-  ParentMap& getParentMap();
+  GRBugReporter(BugReporterData& d, GRExprEngine& eng)
+    : BugReporter(d, GRBugReporterKind), Eng(eng) {}
   
-  void EmitWarning(BugReport& R);
+  virtual ~GRBugReporter();
+  
+  GRExprEngine& getEngine() {
+    return Eng;
+  }
+
+  ExplodedGraph<ValueState>& getGraph();
   
-  void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R);
+  ValueStateManager& getStateManager();
   
-  void addNotableSymbol(SymbolID Sym) { NotableSymbols.insert(Sym); }
-  bool isNotable(SymbolID Sym) const { return (bool) NotableSymbols.count(Sym);}
+  virtual void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R);
+
+  void addNotableSymbol(SymbolID Sym) {
+    NotableSymbols.insert(Sym);
+  }
+  
+  bool isNotable(SymbolID Sym) const {
+    return (bool) NotableSymbols.count(Sym);
+  }
+  
+  static bool classof(const BugReporter* R) {
+    return R->getKind() == GRBugReporterKind;
+  }
 };
   
 } // end clang namespace
diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index 34118f4872..1568e30da9 100644
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -29,6 +29,7 @@ namespace clang {
   class PathDiagnosticClient;
   class Diagnostic;
   class ParentMap;
+  class BugReporterData;
 
 class GRExprEngine {
   
@@ -235,7 +236,7 @@ public:
     BugTypes.push_back(B);
   }
   
-  void EmitWarnings(Diagnostic& Diag, PathDiagnosticClient* PD);  
+  void EmitWarnings(BugReporterData& BRData);  
   
   bool isRetStackAddr(const NodeTy* N) const {
     return N->isSink() && RetsStackAddr.count(const_cast<NodeTy*>(N)) != 0;
diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp
index 26dba4e7e8..1741e7dc9b 100644
--- a/lib/Analysis/BugReporter.cpp
+++ b/lib/Analysis/BugReporter.cpp
@@ -27,22 +27,20 @@
 using namespace clang;
 
 BugReporter::~BugReporter() {}
+GRBugReporter::~GRBugReporter() {}
+BugReporterData::~BugReporterData() {}
 BugType::~BugType() {}
 BugReport::~BugReport() {}
 RangedBugReport::~RangedBugReport() {}
 
-ExplodedGraph<ValueState>& BugReporter::getGraph() {
+ExplodedGraph<ValueState>& GRBugReporter::getGraph() {
   return Eng.getGraph();
 }
 
-ValueStateManager& BugReporter::getStateManager() { 
+ValueStateManager& GRBugReporter::getStateManager() { 
   return Eng.getStateManager();
 }
 
-ParentMap& BugReporter::getParentMap() {
-  return Eng.getParentMap();
-}
-
 static inline Stmt* GetStmt(const ProgramPoint& P) {
   if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {
     return PS->getStmt();
@@ -338,7 +336,7 @@ static void HandleNotableSymbol(ExplodedNode<ValueState>* N, Stmt* S,
   // specified symbol.  Are any of them not in the previous state.
   
   ValueState* St = N->getState();
-  ValueStateManager& VMgr = BR.getStateManager();
+  ValueStateManager& VMgr = cast<GRBugReporter>(BR).getStateManager();
   
   // FIXME: Later generalize for a broader memory model.
 
@@ -411,8 +409,8 @@ static void HandleNotableSymbol(ExplodedNode<ValueState>* N, Stmt* S,
   }
 }
 
-void BugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
-                                         BugReport& R) {
+void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
+                                           BugReport& R) {
 
   ExplodedNode<ValueState>* N = R.getEndNode();
 
@@ -438,6 +436,7 @@ void BugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
   ExplodedNode<ValueState>* NextNode = N->pred_empty() 
                                        ? NULL : *(N->pred_begin());
   
+  ASTContext& Ctx = getContext();
   SourceManager& SMgr = Ctx.getSourceManager();
   
   while (NextNode) {
@@ -715,6 +714,8 @@ void BugReporter::EmitWarning(BugReport& R) {
 
   // Emit a full diagnostic for the path if we have a PathDiagnosticClient.
   
+  PathDiagnosticClient* PD = getPathDiagnosticClient();
+  
   if (PD && !D->empty()) { 
     PD->HandlePathDiagnostic(D.take());
     return;    
@@ -723,7 +724,7 @@ void BugReporter::EmitWarning(BugReport& R) {
   // We don't have a PathDiagnosticClient, but we can still emit a single
   // line diagnostic.  Determine the location.
   
-  FullSourceLoc L = D->empty() ? R.getLocation(Ctx.getSourceManager())
+  FullSourceLoc L = D->empty() ? R.getLocation(getSourceManager())
                                : D->back()->getLocation();
   
   
@@ -749,7 +750,6 @@ void BugReporter::EmitWarning(BugReport& R) {
   }
   else {
     std::ostringstream os;  
-    os << "[CHECKER] ";
     
     if (D->empty())
       os << R.getDescription();
@@ -757,6 +757,7 @@ void BugReporter::EmitWarning(BugReport& R) {
       os << D->back()->getString();
     
     
+    Diagnostic& Diag = getDiagnostic();
     unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning,
                                               os.str().c_str());
 
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index b190ac20df..323181a302 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -2202,7 +2202,7 @@ PathDiagnosticPiece* CFRefReport::VisitNode(ExplodedNode<ValueState>* N,
   // Add the range by scanning the children of the statement for any bindings
   // to Sym.
   
-  ValueStateManager& VSM = BR.getEngine().getStateManager();
+  ValueStateManager& VSM = cast<GRBugReporter>(BR).getStateManager();
   
   for (Stmt::child_iterator I = S->child_begin(), E = S->child_end(); I!=E; ++I)
     if (Expr* Exp = dyn_cast_or_null<Expr>(*I)) {
@@ -2266,7 +2266,7 @@ PathDiagnosticPiece* CFRefReport::getEndPath(BugReporter& BR,
 
   // Tell the BugReporter to report cases when the tracked symbol is
   // assigned to different variables, etc.
-  BR.addNotableSymbol(Sym);
+  cast<GRBugReporter>(BR).addNotableSymbol(Sym);
   
   if (!getBugType().isLeak())
     return RangedBugReport::getEndPath(BR, EndN);
diff --git a/lib/Analysis/DeadStores.cpp b/lib/Analysis/DeadStores.cpp
index 265679ff7e..3097dc355d 100644
--- a/lib/Analysis/DeadStores.cpp
+++ b/lib/Analysis/DeadStores.cpp
@@ -242,8 +242,8 @@ public:
     DiagCollector C(*this);    
     DeadStoreObs A(BR.getContext(), BR.getDiagnostic(), C, BR.getParentMap());
     
-    GRExprEngine& Eng = BR.getEngine();    
-    Eng.getLiveness().runOnAllBlocks(BR.getCFG(), &A);
+
+    BR.getLiveVariables().runOnAllBlocks(BR.getCFG(), &A);
     
     // Emit the bug reports.
     
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index c9ab8097ba..0c9a1b6df5 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -108,21 +108,21 @@ struct VISIBILITY_HIDDEN SaveOr {
 };
 
 
-void GRExprEngine::EmitWarnings(Diagnostic& Diag, PathDiagnosticClient* PD) {
+void GRExprEngine::EmitWarnings(BugReporterData& BRData) {
   for (bug_type_iterator I = bug_types_begin(), E = bug_types_end(); I!=E; ++I){
-    BugReporter BR(Diag, PD, getContext(), *this);
+    GRBugReporter BR(BRData, *this);
     (*I)->EmitWarnings(BR);
   }
   
   for (SimpleChecksTy::iterator I = CallChecks.begin(), E = CallChecks.end();
        I != E; ++I) {
-    BugReporter BR(Diag, PD, getContext(), *this);
+    GRBugReporter BR(BRData, *this);
     (*I)->EmitWarnings(BR);
   }
   
   for (SimpleChecksTy::iterator I=MsgExprChecks.begin(), E=MsgExprChecks.end();
        I != E; ++I) {
-    BugReporter BR(Diag, PD, getContext(), *this);
+    GRBugReporter BR(BRData, *this);
     (*I)->EmitWarnings(BR);
   }
 }
diff --git a/lib/Analysis/GRSimpleVals.cpp b/lib/Analysis/GRSimpleVals.cpp
index c208f0c7b5..33657dadb1 100644
--- a/lib/Analysis/GRSimpleVals.cpp
+++ b/lib/Analysis/GRSimpleVals.cpp
@@ -66,7 +66,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     GenericEmitWarnings(BR, *this, Eng.null_derefs_begin(),
                         Eng.null_derefs_end());
   }
@@ -83,7 +83,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     GenericEmitWarnings(BR, *this, Eng.undef_derefs_begin(),
                         Eng.undef_derefs_end());
   }
@@ -113,7 +113,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     GenericEmitWarnings(BR, *this, Eng.explicit_bad_divides_begin(),
                         Eng.explicit_bad_divides_end());
   }
@@ -130,7 +130,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     GenericEmitWarnings(BR, *this, Eng.undef_results_begin(),
                         Eng.undef_results_end());
   }
@@ -147,7 +147,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     GenericEmitWarnings(BR, *this, Eng.bad_calls_begin(),
                         Eng.bad_calls_end());
   }
@@ -168,7 +168,7 @@ public:
   }  
 
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
 
     for (GRExprEngine::UndefArgsTy::iterator I = Eng.undef_arg_begin(),
          E = Eng.undef_arg_end(); I!=E; ++I) {
@@ -195,7 +195,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     
     for (GRExprEngine::UndefArgsTy::iterator I=Eng.msg_expr_undef_arg_begin(),
          E = Eng.msg_expr_undef_arg_end(); I!=E; ++I) {
@@ -221,7 +221,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     
     for (GRExprEngine::UndefReceiversTy::iterator I=Eng.undef_receivers_begin(),
          End = Eng.undef_receivers_end(); I!=End; ++I) {
@@ -252,7 +252,7 @@ public:
   }
   
   virtual void EmitWarnings(BugReporter& BR) {
-    GRExprEngine& Eng = BR.getEngine();
+    GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
     GenericEmitWarnings(BR, *this, Eng.ret_stackaddr_begin(),
                         Eng.ret_stackaddr_end());
   }
@@ -291,7 +291,7 @@ struct VISIBILITY_HIDDEN FindUndefExpr {
   
 void UndefBranch::EmitWarnings(BugReporter& BR) {
 
-  GRExprEngine& Eng = BR.getEngine();
+  GRExprEngine& Eng = cast<GRBugReporter>(BR).getEngine();
   
   for (GRExprEngine::undef_branch_iterator I=Eng.undef_branches_begin(),
        E=Eng.undef_branches_end(); I!=E; ++I) {