Static Analyzer Diagnostics: Move the responsibility for generating the endOfPath diagnostic piece from BugReport to BugReporterVisitor. Switch CFRefCount to use visitors in order to generate the endOfPath piece.

llvm-svn: 138184

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

@@ -85,8 +85,6 @@ protected:
   /// for each bug.
   virtual void Profile(llvm::FoldingSetNodeID& hash) const;
 
-  const Stmt *getStmt() const;
-
 public:
   BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode)
     : BT(bt), Description(desc), ErrorNode(errornode),
@@ -121,10 +119,6 @@ public:
     return std::make_pair((const char**)0,(const char**)0);
   }
 
-  /// Provide custom definition for the last diagnostic piece on the path.
-  virtual PathDiagnosticPiece *getEndPath(BugReporterContext &BRC,
-                                          const ExplodedNode *N);
-
   /// \brief Return the "definitive" location of the reported bug.
   ///
   ///  While a bug can span an entire path, usually there is a specific
@@ -132,6 +126,8 @@ public:
   ///  This location is used by clients rendering diagnostics.
   virtual SourceLocation getLocation() const;
 
+  const Stmt *getStmt() const;
+
   /// \brief Add a range to a bug report.
   ///
   /// Ranges are used to highlight regions of interest in the source code.

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h

@@ -42,7 +42,23 @@ public:
                                          BugReporterContext &BRC,
                                          BugReport &BR) = 0;
 
+  /// \brief Provide custom definition for the final diagnostic piece on the
+  /// path - the piece, which is displayed before the path is expanded.
+  ///
+  /// If returns NULL the default implementation will be used.
+  /// Also note that at most one visitor of a BugReport should generate a
+  /// non-NULL end of path diagnostic piece.
+  virtual PathDiagnosticPiece *getEndPath(BugReporterContext &BRC,
+                                          const ExplodedNode *N,
+                                          BugReport &BR);
+
   virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0;
+
+  /// \brief Generates the default final diagnostic piece.
+  static PathDiagnosticPiece *getDefaultEndPath(BugReporterContext &BRC,
+                                                const ExplodedNode *N,
+                                                BugReport &BR);
+
 };
 
 class FindLastStoreBRVisitor : public BugReporterVisitor {

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -1261,45 +1261,6 @@ const Stmt *BugReport::getStmt() const {
   return S;
 }
 
-PathDiagnosticPiece*
-BugReport::getEndPath(BugReporterContext &BRC,
-                      const ExplodedNode *EndPathNode) {
-
-  const ProgramPoint &PP = EndPathNode->getLocation();
-  PathDiagnosticLocation L;
-
-  if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&PP)) {
-    const CFGBlock *block = BE->getBlock();
-    if (block->getBlockID() == 0) {
-      L = PathDiagnosticLocation(
-          EndPathNode->getLocationContext()->getDecl()->getBodyRBrace(),
-          BRC.getSourceManager());
-    }
-  }
-
-  if (!L.isValid()) {
-    const Stmt *S = getStmt();
-
-    if (!S)
-      return NULL;
-
-    L = PathDiagnosticLocation(S, BRC.getSourceManager());
-  }
-
-  BugReport::ranges_iterator Beg, End;
-  llvm::tie(Beg, End) = getRanges();
-
-  // Only add the statement itself as a range if we didn't specify any
-  // special ranges for this report.
-  PathDiagnosticPiece *P = new PathDiagnosticEventPiece(L, getDescription(),
-                                                        Beg == End);
-
-  for (; Beg != End; ++Beg)
-    P->addRange(*Beg);
-
-  return P;
-}
-
 std::pair<BugReport::ranges_iterator, BugReport::ranges_iterator>
 BugReport::getRanges() {
     // If no custom ranges, add the range of the statement corresponding to
@@ -1657,15 +1618,28 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
   // Start building the path diagnostic...
   PathDiagnosticBuilder PDB(*this, R, BackMap.get(), getPathDiagnosticClient());
 
-  if (PathDiagnosticPiece *Piece = R->getEndPath(PDB, N))
-    PD.push_back(Piece);
-  else
-    return;
-
   // Register additional node visitors.
   R->addVisitor(new NilReceiverBRVisitor());
   R->addVisitor(new ConditionBRVisitor());
 
+  // Generate the very last diagnostic piece - the piece is visible before 
+  // the trace is expanded.
+  PathDiagnosticPiece *LastPiece = 0;
+  for (BugReport::visitor_iterator I = R->visitor_begin(),
+                                   E = R->visitor_end(); I!=E; ++I) {
+    if (PathDiagnosticPiece *Piece = (*I)->getEndPath(PDB, N, *R)) {
+      assert (!LastPiece &&
+              "There can only be one final piece in a diagnostic.");
+      LastPiece = Piece;
+    }
+  }
+  if (!LastPiece)
+    LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R);
+  if (LastPiece)
+    PD.push_back(LastPiece);
+  else
+    return;
+
   switch (PDB.getGenerationScheme()) {
     case PathDiagnosticClient::Extensive:
       GenerateExtensivePathDiagnostic(PD, PDB, N);

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

@@ -72,6 +72,54 @@ const Stmt *bugreporter::GetRetValExpr(const ExplodedNode *N) {
 //===----------------------------------------------------------------------===//
 // Definitions for bug reporter visitors.
 //===----------------------------------------------------------------------===//
+
+PathDiagnosticPiece*
+BugReporterVisitor::getEndPath(BugReporterContext &BRC,
+                               const ExplodedNode *EndPathNode,
+                               BugReport &BR) {
+  return 0;
+}
+
+PathDiagnosticPiece*
+BugReporterVisitor::getDefaultEndPath(BugReporterContext &BRC,
+                                      const ExplodedNode *EndPathNode,
+                                      BugReport &BR) {
+  const ProgramPoint &PP = EndPathNode->getLocation();
+  PathDiagnosticLocation L;
+
+  if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&PP)) {
+    const CFGBlock *block = BE->getBlock();
+    if (block->getBlockID() == 0) {
+      L = PathDiagnosticLocation(
+          EndPathNode->getLocationContext()->getDecl()->getBodyRBrace(),
+          BRC.getSourceManager());
+    }
+  }
+
+  if (!L.isValid()) {
+    const Stmt *S = BR.getStmt();
+
+    if (!S)
+      return NULL;
+
+    L = PathDiagnosticLocation(S, BRC.getSourceManager());
+  }
+
+  BugReport::ranges_iterator Beg, End;
+  llvm::tie(Beg, End) = BR.getRanges();
+
+  // Only add the statement itself as a range if we didn't specify any
+  // special ranges for this report.
+  PathDiagnosticPiece *P = new PathDiagnosticEventPiece(L,
+      BR.getDescription(),
+      Beg == End);
+  for (; Beg != End; ++Beg)
+    P->addRange(*Beg);
+
+  return P;
+}
+
+
 void FindLastStoreBRVisitor ::Profile(llvm::FoldingSetNodeID &ID) const {
   static int tag = 0;
   ID.AddPointer(&tag);

clang/lib/StaticAnalyzer/Core/CFRefCount.cpp

@@ -1960,23 +1960,38 @@ namespace {
   //===---------===//
 
   class CFRefReportVisitor : public BugReporterVisitor {
+  protected:
     SymbolRef Sym;
     const CFRefCount &TF;
+    
   public:
-
     CFRefReportVisitor(SymbolRef sym, const CFRefCount &tf)
        : Sym(sym), TF(tf) {}
 
-    void Profile(llvm::FoldingSetNodeID &ID) const {
+    virtual void Profile(llvm::FoldingSetNodeID &ID) const {
       static int x = 0;
       ID.AddPointer(&x);
       ID.AddPointer(Sym);
     }
 
-    PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
+    virtual PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
-                                   const ExplodedNode *PrevN,
+                                           const ExplodedNode *PrevN,
-                                   BugReporterContext &BRC,
+                                           BugReporterContext &BRC,
-                                   BugReport &BR);
+                                           BugReport &BR);
+
+    virtual PathDiagnosticPiece *getEndPath(BugReporterContext &BRC,
+                                            const ExplodedNode *N,
+                                            BugReport &BR);
+  };
+
+  class CFRefLeakReportVisitor : public CFRefReportVisitor {
+  public:
+    CFRefLeakReportVisitor(SymbolRef sym, const CFRefCount &tf)
+       : CFRefReportVisitor(sym, tf) {}
+
+    PathDiagnosticPiece *getEndPath(BugReporterContext &BRC,
+                                    const ExplodedNode *N,
+                                    BugReport &BR);
   };
 
   class CFRefReport : public BugReport {
@@ -1985,9 +2000,10 @@ namespace {
     const CFRefCount &TF;
   public:
     CFRefReport(CFRefBug& D, const CFRefCount &tf,
-                ExplodedNode *n, SymbolRef sym)
+                ExplodedNode *n, SymbolRef sym, bool registerVisitor = true)
       : BugReport(D, D.getDescription(), n), Sym(sym), TF(tf) {
-      addVisitor(new CFRefReportVisitor(sym, tf));
+      if (registerVisitor)
+        addVisitor(new CFRefReportVisitor(sym, tf));
     }
 
     CFRefReport(CFRefBug& D, const CFRefCount &tf,
@@ -2011,23 +2027,18 @@ namespace {
 
     SymbolRef getSymbol() const { return Sym; }
 
-    PathDiagnosticPiece *getEndPath(BugReporterContext &BRC,
-                                    const ExplodedNode *N);
-
     std::pair<const char**,const char**> getExtraDescriptiveText();
   };
 
   class CFRefLeakReport : public CFRefReport {
     SourceLocation AllocSite;
     const MemRegion* AllocBinding;
+
   public:
     CFRefLeakReport(CFRefBug& D, const CFRefCount &tf,
                     ExplodedNode *n, SymbolRef sym,
                     ExprEngine& Eng);
 
-    PathDiagnosticPiece *getEndPath(BugReporterContext &BRC,
-                                    const ExplodedNode *N);
-
     SourceLocation getLocation() const { return AllocSite; }
   };
 } // end anonymous namespace
@@ -2384,17 +2395,19 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N,
 }
 
 PathDiagnosticPiece*
-CFRefReport::getEndPath(BugReporterContext &BRC,
+CFRefReportVisitor::getEndPath(BugReporterContext &BRC,
-                        const ExplodedNode *EndN) {
+                               const ExplodedNode *EndN,
+                               BugReport &BR) {
   // Tell the BugReporterContext to report cases when the tracked symbol is
   // assigned to different variables, etc.
   BRC.addNotableSymbol(Sym);
-  return BugReport::getEndPath(BRC, EndN);
+  return BugReporterVisitor::getDefaultEndPath(BRC, EndN, BR);
 }
 
 PathDiagnosticPiece*
-CFRefLeakReport::getEndPath(BugReporterContext &BRC,
+CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
-                            const ExplodedNode *EndN){
+                                   const ExplodedNode *EndN,
+                                   BugReport &BR) {
 
   // Tell the BugReporterContext to report cases when the tracked symbol is
   // assigned to different variables, etc.
@@ -2493,7 +2506,7 @@ CFRefLeakReport::getEndPath(BugReporterContext &BRC,
 CFRefLeakReport::CFRefLeakReport(CFRefBug& D, const CFRefCount &tf,
                                  ExplodedNode *n,
                                  SymbolRef sym, ExprEngine& Eng)
-: CFRefReport(D, tf, n, sym) {
+: CFRefReport(D, tf, n, sym, false) {
 
   // Most bug reports are cached at the location where they occurred.
   // With leaks, we want to unique them by the location where they were
@@ -2527,7 +2540,7 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug& D, const CFRefCount &tf,
   if (AllocBinding)
     os << " and stored into '" << AllocBinding->getString() << '\'';
 
-  addVisitor(new CFRefReportVisitor(sym, tf));
+  addVisitor(new CFRefLeakReportVisitor(sym, tf));
 }
 
 //===----------------------------------------------------------------------===//