[analyzer] [NFC] Clean up messy handling of bug categories in RetainCountChecker

https://reviews.llvm.org/D56887

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

4 files changed, 112 insertions, 150 deletions

diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 05892b6524..a39cd05128 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -29,6 +29,10 @@ const RefVal *getRefBinding(ProgramStateRef State, SymbolRef Sym) {
   return State->get<RefBindings>(Sym);
 }
 
+} // end namespace retaincountchecker
+} // end namespace ento
+} // end namespace clang
+
 static ProgramStateRef setRefBinding(ProgramStateRef State, SymbolRef Sym,
                                      RefVal Val) {
   assert(Sym != nullptr);
@@ -39,73 +43,6 @@ static ProgramStateRef removeRefBinding(ProgramStateRef State, SymbolRef Sym) {
   return State->remove<RefBindings>(Sym);
 }
 
-class UseAfterRelease : public RefCountBug {
-public:
-  UseAfterRelease(const CheckerBase *checker)
-      : RefCountBug(checker, "Use-after-release") {}
-
-  const char *getDescription() const override {
-    return "Reference-counted object is used after it is released";
-  }
-};
-
-class BadRelease : public RefCountBug {
-public:
-  BadRelease(const CheckerBase *checker) : RefCountBug(checker, "Bad release") {}
-
-  const char *getDescription() const override {
-    return "Incorrect decrement of the reference count of an object that is "
-           "not owned at this point by the caller";
-  }
-};
-
-class DeallocNotOwned : public RefCountBug {
-public:
-  DeallocNotOwned(const CheckerBase *checker)
-      : RefCountBug(checker, "-dealloc sent to non-exclusively owned object") {}
-
-  const char *getDescription() const override {
-    return "-dealloc sent to object that may be referenced elsewhere";
-  }
-};
-
-class OverAutorelease : public RefCountBug {
-public:
-  OverAutorelease(const CheckerBase *checker)
-      : RefCountBug(checker, "Object autoreleased too many times") {}
-
-  const char *getDescription() const override {
-    return "Object autoreleased too many times";
-  }
-};
-
-class ReturnedNotOwnedForOwned : public RefCountBug {
-public:
-  ReturnedNotOwnedForOwned(const CheckerBase *checker)
-      : RefCountBug(checker, "Method should return an owned object") {}
-
-  const char *getDescription() const override {
-    return "Object with a +0 retain count returned to caller where a +1 "
-           "(owning) retain count is expected";
-  }
-};
-
-class Leak : public RefCountBug {
-public:
-  // Leaks should not be reported if they are post-dominated by a sink.
-  Leak(const CheckerBase *checker, StringRef name)
-      : RefCountBug(checker, name,
-                    /*SuppressOnSink=*/true) {}
-
-  const char *getDescription() const override { return ""; }
-
-  bool isLeak() const override { return true; }
-};
-
-} // end namespace retaincountchecker
-} // end namespace ento
-} // end namespace clang
-
 void RefVal::print(raw_ostream &Out) const {
   if (!T.isNull())
     Out << "Tracked " << T.getAsString() << " | ";
@@ -414,20 +351,6 @@ void RetainCountChecker::checkPostCall(const CallEvent &Call,
   checkSummary(*Summ, Call, C);
 }
 
-RefCountBug *
-RetainCountChecker::getLeakWithinFunctionBug(const LangOptions &LOpts) const {
-  if (!leakWithinFunction)
-    leakWithinFunction.reset(new Leak(this, "Leak"));
-  return leakWithinFunction.get();
-}
-
-RefCountBug *
-RetainCountChecker::getLeakAtReturnBug(const LangOptions &LOpts) const {
-  if (!leakAtReturn)
-    leakAtReturn.reset(new Leak(this, "Leak of returned object"));
-  return leakAtReturn.get();
-}
-
 /// GetReturnType - Used to get the return type of a message expression or
 ///  function call with the intention of affixing that type to a tracked symbol.
 ///  While the return type can be queried directly from RetEx, when
@@ -879,6 +802,20 @@ ProgramStateRef RetainCountChecker::updateSymbol(ProgramStateRef state,
   return setRefBinding(state, sym, V);
 }
 
+const RefCountBug &
+RetainCountChecker::errorKindToBugKind(RefVal::Kind ErrorKind) const {
+  switch (ErrorKind) {
+    case RefVal::ErrorUseAfterRelease:
+      return useAfterRelease;
+    case RefVal::ErrorReleaseNotOwned:
+      return releaseNotOwned;
+    case RefVal::ErrorDeallocNotOwned:
+      return deallocNotOwned;
+    default:
+      llvm_unreachable("Unhandled error.");
+  }
+}
+
 void RetainCountChecker::processNonLeakError(ProgramStateRef St,
                                              SourceRange ErrorRange,
                                              RefVal::Kind ErrorKind,
@@ -898,30 +835,8 @@ void RetainCountChecker::processNonLeakError(ProgramStateRef St,
   if (!N)
     return;
 
-  RefCountBug *BT;
-  switch (ErrorKind) {
-    default:
-      llvm_unreachable("Unhandled error.");
-    case RefVal::ErrorUseAfterRelease:
-      if (!useAfterRelease)
-        useAfterRelease.reset(new UseAfterRelease(this));
-      BT = useAfterRelease.get();
-      break;
-    case RefVal::ErrorReleaseNotOwned:
-      if (!releaseNotOwned)
-        releaseNotOwned.reset(new BadRelease(this));
-      BT = releaseNotOwned.get();
-      break;
-    case RefVal::ErrorDeallocNotOwned:
-      if (!deallocNotOwned)
-        deallocNotOwned.reset(new DeallocNotOwned(this));
-      BT = deallocNotOwned.get();
-      break;
-  }
-
-  assert(BT);
   auto report = llvm::make_unique<RefCountReport>(
-      *BT, C.getASTContext().getLangOpts(), N, Sym);
+      errorKindToBugKind(ErrorKind), C.getASTContext().getLangOpts(), N, Sym);
   report->addRange(ErrorRange);
   C.emitReport(std::move(report));
 }
@@ -1124,8 +1039,8 @@ ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
         ExplodedNode *N = C.addTransition(state, Pred, &ReturnOwnLeakTag);
         if (N) {
           const LangOptions &LOpts = C.getASTContext().getLangOpts();
-          auto R = llvm::make_unique<RefLeakReport>(
-              *getLeakAtReturnBug(LOpts), LOpts, N, Sym, C);
+          auto R =
+              llvm::make_unique<RefLeakReport>(leakAtReturn, LOpts, N, Sym, C);
           C.emitReport(std::move(R));
         }
         return N;
@@ -1149,11 +1064,8 @@ ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
 
         ExplodedNode *N = C.addTransition(state, Pred, &ReturnNotOwnedTag);
         if (N) {
-          if (!returnNotOwnedForOwned)
-            returnNotOwnedForOwned.reset(new ReturnedNotOwnedForOwned(this));
-
           auto R = llvm::make_unique<RefCountReport>(
-              *returnNotOwnedForOwned, C.getASTContext().getLangOpts(), N, Sym);
+              returnNotOwnedForOwned, C.getASTContext().getLangOpts(), N, Sym);
           C.emitReport(std::move(R));
         }
         return N;
@@ -1305,12 +1217,9 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state,
       os << "but ";
     os << "has a +" << V.getCount() << " retain count";
 
-    if (!overAutorelease)
-      overAutorelease.reset(new OverAutorelease(this));
-
     const LangOptions &LOpts = Ctx.getASTContext().getLangOpts();
-    auto R = llvm::make_unique<RefCountReport>(*overAutorelease, LOpts, N, Sym,
-                                            os.str());
+    auto R = llvm::make_unique<RefCountReport>(overAutorelease, LOpts, N, Sym,
+                                               os.str());
     Ctx.emitReport(std::move(R));
   }
 
@@ -1356,11 +1265,8 @@ RetainCountChecker::processLeaks(ProgramStateRef state,
 
   if (N) {
     for (SymbolRef L : Leaked) {
-      RefCountBug *BT = Pred ? getLeakWithinFunctionBug(LOpts)
-                          : getLeakAtReturnBug(LOpts);
-      assert(BT && "BugType not initialized.");
-
-      Ctx.emitReport(llvm::make_unique<RefLeakReport>(*BT, LOpts, N, L, Ctx));
+      const RefCountBug &BT = Pred ? leakWithinFunction : leakAtReturn;
+      Ctx.emitReport(llvm::make_unique<RefLeakReport>(BT, LOpts, N, L, Ctx));
     }
   }
 
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
index 280a672f70..a04f9aba82 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
@@ -251,10 +251,14 @@ class RetainCountChecker
                     check::RegionChanges,
                     eval::Assume,
                     eval::Call > {
-  mutable std::unique_ptr<RefCountBug> useAfterRelease, releaseNotOwned;
-  mutable std::unique_ptr<RefCountBug> deallocNotOwned;
-  mutable std::unique_ptr<RefCountBug> overAutorelease, returnNotOwnedForOwned;
-  mutable std::unique_ptr<RefCountBug> leakWithinFunction, leakAtReturn;
+
+  RefCountBug useAfterRelease{this, RefCountBug::UseAfterRelease};
+  RefCountBug releaseNotOwned{this, RefCountBug::ReleaseNotOwned};
+  RefCountBug deallocNotOwned{this, RefCountBug::DeallocNotOwned};
+  RefCountBug overAutorelease{this, RefCountBug::OverAutorelease};
+  RefCountBug returnNotOwnedForOwned{this, RefCountBug::ReturnNotOwnedForOwned};
+  RefCountBug leakWithinFunction{this, RefCountBug::LeakWithinFunction};
+  RefCountBug leakAtReturn{this, RefCountBug::LeakAtReturn};
 
   mutable std::unique_ptr<RetainSummaryManager> Summaries;
 public:
@@ -266,11 +270,7 @@ public:
   /// Track sublcasses of OSObject.
   bool TrackOSObjects = false;
 
-  RetainCountChecker() {}
-
-  RefCountBug *getLeakWithinFunctionBug(const LangOptions &LOpts) const;
-
-  RefCountBug *getLeakAtReturnBug(const LangOptions &LOpts) const;
+  RetainCountChecker() {};
 
   RetainSummaryManager &getSummaryManager(ASTContext &Ctx) const {
     // FIXME: We don't support ARC being turned on and off during one analysis.
@@ -336,6 +336,9 @@ public:
                                RefVal V, ArgEffect E, RefVal::Kind &hasErr,
                                CheckerContext &C) const;
 
+
+  const RefCountBug &errorKindToBugKind(RefVal::Kind ErrorKind) const;
+
   void processNonLeakError(ProgramStateRef St, SourceRange ErrorRange,
                            RefVal::Kind ErrorKind, SymbolRef Sym,
                            CheckerContext &C) const;
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index 85b97ac9c0..0f68f07500 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -19,6 +19,50 @@ using namespace clang;
 using namespace ento;
 using namespace retaincountchecker;
 
+StringRef RefCountBug::bugTypeToName(RefCountBug::RefCountBugType BT) {
+  switch (BT) {
+  case UseAfterRelease:
+    return "Use-after-release";
+  case ReleaseNotOwned:
+    return "Bad release";
+  case DeallocNotOwned:
+    return "-dealloc sent to non-exclusively owned object";
+  case OverAutorelease:
+    return "Object autoreleased too many times";
+  case ReturnNotOwnedForOwned:
+    return "Method should return an owned object";
+  case LeakWithinFunction:
+    return "Leak";
+  case LeakAtReturn:
+    return "Leak of returned object";
+  }
+}
+
+StringRef RefCountBug::getDescription() const {
+  switch (BT) {
+  case UseAfterRelease:
+    return "Reference-counted object is used after it is released";
+  case ReleaseNotOwned:
+    return "Incorrect decrement of the reference count of an object that is "
+           "not owned at this point by the caller";
+  case DeallocNotOwned:
+    return "-dealloc sent to object that may be referenced elsewhere";
+  case OverAutorelease:
+    return "Object autoreleased too many times";
+  case ReturnNotOwnedForOwned:
+    return "Object with a +0 retain count returned to caller where a +1 "
+           "(owning) retain count is expected";
+  case LeakWithinFunction:
+  case LeakAtReturn:
+    return "";
+  }
+}
+
+RefCountBug::RefCountBug(const CheckerBase *Checker, RefCountBugType BT)
+    : BugType(Checker, bugTypeToName(BT), categories::MemoryRefCount,
+              /*SupressOnSink=*/BT == LeakWithinFunction || BT == LeakAtReturn),
+      BT(BT) {}
+
 static bool isNumericLiteralExpression(const Expr *E) {
   // FIXME: This set of cases was copied from SemaExprObjC.
   return isa<IntegerLiteral>(E) ||
@@ -711,15 +755,15 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
   return std::make_shared<PathDiagnosticEventPiece>(L, os.str());
 }
 
-RefCountReport::RefCountReport(RefCountBug &D, const LangOptions &LOpts,
+RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts,
                                ExplodedNode *n, SymbolRef sym,
-                               bool registerVisitor)
-    : BugReport(D, D.getDescription(), n), Sym(sym) {
-  if (registerVisitor)
+                               bool isLeak)
+    : BugReport(D, D.getDescription(), n), Sym(sym), isLeak(isLeak) {
+  if (!isLeak)
     addVisitor(llvm::make_unique<RefCountReportVisitor>(sym));
 }
 
-RefCountReport::RefCountReport(RefCountBug &D, const LangOptions &LOpts,
+RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts,
                                ExplodedNode *n, SymbolRef sym,
                                StringRef endText)
     : BugReport(D, D.getDescription(), endText, n) {
@@ -805,10 +849,10 @@ void RefLeakReport::createDescription(CheckerContext &Ctx) {
   }
 }
 
-RefLeakReport::RefLeakReport(RefCountBug &D, const LangOptions &LOpts,
+RefLeakReport::RefLeakReport(const RefCountBug &D, const LangOptions &LOpts,
                              ExplodedNode *n, SymbolRef sym,
                              CheckerContext &Ctx)
-    : RefCountReport(D, LOpts, n, sym, false) {
+    : RefCountReport(D, LOpts, n, sym, /*isLeak=*/true) {
 
   deriveAllocLocation(Ctx, sym);
   if (!AllocBinding)
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
index 1365620de0..fdfab9d77f 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
@@ -25,34 +25,43 @@ namespace ento {
 namespace retaincountchecker {
 
 class RefCountBug : public BugType {
-protected:
-  RefCountBug(const CheckerBase *checker, StringRef name,
-              bool SuppressOnSink=false)
-      : BugType(checker, name, categories::MemoryRefCount,
-                SuppressOnSink) {}
-
 public:
-  virtual const char *getDescription() const = 0;
+  enum RefCountBugType {
+    UseAfterRelease,
+    ReleaseNotOwned,
+    DeallocNotOwned,
+    OverAutorelease,
+    ReturnNotOwnedForOwned,
+    LeakWithinFunction,
+    LeakAtReturn,
+  };
+  RefCountBug(const CheckerBase *checker, RefCountBugType BT);
+  StringRef getDescription() const;
+  RefCountBugType getBugType() const {
+    return BT;
+  }
 
-  virtual bool isLeak() const { return false; }
+private:
+  RefCountBugType BT;
+  static StringRef bugTypeToName(RefCountBugType BT);
 };
 
 class RefCountReport : public BugReport {
 protected:
   SymbolRef Sym;
+  bool isLeak;
 
 public:
-  RefCountReport(RefCountBug &D, const LangOptions &LOpts,
+  RefCountReport(const RefCountBug &D, const LangOptions &LOpts,
               ExplodedNode *n, SymbolRef sym,
-              bool registerVisitor = true);
+              bool isLeak=false);
 
-  RefCountReport(RefCountBug &D, const LangOptions &LOpts,
+  RefCountReport(const RefCountBug &D, const LangOptions &LOpts,
               ExplodedNode *n, SymbolRef sym,
               StringRef endText);
 
   llvm::iterator_range<ranges_iterator> getRanges() override {
-    const RefCountBug& BugTy = static_cast<const RefCountBug&>(getBugType());
-    if (!BugTy.isLeak())
+    if (!isLeak)
       return BugReport::getRanges();
     return llvm::make_range(ranges_iterator(), ranges_iterator());
   }
@@ -71,7 +80,7 @@ class RefLeakReport : public RefCountReport {
   void createDescription(CheckerContext &Ctx);
 
 public:
-  RefLeakReport(RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n,
+  RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n,
                 SymbolRef sym, CheckerContext &Ctx);
 
   PathDiagnosticLocation getLocation(const SourceManager &SM) const override {