[analyzer][NFC] Cleanup BugType lazy-init patterns (#76655)

Cleanup most of the lazy-init `BugType` legacy.
Some will be preserved, as those are slightly more complicated to
refactor.

Notice, that the default category for `BugType` is `LogicError`. I
omitted setting this explicitly where I could.

Please, actually have a look at the diff. I did this manually, and we
rarely check the bug type descriptions and stuff in tests, so the
testing might be shallow on this one.

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

@@ -13,6 +13,7 @@
 namespace clang {
 namespace ento {
 namespace categories {
+extern const char *const AppleAPIMisuse;
 extern const char *const CoreFoundationObjectiveC;
 extern const char *const LogicError;
 extern const char *const MemoryRefCount;

clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp

@@ -25,7 +25,7 @@ using namespace ento;
 namespace {
 class ArrayBoundChecker :
     public Checker<check::Location> {
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{this, "Out-of-bound array access"};
 
 public:
   void checkLocation(SVal l, bool isLoad, const Stmt* S,
@@ -65,16 +65,13 @@ void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS,
     if (!N)
       return;
 
-    if (!BT)
-      BT.reset(new BugType(this, "Out-of-bound array access"));
-
     // FIXME: It would be nice to eventually make this diagnostic more clear,
     // e.g., by referencing the original declaration or by saying *why* this
     // reference is outside the range.
 
     // Generate a report for this bug.
     auto report = std::make_unique<PathSensitiveBugReport>(
-        *BT, "Access out-of-bound array element (buffer overflow)", N);
+        BT, "Access out-of-bound array element (buffer overflow)", N);
 
     report->addRange(LoadS->getSourceRange());
     C.emitReport(std::move(report));

clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

@@ -44,7 +44,7 @@ namespace {
 class APIMisuse : public BugType {
 public:
   APIMisuse(const CheckerBase *checker, const char *name)
-      : BugType(checker, name, "API Misuse (Apple)") {}
+      : BugType(checker, name, categories::AppleAPIMisuse) {}
 };
 } // end anonymous namespace
 

clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

@@ -25,20 +25,31 @@ using namespace clang;
 using namespace ento;
 
 namespace {
-
 class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
-
-  mutable IdentifierInfo *IILockGuard, *IIUniqueLock;
-
-  CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn,
-                  PthreadLockFn, PthreadTryLockFn, PthreadUnlockFn,
-                  MtxLock, MtxTimedLock, MtxTryLock, MtxUnlock;
-
-  StringRef ClassLockGuard, ClassUniqueLock;
-
-  mutable bool IdentifierInfoInitialized;
-
-  std::unique_ptr<BugType> BlockInCritSectionBugType;
+  mutable IdentifierInfo *IILockGuard = nullptr;
+  mutable IdentifierInfo *IIUniqueLock = nullptr;
+  mutable bool IdentifierInfoInitialized = false;
+
+  const CallDescription LockFn{{"lock"}};
+  const CallDescription UnlockFn{{"unlock"}};
+  const CallDescription SleepFn{{"sleep"}};
+  const CallDescription GetcFn{{"getc"}};
+  const CallDescription FgetsFn{{"fgets"}};
+  const CallDescription ReadFn{{"read"}};
+  const CallDescription RecvFn{{"recv"}};
+  const CallDescription PthreadLockFn{{"pthread_mutex_lock"}};
+  const CallDescription PthreadTryLockFn{{"pthread_mutex_trylock"}};
+  const CallDescription PthreadUnlockFn{{"pthread_mutex_unlock"}};
+  const CallDescription MtxLock{{"mtx_lock"}};
+  const CallDescription MtxTimedLock{{"mtx_timedlock"}};
+  const CallDescription MtxTryLock{{"mtx_trylock"}};
+  const CallDescription MtxUnlock{{"mtx_unlock"}};
+
+  const llvm::StringLiteral ClassLockGuard{"lock_guard"};
+  const llvm::StringLiteral ClassUniqueLock{"unique_lock"};
+
+  const BugType BlockInCritSectionBugType{
+      this, "Call to blocking function in critical section", "Blocking Error"};
 
   void initIdentifierInfo(ASTContext &Ctx) const;
 
@@ -47,8 +58,6 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
                                 CheckerContext &C) const;
 
 public:
-  BlockInCriticalSectionChecker();
-
   bool isBlockingFunction(const CallEvent &Call) const;
   bool isLockFunction(const CallEvent &Call) const;
   bool isUnlockFunction(const CallEvent &Call) const;
@@ -63,22 +72,6 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
 
 REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
 
-BlockInCriticalSectionChecker::BlockInCriticalSectionChecker()
-    : IILockGuard(nullptr), IIUniqueLock(nullptr), LockFn({"lock"}),
-      UnlockFn({"unlock"}), SleepFn({"sleep"}), GetcFn({"getc"}),
-      FgetsFn({"fgets"}), ReadFn({"read"}), RecvFn({"recv"}),
-      PthreadLockFn({"pthread_mutex_lock"}),
-      PthreadTryLockFn({"pthread_mutex_trylock"}),
-      PthreadUnlockFn({"pthread_mutex_unlock"}), MtxLock({"mtx_lock"}),
-      MtxTimedLock({"mtx_timedlock"}), MtxTryLock({"mtx_trylock"}),
-      MtxUnlock({"mtx_unlock"}), ClassLockGuard("lock_guard"),
-      ClassUniqueLock("unique_lock"), IdentifierInfoInitialized(false) {
-  // Initialize the bug type.
-  BlockInCritSectionBugType.reset(
-      new BugType(this, "Call to blocking function in critical section",
-                        "Blocking Error"));
-}
-
 void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const {
   if (!IdentifierInfoInitialized) {
     /* In case of checking C code, or when the corresponding headers are not
@@ -151,7 +144,7 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
   llvm::raw_string_ostream os(msg);
   os << "Call to blocking function '" << Call.getCalleeIdentifier()->getName()
      << "' inside of critical section";
-  auto R = std::make_unique<PathSensitiveBugReport>(*BlockInCritSectionBugType,
+  auto R = std::make_unique<PathSensitiveBugReport>(BlockInCritSectionBugType,
                                                     os.str(), ErrNode);
   R->addRange(Call.getSourceRange());
   R->markInteresting(BlockDescSym);

clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp

@@ -24,7 +24,7 @@ using namespace ento;
 
 namespace {
   class BoolAssignmentChecker : public Checker< check::Bind > {
-    mutable std::unique_ptr<BugType> BT;
+    const BugType BT{this, "Assignment of a non-Boolean value"};
     void emitReport(ProgramStateRef state, CheckerContext &C,
                     bool IsTainted = false) const;
 
@@ -36,12 +36,9 @@ namespace {
 void BoolAssignmentChecker::emitReport(ProgramStateRef state, CheckerContext &C,
                                        bool IsTainted) const {
   if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
-    if (!BT)
-      BT.reset(new BugType(this, "Assignment of a non-Boolean value"));
-
     StringRef Msg = IsTainted ? "Might assign a tainted non-Boolean value"
                               : "Assignment of a non-Boolean value";
-    C.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, Msg, N));
+    C.emitReport(std::make_unique<PathSensitiveBugReport>(BT, Msg, N));
   }
 }
 

clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp

@@ -31,6 +31,7 @@
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -65,7 +66,8 @@ class CXXDeleteChecker : public Checker<check::PreStmt<CXXDeleteExpr>> {
 };
 
 class DeleteWithNonVirtualDtorChecker : public CXXDeleteChecker {
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{
+      this, "Destruction of a polymorphic object with no virtual destructor"};
 
   void
   checkTypedDeleteExpr(const CXXDeleteExpr *DE, CheckerContext &C,
@@ -74,7 +76,8 @@ class DeleteWithNonVirtualDtorChecker : public CXXDeleteChecker {
 };
 
 class CXXArrayDeleteChecker : public CXXDeleteChecker {
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{this,
+                   "Deleting an array of polymorphic objects is undefined"};
 
   void
   checkTypedDeleteExpr(const CXXDeleteExpr *DE, CheckerContext &C,
@@ -123,17 +126,10 @@ void DeleteWithNonVirtualDtorChecker::checkTypedDeleteExpr(
   if (!DerivedClass->isDerivedFrom(BaseClass))
     return;
 
-  if (!BT)
-    BT.reset(new BugType(this,
-                         "Destruction of a polymorphic object with no "
-                         "virtual destructor",
-                         "Logic error"));
-
   ExplodedNode *N = C.generateNonFatalErrorNode();
   if (!N)
     return;
-  auto R =
-      std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
+  auto R = std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N);
 
   // Mark region of problematic base class for later use in the BugVisitor.
   R->markInteresting(BaseClassRegion);
@@ -160,12 +156,6 @@ void CXXArrayDeleteChecker::checkTypedDeleteExpr(
   if (!DerivedClass->isDerivedFrom(BaseClass))
     return;
 
-  if (!BT)
-    BT.reset(new BugType(this,
-                         "Deleting an array of polymorphic objects "
-                         "is undefined",
-                         "Logic error"));
-
   ExplodedNode *N = C.generateNonFatalErrorNode();
   if (!N)
     return;
@@ -182,7 +172,7 @@ void CXXArrayDeleteChecker::checkTypedDeleteExpr(
      << SourceType.getAsString(C.getASTContext().getPrintingPolicy())
      << "' is undefined";
 
-  auto R = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N);
+  auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N);
 
   // Mark region of problematic base class for later use in the BugVisitor.
   R->markInteresting(BaseClassRegion);

clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp

@@ -24,7 +24,7 @@ using namespace ento;
 
 namespace {
 class CastSizeChecker : public Checker< check::PreStmt<CastExpr> > {
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{this, "Cast region with wrong size."};
 
 public:
   void checkPreStmt(const CastExpr *CE, CheckerContext &C) const;
@@ -131,12 +131,10 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const {
     return;
 
   if (ExplodedNode *errorNode = C.generateErrorNode()) {
-    if (!BT)
-      BT.reset(new BugType(this, "Cast region with wrong size."));
     constexpr llvm::StringLiteral Msg =
         "Cast a region whose size is not a multiple of the destination type "
         "size.";
-    auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, errorNode);
+    auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, errorNode);
     R->addRange(CE->getSourceRange());
     C.emitReport(std::move(R));
   }

clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

@@ -99,18 +99,23 @@ class ObjCDeallocChecker
                      check::PointerEscape,
                      check::PreStmt<ReturnStmt>> {
 
-  mutable IdentifierInfo *NSObjectII, *SenTestCaseII, *XCTestCaseII,
-      *Block_releaseII, *CIFilterII;
-
-  mutable Selector DeallocSel, ReleaseSel;
-
-  std::unique_ptr<BugType> MissingReleaseBugType;
-  std::unique_ptr<BugType> ExtraReleaseBugType;
-  std::unique_ptr<BugType> MistakenDeallocBugType;
+  mutable const IdentifierInfo *NSObjectII = nullptr;
+  mutable const IdentifierInfo *SenTestCaseII = nullptr;
+  mutable const IdentifierInfo *XCTestCaseII = nullptr;
+  mutable const IdentifierInfo *Block_releaseII = nullptr;
+  mutable const IdentifierInfo *CIFilterII = nullptr;
+
+  mutable Selector DeallocSel;
+  mutable Selector ReleaseSel;
+
+  const BugType MissingReleaseBugType{this, "Missing ivar release (leak)",
+                                      categories::MemoryRefCount};
+  const BugType ExtraReleaseBugType{this, "Extra ivar release",
+                                    categories::MemoryRefCount};
+  const BugType MistakenDeallocBugType{this, "Mistaken dealloc",
+                                       categories::MemoryRefCount};
 
 public:
-  ObjCDeallocChecker();
-
   void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr,
                     BugReporter &BR) const;
   void checkBeginFunction(CheckerContext &Ctx) const;
@@ -579,7 +584,7 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const {
     OS << " by a synthesized property but not released"
           " before '[super dealloc]'";
 
-    auto BR = std::make_unique<PathSensitiveBugReport>(*MissingReleaseBugType,
+    auto BR = std::make_unique<PathSensitiveBugReport>(MissingReleaseBugType,
                                                        OS.str(), ErrNode);
     C.emitReport(std::move(BR));
   }
@@ -701,7 +706,7 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue,
     OS <<  " property but was released in 'dealloc'";
   }
 
-  auto BR = std::make_unique<PathSensitiveBugReport>(*ExtraReleaseBugType,
+  auto BR = std::make_unique<PathSensitiveBugReport>(ExtraReleaseBugType,
                                                      OS.str(), ErrNode);
   BR->addRange(M.getOriginExpr()->getSourceRange());
 
@@ -743,7 +748,7 @@ bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
   OS << "'" << *PropImpl->getPropertyIvarDecl()
      << "' should be released rather than deallocated";
 
-  auto BR = std::make_unique<PathSensitiveBugReport>(*MistakenDeallocBugType,
+  auto BR = std::make_unique<PathSensitiveBugReport>(MistakenDeallocBugType,
                                                      OS.str(), ErrNode);
   BR->addRange(M.getOriginExpr()->getSourceRange());
 
@@ -752,23 +757,6 @@ bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
   return true;
 }
 
-ObjCDeallocChecker::ObjCDeallocChecker()
-    : NSObjectII(nullptr), SenTestCaseII(nullptr), XCTestCaseII(nullptr),
-      Block_releaseII(nullptr), CIFilterII(nullptr) {
-
-  MissingReleaseBugType.reset(
-      new BugType(this, "Missing ivar release (leak)",
-                  categories::MemoryRefCount));
-
-  ExtraReleaseBugType.reset(
-      new BugType(this, "Extra ivar release",
-                  categories::MemoryRefCount));
-
-  MistakenDeallocBugType.reset(
-      new BugType(this, "Mistaken dealloc",
-                  categories::MemoryRefCount));
-}
-
 void ObjCDeallocChecker::initIdentifierInfoAndSelectors(
     ASTContext &Ctx) const {
   if (NSObjectII)

clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp

@@ -41,7 +41,7 @@ bool isRootChanged(intptr_t k) { return k == ROOT_CHANGED; }
 //                      bug<--foo()--          JAIL_ENTERED<--foo()--
 class ChrootChecker : public Checker<eval::Call, check::PreCall> {
   // This bug refers to possibly break out of a chroot() jail.
-  mutable std::unique_ptr<BugType> BT_BreakJail;
+  const BugType BT_BreakJail{this, "Break out of jail"};
 
   const CallDescription Chroot{{"chroot"}, 1}, Chdir{{"chdir"}, 1};
 
@@ -124,12 +124,10 @@ void ChrootChecker::checkPreCall(const CallEvent &Call,
   if (k)
     if (isRootChanged((intptr_t) *k))
       if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-        if (!BT_BreakJail)
-          BT_BreakJail.reset(new BugType(this, "Break out of jail"));
         constexpr llvm::StringLiteral Msg =
             "No call of chdir(\"/\") immediately after chroot";
         C.emitReport(
-            std::make_unique<PathSensitiveBugReport>(*BT_BreakJail, Msg, N));
+            std::make_unique<PathSensitiveBugReport>(BT_BreakJail, Msg, N));
       }
 }
 

clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp

@@ -35,7 +35,8 @@ class CloneChecker
 
 private:
   mutable CloneDetector Detector;
-  mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
+  const BugType BT_Exact{this, "Exact code clone", "Code clone"};
+  const BugType BT_Suspicious{this, "Suspicious code clone", "Code clone"};
 
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
@@ -107,15 +108,11 @@ static PathDiagnosticLocation makeLocation(const StmtSequence &S,
 void CloneChecker::reportClones(
     BugReporter &BR, AnalysisManager &Mgr,
     std::vector<CloneDetector::CloneGroup> &CloneGroups) const {
-
-  if (!BT_Exact)
-    BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone"));
-
   for (const CloneDetector::CloneGroup &Group : CloneGroups) {
     // We group the clones by printing the first as a warning and all others
     // as a note.
     auto R = std::make_unique<BasicBugReport>(
-        *BT_Exact, "Duplicate code detected", makeLocation(Group.front(), Mgr));
+        BT_Exact, "Duplicate code detected", makeLocation(Group.front(), Mgr));
     R->addRange(Group.front().getSourceRange());
 
     for (unsigned i = 1; i < Group.size(); ++i)
@@ -154,10 +151,6 @@ void CloneChecker::reportSuspiciousClones(
     }
   }
 
-  if (!BT_Suspicious)
-    BT_Suspicious.reset(
-        new BugType(this, "Suspicious code clone", "Code clone"));
-
   ASTContext &ACtx = BR.getContext();
   SourceManager &SM = ACtx.getSourceManager();
   AnalysisDeclContext *ADC =
@@ -170,7 +163,7 @@ void CloneChecker::reportSuspiciousClones(
     // Think how to perform more accurate suggestions?
 
     auto R = std::make_unique<BasicBugReport>(
-        *BT_Suspicious,
+        BT_Suspicious,
         "Potential copy-paste error; did you really mean to use '" +
             Pair.FirstCloneInfo.Variable->getNameAsString() + "' here?",
         PathDiagnosticLocation::createBegin(Pair.FirstCloneInfo.Mention, SM,