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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Donát Nagy (NagyDonat)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/128887.diff


2 Files Affected:

- (modified) clang/include/clang/StaticAnalyzer/Core/CheckerManager.h (+18-22) 
- (modified) clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp (+3-2) 


``````````diff
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index de40b96614dbc..b48a75630fe9b 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
@@ -201,24 +199,24 @@ class CheckerManager {
   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;
+    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() {
@@ -622,9 +620,7 @@ class CheckerManager {
   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 f60221ad7587e..35dd255fce898 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include <memory>
@@ -41,8 +42,8 @@ CheckerManager::CheckerManager(AnalyzerOptions &AOptions,
 }
 
 CheckerManager::~CheckerManager() {
-  for (const auto &CheckerDtor : CheckerDtors)
-    CheckerDtor();
+  // This is declared here to ensure that the destructors of `CheckerBase` and
+  // `CheckerRegistryData` are available.
 }
 
 } // namespace ento

``````````

</details>


https://github.com/llvm/llvm-project/pull/128887