[clang] ac7c8eb - [NFC][analyzer] Simplify ownership of checker objects (#128887)

[via cfe-commits]

Author: Donát Nagy
Date: 2025-02-27T18:33:50+01:00
New Revision: ac7c8eb4de0b0f8f9e01df3a12e9a7f7f20899e9

URL: https://github.com/llvm/llvm-project/commit/ac7c8eb4de0b0f8f9e01df3a12e9a7f7f20899e9
DIFF: https://github.com/llvm/llvm-project/commit/ac7c8eb4de0b0f8f9e01df3a12e9a7f7f20899e9.diff

LOG: [NFC][analyzer] Simplify ownership of checker objects (#128887)

Previously checker objects were created by raw `new` calls, which
necessitated managing and calling their destructors explicitly. This
commit refactors this convoluted logic by introducing `unique_ptr`s that
to manage the ownership of these objects automatically.

This change can be thought of as stand-alone code quality improvement;
but I also have a secondary motivation that I'm planning further changes
in the checker registration/initialization process (to formalize our
tradition of multi-part checker) and this commit "prepares the ground"
for those changes.

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
    clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index de40b96614dbc..7d3120b5395c4 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -185,13 +185,11 @@ class CheckerManager {
                                        StringRef OptionName,
                                        StringRef ExpectedValueDesc) const;
 
-  using CheckerRef = CheckerBase *;
   using CheckerTag = const void *;
-  using CheckerDtor = CheckerFn<void ()>;
 
-//===----------------------------------------------------------------------===//
-// Checker registration.
-//===----------------------------------------------------------------------===//
+  //===--------------------------------------------------------------------===//
+  // Checker registration.
+  //===--------------------------------------------------------------------===//
 
   /// Used to register checkers.
   /// All arguments are automatically passed through to the checker
@@ -200,25 +198,25 @@ class CheckerManager {
   /// \returns a pointer to the checker object.
   template <typename CHECKER, typename... AT>
   CHECKER *registerChecker(AT &&... Args) {
-    CheckerTag tag = getTag<CHECKER>();
-    CheckerRef &ref = CheckerTags[tag];
-    assert(!ref && "Checker already registered, use getChecker!");
-
-    CHECKER *checker = new CHECKER(std::forward<AT>(Args)...);
-    checker->Name = CurrentCheckerName;
-    CheckerDtors.push_back(CheckerDtor(checker, destruct<CHECKER>));
-    CHECKER::_register(checker, *this);
-    ref = checker;
-    return checker;
+    CheckerTag Tag = getTag<CHECKER>();
+    std::unique_ptr<CheckerBase> &Ref = CheckerTags[Tag];
+    assert(!Ref && "Checker already registered, use getChecker!");
+
+    std::unique_ptr<CHECKER> Checker =
+        std::make_unique<CHECKER>(std::forward<AT>(Args)...);
+    Checker->Name = CurrentCheckerName;
+    CHECKER::_register(Checker.get(), *this);
+    Ref = std::move(Checker);
+    return static_cast<CHECKER *>(Ref.get());
   }
 
   template <typename CHECKER>
   CHECKER *getChecker() {
-    CheckerTag tag = getTag<CHECKER>();
-    assert(CheckerTags.count(tag) != 0 &&
-           "Requested checker is not registered! Maybe you should add it as a "
-           "dependency in Checkers.td?");
-    return static_cast<CHECKER *>(CheckerTags[tag]);
+    CheckerTag Tag = getTag<CHECKER>();
+    std::unique_ptr<CheckerBase> &Ref = CheckerTags[Tag];
+    assert(Ref && "Requested checker is not registered! Maybe you should add it"
+                  " as a dependency in Checkers.td?");
+    return static_cast<CHECKER *>(Ref.get());
   }
 
   template <typename CHECKER> bool isRegisteredChecker() {
@@ -462,9 +460,9 @@ class CheckerManager {
                                     unsigned int Space = 0,
                                     bool IsDot = false) const;
 
-  //===----------------------------------------------------------------------===//
+  //===--------------------------------------------------------------------===//
   // Internal registration functions for AST traversing.
-  //===----------------------------------------------------------------------===//
+  //===--------------------------------------------------------------------===//
 
   // Functions used by the registration mechanism, checkers should not touch
   // these directly.
@@ -478,9 +476,9 @@ class CheckerManager {
 
   void _registerForBody(CheckDeclFunc checkfn);
 
-//===----------------------------------------------------------------------===//
-// Internal registration functions for path-sensitive checking.
-//===----------------------------------------------------------------------===//
+  //===--------------------------------------------------------------------===//
+  // Internal registration functions for path-sensitive checking.
+  //===--------------------------------------------------------------------===//
 
   using CheckStmtFunc = CheckerFn<void (const Stmt *, CheckerContext &)>;
 
@@ -582,9 +580,9 @@ class CheckerManager {
 
   void _registerForEndOfTranslationUnit(CheckEndOfTranslationUnit checkfn);
 
-//===----------------------------------------------------------------------===//
-// Internal registration functions for events.
-//===----------------------------------------------------------------------===//
+  //===--------------------------------------------------------------------===//
+  // Internal registration functions for events.
+  //===--------------------------------------------------------------------===//
 
   using EventTag = void *;
   using CheckEventFunc = CheckerFn<void (const void *event)>;
@@ -611,20 +609,15 @@ class CheckerManager {
       Checker(&event);
   }
 
-//===----------------------------------------------------------------------===//
-// Implementation details.
-//===----------------------------------------------------------------------===//
+  //===--------------------------------------------------------------------===//
+  // Implementation details.
+  //===--------------------------------------------------------------------===//
 
 private:
-  template <typename CHECKER>
-  static void destruct(void *obj) { delete static_cast<CHECKER *>(obj); }
-
   template <typename T>
   static void *getTag() { static int tag; return &tag; }
 
-  llvm::DenseMap<CheckerTag, CheckerRef> CheckerTags;
-
-  std::vector<CheckerDtor> CheckerDtors;
+  llvm::DenseMap<CheckerTag, std::unique_ptr<CheckerBase>> CheckerTags;
 
   struct DeclCheckerInfo {
     CheckDeclFunc CheckFn;

diff  --git a/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp b/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
index 7ca7f09b6eb23..5d5049a1172b8 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include <memory>
@@ -42,10 +43,9 @@ CheckerManager::CheckerManager(AnalyzerOptions &AOptions,
   Registry.initializeRegistry(*this);
 }
 
-CheckerManager::~CheckerManager() {
-  for (const auto &CheckerDtor : CheckerDtors)
-    CheckerDtor();
-}
+// This is declared here to ensure that the destructors of `CheckerBase` and
+// `CheckerRegistryData` are available.
+CheckerManager::~CheckerManager() = default;
 
 } // namespace ento
 } // namespace clang