Skip to content

[analyzer] Clean up slightly the messed up ownership model of the analyzer #128368

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

Merged
merged 1 commit into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/include/clang/Analysis/AnalysisDeclContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ class AnalysisDeclContextManager {
bool synthesizeBodies = false, bool addStaticInitBranches = false,
bool addCXXNewAllocator = true, bool addRichCXXConstructors = true,
bool markElidedCXXConstructors = true, bool addVirtualBaseBranches = true,
CodeInjector *injector = nullptr);
std::unique_ptr<CodeInjector> injector = nullptr);

AnalysisDeclContext *getContext(const Decl *D);

Expand Down
24 changes: 14 additions & 10 deletions clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,8 @@ class BugReporterData {
public:
virtual ~BugReporterData() = default;

virtual ArrayRef<PathDiagnosticConsumer*> getPathDiagnosticConsumers() = 0;
virtual ArrayRef<std::unique_ptr<PathDiagnosticConsumer>>
getPathDiagnosticConsumers() = 0;
virtual ASTContext &getASTContext() = 0;
virtual SourceManager &getSourceManager() = 0;
virtual AnalyzerOptions &getAnalyzerOptions() = 0;
Expand Down Expand Up @@ -608,7 +609,8 @@ class BugReporter {
/// Generate and flush diagnostics for all bug reports.
void FlushReports();

ArrayRef<PathDiagnosticConsumer*> getPathDiagnosticConsumers() {
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>>
getPathDiagnosticConsumers() {
return D.getPathDiagnosticConsumers();
}

Expand Down Expand Up @@ -670,9 +672,10 @@ class BugReporter {
protected:
/// Generate the diagnostics for the given bug report.
virtual std::unique_ptr<DiagnosticForConsumerMapTy>
generateDiagnosticForConsumerMap(BugReport *exampleReport,
ArrayRef<PathDiagnosticConsumer *> consumers,
ArrayRef<BugReport *> bugReports);
generateDiagnosticForConsumerMap(
BugReport *exampleReport,
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>> consumers,
ArrayRef<BugReport *> bugReports);
};

/// GRBugReporter is used for generating path-sensitive reports.
Expand All @@ -684,10 +687,11 @@ class PathSensitiveBugReporter final : public BugReporter {
SmallVectorImpl<BugReport *> &bugReports) override;

/// Generate the diagnostics for the given bug report.
std::unique_ptr<DiagnosticForConsumerMapTy>
generateDiagnosticForConsumerMap(BugReport *exampleReport,
ArrayRef<PathDiagnosticConsumer *> consumers,
ArrayRef<BugReport *> bugReports) override;
std::unique_ptr<DiagnosticForConsumerMapTy> generateDiagnosticForConsumerMap(
BugReport *exampleReport,
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>> consumers,
ArrayRef<BugReport *> bugReports) override;

public:
PathSensitiveBugReporter(BugReporterData& d, ExprEngine& eng)
: BugReporter(d), Eng(eng) {}
Expand All @@ -706,7 +710,7 @@ class PathSensitiveBugReporter final : public BugReporter {
/// Iterates through the bug reports within a single equivalence class,
/// stops at a first non-invalidated report.
std::unique_ptr<DiagnosticForConsumerMapTy> generatePathDiagnostics(
ArrayRef<PathDiagnosticConsumer *> consumers,
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>> consumers,
ArrayRef<PathSensitiveBugReport *> &bugReports);

void emitReport(std::unique_ptr<BugReport> R) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class CrossTranslationUnitContext;
namespace ento {

class PathDiagnosticConsumer;
typedef std::vector<PathDiagnosticConsumer*> PathDiagnosticConsumers;
using PathDiagnosticConsumers =
std::vector<std::unique_ptr<PathDiagnosticConsumer>>;

#define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN) \
void CREATEFN(PathDiagnosticConsumerOptions Diagopts, \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ class AnalysisManager : public BugReporterData {
AnalyzerOptions &options;

AnalysisManager(ASTContext &ctx, Preprocessor &PP,
const PathDiagnosticConsumers &Consumers,
PathDiagnosticConsumers Consumers,
StoreManagerCreator storemgr,
ConstraintManagerCreator constraintmgr,
CheckerManager *checkerMgr, AnalyzerOptions &Options,
CodeInjector *injector = nullptr);
std::unique_ptr<CodeInjector> injector = nullptr);

~AnalysisManager() override;

Expand Down Expand Up @@ -91,7 +91,8 @@ class AnalysisManager : public BugReporterData {
return LangOpts;
}

ArrayRef<PathDiagnosticConsumer*> getPathDiagnosticConsumers() override {
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>>
getPathDiagnosticConsumers() override {
return PathConsumers;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class CheckerRegistry;

class AnalysisASTConsumer : public ASTConsumer {
public:
virtual void AddDiagnosticConsumer(PathDiagnosticConsumer *Consumer) = 0;
virtual void
AddDiagnosticConsumer(std::unique_ptr<PathDiagnosticConsumer> Consumer) = 0;

/// This method allows registering statically linked custom checkers that are
/// not a part of the Clang tree. It employs the same mechanism that is used
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Analysis/AnalysisDeclContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ AnalysisDeclContextManager::AnalysisDeclContextManager(
bool addLoopExit, bool addScopes, bool synthesizeBodies,
bool addStaticInitBranch, bool addCXXNewAllocator,
bool addRichCXXConstructors, bool markElidedCXXConstructors,
bool addVirtualBaseBranches, CodeInjector *injector)
: Injector(injector), FunctionBodyFarm(ASTCtx, injector),
bool addVirtualBaseBranches, std::unique_ptr<CodeInjector> injector)
: Injector(std::move(injector)), FunctionBodyFarm(ASTCtx, Injector.get()),
SynthesizeBodies(synthesizeBodies) {
cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
Expand Down
23 changes: 8 additions & 15 deletions clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,28 @@ using namespace ento;
void AnalysisManager::anchor() { }

AnalysisManager::AnalysisManager(ASTContext &ASTCtx, Preprocessor &PP,
const PathDiagnosticConsumers &PDC,
PathDiagnosticConsumers PDC,
StoreManagerCreator storemgr,
ConstraintManagerCreator constraintmgr,
CheckerManager *checkerMgr,
AnalyzerOptions &Options,
CodeInjector *injector)
std::unique_ptr<CodeInjector> injector)
: AnaCtxMgr(
ASTCtx, Options.UnoptimizedCFG,
Options.ShouldIncludeImplicitDtorsInCFG,
/*addInitializers=*/true,
Options.ShouldIncludeTemporaryDtorsInCFG,
/*addInitializers=*/true, Options.ShouldIncludeTemporaryDtorsInCFG,
Options.ShouldIncludeLifetimeInCFG,
// Adding LoopExit elements to the CFG is a requirement for loop
// unrolling.
Options.ShouldIncludeLoopExitInCFG ||
Options.ShouldUnrollLoops,
Options.ShouldIncludeScopesInCFG,
Options.ShouldSynthesizeBodies,
Options.ShouldIncludeLoopExitInCFG || Options.ShouldUnrollLoops,
Options.ShouldIncludeScopesInCFG, Options.ShouldSynthesizeBodies,
Options.ShouldConditionalizeStaticInitializers,
/*addCXXNewAllocator=*/true,
Options.ShouldIncludeRichConstructorsInCFG,
Options.ShouldElideConstructors,
/*addVirtualBaseBranches=*/true,
injector),
/*addVirtualBaseBranches=*/true, std::move(injector)),
Ctx(ASTCtx), PP(PP), LangOpts(ASTCtx.getLangOpts()),
PathConsumers(PDC), CreateStoreMgr(storemgr),
PathConsumers(std::move(PDC)), CreateStoreMgr(storemgr),
CreateConstraintMgr(constraintmgr), CheckerMgr(checkerMgr),
options(Options) {
AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
Expand All @@ -50,14 +46,11 @@ AnalysisManager::AnalysisManager(ASTContext &ASTCtx, Preprocessor &PP,

AnalysisManager::~AnalysisManager() {
FlushDiagnostics();
for (PathDiagnosticConsumer *Consumer : PathConsumers) {
delete Consumer;
}
}

void AnalysisManager::FlushDiagnostics() {
PathDiagnosticConsumer::FilesMade filesMade;
for (PathDiagnosticConsumer *Consumer : PathConsumers) {
for (const auto &Consumer : PathConsumers) {
Consumer->FlushDiagnostics(&filesMade);
}
}
22 changes: 11 additions & 11 deletions clang/lib/StaticAnalyzer/Core/BugReporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2958,7 +2958,7 @@ std::optional<PathDiagnosticBuilder> PathDiagnosticBuilder::findValidReport(

std::unique_ptr<DiagnosticForConsumerMapTy>
PathSensitiveBugReporter::generatePathDiagnostics(
ArrayRef<PathDiagnosticConsumer *> consumers,
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>> consumers,
ArrayRef<PathSensitiveBugReport *> &bugReports) {
assert(!bugReports.empty());

Expand All @@ -2968,9 +2968,9 @@ PathSensitiveBugReporter::generatePathDiagnostics(
PathDiagnosticBuilder::findValidReport(bugReports, *this);

if (PDB) {
for (PathDiagnosticConsumer *PC : consumers) {
if (std::unique_ptr<PathDiagnostic> PD = PDB->generate(PC)) {
(*Out)[PC] = std::move(PD);
for (const auto &PC : consumers) {
if (std::unique_ptr<PathDiagnostic> PD = PDB->generate(PC.get())) {
(*Out)[PC.get()] = std::move(PD);
}
}
}
Expand Down Expand Up @@ -3164,7 +3164,7 @@ void BugReporter::FlushReport(BugReportEquivClass &EQ) {
return;
}

ArrayRef<PathDiagnosticConsumer*> Consumers = getPathDiagnosticConsumers();
ArrayRef Consumers = getPathDiagnosticConsumers();
std::unique_ptr<DiagnosticForConsumerMapTy> Diagnostics =
generateDiagnosticForConsumerMap(report, Consumers, bugReports);

Expand Down Expand Up @@ -3298,12 +3298,13 @@ findExecutedLines(const SourceManager &SM, const ExplodedNode *N) {

std::unique_ptr<DiagnosticForConsumerMapTy>
BugReporter::generateDiagnosticForConsumerMap(
BugReport *exampleReport, ArrayRef<PathDiagnosticConsumer *> consumers,
BugReport *exampleReport,
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>> consumers,
ArrayRef<BugReport *> bugReports) {
auto *basicReport = cast<BasicBugReport>(exampleReport);
auto Out = std::make_unique<DiagnosticForConsumerMapTy>();
for (auto *Consumer : consumers)
(*Out)[Consumer] =
for (const auto &Consumer : consumers)
(*Out)[Consumer.get()] =
generateDiagnosticForBasicReport(basicReport, AnalysisEntryPoint);
return Out;
}
Expand Down Expand Up @@ -3371,11 +3372,10 @@ static void resetDiagnosticLocationToMainFile(PathDiagnostic &PD) {
}
}



std::unique_ptr<DiagnosticForConsumerMapTy>
PathSensitiveBugReporter::generateDiagnosticForConsumerMap(
BugReport *exampleReport, ArrayRef<PathDiagnosticConsumer *> consumers,
BugReport *exampleReport,
ArrayRef<std::unique_ptr<PathDiagnosticConsumer>> consumers,
ArrayRef<BugReport *> bugReports) {
std::vector<BasicBugReport *> BasicBugReports;
std::vector<PathSensitiveBugReport *> PathSensitiveBugReports;
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ void ento::createHTMLDiagnosticConsumer(
if (OutputDir.empty())
return;

C.push_back(new HTMLDiagnostics(std::move(DiagOpts), OutputDir, PP, true));
C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts),
OutputDir, PP, true));
}

void ento::createHTMLSingleFileDiagnosticConsumer(
Expand All @@ -208,7 +209,8 @@ void ento::createHTMLSingleFileDiagnosticConsumer(
if (OutputDir.empty())
return;

C.push_back(new HTMLDiagnostics(std::move(DiagOpts), OutputDir, PP, false));
C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts),
OutputDir, PP, false));
}

void ento::createPlistHTMLDiagnosticConsumer(
Expand Down
12 changes: 6 additions & 6 deletions clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,9 +541,9 @@ void ento::createPlistDiagnosticConsumer(
if (OutputFile.empty())
return;

C.push_back(new PlistDiagnostics(DiagOpts, OutputFile, PP, CTU,
MacroExpansions,
/*supportsMultipleFiles=*/false));
C.push_back(std::make_unique<PlistDiagnostics>(
DiagOpts, OutputFile, PP, CTU, MacroExpansions,
/*supportsMultipleFiles=*/false));
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
PP, CTU, MacroExpansions);
}
Expand All @@ -558,9 +558,9 @@ void ento::createPlistMultiFileDiagnosticConsumer(
if (OutputFile.empty())
return;

C.push_back(new PlistDiagnostics(DiagOpts, OutputFile, PP, CTU,
MacroExpansions,
/*supportsMultipleFiles=*/true));
C.push_back(std::make_unique<PlistDiagnostics>(
DiagOpts, OutputFile, PP, CTU, MacroExpansions,
/*supportsMultipleFiles=*/true));
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
PP, CTU, MacroExpansions);
}
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/Path.h"
#include <memory>

using namespace llvm;
using namespace clang;
Expand Down Expand Up @@ -60,8 +61,8 @@ void ento::createSarifDiagnosticConsumer(
if (Output.empty())
return;

C.push_back(
new SarifDiagnostics(Output, PP.getLangOpts(), PP.getSourceManager()));
C.push_back(std::make_unique<SarifDiagnostics>(Output, PP.getLangOpts(),
PP.getSourceManager()));
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Output, PP,
CTU, MacroExpansions);
}
Expand Down
21 changes: 11 additions & 10 deletions clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class AnalysisConsumer : public AnalysisASTConsumer,
const std::string OutDir;
AnalyzerOptions &Opts;
ArrayRef<std::string> Plugins;
CodeInjector *Injector;
std::unique_ptr<CodeInjector> Injector;
cross_tu::CrossTranslationUnitContext CTU;

/// Stores the declarations from the local translation unit.
Expand Down Expand Up @@ -123,10 +123,10 @@ class AnalysisConsumer : public AnalysisASTConsumer,

AnalysisConsumer(CompilerInstance &CI, const std::string &outdir,
AnalyzerOptions &opts, ArrayRef<std::string> plugins,
CodeInjector *injector)
std::unique_ptr<CodeInjector> injector)
: RecVisitorMode(0), RecVisitorBR(nullptr), Ctx(nullptr),
PP(CI.getPreprocessor()), OutDir(outdir), Opts(opts),
Plugins(plugins), Injector(injector), CTU(CI),
PP(CI.getPreprocessor()), OutDir(outdir), Opts(opts), Plugins(plugins),
Injector(std::move(injector)), CTU(CI),
MacroExpansions(CI.getLangOpts()) {
DigestAnalyzerOptions();
if (Opts.AnalyzerDisplayProgress || Opts.PrintStats ||
Expand Down Expand Up @@ -229,9 +229,9 @@ class AnalysisConsumer : public AnalysisASTConsumer,
checkerMgr = std::make_unique<CheckerManager>(*Ctx, Opts, PP, Plugins,
CheckerRegistrationFns);

Mgr = std::make_unique<AnalysisManager>(*Ctx, PP, PathConsumers,
CreateStoreMgr, CreateConstraintMgr,
checkerMgr.get(), Opts, Injector);
Mgr = std::make_unique<AnalysisManager>(
*Ctx, PP, std::move(PathConsumers), CreateStoreMgr, CreateConstraintMgr,
checkerMgr.get(), Opts, std::move(Injector));
}

/// Store the top level decls in the set to be processed later on.
Expand Down Expand Up @@ -342,8 +342,9 @@ class AnalysisConsumer : public AnalysisASTConsumer,
return true;
}

void AddDiagnosticConsumer(PathDiagnosticConsumer *Consumer) override {
PathConsumers.push_back(Consumer);
void AddDiagnosticConsumer(
std::unique_ptr<PathDiagnosticConsumer> Consumer) override {
PathConsumers.push_back(std::move(Consumer));
}

void AddCheckerRegistrationFn(std::function<void(CheckerRegistry&)> Fn) override {
Expand Down Expand Up @@ -794,5 +795,5 @@ ento::CreateAnalysisConsumer(CompilerInstance &CI) {
return std::make_unique<AnalysisConsumer>(
CI, CI.getFrontendOpts().OutputFile, analyzerOpts,
CI.getFrontendOpts().Plugins,
hasModelPath ? new ModelInjector(CI) : nullptr);
hasModelPath ? std::make_unique<ModelInjector>(CI) : nullptr);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
#include <memory>

using namespace clang;
using namespace ento;
Expand Down Expand Up @@ -114,8 +115,9 @@ class TestAction : public ASTFrontendAction {
StringRef File) override {
std::unique_ptr<AnalysisASTConsumer> AnalysisConsumer =
CreateAnalysisConsumer(Compiler);
AnalysisConsumer->AddDiagnosticConsumer(new VerifyPathDiagnosticConsumer(
std::move(ExpectedDiags), Compiler.getSourceManager()));
AnalysisConsumer->AddDiagnosticConsumer(
std::make_unique<VerifyPathDiagnosticConsumer>(
std::move(ExpectedDiags), Compiler.getSourceManager()));
AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
Registry.addChecker<InterestingnessTestChecker>("test.Interestingness",
"Description", "");
Expand Down
4 changes: 2 additions & 2 deletions clang/unittests/StaticAnalyzer/CheckerRegistration.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ template <AddCheckerFn... Fns> class TestAction : public ASTFrontendAction {
CreateAnalysisConsumer(Compiler);
if (OnlyEmitWarnings)
AnalysisConsumer->AddDiagnosticConsumer(
new OnlyWarningsDiagConsumer(DiagsOutput));
std::make_unique<OnlyWarningsDiagConsumer>(DiagsOutput));
else
AnalysisConsumer->AddDiagnosticConsumer(
new PathDiagConsumer(DiagsOutput));
std::make_unique<PathDiagConsumer>(DiagsOutput));
addChecker<Fns...>(*AnalysisConsumer, Compiler.getAnalyzerOpts());
return std::move(AnalysisConsumer);
}
Expand Down
Loading
Loading