Skip to content

Commit 76f8417

Browse files
committed
[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.
1 parent a005861 commit 76f8417

File tree

2 files changed

+21
-24
lines changed

2 files changed

+21
-24
lines changed

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,11 @@ class CheckerManager {
185185
StringRef OptionName,
186186
StringRef ExpectedValueDesc) const;
187187

188-
using CheckerRef = CheckerBase *;
189188
using CheckerTag = const void *;
190-
using CheckerDtor = CheckerFn<void ()>;
191189

192-
//===----------------------------------------------------------------------===//
193-
// Checker registration.
194-
//===----------------------------------------------------------------------===//
190+
//===----------------------------------------------------------------------===//
191+
// Checker registration.
192+
//===----------------------------------------------------------------------===//
195193

196194
/// Used to register checkers.
197195
/// All arguments are automatically passed through to the checker
@@ -201,24 +199,24 @@ class CheckerManager {
201199
template <typename CHECKER, typename... AT>
202200
CHECKER *registerChecker(AT &&... Args) {
203201
CheckerTag tag = getTag<CHECKER>();
204-
CheckerRef &ref = CheckerTags[tag];
205-
assert(!ref && "Checker already registered, use getChecker!");
206-
207-
CHECKER *checker = new CHECKER(std::forward<AT>(Args)...);
208-
checker->Name = CurrentCheckerName;
209-
CheckerDtors.push_back(CheckerDtor(checker, destruct<CHECKER>));
210-
CHECKER::_register(checker, *this);
211-
ref = checker;
212-
return checker;
202+
std::unique_ptr<CheckerBase> &Ref = CheckerTags[tag];
203+
assert(!Ref && "Checker already registered, use getChecker!");
204+
205+
std::unique_ptr<CHECKER> Checker =
206+
std::make_unique<CHECKER>(std::forward<AT>(Args)...);
207+
Checker->Name = CurrentCheckerName;
208+
CHECKER::_register(Checker.get(), *this);
209+
Ref = std::move(Checker);
210+
return static_cast<CHECKER *>(Ref.get());
213211
}
214212

215213
template <typename CHECKER>
216214
CHECKER *getChecker() {
217-
CheckerTag tag = getTag<CHECKER>();
218-
assert(CheckerTags.count(tag) != 0 &&
219-
"Requested checker is not registered! Maybe you should add it as a "
220-
"dependency in Checkers.td?");
221-
return static_cast<CHECKER *>(CheckerTags[tag]);
215+
CheckerTag Tag = getTag<CHECKER>();
216+
std::unique_ptr<CheckerBase> &Ref = CheckerTags[Tag];
217+
assert(Ref && "Requested checker is not registered! Maybe you should add it"
218+
" as a dependency in Checkers.td?");
219+
return static_cast<CHECKER *>(Ref.get());
222220
}
223221

224222
template <typename CHECKER> bool isRegisteredChecker() {
@@ -622,9 +620,7 @@ class CheckerManager {
622620
template <typename T>
623621
static void *getTag() { static int tag; return &tag; }
624622

625-
llvm::DenseMap<CheckerTag, CheckerRef> CheckerTags;
626-
627-
std::vector<CheckerDtor> CheckerDtors;
623+
llvm::DenseMap<CheckerTag, std::unique_ptr<CheckerBase>> CheckerTags;
628624

629625
struct DeclCheckerInfo {
630626
CheckDeclFunc CheckFn;

clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include "clang/StaticAnalyzer/Core/Checker.h"
1314
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
1415
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
1516
#include <memory>
@@ -41,8 +42,8 @@ CheckerManager::CheckerManager(AnalyzerOptions &AOptions,
4142
}
4243

4344
CheckerManager::~CheckerManager() {
44-
for (const auto &CheckerDtor : CheckerDtors)
45-
CheckerDtor();
45+
// This is declared here to ensure that the destructors of `CheckerBase` and
46+
// `CheckerRegistryData` are available.
4647
}
4748

4849
} // namespace ento

0 commit comments

Comments
 (0)