llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety @llvm/pr-subscribers-clang-analysis Author: NeKon69 <details> <summary>Changes</summary> Now in `CFGFullExprCleanup` we also store a cleanup statement to be able to get an accurate location where destruction happened. This helps user understand lifetime semantics of objects better. Closes #<!-- -->195503 --- Full diff: https://github.com/llvm/llvm-project/pull/200568.diff 4 Files Affected: - (modified) clang/include/clang/Analysis/CFG.h (+10-4) - (modified) clang/lib/Analysis/CFG.cpp (+14-11) - (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+4-3) - (modified) clang/test/Sema/warn-lifetime-safety.cpp (+17-2) ``````````diff diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h index 72b7ac013ccc2..bd819247950db 100644 --- a/clang/include/clang/Analysis/CFG.h +++ b/clang/include/clang/Analysis/CFG.h @@ -318,8 +318,14 @@ class CFGFullExprCleanup : public CFGElement { public: using MTEVecTy = BumpVector<const MaterializeTemporaryExpr *>; - explicit CFGFullExprCleanup(const MTEVecTy *MTEs) - : CFGElement(FullExprCleanup, MTEs, nullptr) {} + explicit CFGFullExprCleanup(const MTEVecTy *MTEs, const Stmt *CleanupStmt) + : CFGElement(FullExprCleanup, MTEs, CleanupStmt) {} + + const Stmt *getCleanupStmt() const { + return static_cast<const Stmt *>(Data2.getPointer()); + } + + SourceLocation getCleanupLoc() const { return getCleanupStmt()->getEndLoc(); } ArrayRef<const MaterializeTemporaryExpr *> getExpiringMTEs() const { const MTEVecTy *ExpiringMTEs = @@ -1211,8 +1217,8 @@ class CFGBlock { } void appendFullExprCleanup(BumpVector<const MaterializeTemporaryExpr *> *BV, - BumpVectorContext &C) { - Elements.push_back(CFGFullExprCleanup(BV), C); + const Stmt *CleanupStmt, BumpVectorContext &C) { + Elements.push_back(CFGFullExprCleanup(BV, CleanupStmt), C); } void appendLoopExit(const Stmt *LoopStmt, BumpVectorContext &C) { diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 1b4d10b72e249..c1ccdd3cf9c35 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -813,7 +813,8 @@ class CFGBuilder { void addScopeChangesHandling(LocalScope::const_iterator SrcPos, LocalScope::const_iterator DstPos, Stmt *S); - void addFullExprCleanupMarker(TempDtorContext &Context); + void addFullExprCleanupMarker(TempDtorContext &Context, + const Stmt *CleanupStmt); CFGBlock *createScopeChangesHandlingBlock(LocalScope::const_iterator SrcPos, CFGBlock *SrcBlk, LocalScope::const_iterator DstPost, @@ -1841,10 +1842,10 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) { (BuildOpts.AddTemporaryDtors || BuildOpts.AddLifetime)) { // Generate destructors for temporaries in initialization expression. TempDtorContext Context; - VisitForTemporaries(cast<ExprWithCleanups>(ActualInit)->getSubExpr(), - /*ExternallyDestructed=*/false, Context); + Expr *FullExpr = cast<ExprWithCleanups>(ActualInit)->getSubExpr(); + VisitForTemporaries(FullExpr, /*ExternallyDestructed=*/false, Context); - addFullExprCleanupMarker(Context); + addFullExprCleanupMarker(Context, FullExpr); } } @@ -2096,7 +2097,8 @@ void CFGBuilder::addScopeChangesHandling(LocalScope::const_iterator SrcPos, addAutomaticObjHandling(SrcPos, BasePos, S); } -void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context) { +void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context, + const Stmt *CleanupStmt) { CFGFullExprCleanup::MTEVecTy *ExpiringMTEs = nullptr; BumpVectorContext &BVC = cfg->getBumpVectorContext(); @@ -2107,7 +2109,7 @@ void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context) { CFGFullExprCleanup::MTEVecTy(BVC, NumCollected); for (const MaterializeTemporaryExpr *MTE : Context.CollectedMTEs) ExpiringMTEs->push_back(MTE, BVC); - Block->appendFullExprCleanup(ExpiringMTEs, BVC); + Block->appendFullExprCleanup(ExpiringMTEs, CleanupStmt, BVC); } } @@ -3173,10 +3175,10 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) { (BuildOpts.AddTemporaryDtors || BuildOpts.AddLifetime)) { // Generate destructors for temporaries in initialization expression. TempDtorContext Context; - VisitForTemporaries(cast<ExprWithCleanups>(Init)->getSubExpr(), - /*ExternallyDestructed=*/true, Context); + Expr *FullExpr = cast<ExprWithCleanups>(Init)->getSubExpr(); + VisitForTemporaries(FullExpr, /*ExternallyDestructed=*/true, Context); - addFullExprCleanupMarker(Context); + addFullExprCleanupMarker(Context, FullExpr); } } @@ -4990,9 +4992,10 @@ CFGBlock *CFGBuilder::VisitExprWithCleanups(ExprWithCleanups *E, // If adding implicit destructors visit the full expression for adding // destructors of temporaries. TempDtorContext Context; - VisitForTemporaries(E->getSubExpr(), ExternallyDestructed, Context); + Expr *FullExpr = E->getSubExpr(); + VisitForTemporaries(FullExpr, ExternallyDestructed, Context); - addFullExprCleanupMarker(Context); + addFullExprCleanupMarker(Context, FullExpr); // Full expression has to be added as CFGStmt so it will be sequenced // before destructors of it's temporaries. diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index b34a04f1f3bda..5681d0ce35e90 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -725,9 +725,10 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { void FactsGenerator::handleFullExprCleanup( const CFGFullExprCleanup &FullExprCleanup) { - for (const auto *MTE : FullExprCleanup.getExpiringMTEs()) - CurrentBlockFacts.push_back( - FactMgr.createFact<ExpireFact>(AccessPath(MTE), MTE->getEndLoc())); + for (const auto *MTE : FullExprCleanup.getExpiringMTEs()) { + CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( + AccessPath(MTE), FullExprCleanup.getCleanupLoc())); + } } void FactsGenerator::handleExitBlock() { diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 06097d4600af5..461287b4ecb9a 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1287,6 +1287,21 @@ void use_trivial_temporary_after_destruction() { use(a); // expected-note {{later used here}} } +namespace FullExprCleanupLoc { +void var_initializer() { + View v = non_trivially_destructed_temporary() // expected-warning {{local temporary object does not live long enough}} + .getView(); // expected-note {{destroyed here}} + v.use(); // expected-note {{later used here}} +} + +void expr_statement() { + View v; + v = non_trivially_destructed_temporary() // expected-warning {{local temporary object does not live long enough}} + .getView(); // expected-note {{destroyed here}} + v.use(); // expected-note {{later used here}} +} +} // namespace FullExprCleanupLoc + namespace GH162834 { // https://github.com/llvm/llvm-project/issues/162834 template <class T> @@ -2299,8 +2314,8 @@ struct S { void indexing_with_static_operator() { S()(1, 2); S& x = S()("1", - 2, // expected-warning {{local temporary object does not live long enough}} expected-note {{destroyed here}} - 3); // expected-warning {{local temporary object does not live long enough}} expected-note {{destroyed here}} + 2, // expected-warning {{local temporary object does not live long enough}} + 3); // expected-warning {{local temporary object does not live long enough}} expected-note 2 {{destroyed here}} (void)x; // expected-note 2 {{later used here}} `````````` </details> https://github.com/llvm/llvm-project/pull/200568 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
