https://github.com/NagyDonat created
https://github.com/llvm/llvm-project/pull/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.
From 76f8417b8b46e7d036d98fa92890469654158e20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Wed, 26 Feb 2025 15:41:46 +0100
Subject: [PATCH] [NFC][analyzer] Simplify ownership of checker objects
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.
---
.../StaticAnalyzer/Core/CheckerManager.h | 40 +++++++++----------
.../Frontend/CreateCheckerManager.cpp | 5 ++-
2 files changed, 21 insertions(+), 24 deletions(-)
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
