NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
This is a refactoring patch; no functional change intended.
`ConstructionContext` is composed of `ConstructionContextLayer`s, but the layer
information is explicitly discarded when `ConstructionContext` is created, in
favor of a more user-friendly and explicit interface for accessing the
interesting items in the `ConstructionContext`. Later, however - in the
Analyzer, which is currently the only user of this API - there appears
`ConstructedObjectKey` which helps tracking path-sensitive information about
the constructed object. Apparently, `ConstructedObjectKey` has a lot in common
with `ConstructionContextLayer`: it captures a single item that requires
attention during object construction.
The common part of these two structures is factored out into a new structure,
`ConstructionContextItem`. With that, `ConstructionContextLayer` becomes a
wrapper around `ConstructionContextItem` that additionally temporarily
organizes the items as a linked list, and `ConstructedObjectKey` becomes a
wrapper around `ConstructionContextItem`that additionally carries
path-sensitive stack frame information.
Once `ConstructionContextItem` is factored out, i also made it a bit more
explicit when it comes to what sort of information does it carry. It now
contains an enumeration of item kinds ("variable", "destructor",
"materialization", "elidable copy"). Such clarification was necessary for
supporting argument constructors because otherwise it's hard to discriminate
between an elidable copy layer (identified by a construct-expression and,
occasionally, index 0) and a layer for the first argument of a constructor
(identified by a construct-expression and, intentionally, index 0). I believe
it was a good idea in general to be more explicit about what the layer means,
rather than give its clients an ad-hoc pile of raw data to work with. It also
allows fancy switches in `createFromLayers()` that check that all cases were
handled, and many assertions become more explicit.
I did not, however, want to introduce a full-scale class hierarchy of
`ConstructionContextItem`s because it'd result in a ridiculous amount of
boilerplate. Having them easy to construct via implicit conversion, on the
other hand, seems very user-friendly and i'm not seeing much error surface.
One of the questionable ideas here is to use a special item kind
`ElidedDestructor` that is only used by the analyzer (to track destructors that
correspond to elided constructors) and is never provided by the CFG (so unlike
other items it has only one use). This allows us to stop using an extra program
state set for elided destructors, but i'm not sure it was a beautiful thing to
pursue.
Repository:
rC Clang
https://reviews.llvm.org/D49210
Files:
include/clang/Analysis/ConstructionContext.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/Analysis/CFG.cpp
lib/Analysis/ConstructionContext.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -117,56 +117,40 @@
/// the construction context was present and contained references to these
/// AST nodes.
class ConstructedObjectKey {
- typedef std::pair<
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *>,
- const LocationContext *> ConstructedObjectKeyImpl;
+ typedef std::pair<ConstructionContextItem, const LocationContext *>
+ ConstructedObjectKeyImpl;
- ConstructedObjectKeyImpl Impl;
+ const ConstructedObjectKeyImpl Impl;
const void *getAnyASTNodePtr() const {
- if (const Stmt *S = getStmt())
+ if (const Stmt *S = getItem().getStmt())
return S;
else
- return getCXXCtorInitializer();
+ return getItem().getCXXCtorInitializer();
}
public:
- ConstructedObjectKey(
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
- const LocationContext *LC)
- : Impl(P, LC) {
- // This is the full list of statements that require additional actions when
- // encountered. This list may be expanded when new actions are implemented.
- assert(getCXXCtorInitializer() || isa<DeclStmt>(getStmt()) ||
- isa<CXXNewExpr>(getStmt()) || isa<CXXBindTemporaryExpr>(getStmt()) ||
- isa<MaterializeTemporaryExpr>(getStmt()) ||
- isa<CXXConstructExpr>(getStmt()));
- }
-
- const Stmt *getStmt() const {
- return Impl.first.dyn_cast<const Stmt *>();
- }
-
- const CXXCtorInitializer *getCXXCtorInitializer() const {
- return Impl.first.dyn_cast<const CXXCtorInitializer *>();
- }
+ explicit ConstructedObjectKey(const ConstructionContextItem &Item,
+ const LocationContext *LC)
+ : Impl(Item, LC) {}
- const LocationContext *getLocationContext() const {
- return Impl.second;
- }
+ const ConstructionContextItem &getItem() const { return Impl.first; }
+ const LocationContext *getLocationContext() const { return Impl.second; }
void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) {
OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ") ";
- if (const Stmt *S = getStmt()) {
+ if (const Stmt *S = getItem().getStmtOrNull()) {
S->printPretty(OS, Helper, PP);
+ if (getItem().getKind() == ConstructionContextItem::ElidedDestructorKind)
+ OS << " (skip destructor for an elided constructor)";
} else {
- const CXXCtorInitializer *I = getCXXCtorInitializer();
+ const CXXCtorInitializer *I = getItem().getCXXCtorInitializer();
OS << I->getAnyMember()->getNameAsString();
}
}
void Profile(llvm::FoldingSetNodeID &ID) const {
- ID.AddPointer(Impl.first.getOpaqueValue());
+ ID.Add(Impl.first);
ID.AddPointer(Impl.second);
}
@@ -184,15 +168,6 @@
REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction,
ObjectsUnderConstructionMap)
-// Additionally, track a set of destructors that correspond to elided
-// constructors when copy elision occurs.
-typedef std::pair<const CXXBindTemporaryExpr *, const LocationContext *>
- ElidedDestructorItem;
-typedef llvm::ImmutableSet<ElidedDestructorItem>
- ElidedDestructorSet;
-REGISTER_TRAIT_WITH_PROGRAMSTATE(ElidedDestructors,
- ElidedDestructorSet);
-
//===----------------------------------------------------------------------===//
// Engine construction and deletion.
//===----------------------------------------------------------------------===//
@@ -449,57 +424,58 @@
return State;
}
-ProgramStateRef ExprEngine::addObjectUnderConstruction(
- ProgramStateRef State,
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
- const LocationContext *LC, SVal V) {
- ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
+ProgramStateRef
+ExprEngine::addObjectUnderConstruction(ProgramStateRef State,
+ const ConstructionContextItem &Item,
+ const LocationContext *LC, SVal V) {
+ ConstructedObjectKey Key(Item, LC->getCurrentStackFrame());
// FIXME: Currently the state might already contain the marker due to
// incorrect handling of temporaries bound to default parameters.
assert(!State->get<ObjectsUnderConstruction>(Key) ||
- isa<CXXBindTemporaryExpr>(Key.getStmt()));
+ Key.getItem().getKind() ==
+ ConstructionContextItem::TemporaryDestructorKind);
return State->set<ObjectsUnderConstruction>(Key, V);
}
-Optional<SVal> ExprEngine::getObjectUnderConstruction(
- ProgramStateRef State,
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
- const LocationContext *LC) {
- ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
+Optional<SVal>
+ExprEngine::getObjectUnderConstruction(ProgramStateRef State,
+ const ConstructionContextItem &Item,
+ const LocationContext *LC) {
+ ConstructedObjectKey Key(Item, LC->getCurrentStackFrame());
return Optional<SVal>::create(State->get<ObjectsUnderConstruction>(Key));
}
-ProgramStateRef ExprEngine::finishObjectConstruction(
- ProgramStateRef State,
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
- const LocationContext *LC) {
- ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
+ProgramStateRef
+ExprEngine::finishObjectConstruction(ProgramStateRef State,
+ const ConstructionContextItem &Item,
+ const LocationContext *LC) {
+ ConstructedObjectKey Key(Item, LC->getCurrentStackFrame());
assert(State->contains<ObjectsUnderConstruction>(Key));
return State->remove<ObjectsUnderConstruction>(Key);
}
ProgramStateRef ExprEngine::elideDestructor(ProgramStateRef State,
const CXXBindTemporaryExpr *BTE,
const LocationContext *LC) {
- ElidedDestructorItem I(BTE, LC);
- assert(!State->contains<ElidedDestructors>(I));
- return State->add<ElidedDestructors>(I);
+ ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC);
+ assert(!State->contains<ObjectsUnderConstruction>(Key));
+ return State->set<ObjectsUnderConstruction>(Key, UnknownVal());
}
ProgramStateRef
ExprEngine::cleanupElidedDestructor(ProgramStateRef State,
const CXXBindTemporaryExpr *BTE,
const LocationContext *LC) {
- ElidedDestructorItem I(BTE, LC);
- assert(State->contains<ElidedDestructors>(I));
- return State->remove<ElidedDestructors>(I);
+ ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC);
+ assert(State->contains<ObjectsUnderConstruction>(Key));
+ return State->remove<ObjectsUnderConstruction>(Key);
}
bool ExprEngine::isDestructorElided(ProgramStateRef State,
const CXXBindTemporaryExpr *BTE,
const LocationContext *LC) {
- ElidedDestructorItem I(BTE, LC);
- return State->contains<ElidedDestructors>(I);
+ ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC);
+ return State->contains<ObjectsUnderConstruction>(Key);
}
bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State,
@@ -512,10 +488,6 @@
if (I.first.getLocationContext() == LC)
return false;
- for (auto I: State->get<ElidedDestructors>())
- if (I.second == LC)
- return false;
-
LC = LC->getParent();
}
return true;
@@ -560,14 +532,6 @@
Key.print(Out, nullptr, PP);
Out << " : " << Value << NL;
}
-
- for (auto I : State->get<ElidedDestructors>()) {
- if (I.second != LC)
- continue;
- Out << '(' << I.second << ',' << (const void *)I.first << ") ";
- I.first->printPretty(Out, nullptr, PP);
- Out << " : (constructor elided)" << NL;
- }
}
void ExprEngine::printState(raw_ostream &Out, ProgramStateRef State,
@@ -2252,25 +2216,14 @@
for (auto I : State->get<ObjectsUnderConstruction>())
if (I.first.getLocationContext() == LC) {
// The comment above only pardons us for not cleaning up a
- // CXXBindTemporaryExpr. If any other statements are found here,
+ // temporary destructor. If any other statements are found here,
// it must be a separate problem.
- assert(isa<CXXBindTemporaryExpr>(I.first.getStmt()));
+ assert(I.first.getItem().getKind() ==
+ ConstructionContextItem::TemporaryDestructorKind ||
+ I.first.getItem().getKind() ==
+ ConstructionContextItem::ElidedDestructorKind);
State = State->remove<ObjectsUnderConstruction>(I.first);
- // Also cleanup the elided destructor info.
- ElidedDestructorItem Item(
- cast<CXXBindTemporaryExpr>(I.first.getStmt()),
- I.first.getLocationContext());
- State = State->remove<ElidedDestructors>(Item);
}
-
- // Also suppress the assertion for elided destructors when temporary
- // destructors are not provided at all by the CFG, because there's no
- // good place to clean them up.
- if (!AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG())
- for (auto I : State->get<ElidedDestructors>())
- if (I.second == LC)
- State = State->remove<ElidedDestructors>(I);
-
LC = LC->getParent();
}
if (State != Pred->getState()) {
Index: lib/Analysis/ConstructionContext.cpp
===================================================================
--- lib/Analysis/ConstructionContext.cpp
+++ lib/Analysis/ConstructionContext.cpp
@@ -20,178 +20,198 @@
using namespace clang;
const ConstructionContextLayer *
-ConstructionContextLayer::create(BumpVectorContext &C, TriggerTy Trigger,
- unsigned Index,
+ConstructionContextLayer::create(BumpVectorContext &C,
+ const ConstructionContextItem &Item,
const ConstructionContextLayer *Parent) {
ConstructionContextLayer *CC =
C.getAllocator().Allocate<ConstructionContextLayer>();
- return new (CC) ConstructionContextLayer(Trigger, Index, Parent);
+ return new (CC) ConstructionContextLayer(Item, Parent);
}
bool ConstructionContextLayer::isStrictlyMoreSpecificThan(
const ConstructionContextLayer *Other) const {
const ConstructionContextLayer *Self = this;
while (true) {
if (!Other)
return Self;
- if (!Self || !Self->isSameLayer(Other))
+ if (!Self || !(Self->Item == Other->Item))
return false;
Self = Self->getParent();
Other = Other->getParent();
}
llvm_unreachable("The above loop can only be terminated via return!");
}
+const ConstructionContext *
+ConstructionContext::createMaterializedTemporaryFromLayers(
+ BumpVectorContext &C, const MaterializeTemporaryExpr *MTE,
+ const CXXBindTemporaryExpr *BTE,
+ const ConstructionContextLayer *ParentLayer) {
+ assert(MTE);
+
+ // If the object requires destruction and is not lifetime-extended,
+ // then it must have a BTE within its MTE, otherwise it shouldn't.
+ // FIXME: This should be an assertion.
+ if (!BTE && !(MTE->getType().getCanonicalType()->getAsCXXRecordDecl()
+ ->hasTrivialDestructor() ||
+ MTE->getStorageDuration() != SD_FullExpression)) {
+ return nullptr;
+ }
+
+ // If the temporary is lifetime-extended, don't save the BTE,
+ // because we don't need a temporary destructor, but an automatic
+ // destructor.
+ if (MTE->getStorageDuration() != SD_FullExpression) {
+ BTE = nullptr;
+ }
+
+ // Handle pre-C++17 copy and move elision.
+ const CXXConstructExpr *ElidedCE = nullptr;
+ const ConstructionContext *ElidedCC = nullptr;
+ if (ParentLayer) {
+ const ConstructionContextItem &ElidedItem = ParentLayer->getItem();
+ assert(ElidedItem.getKind() ==
+ ConstructionContextItem::ElidableConstructorKind);
+ ElidedCE = cast<CXXConstructExpr>(ElidedItem.getStmt());
+ assert(ElidedCE->isElidable());
+ // We're creating a construction context that might have already
+ // been created elsewhere. Maybe we should unique our construction
+ // contexts. That's what we often do, but in this case it's unlikely
+ // to bring any benefits.
+ ElidedCC = createFromLayers(C, ParentLayer->getParent());
+ if (!ElidedCC) {
+ // We may fail to create the elided construction context.
+ // In this case, skip copy elision entirely.
+ return create<SimpleTemporaryObjectConstructionContext>(C, BTE, MTE);
+ }
+ return create<ElidedTemporaryObjectConstructionContext>(
+ C, BTE, MTE, ElidedCE, ElidedCC);
+ }
+
+ // This is a normal temporary.
+ assert(!ParentLayer);
+ return create<SimpleTemporaryObjectConstructionContext>(C, BTE, MTE);
+}
+
+const ConstructionContext *ConstructionContext::createBoundTemporaryFromLayers(
+ BumpVectorContext &C, const CXXBindTemporaryExpr *BTE,
+ const ConstructionContextLayer *ParentLayer) {
+ if (!ParentLayer) {
+ // A temporary object that doesn't require materialization.
+ // In particular, it shouldn't require copy elision, because
+ // copy/move constructors take a reference, which requires
+ // materialization to obtain the glvalue.
+ return create<SimpleTemporaryObjectConstructionContext>(C, BTE,
+ /*MTE=*/nullptr);
+ }
+
+ const ConstructionContextItem &ParentItem = ParentLayer->getItem();
+ switch (ParentItem.getKind()) {
+ case ConstructionContextItem::VariableKind: {
+ const auto *DS = cast<DeclStmt>(ParentItem.getStmt());
+ assert(!cast<VarDecl>(DS->getSingleDecl())->getType().getCanonicalType()
+ ->getAsCXXRecordDecl()->hasTrivialDestructor());
+ return create<CXX17ElidedCopyVariableConstructionContext>(C, DS, BTE);
+ }
+ case ConstructionContextItem::NewAllocatorKind: {
+ llvm_unreachable("This context does not accept a bound temporary!");
+ }
+ case ConstructionContextItem::ReturnKind: {
+ assert(ParentLayer->isLast());
+ const auto *RS = cast<ReturnStmt>(ParentItem.getStmt());
+ assert(!RS->getRetValue()->getType().getCanonicalType()
+ ->getAsCXXRecordDecl()->hasTrivialDestructor());
+ return create<CXX17ElidedCopyReturnedValueConstructionContext>(C, RS,
+ BTE);
+ }
+
+ case ConstructionContextItem::MaterializationKind: {
+ // No assert. We may have an elidable copy on the grandparent layer.
+ const auto *MTE = cast<MaterializeTemporaryExpr>(ParentItem.getStmt());
+ return createMaterializedTemporaryFromLayers(C, MTE, BTE,
+ ParentLayer->getParent());
+ }
+ case ConstructionContextItem::TemporaryDestructorKind: {
+ llvm_unreachable("Duplicate CXXBindTemporaryExpr in the AST!");
+ }
+ case ConstructionContextItem::ElidedDestructorKind: {
+ llvm_unreachable("Elided destructor items are not produced by the CFG!");
+ }
+ case ConstructionContextItem::ElidableConstructorKind: {
+ llvm_unreachable("Materialization is necessary to put temporary into a "
+ "copy or move constructor!");
+ }
+ case ConstructionContextItem::ArgumentKind: {
+ assert(ParentLayer->isLast());
+ const auto *E = cast<Expr>(ParentItem.getStmt());
+ assert(isa<CallExpr>(E) || isa<CXXConstructExpr>(E) ||
+ isa<ObjCMessageExpr>(E));
+ return create<ArgumentConstructionContext>(C, E, ParentItem.getIndex(),
+ BTE);
+ }
+ case ConstructionContextItem::InitializerKind: {
+ assert(ParentLayer->isLast());
+ const auto *I = ParentItem.getCXXCtorInitializer();
+ assert(!I->getAnyMember()->getType().getCanonicalType()
+ ->getAsCXXRecordDecl()->hasTrivialDestructor());
+ return create<CXX17ElidedCopyConstructorInitializerConstructionContext>(
+ C, I, BTE);
+ }
+ } // switch (ParentItem.getKind())
+
+ llvm_unreachable("Unexpected construction context with destructor!");
+}
+
const ConstructionContext *ConstructionContext::createFromLayers(
BumpVectorContext &C, const ConstructionContextLayer *TopLayer) {
// Before this point all we've had was a stockpile of arbitrary layers.
// Now validate that it is shaped as one of the finite amount of expected
// patterns.
- if (const Stmt *S = TopLayer->getTriggerStmt()) {
- if (const auto *DS = dyn_cast<DeclStmt>(S)) {
- assert(TopLayer->isLast());
- return create<SimpleVariableConstructionContext>(C, DS);
- }
- if (const auto *NE = dyn_cast<CXXNewExpr>(S)) {
- assert(TopLayer->isLast());
- return create<NewAllocatedObjectConstructionContext>(C, NE);
- }
- if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(S)) {
- const MaterializeTemporaryExpr *MTE = nullptr;
- assert(BTE->getType().getCanonicalType()
- ->getAsCXXRecordDecl()->hasNonTrivialDestructor());
- // For temporaries with destructors, there may or may not be
- // lifetime extension on the parent layer.
- if (const ConstructionContextLayer *ParentLayer = TopLayer->getParent()) {
- // C++17 *requires* elision of the constructor at the return site
- // and at variable/member initialization site, while previous standards
- // were allowing an optional elidable constructor.
- // This is the C++17 copy-elided construction into a ctor initializer.
- if (const CXXCtorInitializer *I = ParentLayer->getTriggerInit()) {
- return create<
- CXX17ElidedCopyConstructorInitializerConstructionContext>(C,
- I, BTE);
- }
- assert(ParentLayer->getTriggerStmt() &&
- "Non-statement-based layers have been handled above!");
- // This is the normal, non-C++17 case: a temporary object which has
- // both destruction and materialization info attached to it in the AST.
- if ((MTE = dyn_cast<MaterializeTemporaryExpr>(
- ParentLayer->getTriggerStmt()))) {
- if (MTE->getStorageDuration() != SD_FullExpression) {
- // If the temporary is lifetime-extended, don't save the BTE,
- // because we don't need a temporary destructor, but an automatic
- // destructor.
- BTE = nullptr;
- }
-
- // Handle pre-C++17 copy and move elision.
- const CXXConstructExpr *ElidedCE = nullptr;
- const ConstructionContext *ElidedCC = nullptr;
- if (const ConstructionContextLayer *ElidedLayer =
- ParentLayer->getParent()) {
- ElidedCE = cast<CXXConstructExpr>(ElidedLayer->getTriggerStmt());
- assert(ElidedCE->isElidable());
- // We're creating a construction context that might have already
- // been created elsewhere. Maybe we should unique our construction
- // contexts. That's what we often do, but in this case it's unlikely
- // to bring any benefits.
- ElidedCC = createFromLayers(C, ElidedLayer->getParent());
- if (!ElidedCC) {
- // We may fail to create the elided construction context.
- // In this case, skip copy elision entirely.
- return create<SimpleTemporaryObjectConstructionContext>(C, BTE,
- MTE);
- } else {
- return create<ElidedTemporaryObjectConstructionContext>(
- C, BTE, MTE, ElidedCE, ElidedCC);
- }
- }
- assert(ParentLayer->isLast());
- return create<SimpleTemporaryObjectConstructionContext>(C, BTE, MTE);
- }
- assert(ParentLayer->isLast());
-
- // This is a constructor into a function argument.
- if (isa<CallExpr>(ParentLayer->getTriggerStmt()) ||
- isa<CXXConstructExpr>(ParentLayer->getTriggerStmt()) ||
- isa<ObjCMessageExpr>(ParentLayer->getTriggerStmt())) {
- return create<ArgumentConstructionContext>(
- C, cast<Expr>(ParentLayer->getTriggerStmt()),
- ParentLayer->getIndex(), BTE);
- }
- // This is C++17 copy-elided construction into return statement.
- if (auto *RS = dyn_cast<ReturnStmt>(ParentLayer->getTriggerStmt())) {
- assert(!RS->getRetValue()->getType().getCanonicalType()
- ->getAsCXXRecordDecl()->hasTrivialDestructor());
- return create<CXX17ElidedCopyReturnedValueConstructionContext>(C,
- RS, BTE);
- }
- // This is C++17 copy-elided construction into a simple variable.
- if (auto *DS = dyn_cast<DeclStmt>(ParentLayer->getTriggerStmt())) {
- assert(!cast<VarDecl>(DS->getSingleDecl())->getType()
- .getCanonicalType()->getAsCXXRecordDecl()
- ->hasTrivialDestructor());
- return create<CXX17ElidedCopyVariableConstructionContext>(C, DS, BTE);
- }
- llvm_unreachable("Unexpected construction context with destructor!");
- }
- // A temporary object that doesn't require materialization.
- // In particular, it shouldn't require copy elision, because
- // copy/move constructors take a reference, which requires
- // materialization to obtain the glvalue.
- return create<SimpleTemporaryObjectConstructionContext>(C, BTE,
- /*MTE=*/nullptr);
- }
- if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(S)) {
- // If the object requires destruction and is not lifetime-extended,
- // then it must have a BTE within its MTE.
- // FIXME: This should be an assertion.
- if (!(MTE->getType().getCanonicalType()
- ->getAsCXXRecordDecl()->hasTrivialDestructor() ||
- MTE->getStorageDuration() != SD_FullExpression))
- return nullptr;
-
- // Handle pre-C++17 copy and move elision.
- const CXXConstructExpr *ElidedCE = nullptr;
- const ConstructionContext *ElidedCC = nullptr;
- if (const ConstructionContextLayer *ElidedLayer = TopLayer->getParent()) {
- ElidedCE = cast<CXXConstructExpr>(ElidedLayer->getTriggerStmt());
- assert(ElidedCE->isElidable());
- // We're creating a construction context that might have already
- // been created elsewhere. Maybe we should unique our construction
- // contexts. That's what we often do, but in this case it's unlikely
- // to bring any benefits.
- ElidedCC = createFromLayers(C, ElidedLayer->getParent());
- if (!ElidedCC) {
- // We may fail to create the elided construction context.
- // In this case, skip copy elision entirely.
- return create<SimpleTemporaryObjectConstructionContext>(C, nullptr,
- MTE);
- }
- return create<ElidedTemporaryObjectConstructionContext>(
- C, nullptr, MTE, ElidedCE, ElidedCC);
- }
- assert(TopLayer->isLast());
- return create<SimpleTemporaryObjectConstructionContext>(C, nullptr, MTE);
- }
- if (const auto *RS = dyn_cast<ReturnStmt>(S)) {
- assert(TopLayer->isLast());
- return create<SimpleReturnedValueConstructionContext>(C, RS);
- }
- // This is a constructor into a function argument.
- if (isa<CallExpr>(TopLayer->getTriggerStmt()) ||
- isa<CXXConstructExpr>(TopLayer->getTriggerStmt()) ||
- isa<ObjCMessageExpr>(TopLayer->getTriggerStmt())) {
- assert(TopLayer->isLast());
- return create<ArgumentConstructionContext>(
- C, cast<Expr>(TopLayer->getTriggerStmt()), TopLayer->getIndex(),
- /*BTE=*/nullptr);
- }
- llvm_unreachable("Unexpected construction context with statement!");
- } else if (const CXXCtorInitializer *I = TopLayer->getTriggerInit()) {
+ const ConstructionContextItem &TopItem = TopLayer->getItem();
+ switch (TopItem.getKind()) {
+ case ConstructionContextItem::VariableKind: {
assert(TopLayer->isLast());
+ const auto *DS = cast<DeclStmt>(TopItem.getStmt());
+ return create<SimpleVariableConstructionContext>(C, DS);
+ }
+ case ConstructionContextItem::NewAllocatorKind: {
+ assert(TopLayer->isLast());
+ const auto *NE = cast<CXXNewExpr>(TopItem.getStmt());
+ return create<NewAllocatedObjectConstructionContext>(C, NE);
+ }
+ case ConstructionContextItem::ReturnKind: {
+ assert(TopLayer->isLast());
+ const auto *RS = cast<ReturnStmt>(TopItem.getStmt());
+ return create<SimpleReturnedValueConstructionContext>(C, RS);
+ }
+ case ConstructionContextItem::MaterializationKind: {
+ const auto *MTE = cast<MaterializeTemporaryExpr>(TopItem.getStmt());
+ return createMaterializedTemporaryFromLayers(C, MTE, /*BTE=*/nullptr,
+ TopLayer->getParent());
+ }
+ case ConstructionContextItem::TemporaryDestructorKind: {
+ const auto *BTE = cast<CXXBindTemporaryExpr>(TopItem.getStmt());
+ assert(BTE->getType().getCanonicalType()->getAsCXXRecordDecl()
+ ->hasNonTrivialDestructor());
+ return createBoundTemporaryFromLayers(C, BTE, TopLayer->getParent());
+ }
+ case ConstructionContextItem::ElidedDestructorKind: {
+ llvm_unreachable("Elided destructor items are not produced by the CFG!");
+ }
+ case ConstructionContextItem::ElidableConstructorKind: {
+ llvm_unreachable("The argument needs to be materialized first!");
+ }
+ case ConstructionContextItem::InitializerKind: {
+ assert(TopLayer->isLast());
+ const CXXCtorInitializer *I = TopItem.getCXXCtorInitializer();
return create<SimpleConstructorInitializerConstructionContext>(C, I);
}
+ case ConstructionContextItem::ArgumentKind: {
+ assert(TopLayer->isLast());
+ const auto *E = cast<Expr>(TopItem.getStmt());
+ return create<ArgumentConstructionContext>(C, E, TopItem.getIndex(),
+ /*BTE=*/nullptr);
+ }
+ } // switch (TopItem.getKind())
llvm_unreachable("Unexpected construction context!");
}
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -697,7 +697,8 @@
Expr *Arg = E->getArg(i);
if (Arg->getType()->getAsCXXRecordDecl() && !Arg->isGLValue())
findConstructionContexts(
- ConstructionContextLayer::create(cfg->getBumpVectorContext(), E, i),
+ ConstructionContextLayer::create(cfg->getBumpVectorContext(),
+ ConstructionContextItem(E, i)),
Arg);
}
}
@@ -1286,9 +1287,9 @@
if (!Child)
return;
- auto withExtraLayer = [this, Layer](Stmt *S, unsigned Index = 0) {
- return ConstructionContextLayer::create(cfg->getBumpVectorContext(), S,
- Index, Layer);
+ auto withExtraLayer = [this, Layer](const ConstructionContextItem &Item) {
+ return ConstructionContextLayer::create(cfg->getBumpVectorContext(), Item,
+ Layer);
};
switch(Child->getStmtClass()) {
@@ -1349,18 +1350,17 @@
// it indicates the beginning of a temporary object construction context,
// so it shouldn't be found in the middle. However, if it is the beginning
// of an elidable copy or move construction context, we need to include it.
- if (const auto *CE =
- dyn_cast_or_null<CXXConstructExpr>(Layer->getTriggerStmt())) {
- if (CE->isElidable()) {
- auto *MTE = cast<MaterializeTemporaryExpr>(Child);
- findConstructionContexts(withExtraLayer(MTE), MTE->GetTemporaryExpr());
- }
+ if (Layer->getItem().getKind() ==
+ ConstructionContextItem::ElidableConstructorKind) {
+ auto *MTE = cast<MaterializeTemporaryExpr>(Child);
+ findConstructionContexts(withExtraLayer(MTE), MTE->GetTemporaryExpr());
}
break;
}
case Stmt::ConditionalOperatorClass: {
auto *CO = cast<ConditionalOperator>(Child);
- if (!dyn_cast_or_null<MaterializeTemporaryExpr>(Layer->getTriggerStmt())) {
+ if (Layer->getItem().getKind() !=
+ ConstructionContextItem::MaterializationKind) {
// If the object returned by the conditional operator is not going to be a
// temporary object that needs to be immediately materialized, then
// it must be C++17 with its mandatory copy elision. Do not yet promise
@@ -3222,8 +3222,7 @@
const DeclStmt *DS = F->getConditionVariableDeclStmt();
assert(DS->isSingleDecl());
findConstructionContexts(
- ConstructionContextLayer::create(cfg->getBumpVectorContext(),
- const_cast<DeclStmt *>(DS)),
+ ConstructionContextLayer::create(cfg->getBumpVectorContext(), DS),
Init);
appendStmt(Block, DS);
EntryConditionBlock = addStmt(Init);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -761,24 +761,24 @@
/// This allows, among other things, to keep bindings to variable's fields
/// made within the constructor alive until its declaration actually
/// goes into scope.
- static ProgramStateRef addObjectUnderConstruction(
- ProgramStateRef State,
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
- const LocationContext *LC, SVal V);
+ static ProgramStateRef
+ addObjectUnderConstruction(ProgramStateRef State,
+ const ConstructionContextItem &Item,
+ const LocationContext *LC, SVal V);
/// Mark the object sa fully constructed, cleaning up the state trait
/// that tracks objects under construction.
- static ProgramStateRef finishObjectConstruction(
- ProgramStateRef State,
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
- const LocationContext *LC);
+ static ProgramStateRef
+ finishObjectConstruction(ProgramStateRef State,
+ const ConstructionContextItem &Item,
+ const LocationContext *LC);
/// If the given statement corresponds to an object under construction,
/// being part of its construciton context, retrieve that object's location.
- static Optional<SVal> getObjectUnderConstruction(
- ProgramStateRef State,
- llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
- const LocationContext *LC);
+ static Optional<SVal>
+ getObjectUnderConstruction(ProgramStateRef State,
+ const ConstructionContextItem &Item,
+ const LocationContext *LC);
/// If the given expression corresponds to a temporary that was used for
/// passing into an elidable copy/move constructor and that constructor
Index: include/clang/Analysis/ConstructionContext.h
===================================================================
--- include/clang/Analysis/ConstructionContext.h
+++ include/clang/Analysis/ConstructionContext.h
@@ -22,74 +22,175 @@
namespace clang {
-/// Construction context is a linked list of multiple layers. Layers are
-/// created gradually while traversing the AST, and layers that represent
-/// the outmost AST nodes are built first, while the node that immediately
-/// contains the constructor would be built last and capture the previous
-/// layers as its parents. Construction context captures the last layer
-/// (which has links to the previous layers) and classifies the seemingly
-/// arbitrary chain of layers into one of the possible ways of constructing
-/// an object in C++ for user-friendly experience.
-class ConstructionContextLayer {
+/// Represents a single point (AST node) in the program that requires attention
+/// during construction of an object. ConstructionContext would be represented
+/// as a list of such items.
+class ConstructionContextItem {
public:
- typedef llvm::PointerUnion<Stmt *, CXXCtorInitializer *> TriggerTy;
+ enum ItemKind {
+ VariableKind,
+ NewAllocatorKind,
+ ReturnKind,
+ MaterializationKind,
+ TemporaryDestructorKind,
+ ElidedDestructorKind,
+ ElidableConstructorKind,
+ ArgumentKind,
+ STATEMENT_WITH_INDEX_KIND_BEGIN=ArgumentKind,
+ STATEMENT_WITH_INDEX_KIND_END=ArgumentKind,
+ STATEMENT_KIND_BEGIN = VariableKind,
+ STATEMENT_KIND_END = ArgumentKind,
+ InitializerKind,
+ INITIALIZER_KIND_BEGIN=InitializerKind,
+ INITIALIZER_KIND_END=InitializerKind
+ };
private:
+ const void *const Data;
+ const ItemKind Kind;
+ const unsigned Index = 0;
+
+ bool hasStatement() const {
+ return Kind >= STATEMENT_KIND_BEGIN &&
+ Kind <= STATEMENT_KIND_END;
+ }
+
+ bool hasIndex() const {
+ return Kind >= STATEMENT_WITH_INDEX_KIND_BEGIN &&
+ Kind >= STATEMENT_WITH_INDEX_KIND_END;
+ }
+
+ bool hasInitializer() const {
+ return Kind >= INITIALIZER_KIND_BEGIN &&
+ Kind <= INITIALIZER_KIND_END;
+ }
+
+public:
+ // ConstructionContextItem should be simple enough so that it was easy to
+ // re-construct it from the AST node it captures. For that reason we provide
+ // simple implicit conversions from all sorts of supported AST nodes.
+ ConstructionContextItem(const DeclStmt *DS)
+ : Data(DS), Kind(VariableKind) {}
+
+ ConstructionContextItem(const CXXNewExpr *NE)
+ : Data(NE), Kind(NewAllocatorKind) {}
+
+ ConstructionContextItem(const ReturnStmt *RS)
+ : Data(RS), Kind(ReturnKind) {}
+
+ ConstructionContextItem(const MaterializeTemporaryExpr *MTE)
+ : Data(MTE), Kind(MaterializationKind) {}
+
+ ConstructionContextItem(const CXXBindTemporaryExpr *BTE,
+ bool IsElided = false)
+ : Data(BTE),
+ Kind(IsElided ? ElidedDestructorKind : TemporaryDestructorKind) {}
+
+ ConstructionContextItem(const CXXConstructExpr *CE)
+ : Data(CE), Kind(ElidableConstructorKind) {}
+
+ ConstructionContextItem(const CallExpr *CE, unsigned Index)
+ : Data(CE), Kind(ArgumentKind), Index(Index) {}
+
+ ConstructionContextItem(const CXXConstructExpr *CE, unsigned Index)
+ : Data(CE), Kind(ArgumentKind), Index(Index) {}
+
+ ConstructionContextItem(const ObjCMessageExpr *ME, unsigned Index)
+ : Data(ME), Kind(ArgumentKind), Index(Index) {}
+
+ ConstructionContextItem(const CXXCtorInitializer *Init)
+ : Data(Init), Kind(InitializerKind), Index(0) {}
+
+ ItemKind getKind() const { return Kind; }
+
/// The construction site - the statement that triggered the construction
/// for one of its parts. For instance, stack variable declaration statement
/// triggers construction of itself or its elements if it's an array,
/// new-expression triggers construction of the newly allocated object(s).
- TriggerTy Trigger;
+ const Stmt *getStmt() const {
+ assert(hasStatement());
+ return static_cast<const Stmt *>(Data);
+ }
+
+ const Stmt *getStmtOrNull() const {
+ return hasStatement() ? getStmt() : nullptr;
+ }
+
+ /// The construction site is not necessarily a statement. It may also be a
+ /// CXXCtorInitializer, which means that a member variable is being
+ /// constructed during initialization of the object that contains it.
+ const CXXCtorInitializer *getCXXCtorInitializer() const {
+ assert(hasInitializer());
+ return static_cast<const CXXCtorInitializer *>(Data);
+ }
/// If a single trigger statement triggers multiple constructors, they are
/// usually being enumerated. This covers function argument constructors
/// triggered by a call-expression and items in an initializer list triggered
/// by an init-list-expression.
- unsigned Index;
-
- /// Sometimes a single trigger is not enough to describe the construction
- /// site. In this case we'd have a chain of "partial" construction context
- /// layers.
- /// 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.
+ unsigned getIndex() const {
+ // This is a fairly specific request. Let's make sure the user knows
+ // what he's doing.
+ assert(hasIndex());
+ return Index;
+ }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(Data);
+ ID.AddInteger(Kind);
+ ID.AddInteger(Index);
+ }
+
+ bool operator==(const ConstructionContextItem &Other) const {
+ // For most kinds the Index comparison is trivially true, but
+ // checking kind separately doesn't seem to be less expensive
+ // than checking Index. Same in operator<().
+ return std::make_tuple(Data, Kind, Index) ==
+ std::make_tuple(Other.Data, Other.Kind, Other.Index);
+ }
+
+ bool operator<(const ConstructionContextItem &Other) const {
+ return std::make_tuple(Data, Kind, Index) <
+ std::make_tuple(Other.Data, Other.Kind, Other.Index);
+ }
+};
+
+/// Construction context can be seen as a linked list of multiple layers.
+/// Sometimes a single trigger is not enough to describe the construction
+/// site. That's what causing us to have a chain of "partial" construction
+/// context layers. 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.
+/// Layers are created gradually while traversing the AST, and layers that
+/// represent the outmost AST nodes are built first, while the node that
+/// immediately contains the constructor would be built last and capture the
+/// previous layers as its parents. Construction context captures the last layer
+/// (which has links to the previous layers) and classifies the seemingly
+/// arbitrary chain of layers into one of the possible ways of constructing
+/// an object in C++ for user-friendly experience.
+class ConstructionContextLayer {
const ConstructionContextLayer *Parent = nullptr;
+ ConstructionContextItem Item;
- ConstructionContextLayer(TriggerTy Trigger, unsigned Index,
+ ConstructionContextLayer(ConstructionContextItem Item,
const ConstructionContextLayer *Parent)
- : Trigger(Trigger), Index(Index), Parent(Parent) {}
+ : Parent(Parent), Item(Item) {}
public:
static const ConstructionContextLayer *
- create(BumpVectorContext &C, TriggerTy Trigger, unsigned Index = 0,
+ create(BumpVectorContext &C, const ConstructionContextItem &Item,
const ConstructionContextLayer *Parent = nullptr);
+ const ConstructionContextItem &getItem() const { return Item; }
const ConstructionContextLayer *getParent() const { return Parent; }
bool isLast() const { return !Parent; }
- const Stmt *getTriggerStmt() const {
- return Trigger.dyn_cast<Stmt *>();
- }
-
- const CXXCtorInitializer *getTriggerInit() const {
- return Trigger.dyn_cast<CXXCtorInitializer *>();
- }
-
- unsigned getIndex() const { return Index; }
-
- /// Returns true if these layers are equal as individual layers, even if
- /// their parents are different.
- bool isSameLayer(const ConstructionContextLayer *Other) const {
- assert(Other);
- return (Trigger == Other->Trigger && Index == Other->Index);
- }
-
/// 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.,
@@ -141,6 +242,23 @@
return new (CC) T(Args...);
}
+ // A sub-routine of createFromLayers() that deals with temporary objects
+ // that need to be materialized. The BTE argument is for the situation when
+ // the object also needs to be bound for destruction.
+ static const ConstructionContext *createMaterializedTemporaryFromLayers(
+ BumpVectorContext &C, const MaterializeTemporaryExpr *MTE,
+ const CXXBindTemporaryExpr *BTE,
+ const ConstructionContextLayer *ParentLayer);
+
+ // A sub-routine of createFromLayers() that deals with temporary objects
+ // that need to be bound for destruction. Automatically finds out if the
+ // object also needs to be materialized and delegates to
+ // createMaterializedTemporaryFromLayers() if necessary.
+ static const ConstructionContext *
+ createBoundTemporaryFromLayers(
+ BumpVectorContext &C, const CXXBindTemporaryExpr *BTE,
+ const ConstructionContextLayer *ParentLayer);
+
public:
/// Consume the construction context layer, together with its parent layers,
/// and wrap it up into a complete construction context. May return null
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
