Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Jan 1, 2024

Conversation

steakhal
Copy link
Contributor

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.

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.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Dec 31, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 31, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balazs Benics (steakhal)

Changes

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.


Patch is 68.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76655.diff

39 Files Affected:

  • (modified) clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h (+1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp (+2-5)
  • (modified) clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp (+2-5)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp (+7-17)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp (+2-4)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp (+2-4)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp (+2-5)
  • (modified) clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp (+5-11)
  • (modified) clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp (+2-8)
  • (modified) clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp (+2-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp (+2-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp (+2-4)
  • (modified) clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp (+3-9)
  • (modified) clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp (+6-16)
  • (modified) clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp (+4-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp (+9-7)
  • (modified) clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp (+3-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp (+6-15)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp (+6-11)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp (+3-8)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp (+3-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp (+2-7)
  • (modified) clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp (+4-8)
  • (modified) clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp (+2-4)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp (+4-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp (+5-10)
  • (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp (+2-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp (+4-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp (+42-47)
  • (modified) clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp (+2-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp (+2-6)
  • (modified) clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp (+2-5)
  • (modified) clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp (+3-8)
  • (modified) clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp (+11-21)
  • (modified) clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp (+6-14)
  • (modified) clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp (+2-5)
  • (modified) clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp (+1)
  • (modified) clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp (+2-5)
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
index 5d2c96e5bc9de3..45187433c069fd 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
+++ b/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;
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index ce126541265551..c990ad138f8905 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/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));
diff --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index 5e25153a148fea..c72a97cc01e914 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/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
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
index 361a4eed922108..a09db6d2d0ec5b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
+++ b/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));
   }
 }
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
index eb265f4dde68bc..b4dee1e300e886 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
+++ b/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);
diff --git a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
index d1d4f3baf6a852..a50772f881f7d0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
+++ b/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));
   }
diff --git a/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
index 9e11d8d9ecbc3c..be7be15022d360 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
+++ b/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));
       }
 }
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
index 8b34b41bab21c3..eca8d3cc072292 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -42,7 +42,7 @@ class ConversionChecker : public Checker<check::PreStmt<ImplicitCastExpr>> {
   void checkPreStmt(const ImplicitCastExpr *Cast, CheckerContext &C) const;
 
 private:
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{this, "Conversion"};
 
   bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType,
                          CheckerContext &C) const;
@@ -126,11 +126,8 @@ void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast,
 
 void ConversionChecker::reportBug(ExplodedNode *N, const Expr *E,
                                   CheckerContext &C, const char Msg[]) const {
-  if (!BT)
-    BT.reset(new BugType(this, "Conversion"));
-
   // Generate a report for this bug.
-  auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+  auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
   bugreporter::trackExpressionValue(N, E, *R);
   C.emitReport(std::move(R));
 }
diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
index 5331d9574743fb..5496f087447fbe 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
@@ -14,6 +14,7 @@
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/Taint.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/CheckerContext.h"
@@ -25,8 +26,8 @@ using namespace taint;
 
 namespace {
 class DivZeroChecker : public Checker< check::PreStmt<BinaryOperator> > {
-  mutable std::unique_ptr<BugType> BT;
-  mutable std::unique_ptr<BugType> TaintBT;
+  const BugType BT{this, "Division by zero"};
+  const BugType TaintBT{this, "Division by zero", categories::TaintedData};
   void reportBug(StringRef Msg, ProgramStateRef StateZero,
                  CheckerContext &C) const;
   void reportTaintBug(StringRef Msg, ProgramStateRef StateZero,
@@ -48,10 +49,7 @@ static const Expr *getDenomExpr(const ExplodedNode *N) {
 void DivZeroChecker::reportBug(StringRef Msg, ProgramStateRef StateZero,
                                CheckerContext &C) const {
   if (ExplodedNode *N = C.generateErrorNode(StateZero)) {
-    if (!BT)
-      BT.reset(new BugType(this, "Division by zero", categories::LogicError));
-
-    auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+    auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
     bugreporter::trackExpressionValue(N, getDenomExpr(N), *R);
     C.emitReport(std::move(R));
   }
@@ -61,11 +59,7 @@ void DivZeroChecker::reportTaintBug(
     StringRef Msg, ProgramStateRef StateZero, CheckerContext &C,
     llvm::ArrayRef<SymbolRef> TaintedSyms) const {
   if (ExplodedNode *N = C.generateErrorNode(StateZero)) {
-    if (!TaintBT)
-      TaintBT.reset(
-          new BugType(this, "Division by zero", categories::TaintedData));
-
-    auto R = std::make_unique<PathSensitiveBugReport>(*TaintBT, Msg, N);
+    auto R = std::make_unique<PathSensitiveBugReport>(TaintBT, Msg, N);
     bugreporter::trackExpressionValue(N, getDenomExpr(N), *R);
     for (auto Sym : TaintedSyms)
       R->markInteresting(Sym);
diff --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
index dbc930d7d37b7c..0ad307d3ebd504 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
@@ -30,12 +30,7 @@ using namespace ento;
 
 namespace {
 class DynamicTypeChecker : public Checker<check::PostStmt<ImplicitCastExpr>> {
-  mutable std::unique_ptr<BugType> BT;
-  void initBugType() const {
-    if (!BT)
-      BT.reset(
-          new BugType(this, "Dynamic and static type mismatch", "Type Error"));
-  }
+  const BugType BT{this, "Dynamic and static type mismatch", "Type Error"};
 
   class DynamicTypeBugVisitor : public BugReporterVisitor {
   public:
@@ -70,7 +65,6 @@ void DynamicTypeChecker::reportTypeError(QualType DynamicType,
                                          const MemRegion *Reg,
                                          const Stmt *ReportedNode,
                                          CheckerContext &C) const {
-  initBugType();
   SmallString<192> Buf;
   llvm::raw_svector_ostream OS(Buf);
   OS << "Object has a dynamic type '";
@@ -81,7 +75,7 @@ void DynamicTypeChecker::reportTypeError(QualType DynamicType,
                   llvm::Twine());
   OS << "'";
   auto R = std::make_unique<PathSensitiveBugReport>(
-      *BT, OS.str(), C.generateNonFatalErrorNode());
+      BT, OS.str(), C.generateNonFatalErrorNode());
   R->markInteresting(Reg);
   R->addVisitor(std::make_unique<DynamicTypeBugVisitor>(Reg));
   R->addRange(ReportedNode->getSourceRange());
diff --git a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
index 7c51673422a0a2..0fa20428c1b560 100644
--- a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
@@ -60,7 +60,7 @@ class ConstraintBasedEQEvaluator {
 // Being conservative, it does not warn if there is slight possibility the
 // value can be matching.
 class EnumCastOutOfRangeChecker : public Checker<check::PreStmt<CastExpr>> {
-  mutable std::unique_ptr<BugType> EnumValueCastOutOfRange;
+  const BugType EnumValueCastOutOfRange{this, "Enum cast out of range"};
   void reportWarning(CheckerContext &C, const CastExpr *CE,
                      const EnumDecl *E) const;
 
@@ -85,10 +85,6 @@ void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C,
                                               const EnumDecl *E) const {
   assert(E && "valid EnumDecl* is expected");
   if (const ExplodedNode *N = C.generateNonFatalErrorNode()) {
-    if (!EnumValueCastOutOfRange)
-      EnumValueCastOutOfRange.reset(
-          new BugType(this, "Enum cast out of range"));
-
     std::string ValueStr = "", NameStr = "the enum";
 
     // Try to add details to the message:
@@ -105,7 +101,7 @@ void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C,
                               "not in the valid range of values for {1}",
                               ValueStr, NameStr);
 
-    auto BR = std::make_unique<PathSensitiveBugReport>(*EnumValueCastOutOfRange,
+    auto BR = std::make_unique<PathSensitiveBugReport>(EnumValueCastOutOfRange,
                                                        Msg, N);
     bugreporter::trackExpressionValue(N, CE->getSubExpr(), *BR);
     BR->addNote("enum declared here",
diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 2c7bacac33a687..3096999e9fd163 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -25,7 +25,7 @@ using namespace ento;
 namespace {
 class ExprInspectionChecker
     : public Checker<eval::Call, check::DeadSymbols, check::EndAnalysis> {
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{this, "Checking analyzer assumptions", "debug"};
 
   // These stats are per-analysis, not per-branch, hence they shouldn't
   // stay inside the program state.
@@ -176,11 +176,7 @@ ExprInspectionChecker::reportBug(llvm::StringRef Msg, BugReporter &BR,
                                  std::optional<SVal> ExprVal) const {
   if (!N)
     return nullptr;
-
-  if (!BT)
-    BT.reset(new BugType(this, "Checking analyzer assumptions", "debug"));
-
-  auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+  auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
   if (ExprVal) {
     R->markInteresting(*ExprVal);
   }
diff --git a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
index 2ee201b640089d..7aefcdc6d358aa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
@@ -24,7 +24,7 @@ using namespace ento;
 namespace {
 class FixedAddressChecker
   : public Checker< check::PreStmt<BinaryOperator> > {
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{this, "Use fixed address"};
 
 public:
   void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
@@ -50,12 +50,10 @@ void FixedAddressChecker::checkPreStmt(const BinaryOperator *B,
 
   if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     // FIXME: improve grammar in the following strings:
-    if (!BT)
-      BT.reset(new BugType(this, "Use fixed address"));
     constexpr llvm::StringLiteral Msg =
         "Using a fixed address is not portable because that address will "
         "probably not be valid in all environments or platforms.";
-    auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+    auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
     R->addRange(B->getRHS()->getSourceRange());
     C.emitReport(std::move(R));
   }
diff --git a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
index 70f911fc66abca..de9efe17d220b1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
@@ -62,7 +62,8 @@ class NonLocalizedStringChecker
                      check::PostObjCMessage,
                      check::PostStmt<ObjCStringLiteral>> {
 
-  mutable std::unique_ptr<BugType> BT;
+  const BugType BT{this, "Unlocalizable string",
+                   "Localizability Issue (Apple)"};
 
   // Methods that require a localized string
   mutable llvm::DenseMap<const IdentifierInfo *,
@@ -89,8 +90,6 @@ class NonLocalizedStringChecker
                                       Selector S) const;
 
 public:
-  NonLocalizedStringChecker();
-
   // When this parameter is set to true, the checker assumes all
   // methods that return NSStrings are unlocalized. Thus, more false
   // positives will be reported.
@@ -108,11 +107,6 @@ class NonLocalizedStringChecker
 REGISTER_MAP_WITH_PROGRAMSTATE(LocalizedMemMap, const MemRegion *,
                                LocalizedState)
 
-NonLocalizedStringChecker::NonLocalizedStringChecker() {
-  BT.reset(new BugType(this, "Unlocalizable string",
-                       "Localizability Issue (Apple)"));
-}
-
 namespace {
 class NonLo...
[truncated]

CheckerContext &C, ExplodedNode *ErrNode) const;
public:
InvalidatedIteratorChecker();
void reportBug(const StringRef &Message, const SVal &Val, CheckerContext &C,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are changing this code, I guess we do not want to take StringRefs as const references.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for the SVal. Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup PR for the analyzer-wide refactor: #76688

MissingWaitBugType.reset(new BugType(&CB, "Missing wait", MPIError));
}
MPIBugReporter(const CheckerBase &CB)
: UnmatchedWaitBugType(&CB, "Unmatched wait", MPIError),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to field initializers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, but couldn't as CB is a ctor param. I don't have that in a field init context.

@@ -52,10 +52,13 @@ class SimpleStreamChecker : public Checker<check::PostCall,
check::PreCall,
check::DeadSymbols,
check::PointerEscape> {
CallDescription OpenFn, CloseFn;
const CallDescription OpenFn{{"fopen"}, 2};
const CallDescription CloseFn{{"fclose"}, 1};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we mostly use CallDescriptions with compile time known inputs, it would be nice to make their ctors constexpr. This is not for this PR, just a note for the future.

Copy link
Contributor Author

@steakhal steakhal Jan 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it seems like StringRef is not constexpr for some reason - and I'm using that inside.
Consequently, that's a blocker.

EDIT: I've just checked and it seems like its constexpr, yet I could not make CallDescription constexpr.
Let me give it another try.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is why we can't have CallDescriptions constexpr:
It's an owning type, holding a std::vector<std::string>, and they are themselves non-constexpr, until IDK cpp20+?
So, this is resolved.

@steakhal steakhal merged commit 18f219c into llvm:main Jan 1, 2024
3 of 4 checks passed
@steakhal steakhal deleted the bb/cleanup-bugtype-usages branch January 1, 2024 17:57
@NagyDonat
Copy link
Contributor

Thanks for cleaning this up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants