r368689 - [analyzer][NFC] Refactoring BugReporter.cpp P1.: Store interesting symbols/regions in a simple set
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 13 06:09:48 PDT 2019
Author: szelethus
Date: Tue Aug 13 06:09:48 2019
New Revision: 368689
URL: http://llvm.org/viewvc/llvm-project?rev=368689&view=rev
Log:
[analyzer][NFC] Refactoring BugReporter.cpp P1.: Store interesting symbols/regions in a simple set
The goal of this refactoring effort was to better understand how interestingness
was propagated in BugReporter.cpp, which eventually turned out to be a dead end,
but with such a twist, I wouldn't even want to spoil it ahead of time. However,
I did get to learn a lot about how things are working in there.
In these series of patches, as well as cleaning up the code big time, I invite
you to study how BugReporter.cpp operates, and discuss how we could design this
file to reduce the horrible mess that it is.
This patch reverts a great part of rC162028, which holds the title "Allow
multiple PathDiagnosticConsumers to be used with a BugReporter at the same
time.". This, however doesn't imply that there's any need for multiple "layers"
or stacks of interesting symbols and regions, quite the contrary, I would argue
that we would like to generate the same amount of information for all output
types, and only process them differently.
Differential Revision: https://reviews.llvm.org/D65378
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=368689&r1=368688&r2=368689&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Tue Aug 13 06:09:48 2019
@@ -107,22 +107,19 @@ protected:
ExtraTextList ExtraText;
NoteList Notes;
- using Symbols = llvm::DenseSet<SymbolRef>;
- using Regions = llvm::DenseSet<const MemRegion *>;
-
/// A (stack of) a set of symbols that are registered with this
/// report as being "interesting", and thus used to help decide which
/// diagnostics to include when constructing the final path diagnostic.
/// The stack is largely used by BugReporter when generating PathDiagnostics
/// for multiple PathDiagnosticConsumers.
- SmallVector<Symbols *, 2> interestingSymbols;
+ llvm::DenseSet<SymbolRef> InterestingSymbols;
/// A (stack of) set of regions that are registered with this report as being
/// "interesting", and thus used to help decide which diagnostics
/// to include when constructing the final path diagnostic.
/// The stack is largely used by BugReporter when generating PathDiagnostics
/// for multiple PathDiagnosticConsumers.
- SmallVector<Regions *, 2> interestingRegions;
+ llvm::DenseSet<const MemRegion *> InterestingRegions;
/// A set of location contexts that correspoind to call sites which should be
/// considered "interesting".
@@ -156,15 +153,6 @@ protected:
/// Conditions we're already tracking.
llvm::SmallSet<const ExplodedNode *, 4> TrackedConditions;
-private:
- // Used internally by BugReporter.
- Symbols &getInterestingSymbols();
- Regions &getInterestingRegions();
-
- void lazyInitializeInterestingSets();
- void pushInterestingSymbolsAndRegions();
- void popInterestingSymbolsAndRegions();
-
public:
BugReport(const BugType& bt, StringRef desc, const ExplodedNode *errornode)
: BT(bt), Description(desc), ErrorNode(errornode) {}
@@ -189,7 +177,7 @@ public:
: BT(bt), Description(desc), UniqueingLocation(LocationToUnique),
UniqueingDecl(DeclToUnique), ErrorNode(errornode) {}
- virtual ~BugReport();
+ virtual ~BugReport() = default;
const BugType& getBugType() const { return BT; }
//BugType& getBugType() { return BT; }
@@ -381,7 +369,6 @@ class BugReportEquivClass : public llvm:
public:
BugReportEquivClass(std::unique_ptr<BugReport> R) { AddReport(std::move(R)); }
- ~BugReportEquivClass();
void Profile(llvm::FoldingSetNodeID& ID) const {
assert(!Reports.empty());
@@ -404,7 +391,7 @@ public:
class BugReporterData {
public:
- virtual ~BugReporterData();
+ virtual ~BugReporterData() = default;
virtual DiagnosticsEngine& getDiagnostic() = 0;
virtual ArrayRef<PathDiagnosticConsumer*> getPathDiagnosticConsumers() = 0;
@@ -526,7 +513,7 @@ public:
GRBugReporter(BugReporterData& d, ExprEngine& eng)
: BugReporter(d, GRBugReporterKind), Eng(eng) {}
- ~GRBugReporter() override;
+ ~GRBugReporter() override = default;;
/// getGraph - Get the exploded graph created by the analysis engine
/// for the analyzed method or function.
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=368689&r1=368688&r2=368689&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Tue Aug 13 06:09:48 2019
@@ -2058,12 +2058,6 @@ void BugReport::clearVisitors() {
Callbacks.clear();
}
-BugReport::~BugReport() {
- while (!interestingSymbols.empty()) {
- popInterestingSymbolsAndRegions();
- }
-}
-
const Decl *BugReport::getDeclWithIssue() const {
if (DeclWithIssue)
return DeclWithIssue;
@@ -2101,10 +2095,10 @@ void BugReport::markInteresting(SymbolRe
if (!sym)
return;
- getInterestingSymbols().insert(sym);
+ InterestingSymbols.insert(sym);
if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
- getInterestingRegions().insert(meta->getRegion());
+ InterestingRegions.insert(meta->getRegion());
}
void BugReport::markInteresting(const MemRegion *R) {
@@ -2112,10 +2106,10 @@ void BugReport::markInteresting(const Me
return;
R = R->getBaseRegion();
- getInterestingRegions().insert(R);
+ InterestingRegions.insert(R);
if (const auto *SR = dyn_cast<SymbolicRegion>(R))
- getInterestingSymbols().insert(SR->getSymbol());
+ InterestingSymbols.insert(SR->getSymbol());
}
void BugReport::markInteresting(SVal V) {
@@ -2138,18 +2132,18 @@ bool BugReport::isInteresting(SymbolRef
return false;
// We don't currently consider metadata symbols to be interesting
// even if we know their region is interesting. Is that correct behavior?
- return getInterestingSymbols().count(sym);
+ return InterestingSymbols.count(sym);
}
bool BugReport::isInteresting(const MemRegion *R) {
if (!R)
return false;
R = R->getBaseRegion();
- bool b = getInterestingRegions().count(R);
+ bool b = InterestingRegions.count(R);
if (b)
return true;
if (const auto *SR = dyn_cast<SymbolicRegion>(R))
- return getInterestingSymbols().count(SR->getSymbol());
+ return InterestingSymbols.count(SR->getSymbol());
return false;
}
@@ -2159,33 +2153,6 @@ bool BugReport::isInteresting(const Loca
return InterestingLocationContexts.count(LC);
}
-void BugReport::lazyInitializeInterestingSets() {
- if (interestingSymbols.empty()) {
- interestingSymbols.push_back(new Symbols());
- interestingRegions.push_back(new Regions());
- }
-}
-
-BugReport::Symbols &BugReport::getInterestingSymbols() {
- lazyInitializeInterestingSets();
- return *interestingSymbols.back();
-}
-
-BugReport::Regions &BugReport::getInterestingRegions() {
- lazyInitializeInterestingSets();
- return *interestingRegions.back();
-}
-
-void BugReport::pushInterestingSymbolsAndRegions() {
- interestingSymbols.push_back(new Symbols(getInterestingSymbols()));
- interestingRegions.push_back(new Regions(getInterestingRegions()));
-}
-
-void BugReport::popInterestingSymbolsAndRegions() {
- delete interestingSymbols.pop_back_val();
- delete interestingRegions.pop_back_val();
-}
-
const Stmt *BugReport::getStmt() const {
if (!ErrorNode)
return nullptr;
@@ -2236,12 +2203,6 @@ PathDiagnosticLocation BugReport::getLoc
// Methods for BugReporter and subclasses.
//===----------------------------------------------------------------------===//
-BugReportEquivClass::~BugReportEquivClass() = default;
-
-GRBugReporter::~GRBugReporter() = default;
-
-BugReporterData::~BugReporterData() = default;
-
ExplodedGraph &GRBugReporter::getGraph() { return Eng.getGraph(); }
ProgramStateManager&
More information about the cfe-commits
mailing list