This revision was automatically updated to reflect the committed changes.
Closed by commit rL325966: [CFG] [analyzer] NFC: Allow more complicated
construction contexts. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43428?vs=134785&id=135701#toc
Repository:
rL LLVM
https://reviews.llvm.org/D43428
Files:
cfe/trunk/include/clang/Analysis/CFG.h
cfe/trunk/lib/Analysis/CFG.cpp
Index: cfe/trunk/include/clang/Analysis/CFG.h
===================================================================
--- cfe/trunk/include/clang/Analysis/CFG.h
+++ cfe/trunk/include/clang/Analysis/CFG.h
@@ -155,14 +155,35 @@
// new-expression triggers construction of the newly allocated object(s).
TriggerTy Trigger;
-public:
+ // Sometimes a single trigger is not enough to describe the construction site.
+ // In this case we'd have a chain of "partial" construction contexts.
+ // Some examples:
+ // - A constructor within in an aggregate initializer list within a variable
+ // would have a construction context of the initializer list with the parent
+ // construction context of a variable.
+ // - A constructor for a temporary that needs to be both destroyed
+ // and materialized into an elidable copy constructor would have a
+ // construction context of a CXXBindTemporaryExpr with the parent
+ // construction context of a MaterializeTemproraryExpr.
+ // Not all of these are currently supported.
+ const ConstructionContext *Parent = nullptr;
+
ConstructionContext() = default;
- ConstructionContext(TriggerTy Trigger)
- : Trigger(Trigger) {}
+ ConstructionContext(TriggerTy Trigger, const ConstructionContext *Parent)
+ : Trigger(Trigger), Parent(Parent) {}
+
+public:
+ static const ConstructionContext *
+ create(BumpVectorContext &C, TriggerTy Trigger,
+ const ConstructionContext *Parent = nullptr) {
+ ConstructionContext *CC = C.getAllocator().Allocate<ConstructionContext>();
+ return new (CC) ConstructionContext(Trigger, Parent);
+ }
bool isNull() const { return Trigger.isNull(); }
TriggerTy getTrigger() const { return Trigger; }
+ const ConstructionContext *getParent() const { return Parent; }
const Stmt *getTriggerStmt() const {
return Trigger.dyn_cast<Stmt *>();
@@ -172,10 +193,27 @@
return Trigger.dyn_cast<CXXCtorInitializer *>();
}
- const ConstructionContext *getPersistentCopy(BumpVectorContext &C) const {
- ConstructionContext *CC = C.getAllocator().Allocate<ConstructionContext>();
- *CC = *this;
- return CC;
+ bool isSameAsPartialContext(const ConstructionContext *Other) const {
+ assert(Other);
+ return (Trigger == Other->Trigger);
+ }
+
+ // See if Other is a proper initial segment of this construction context
+ // in terms of the parent chain - i.e. a few first parents coincide and
+ // then the other context terminates but our context goes further - i.e.,
+ // we are providing the same context that the other context provides,
+ // and a bit more above that.
+ bool isStrictlyMoreSpecificThan(const ConstructionContext *Other) const {
+ const ConstructionContext *Self = this;
+ while (true) {
+ if (!Other)
+ return Self;
+ if (!Self || !Self->isSameAsPartialContext(Other))
+ return false;
+ Self = Self->getParent();
+ Other = Other->getParent();
+ }
+ llvm_unreachable("The above loop can only be terminated via return!");
}
};
@@ -834,9 +872,9 @@
Elements.push_back(CFGStmt(statement), C);
}
- void appendConstructor(CXXConstructExpr *CE, const ConstructionContext &CC,
+ void appendConstructor(CXXConstructExpr *CE, const ConstructionContext *CC,
BumpVectorContext &C) {
- Elements.push_back(CFGConstructor(CE, CC.getPersistentCopy(C)), C);
+ Elements.push_back(CFGConstructor(CE, CC), C);
}
void appendInitializer(CXXCtorInitializer *initializer,
Index: cfe/trunk/lib/Analysis/CFG.cpp
===================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -475,7 +475,8 @@
// Information about the currently visited C++ object construction site.
// This is set in the construction trigger and read when the constructor
// itself is being visited.
- ConstructionContext CurrentConstructionContext = {};
+ llvm::DenseMap<CXXConstructExpr *, const ConstructionContext *>
+ ConstructionContextMap{};
bool badCFG = false;
const CFG::BuildOptions &BuildOpts;
@@ -654,12 +655,12 @@
// to the trigger statement. The construction context will be unset once
// it is consumed when the CFG building procedure processes the
// construct-expression and adds the respective CFGConstructor element.
- void EnterConstructionContextIfNecessary(
- ConstructionContext::TriggerTy Trigger, Stmt *Child);
+ void findConstructionContexts(const ConstructionContext *ContextSoFar,
+ Stmt *Child);
// Unset the construction context after consuming it. This is done immediately
// after adding the CFGConstructor element, so there's no need to
// do this manually in every Visit... function.
- void ExitConstructionContext();
+ void cleanupConstructionContext(CXXConstructExpr *CE);
void autoCreateBlock() { if (!Block) Block = createBlock(); }
CFGBlock *createBlock(bool add_successor = true);
@@ -702,10 +703,9 @@
void appendConstructor(CFGBlock *B, CXXConstructExpr *CE) {
if (BuildOpts.AddRichCXXConstructors) {
- if (!CurrentConstructionContext.isNull()) {
- B->appendConstructor(CE, CurrentConstructionContext,
- cfg->getBumpVectorContext());
- ExitConstructionContext();
+ if (const ConstructionContext *CC = ConstructionContextMap.lookup(CE)) {
+ B->appendConstructor(CE, CC, cfg->getBumpVectorContext());
+ cleanupConstructionContext(CE);
return;
}
}
@@ -1148,25 +1148,38 @@
return nullptr;
}
-void CFGBuilder::EnterConstructionContextIfNecessary(
- ConstructionContext::TriggerTy Trigger, Stmt *Child) {
+void CFGBuilder::findConstructionContexts(
+ const ConstructionContext *ContextSoFar, Stmt *Child) {
if (!BuildOpts.AddRichCXXConstructors)
return;
if (!Child)
return;
- if (isa<CXXConstructExpr>(Child)) {
- assert(CurrentConstructionContext.isNull() &&
- "Already within a construction context!");
- CurrentConstructionContext = ConstructionContext(Trigger);
+ if (auto *CE = dyn_cast<CXXConstructExpr>(Child)) {
+ if (const ConstructionContext *PreviousContext =
+ ConstructionContextMap.lookup(CE)) {
+ // We might have visited this child when we were finding construction
+ // contexts within its parents.
+ assert(PreviousContext->isStrictlyMoreSpecificThan(ContextSoFar) &&
+ "Already within a different construction context!");
+ } else {
+ ConstructionContextMap[CE] = ContextSoFar;
+ }
} else if (auto *Cleanups = dyn_cast<ExprWithCleanups>(Child)) {
- EnterConstructionContextIfNecessary(Trigger, Cleanups->getSubExpr());
+ findConstructionContexts(ContextSoFar, Cleanups->getSubExpr());
+ } else if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Child)) {
+ findConstructionContexts(
+ ConstructionContext::create(cfg->getBumpVectorContext(), BTE,
+ ContextSoFar),
+ BTE->getSubExpr());
}
}
-void CFGBuilder::ExitConstructionContext() {
- assert(!CurrentConstructionContext.isNull() &&
+void CFGBuilder::cleanupConstructionContext(CXXConstructExpr *CE) {
+ assert(BuildOpts.AddRichCXXConstructors &&
+ "We should not be managing construction contexts!");
+ assert(ConstructionContextMap.count(CE) &&
"Cannot exit construction context without the context!");
- CurrentConstructionContext = ConstructionContext();
+ ConstructionContextMap.erase(CE);
}
@@ -1250,6 +1263,10 @@
// Create an empty entry block that has no predecessors.
cfg->setEntry(createBlock());
+ if (BuildOpts.AddRichCXXConstructors)
+ assert(ConstructionContextMap.empty() &&
+ "Not all construction contexts were cleaned up!");
+
return std::move(cfg);
}
@@ -1297,7 +1314,9 @@
appendInitializer(Block, I);
if (Init) {
- EnterConstructionContextIfNecessary(I, Init);
+ findConstructionContexts(
+ ConstructionContext::create(cfg->getBumpVectorContext(), I),
+ Init);
if (HasTemporaries) {
// For expression with temporaries go directly to subexpression to omit
@@ -2383,7 +2402,9 @@
autoCreateBlock();
appendStmt(Block, DS);
- EnterConstructionContextIfNecessary(DS, Init);
+ findConstructionContexts(
+ ConstructionContext::create(cfg->getBumpVectorContext(), DS),
+ Init);
// Keep track of the last non-null block, as 'Block' can be nulled out
// if the initializer expression is something like a 'while' in a
@@ -2575,7 +2596,9 @@
addAutomaticObjHandling(ScopePos, LocalScope::const_iterator(), R);
- EnterConstructionContextIfNecessary(R, R->getRetValue());
+ findConstructionContexts(
+ ConstructionContext::create(cfg->getBumpVectorContext(), R),
+ R->getRetValue());
// If the one of the destructors does not return, we already have the Exit
// block as a successor.
@@ -3923,7 +3946,9 @@
autoCreateBlock();
appendStmt(Block, E);
- EnterConstructionContextIfNecessary(E, E->getSubExpr());
+ findConstructionContexts(
+ ConstructionContext::create(cfg->getBumpVectorContext(), E),
+ E->getSubExpr());
// We do not want to propagate the AlwaysAdd property.
asc = asc.withAlwaysAdd(false);
@@ -3944,8 +3969,9 @@
autoCreateBlock();
appendStmt(Block, NE);
- EnterConstructionContextIfNecessary(
- NE, const_cast<CXXConstructExpr *>(NE->getConstructExpr()));
+ findConstructionContexts(
+ ConstructionContext::create(cfg->getBumpVectorContext(), NE),
+ const_cast<CXXConstructExpr *>(NE->getConstructExpr()));
if (NE->getInitializer())
Block = Visit(NE->getInitializer());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
