Hi jordan_rose,
Fix handling of lifetime extended temporaries by using the
MaterializeTemporaryExpr as indication of whether a temporary was actually
extended. As lifetime extended temporaries do not necessarily have VarDecls,
changes the type of the destructors for those to reuse CFGTemopraryDtor.
Open problems:
- temporary dtors are not yet inlined, but some tests rely on dtor inlining
for lifetime extended temporaries
- there are still missing cases for when we do not want to continue digging
through the AST for the CXXBindTemporaryExpr after hitting a
MaterializeTemporaryExpr; we now have 3 different places in the code that
encodes this information; some refactoring is needed to clean that up
- more tests needed
This is still rough, but I wanted to get early feedback about the direction this
is taking.
http://reviews.llvm.org/D4706
Files:
lib/Analysis/CFG.cpp
test/Analysis/cfg.cpp
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -435,6 +435,9 @@
return Visit(S, AddStmtChoice::AlwaysAdd);
}
CFGBlock *addInitializer(CXXCtorInitializer *I);
+ void addAutomaticObjDtorsForLifetimeExtendedTemporaries(
+ Stmt *E, bool LifetimeExtended = false);
+ void addAutomaticObjDtorsForVarDecl(VarDecl *VD, Stmt* S);
void addAutomaticObjDtors(LocalScope::const_iterator B,
LocalScope::const_iterator E, Stmt *S);
void addImplicitDtorsForDestructor(const CXXDestructorDecl *DD);
@@ -445,6 +448,9 @@
void addLocalScopeForStmt(Stmt *S);
LocalScope* addLocalScopeForDeclStmt(DeclStmt *DS,
LocalScope* Scope = nullptr);
+ bool needsLocalScopeForLifetimeExtendedTemporary(const Stmt* E);
+ bool needsLocalScopeForVarDecl(const VarDecl* VD);
+ bool needsLocalScopeForType(QualType QT);
LocalScope* addLocalScopeForVarDecl(VarDecl *VD, LocalScope* Scope = nullptr);
void addLocalScopeAndDtors(Stmt *S);
@@ -1033,52 +1039,68 @@
return Block;
}
-/// \brief Retrieve the type of the temporary object whose lifetime was
-/// extended by a local reference with the given initializer.
-static QualType getReferenceInitTemporaryType(ASTContext &Context,
- const Expr *Init) {
- while (true) {
- // Skip parentheses.
- Init = Init->IgnoreParens();
-
- // Skip through cleanups.
- if (const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(Init)) {
- Init = EWC->getSubExpr();
- continue;
+void CFGBuilder::addAutomaticObjDtorsForLifetimeExtendedTemporaries(
+ Stmt *E, bool LifetimeExtended) {
+ switch (E->getStmtClass()) {
+ // FIXME: What do we want to do for conditional operators and binary
+ // conditional operators?
+ case Stmt::MaterializeTemporaryExprClass: {
+ const MaterializeTemporaryExpr * MTE = cast<MaterializeTemporaryExpr>(E);
+ if (MTE->getStorageDuration() != SD_FullExpression) {
+ SmallVector<const Expr *, 2> CommaLHSs;
+ SmallVector<SubobjectAdjustment, 2> Adjustments;
+ assert(MTE->GetTemporaryExpr());
+ const Expr *ExtendedE =
+ MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments(CommaLHSs,
+ Adjustments);
+ addAutomaticObjDtorsForLifetimeExtendedTemporaries(
+ const_cast<Expr*>(ExtendedE), /*LifetimeExtended=*/true);
}
-
- // Skip through the temporary-materialization expression.
- if (const MaterializeTemporaryExpr *MTE
- = dyn_cast<MaterializeTemporaryExpr>(Init)) {
- Init = MTE->GetTemporaryExpr();
- continue;
+ } break;
+ case Stmt::CXXBindTemporaryExprClass:
+ if (LifetimeExtended) {
+ const CXXDestructorDecl *Dtor =
+ cast<CXXBindTemporaryExpr>(E)->getTemporary()->getDestructor();
+ if (Dtor->isNoReturn()) {
+ Block = createNoReturnBlock();
+ } else {
+ autoCreateBlock();
}
-
- // Skip derived-to-base and no-op casts.
- if (const CastExpr *CE = dyn_cast<CastExpr>(Init)) {
- if ((CE->getCastKind() == CK_DerivedToBase ||
- CE->getCastKind() == CK_UncheckedDerivedToBase ||
- CE->getCastKind() == CK_NoOp) &&
- Init->getType()->isRecordType()) {
- Init = CE->getSubExpr();
- continue;
- }
+
+ appendTemporaryDtor(Block, cast<CXXBindTemporaryExpr>(E));
}
-
- // Skip member accesses into rvalues.
- if (const MemberExpr *ME = dyn_cast<MemberExpr>(Init)) {
- if (!ME->isArrow() && ME->getBase()->isRValue()) {
- Init = ME->getBase();
- continue;
- }
+ break;
+ default:
+ for (Stmt *Child : E->children()) {
+ if (Child)
+ addAutomaticObjDtorsForLifetimeExtendedTemporaries(Child,
+ LifetimeExtended);
}
-
break;
}
+}
- return Init->getType();
+void CFGBuilder::addAutomaticObjDtorsForVarDecl(VarDecl *VD,
+ Stmt *S) {
+ if (!VD->getType()->isReferenceType() &&
+ needsLocalScopeForType(VD->getType())) {
+ QualType QT = VD->getType();
+ QT = Context->getBaseElementType(QT);
+
+ const CXXDestructorDecl *Dtor = QT->getAsCXXRecordDecl()->getDestructor();
+ if (Dtor->isNoReturn())
+ Block = createNoReturnBlock();
+ else
+ autoCreateBlock();
+
+ appendAutomaticObjDtor(Block, VD, S);
+ }
+ if (const Expr *Init = VD->getInit()) {
+ addAutomaticObjDtorsForLifetimeExtendedTemporaries(
+ const_cast<Expr *>(Init));
+ }
}
-
+
/// addAutomaticObjDtors - Add to current block automatic objects destructors
/// for objects in range of local scope positions. Use S as trigger statement
/// for destructors.
@@ -1102,22 +1124,7 @@
for (SmallVectorImpl<VarDecl*>::reverse_iterator I = Decls.rbegin(),
E = Decls.rend();
I != E; ++I) {
- // If this destructor is marked as a no-return destructor, we need to
- // create a new block for the destructor which does not have as a successor
- // anything built thus far: control won't flow out of this block.
- QualType Ty = (*I)->getType();
- if (Ty->isReferenceType()) {
- Ty = getReferenceInitTemporaryType(*Context, (*I)->getInit());
- }
- Ty = Context->getBaseElementType(Ty);
-
- const CXXDestructorDecl *Dtor = Ty->getAsCXXRecordDecl()->getDestructor();
- if (Dtor->isNoReturn())
- Block = createNoReturnBlock();
- else
- autoCreateBlock();
-
- appendAutomaticObjDtor(Block, *I, S);
+ addAutomaticObjDtorsForVarDecl(*I, S);
}
}
@@ -1215,6 +1222,55 @@
return Scope;
}
+bool CFGBuilder::needsLocalScopeForType(QualType QT) {
+ // Check for constant size array. Set type to array element type.
+ while (const ConstantArrayType *AT = Context->getAsConstantArrayType(QT)) {
+ if (AT->getSize() == 0)
+ return false;
+ QT = AT->getElementType();
+ }
+
+ // Check if type is a C++ class with non-trivial destructor.
+ if (const CXXRecordDecl *CD = QT->getAsCXXRecordDecl())
+ return !CD->hasTrivialDestructor();
+ return false;
+}
+
+bool CFGBuilder::needsLocalScopeForLifetimeExtendedTemporary(const Stmt* E) {
+ switch (E->getStmtClass()) {
+ case Stmt::MaterializeTemporaryExprClass: {
+ const MaterializeTemporaryExpr * MTE = cast<MaterializeTemporaryExpr>(E);
+ if (MTE->getStorageDuration() != SD_FullExpression) {
+ SmallVector<const Expr *, 2> CommaLHSs;
+ SmallVector<SubobjectAdjustment, 2> Adjustments;
+ assert(MTE->GetTemporaryExpr());
+ const Expr *ExtendedE =
+ MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments(CommaLHSs,
+ Adjustments);
+ return needsLocalScopeForType(ExtendedE->getType());
+ }
+ } break;
+ default:
+ for (const Stmt *Child : E->children()) {
+ if (Child && needsLocalScopeForLifetimeExtendedTemporary(Child))
+ return true;
+ }
+ break;
+ }
+ return false;
+}
+
+bool CFGBuilder::needsLocalScopeForVarDecl(const VarDecl* VD) {
+ if (!VD->getType()->isReferenceType() &&
+ needsLocalScopeForType(VD->getType())) {
+ return true;
+ }
+ if (const Expr *Init = VD->getInit()) {
+ return needsLocalScopeForLifetimeExtendedTemporary(Init);
+ }
+ return false;
+}
+
/// addLocalScopeForVarDecl - Add LocalScope for variable declaration. It will
/// create add scope for automatic objects and temporary objects bound to
/// const reference. Will reuse Scope if not NULL.
@@ -1232,43 +1288,11 @@
default: return Scope;
}
- // Check for const references bound to temporary. Set type to pointee.
- QualType QT = VD->getType();
- if (QT.getTypePtr()->isReferenceType()) {
- // Attempt to determine whether this declaration lifetime-extends a
- // temporary.
- //
- // FIXME: This is incorrect. Non-reference declarations can lifetime-extend
- // temporaries, and a single declaration can extend multiple temporaries.
- // We should look at the storage duration on each nested
- // MaterializeTemporaryExpr instead.
- const Expr *Init = VD->getInit();
- if (!Init)
- return Scope;
- if (const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(Init))
- Init = EWC->getSubExpr();
- if (!isa<MaterializeTemporaryExpr>(Init))
- return Scope;
-
- // Lifetime-extending a temporary.
- QT = getReferenceInitTemporaryType(*Context, Init);
- }
-
- // Check for constant size array. Set type to array element type.
- while (const ConstantArrayType *AT = Context->getAsConstantArrayType(QT)) {
- if (AT->getSize() == 0)
- return Scope;
- QT = AT->getElementType();
- }
-
- // Check if type is a C++ class with non-trivial destructor.
- if (const CXXRecordDecl *CD = QT->getAsCXXRecordDecl())
- if (!CD->hasTrivialDestructor()) {
- // Add the variable to scope
- Scope = createOrReuseLocalScope(Scope);
- Scope->addVar(VD);
- ScopePos = Scope->begin();
- }
+ if (!needsLocalScopeForVarDecl(VD))
+ return Scope;
+ Scope = createOrReuseLocalScope(Scope);
+ Scope->addVar(VD);
+ ScopePos = Scope->begin();
return Scope;
}
Index: test/Analysis/cfg.cpp
===================================================================
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -376,6 +376,20 @@
MyClass* obj = new (buffer) MyClass[5];
}
+// CHECK-LABEL: void test_lifetime_extended_temporaries()
+// CHECK: [B1]
+
+struct LifetimeExtend { LifetimeExtend(int); ~LifetimeExtend(); int i; };
+void test_lifetime_extended_temporaries() {
+ // CHECK: LifetimeExtend(1)
+ // CHECK-NEXT: : 1
+ // CHECK-NEXT: ~LifetimeExtend()
+ // CHECK-NOT: ~LifetimeExtend()
+ {
+ const int &i = LifetimeExtend(1).i;
+ 1;
+ }
+}
// CHECK-LABEL: int *PR18472()
// CHECK: [B2 (ENTRY)]
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
