Overhaul BugReporter interface and implementation. The new interface cleans up

the ownership of BugTypes and BugReports. Now BugReports are owned by BugTypes,
and BugTypes are owned by the BugReporter object.

The major functionality change in this patch is that reports are not immediately
emitted by a call to BugReporter::EmitWarning (now called EmitReport), but
instead of queued up in report "equivalence classes". When
BugReporter::FlushReports() is called, it emits one diagnostic per report
equivalence class. This provides a nice cleanup with the caching of reports as
well as enables the BugReporter engine to select the "best" path for reporting a
path-sensitive bug based on all the locations in the ExplodedGraph that the same
bug could occur.

Along with this patch, Leaks are now coalesced into a common equivalence class
by their allocation site, and the "summary" diagnostic for leaks now reports the
allocation site as the location of the bug (this may later be augmented to also
provide an example location where the leak occurs).


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

9 files changed, 569 insertions, 702 deletions

diff --git a/lib/Analysis/BasicObjCFoundationChecks.cpp b/lib/Analysis/BasicObjCFoundationChecks.cpp
index be729bee6c..8a5c3e0a84 100644
--- a/lib/Analysis/BasicObjCFoundationChecks.cpp
+++ b/lib/Analysis/BasicObjCFoundationChecks.cpp
@@ -23,15 +23,12 @@
 #include "clang/Analysis/PathSensitive/MemRegion.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Analysis/LocalCheckers.h"
-
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ASTContext.h"
 #include "llvm/Support/Compiler.h"
 
-#include <sstream>
-
 using namespace clang;
 
 static ObjCInterfaceType* GetReceiverType(ObjCMessageExpr* ME) {
@@ -60,64 +57,17 @@ static const char* GetReceiverNameType(ObjCMessageExpr* ME) {
 
 namespace {
 
-class VISIBILITY_HIDDEN APIMisuse : public BugTypeCacheLocation {
-public:  
-  const char* getCategory() const {
-    return "API Misuse (Apple)";
-  }
-};
-  
-class VISIBILITY_HIDDEN NilArg : public APIMisuse {
+class VISIBILITY_HIDDEN APIMisuse : public BugType {
 public:
-  virtual ~NilArg() {}
-  
-  virtual const char* getName() const {
-    return "nil argument";
-  }  
-  
-  class Report : public BugReport {
-    std::string Msg;
-    const char* s;
-    SourceRange R;
-  public:
-    
-    Report(NilArg& Desc, ExplodedNode<GRState>* N, 
-           ObjCMessageExpr* ME, unsigned Arg)
-      : BugReport(Desc, N) {
-      
-      Expr* E = ME->getArg(Arg);
-      R = E->getSourceRange();
-      
-      std::ostringstream os;
-      
-      os << "Argument to '" << GetReceiverNameType(ME) << "' method '"
-      << ME->getSelector().getAsString() << "' cannot be nil.";
-      
-      Msg = os.str();
-      s = Msg.c_str();      
-    }
-    
-    virtual ~Report() {}
-    
-    virtual const char* getDescription() const { return s; }
-    
-    virtual void getRanges(BugReporter& BR,
-                           const SourceRange*& B, const SourceRange*& E) {
-      B = &R;
-      E = B+1;
-    }
-  };  
+  APIMisuse(const char* name) : BugType(name, "API Misuse (Apple)") {}
 };
-
   
 class VISIBILITY_HIDDEN BasicObjCFoundationChecks : public GRSimpleAPICheck {
-  NilArg Desc;
+  APIMisuse *BT;
+  BugReporter& BR;
   ASTContext &Ctx;
   GRStateManager* VMgr;
-  
-  typedef std::vector<BugReport*> ErrorsTy;
-  ErrorsTy Errors;
-      
+
   SVal GetSVal(const GRState* St, Expr* E) { return VMgr->GetSVal(St, E); }
       
   bool isNSString(ObjCInterfaceType* T, const char* suffix);
@@ -129,26 +79,26 @@ class VISIBILITY_HIDDEN BasicObjCFoundationChecks : public GRSimpleAPICheck {
   bool CheckNilArg(NodeTy* N, unsigned Arg);
 
 public:
-  BasicObjCFoundationChecks(ASTContext& ctx, GRStateManager* vmgr) 
-    : Ctx(ctx), VMgr(vmgr) {}
-      
-  virtual ~BasicObjCFoundationChecks() {
-    for (ErrorsTy::iterator I = Errors.begin(), E = Errors.end(); I!=E; ++I)
-      delete *I;    
-  }
-  
-  virtual bool Audit(ExplodedNode<GRState>* N, GRStateManager&);
-  
-  virtual void EmitWarnings(BugReporter& BR);
-  
-private:
-  
-  void AddError(BugReport* R) {
-    Errors.push_back(R);
-  }
-  
-  void WarnNilArg(NodeTy* N, ObjCMessageExpr* ME, unsigned Arg) {
-    AddError(new NilArg::Report(Desc, N, ME, Arg));
+  BasicObjCFoundationChecks(ASTContext& ctx, GRStateManager* vmgr,
+                            BugReporter& br) 
+    : BT(0), BR(br), Ctx(ctx), VMgr(vmgr) {}
+        
+  bool Audit(ExplodedNode<GRState>* N, GRStateManager&);
+  
+private:  
+  void WarnNilArg(NodeTy* N, ObjCMessageExpr* ME, unsigned Arg) {    
+    std::string sbuf;
+    llvm::raw_string_ostream os(sbuf);
+    os << "Argument to '" << GetReceiverNameType(ME) << "' method '"
+       << ME->getSelector().getAsString() << "' cannot be nil.";
+    
+    // Lazily create the BugType object for NilArg.  This will be owned
+    // by the BugReporter object 'BR' once we call BR.EmitWarning.
+    if (!BT) BT = new APIMisuse("nil argument");
+    
+    RangedBugReport *R = new RangedBugReport(*BT, os.str().c_str(), N);
+    R->addRange(ME->getArg(Arg)->getSourceRange());
+    BR.EmitReport(R);
   }
 };
   
@@ -157,9 +107,9 @@ private:
 
 GRSimpleAPICheck*
 clang::CreateBasicObjCFoundationChecks(ASTContext& Ctx,
-                                       GRStateManager* VMgr) {
+                                       GRStateManager* VMgr, BugReporter& BR) {
   
-  return new BasicObjCFoundationChecks(Ctx, VMgr);  
+  return new BasicObjCFoundationChecks(Ctx, VMgr, BR);  
 }
 
 
@@ -201,13 +151,6 @@ static inline bool isNil(SVal X) {
 // Error reporting.
 //===----------------------------------------------------------------------===//
 
-
-void BasicObjCFoundationChecks::EmitWarnings(BugReporter& BR) {    
-                 
-  for (ErrorsTy::iterator I=Errors.begin(), E=Errors.end(); I!=E; ++I)    
-    BR.EmitWarning(**I);
-}
-
 bool BasicObjCFoundationChecks::CheckNilArg(NodeTy* N, unsigned Arg) {
   ObjCMessageExpr* ME =
     cast<ObjCMessageExpr>(cast<PostStmt>(N->getLocation()).getStmt());
@@ -307,41 +250,9 @@ bool BasicObjCFoundationChecks::AuditNSString(NodeTy* N,
 //===----------------------------------------------------------------------===//
 
 namespace {
-  
-class VISIBILITY_HIDDEN BadCFNumberCreate : public APIMisuse{
-public:
-  typedef std::vector<BugReport*> AllErrorsTy;
-  AllErrorsTy AllErrors;
-  
-  virtual const char* getName() const {
-    return "Bad use of CFNumberCreate";
-  }
-    
-  virtual void EmitWarnings(BugReporter& BR) {
-    // FIXME: Refactor this method.
-    for (AllErrorsTy::iterator I=AllErrors.begin(), E=AllErrors.end(); I!=E;++I)
-      BR.EmitWarning(**I);
-  }
-};
 
-  // FIXME: This entire class should be refactored into the common
-  //  BugReporter classes.
-class VISIBILITY_HIDDEN StrBugReport : public RangedBugReport {
-  std::string str;
-  const char* cstr;
-public:
-  StrBugReport(BugType& D, ExplodedNode<GRState>* N, std::string s)
-    : RangedBugReport(D, N), str(s) {
-      cstr = str.c_str();
-    }
-  
-  virtual const char* getDescription() const { return cstr; }
-};
-
-  
 class VISIBILITY_HIDDEN AuditCFNumberCreate : public GRSimpleAPICheck {
-  // FIXME: Who should own this?
-  BadCFNumberCreate Desc;
+  APIMisuse* BT;
   
   // FIXME: Either this should be refactored into GRSimpleAPICheck, or
   //   it should always be passed with a call to Audit.  The latter
@@ -349,24 +260,19 @@ class VISIBILITY_HIDDEN AuditCFNumberCreate : public GRSimpleAPICheck {
   ASTContext& Ctx;
   IdentifierInfo* II;
   GRStateManager* VMgr;
+  BugReporter& BR;
     
   SVal GetSVal(const GRState* St, Expr* E) { return VMgr->GetSVal(St, E); }
   
 public:
-
-  AuditCFNumberCreate(ASTContext& ctx, GRStateManager* vmgr) 
-  : Ctx(ctx), II(&Ctx.Idents.get("CFNumberCreate")), VMgr(vmgr) {}
-  
-  virtual ~AuditCFNumberCreate() {}
+  AuditCFNumberCreate(ASTContext& ctx, GRStateManager* vmgr, BugReporter& br) 
+  : BT(0), Ctx(ctx), II(&Ctx.Idents.get("CFNumberCreate")), VMgr(vmgr), BR(br){}
   
-  virtual bool Audit(ExplodedNode<GRState>* N, GRStateManager&);
+  ~AuditCFNumberCreate() {}
   
-  virtual void EmitWarnings(BugReporter& BR) {
-    Desc.EmitWarnings(BR);
-  }
+  bool Audit(ExplodedNode<GRState>* N, GRStateManager&);
   
 private:
-  
   void AddError(const TypedRegion* R, Expr* Ex, ExplodedNode<GRState> *N,
                 uint64_t SourceSize, uint64_t TargetSize, uint64_t NumberKind);  
 };
@@ -537,8 +443,9 @@ void AuditCFNumberCreate::AddError(const TypedRegion* R, Expr* Ex,
                                    ExplodedNode<GRState> *N,
                                    uint64_t SourceSize, uint64_t TargetSize,
                                    uint64_t NumberKind) {
-
-  std::ostringstream os;
+  
+  std::string sbuf;
+  llvm::raw_string_ostream os(sbuf);
   
   os << (SourceSize == 8 ? "An " : "A ")
      << SourceSize << " bit integer is used to initialize a CFNumber "
@@ -553,16 +460,18 @@ void AuditCFNumberCreate::AddError(const TypedRegion* R, Expr* Ex,
     os << (SourceSize - TargetSize)
        << " bits of the input integer will be lost.";
          
-  StrBugReport* B = new StrBugReport(Desc, N, os.str());
-  B->addRange(Ex->getSourceRange());
-  Desc.AllErrors.push_back(B);
+  // Lazily create the BugType object.  This will be owned
+  // by the BugReporter object 'BR' once we call BR.EmitWarning.
+  if (!BT) BT = new APIMisuse("Bad use of CFNumberCreate");
+  RangedBugReport *report = new RangedBugReport(*BT, os.str().c_str(), N);
+  report->addRange(Ex->getSourceRange());
+  BR.EmitReport(report);
 }
 
 GRSimpleAPICheck*
 clang::CreateAuditCFNumberCreate(ASTContext& Ctx,
-                                 GRStateManager* VMgr) {
-  
-  return new AuditCFNumberCreate(Ctx, VMgr);  
+                                 GRStateManager* VMgr, BugReporter& BR) {  
+  return new AuditCFNumberCreate(Ctx, VMgr, BR);
 }
 
 //===----------------------------------------------------------------------===//
@@ -571,12 +480,13 @@ clang::CreateAuditCFNumberCreate(ASTContext& Ctx,
 void clang::RegisterAppleChecks(GRExprEngine& Eng) {
   ASTContext& Ctx = Eng.getContext();
   GRStateManager* VMgr = &Eng.getStateManager();
+  BugReporter &BR = Eng.getBugReporter();
 
-  Eng.AddCheck(CreateBasicObjCFoundationChecks(Ctx, VMgr),
+  Eng.AddCheck(CreateBasicObjCFoundationChecks(Ctx, VMgr, BR),
                Stmt::ObjCMessageExprClass);
 
-  Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr),
+  Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr, BR),
                Stmt::CallExprClass);
   
-  Eng.Register(CreateNSErrorCheck());
+  RegisterNSErrorChecks(BR, Eng);
 }
diff --git a/lib/Analysis/BasicObjCFoundationChecks.h b/lib/Analysis/BasicObjCFoundationChecks.h
index 3ad2ca57aa..6c594ea721 100644
--- a/lib/Analysis/BasicObjCFoundationChecks.h
+++ b/lib/Analysis/BasicObjCFoundationChecks.h
@@ -29,15 +29,18 @@ namespace clang {
 class GRSimpleAPICheck;
 class ASTContext;
 class GRStateManager;  
-class BugType;
+class BugReporter;
+class GRExprEngine;
   
 GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx,
-                                                  GRStateManager* VMgr);
+                                                  GRStateManager* VMgr,
+                                                  BugReporter& BR);
   
 GRSimpleAPICheck* CreateAuditCFNumberCreate(ASTContext& Ctx,
-                                            GRStateManager* VMgr);
+                                            GRStateManager* VMgr,
+                                            BugReporter& BR);
   
-BugType* CreateNSErrorCheck();
+void RegisterNSErrorChecks(BugReporter& BR, GRExprEngine &Eng);
   
 } // end clang namespace
 
diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp
index f59f89a1a8..c42218e506 100644
--- a/lib/Analysis/BugReporter.cpp
+++ b/lib/Analysis/BugReporter.cpp
@@ -23,24 +23,14 @@
 #include "clang/Analysis/PathDiagnostic.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLExtras.h"
 #include <sstream>
 
 using namespace clang;
 
-BugReporter::~BugReporter() {}
-GRBugReporter::~GRBugReporter() {}
-BugReporterData::~BugReporterData() {}
-BugType::~BugType() {}
-BugReport::~BugReport() {}
-RangedBugReport::~RangedBugReport() {}
-
-ExplodedGraph<GRState>& GRBugReporter::getGraph() {
-  return Eng.getGraph();
-}
-
-GRStateManager& GRBugReporter::getStateManager() { 
-  return Eng.getStateManager();
-}
+//===----------------------------------------------------------------------===//
+// static functions.
+//===----------------------------------------------------------------------===//
 
 static inline Stmt* GetStmt(const ProgramPoint& P) {
   if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {
@@ -85,7 +75,6 @@ static inline Stmt* GetStmt(const ExplodedNode<GRState>* N) {
   return isa<BlockEntrance>(ProgP) ? GetLastStmt(N) : GetStmt(ProgP);
 }
 
-
 static void ExecutionContinues(std::ostringstream& os, SourceManager& SMgr,
                                const Stmt* S) {
   
@@ -97,36 +86,42 @@ static void ExecutionContinues(std::ostringstream& os, SourceManager& SMgr,
     os << ' ';
   
   os << "Execution continues on line "
-     << SMgr.getInstantiationLineNumber(S->getLocStart()) << '.';
+  << SMgr.getInstantiationLineNumber(S->getLocStart()) << '.';
 }
 
-
 static inline void ExecutionContinues(std::ostringstream& os,
                                       SourceManager& SMgr,
                                       const ExplodedNode<GRState>* N) {
-
   ExecutionContinues(os, SMgr, GetStmt(N->getLocation()));
 }
 
 static inline void ExecutionContinues(std::ostringstream& os,
                                       SourceManager& SMgr,
                                       const CFGBlock* B) {  
-  
   ExecutionContinues(os, SMgr, GetStmt(B));
 }
 
+//===----------------------------------------------------------------------===//
+// Methods for BugType and subclasses.
+//===----------------------------------------------------------------------===//
+BugType::~BugType() {}
+void BugType::FlushReports(BugReporter &BR) {}
 
-Stmt* BugReport::getStmt(BugReporter& BR) const {
-  
+//===----------------------------------------------------------------------===//
+// Methods for BugReport and subclasses.
+//===----------------------------------------------------------------------===//
+BugReport::~BugReport() {}
+RangedBugReport::~RangedBugReport() {}
+
+Stmt* BugReport::getStmt(BugReporter& BR) const {  
   ProgramPoint ProgP = EndNode->getLocation();  
   Stmt *S = NULL;
   
-  if (BlockEntrance* BE = dyn_cast<BlockEntrance>(&ProgP))
-    if (BE->getBlock() == &BR.getCFG()->getExit())
-      S = GetLastStmt(EndNode);
-  if (!S)
-    S = GetStmt(ProgP);  
-
+  if (BlockEntrance* BE = dyn_cast<BlockEntrance>(&ProgP)) {
+    if (BE->getBlock() == &BR.getCFG()->getExit()) S = GetLastStmt(EndNode);
+  }
+  if (!S) S = GetStmt(ProgP);  
+  
   return S;  
 }
 
@@ -144,7 +139,7 @@ BugReport::getEndPath(BugReporter& BR,
   
   const SourceRange *Beg, *End;
   getRanges(BR, Beg, End);  
-
+  
   for (; Beg != End; ++Beg)
     P->addRange(*Beg);
   
@@ -163,17 +158,12 @@ void BugReport::getRanges(BugReporter& BR, const SourceRange*& beg,
     beg = end = 0;
 }
 
-FullSourceLoc BugReport::getLocation(SourceManager& Mgr) {
-  
-  if (!EndNode)
-    return FullSourceLoc();
-  
-  Stmt* S = GetStmt(EndNode);
-  
-  if (!S)
-    return FullSourceLoc();
-  
-  return FullSourceLoc(S->getLocStart(), Mgr);
+SourceLocation BugReport::getLocation() const {  
+  if (EndNode)
+    if (Stmt* S = GetStmt(EndNode))
+      return S->getLocStart();
+
+  return FullSourceLoc();
 }
 
 PathDiagnosticPiece* BugReport::VisitNode(const ExplodedNode<GRState>* N,
@@ -183,35 +173,101 @@ PathDiagnosticPiece* BugReport::VisitNode(const ExplodedNode<GRState>* N,
   return NULL;
 }
 
-static std::pair<ExplodedGraph<GRState>*, ExplodedNode<GRState>*>
-MakeReportGraph(const ExplodedGraph<GRState>* G,
-                const ExplodedNode<GRState>* N) {
-  
-  llvm::OwningPtr< ExplodedGraph<GRState> > GTrim(G->Trim(&N, &N+1));    
-    
-  // Find the error node in the trimmed graph.  
-  
-  const ExplodedNode<GRState>* NOld = N;
-  N = 0;
-  
-  for (ExplodedGraph<GRState>::node_iterator
-       I = GTrim->nodes_begin(), E = GTrim->nodes_end(); I != E; ++I) {
+//===----------------------------------------------------------------------===//
+// Methods for BugReporter and subclasses.
+//===----------------------------------------------------------------------===//
+
+BugReportEquivClass::~BugReportEquivClass() {
+  for (iterator I=begin(), E=end(); I!=E; ++I) delete *I;  
+}
+
+GRBugReporter::~GRBugReporter() { FlushReports(); }
+BugReporterData::~BugReporterData() {}
+
+ExplodedGraph<GRState>&
+GRBugReporter::getGraph() { return Eng.getGraph(); }
+
+GRStateManager&
+GRBugReporter::getStateManager() { return Eng.getStateManager(); }
+
+BugReporter::~BugReporter() { FlushReports(); }
+
+void BugReporter::FlushReports() {
+  if (BugTypes.isEmpty())
+    return;
+
+  // First flush the warnings for each BugType.  This may end up creating new
+  // warnings and new BugTypes.  Because ImmutableSet is a functional data
+  // structure, we do not need to worry about the iterators being invalidated.
+  for (BugTypesTy::iterator I=BugTypes.begin(), E=BugTypes.end(); I!=E; ++I)
+    const_cast<BugType*>(*I)->FlushReports(*this);
+
+  // Iterate through BugTypes a second time.  BugTypes may have been updated
+  // with new BugType objects and new warnings.
+  for (BugTypesTy::iterator I=BugTypes.begin(), E=BugTypes.end(); I!=E; ++I) {
+    BugType *BT = const_cast<BugType*>(*I);
+
+    typedef llvm::FoldingSet<BugReportEquivClass> SetTy;
+    SetTy& EQClasses = BT->EQClasses;
+
+    for (SetTy::iterator EI=EQClasses.begin(), EE=EQClasses.end(); EI!=EE;++EI){
+      BugReportEquivClass& EQ = *EI;
+      FlushReport(EQ);
+    }
     
-    if (I->getState() == NOld->getState() &&
-        I->getLocation() == NOld->getLocation()) {
-      N = &*I;
-      break;
-    }    
+    // Delete the BugType object.  This will also delete the equivalence
+    // classes.
+    delete BT;
   }
+
+  // Remove all references to the BugType objects.
+  BugTypes = F.GetEmptySet();
+}
+
+//===----------------------------------------------------------------------===//
+// PathDiagnostics generation.
+//===----------------------------------------------------------------------===//
+
+static std::pair<ExplodedGraph<GRState>*,
+                 std::pair<ExplodedNode<GRState>*, unsigned> >
+MakeReportGraph(const ExplodedGraph<GRState>* G,
+                const ExplodedNode<GRState>** NStart,
+                const ExplodedNode<GRState>** NEnd) {
+  
+  // Create the trimmed graph.  It will contain the shortest paths from the
+  // error nodes to the root.  In the new graph we should only have one 
+  // error node unless there are two or more error nodes with the same minimum
+  // path length.
+  ExplodedGraph<GRState>* GTrim;
+  InterExplodedGraphMap<GRState>* NMap;
+  llvm::tie(GTrim, NMap) = G->Trim(NStart, NEnd);
+  
+  // Create owning pointers for GTrim and NMap just to ensure that they are
+  // released when this function exists.
+  llvm::OwningPtr<ExplodedGraph<GRState> > AutoReleaseGTrim(GTrim);
+  llvm::OwningPtr<InterExplodedGraphMap<GRState> > AutoReleaseNMap(NMap);
+  
+  // Find the (first) error node in the trimmed graph.  We just need to consult
+  // the node map (NMap) which maps from nodes in the original graph to nodes
+  // in the new graph.
+  const ExplodedNode<GRState>* N = 0;
+  unsigned NodeIndex = 0;
+
+  for (const ExplodedNode<GRState>** I = NStart; I != NEnd; ++I)
+    if ((N = NMap->getMappedNode(*I))) {
+      NodeIndex = (I - NStart) / sizeof(*I);
+      break;
+    }
   
-  assert(N);
-    
-  // Create a new graph with a single path.
+  assert(N && "No error node found in the trimmed graph.");
+
+  // Create a new (third!) graph with a single path.  This is the graph
+  // that will be returned to the caller.
   ExplodedGraph<GRState> *GNew =
-    new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(),
-                               GTrim->getContext());
-                                     
-  // Sometimes TrimGraph can contain a cycle.  Perform a reverse DFS
+  new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(),
+                             GTrim->getContext());
+  
+  // Sometimes the trimmed graph can contain a cycle.  Perform a reverse DFS
   // to the root node, and then construct a new graph that contains only
   // a single path.
   llvm::DenseMap<const void*,unsigned> Visited;
@@ -238,13 +294,13 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
          E=Node->pred_end(); I!=E; ++I)
       WS.push_back(*I);
   }
-
+  
   assert (Root);
   
   // Now walk from the root down the DFS path, always taking the successor
   // with the lowest number.
   ExplodedNode<GRState> *Last = 0, *First = 0;  
-    
+  
   for ( N = Root ;;) {
     // Lookup the number associated with the current node.
     llvm::DenseMap<const void*,unsigned>::iterator I = Visited.find(N);
@@ -253,20 +309,20 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
     // Create the equivalent node in the new graph with the same state
     // and location.
     ExplodedNode<GRState>* NewN =
-      GNew->getNode(N->getLocation(), N->getState());    
-
+    GNew->getNode(N->getLocation(), N->getState());    
+    
     // Link up the new node with the previous node.
     if (Last)
       NewN->addPredecessor(Last);
     
     Last = NewN;
-
+    
     // Are we at the final node?
     if (I->second == 0) {
       First = NewN;
       break;
     }
-
+    
     // Find the next successor node.  We choose the node that is marked
     // with the lowest DFS number.
     ExplodedNode<GRState>::const_succ_iterator SI = N->succ_begin();
@@ -274,7 +330,7 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
     N = 0;
     
     for (unsigned MinVal = 0; SI != SE; ++SI) {
-
+      
       I = Visited.find(*SI);
       
       if (I == Visited.end())
@@ -285,12 +341,12 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
         MinVal = I->second;
       }
     }
-
+    
     assert (N);
   }
-
+  
   assert (First);
-  return std::make_pair(GNew, First);
+  return std::make_pair(GNew, std::make_pair(First, NodeIndex));
 }
 
 static const VarDecl*
@@ -300,12 +356,12 @@ GetMostRecentVarDeclBinding(const ExplodedNode<GRState>* N,
   for ( ; N ; N = N->pred_empty() ? 0 : *N->pred_begin()) {
     
     ProgramPoint P = N->getLocation();
-
+    
     if (!isa<PostStmt>(P))
       continue;
     
     DeclRefExpr* DR = dyn_cast<DeclRefExpr>(cast<PostStmt>(P).getStmt());
-
+    
     if (!DR)
       continue;
     
@@ -474,25 +530,37 @@ public:
 } // end anonymous namespace
 
 void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
-                                           BugReport& R) {
-
-  const ExplodedNode<GRState>* N = R.getEndNode();
+                                           BugReportEquivClass& EQ) {
+  
+  std::vector<const ExplodedNode<GRState>*> Nodes;
 
-  if (!N) return;
+  for (BugReportEquivClass::iterator I=EQ.begin(), E=EQ.end(); I!=E; ++I) {
+    const ExplodedNode<GRState>* N = I->getEndNode();
+    if (N) Nodes.push_back(N);
+  }
+  
+  if (Nodes.empty())
+    return;
   
   // Construct a new graph that contains only a single path from the error
-  // node to a root.
+  // node to a root.  
+  const std::pair<ExplodedGraph<GRState>*,
+                  std::pair<ExplodedNode<GRState>*, unsigned> >&
+    GPair = MakeReportGraph(&getGraph(), &Nodes[0], &Nodes[0] + Nodes.size());
+  
+  // Find the BugReport with the original location.
+  BugReport *R = 0;
+  unsigned i = 0;
+  for (BugReportEquivClass::iterator I=EQ.begin(), E=EQ.end(); I!=E; ++I, ++i)
+    if (i == GPair.second.second) { R = *I; break; }
   
-  const std::pair<ExplodedGraph<GRState>*, ExplodedNode<GRState>*>
-    GPair = MakeReportGraph(&getGraph(), N);
+  assert(R && "No original report found for sliced graph.");
   
   llvm::OwningPtr<ExplodedGraph<GRState> > ReportGraph(GPair.first);
-  assert(GPair.second->getLocation() == N->getLocation());
-  N = GPair.second;
+  const ExplodedNode<GRState> *N = GPair.second.first;
 
-  // Start building the path diagnostic...
-  
-  if (PathDiagnosticPiece* Piece = R.getEndPath(*this, N))
+  // Start building the path diagnostic...  
+  if (PathDiagnosticPiece* Piece = R->getEndPath(*this, N))
     PD.push_back(Piece);
   else
     return;
@@ -686,7 +754,7 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
       }
     }
 
-    if (PathDiagnosticPiece* p = R.VisitNode(N, NextNode, *ReportGraph, *this))
+    if (PathDiagnosticPiece* p = R->VisitNode(N, NextNode, *ReportGraph, *this))
       PD.push_front(p);
     
     if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {      
@@ -699,37 +767,41 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
 }
 
 
-bool BugTypeCacheLocation::isCached(BugReport& R) {
-  
-  const ExplodedNode<GRState>* N = R.getEndNode();
-  
-  if (!N)
-    return false;
-
-  // Cache the location of the error.  Don't emit the same
-  // warning for the same error type that occurs at the same program
-  // location but along a different path.
-  
-  return isCached(N->getLocation());
+void BugReporter::Register(BugType *BT) {
+  BugTypes = F.Add(BugTypes, BT);
 }
 
-bool BugTypeCacheLocation::isCached(ProgramPoint P) {
-  if (CachedErrors.count(P))
-    return true;
+void BugReporter::EmitReport(BugReport* R) {  
+  // Compute the bug report's hash to determine its equivalence class.
+  llvm::FoldingSetNodeID ID;
+  R->Profile(ID);
   
-  CachedErrors.insert(P);  
-  return false;
+  // Lookup the equivance class.  If there isn't one, create it.  
+  BugType& BT = R->getBugType();
+  Register(&BT);
+  void *InsertPos;
+  BugReportEquivClass* EQ = BT.EQClasses.FindNodeOrInsertPos(ID, InsertPos);  
+  
+  if (!EQ) {
+    EQ = new BugReportEquivClass(R);
+    BT.EQClasses.InsertNode(EQ, InsertPos);
+  }
+  else
+    EQ->AddReport(R);
 }
 
-void BugReporter::EmitWarning(BugReport& R) {
-
-  if (R.getBugType().isCached(R))
-    return;
-
-  llvm::OwningPtr<PathDiagnostic> D(new PathDiagnostic(R.getName(),
-                                                       R.getDescription(),
-                                                       R.getCategory()));
-  GeneratePathDiagnostic(*D.get(), R);
+void BugReporter::FlushReport(BugReportEquivClass& EQ) {
+  assert(!EQ.Reports.empty());
+  BugReport &R = **EQ.begin();
+  
+  // FIXME: Make sure we use the 'R' for the path that was actually used.
+  // Probably doesn't make a difference in practice.  
+  BugType& BT = R.getBugType();
+  
+  llvm::OwningPtr<PathDiagnostic> D(new PathDiagnostic(R.getBugType().getName(),
+                                                R.getDescription(),
+                                                BT.getCategory()));
+  GeneratePathDiagnostic(*D.get(), EQ);
   
   // Get the meta data.
   std::pair<const char**, const char**> Meta = R.getExtraDescriptiveText();
@@ -740,9 +812,9 @@ void BugReporter::EmitWarning(BugReport& R) {
   const SourceRange *Beg = 0, *End = 0;
   R.getRanges(*this, Beg, End);    
   Diagnostic& Diag = getDiagnostic();
-  FullSourceLoc L = R.getLocation(getSourceManager());  
-  const char *msg = PD ? R.getBugType().getName() : R.getDescription();  
-  unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning, msg);
+  FullSourceLoc L(R.getLocation(), getSourceManager());  
+  const std::string &msg = PD ? R.getBugType().getName() : R.getDescription();  
+  unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning, msg.c_str());
 
   switch (End-Beg) {
     default: assert(0 && "Don't handle this many ranges yet!");
@@ -770,73 +842,15 @@ void BugReporter::EmitBasicReport(const char* name, const char* str,
                                   SourceRange* RBeg, unsigned NumRanges) {
   EmitBasicReport(name, "", str, Loc, RBeg, NumRanges);
 }
-  
+
 void BugReporter::EmitBasicReport(const char* name, const char* category,
                                   const char* str, SourceLocation Loc,
                                   SourceRange* RBeg, unsigned NumRanges) {
   
-  SimpleBugType BT(name, category, 0);
-  DiagCollector C(BT);
-  Diagnostic& Diag = getDiagnostic();
-  
-  DiagnosticClient *OldClient = Diag.getClient();
-  Diag.setClient(&C);
+  // 'BT' will be owned by BugReporter as soon as we call 'EmitReport'.
+  BugType *BT = new BugType(name, category);
   FullSourceLoc L = getContext().getFullLoc(Loc);
-  unsigned DiagID = Diag.getCustomDiagID(Diagnostic::Warning, str);
-  
-  switch (NumRanges) {
-  default: assert(0 && "Don't handle this many ranges yet!");
-  case 0: Diag.Report(L, DiagID); break;
-  case 1: Diag.Report(L, DiagID) << RBeg[0]; break;
-  case 2: Diag.Report(L, DiagID) << RBeg[0] << RBeg[1]; break;
-  case 3: Diag.Report(L, DiagID) << RBeg[0] << RBeg[1] << RBeg[2]; break;
-  }
-  
-  Diag.setClient(OldClient);
-  
-  for (DiagCollector::iterator I = C.begin(), E = C.end(); I != E; ++I)
-    EmitWarning(*I);
-}
-
-void DiagCollector::HandleDiagnostic(Diagnostic::Level DiagLevel,
-                                     const DiagnosticInfo &Info) {
-  
-  // FIXME: Use a map from diag::kind to BugType, instead of having just
-  //  one BugType.
-  const char *Desc = Info.getDiags()->getDescription(Info.getID());
-  Reports.push_back(DiagBugReport(Desc, D, Info.getLocation()));
-  DiagBugReport& R = Reports.back();
-  
-  for (unsigned i = 0, e = Info.getNumRanges(); i != e; ++i)
-    R.addRange(Info.getRange(i));
-  
-  // FIXME: This is losing/ignoring formatting.
-  for (unsigned i = 0, e = Info.getNumArgs(); i != e; ++i) {
-    switch (Info.getArgKind(i)) {
-      case Diagnostic::ak_std_string:   
-        R.addString(Info.getArgStdStr(i));
-        break;
-      case Diagnostic::ak_c_string:   
-        R.addString(Info.getArgCStr(i));
-        break;
-      case Diagnostic::ak_sint:
-        R.addString(llvm::itostr(Info.getArgSInt(i)));
-        break;
-      case Diagnostic::ak_uint:
-        R.addString(llvm::utostr_32(Info.getArgUInt(i)));
-        break;
-      case Diagnostic::ak_identifierinfo:
-        R.addString(Info.getArgIdentifier(i)->getName());
-        break;
-      case Diagnostic::ak_qualtype:
-      case Diagnostic::ak_declarationname: {
-        llvm::SmallString<64> Str;
-        Info.getDiags()->ConvertArgToString(Info.getArgKind(i),
-                                            Info.getRawArg(i), 0, 0, 0, 0, Str);
-        R.addString(std::string(Str.begin(), Str.end()));
-        break;
-      }
-    }
-  }
+  RangedBugReport *R = new DiagBugReport(*BT, str, L);
+  for ( ; NumRanges > 0 ; --NumRanges, ++RBeg) R->addRange(*RBeg);
+  EmitReport(R);
 }
-                                  
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index ab9d409dd2..e42fa8614c 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -1333,7 +1333,7 @@ public:
       delete I->second;
   }
   
-  virtual void RegisterChecks(GRExprEngine& Eng);
+  void RegisterChecks(BugReporter &BR);
  
   virtual void RegisterPrinters(std::vector<GRState::Printer*>& Printers) {
     Printers.push_back(new BindingsPrinter());
@@ -2154,91 +2154,73 @@ namespace {
   // Bug Descriptions. //
   //===-------------===//  
   
-  class VISIBILITY_HIDDEN CFRefBug : public BugTypeCacheLocation {
+  class VISIBILITY_HIDDEN CFRefBug : public BugType {
   protected:
     CFRefCount& TF;
-    
+
+    CFRefBug(CFRefCount* tf, const char* name) 
+      : BugType(name, "Memory (Core Foundation/Objective-C)"), TF(*tf) {}    
   public:
-    CFRefBug(CFRefCount& tf) : TF(tf) {}
     
     CFRefCount& getTF() { return TF; }
     const CFRefCount& getTF() const { return TF; }
 
+    // FIXME: Eventually remove.
+    virtual const char* getDescription() const = 0;
+    
     virtual bool isLeak() const { return false; }
-
-    const char* getCategory() const { 
-      return "Memory (Core Foundation/Objective-C)";
-    }
   };
   
   class VISIBILITY_HIDDEN UseAfterRelease : public CFRefBug {
   public:
-    UseAfterRelease(CFRefCount& tf) : CFRefBug(tf) {}
+    UseAfterRelease(CFRefCount* tf)
+      : CFRefBug(tf, "use-after-release") {}
     
-    virtual const char* getName() const {
-      return "use-after-release";
-    }
-    virtual const char* getDescription() const {
+    const char* getDescription() const {
       return "Reference-counted object is used after it is released.";
     }
     
-    virtual void EmitWarnings(BugReporter& BR);
+    virtual void FlushReports(BugReporter& BR);
   };
   
   class VISIBILITY_HIDDEN BadRelease : public CFRefBug {
   public:
-    BadRelease(CFRefCount&a