llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Utkarsh Saxena (usx95) <details> <summary>Changes</summary> Followup on https://github.com/llvm/llvm-project/pull/107627 Fixes https://github.com/llvm/llvm-project/issues/62072 Fixes https://github.com/llvm/llvm-project/issues/172013 This PR adds support for merging the `lifetimebound` attribute on the implicit `this` parameter when merging method declarations. Previously, if a method was declared with `lifetimebound` on its function type (which represents the implicit `this` parameter), this attribute would not be propagated to the method definition, causing lifetime safety warnings to be missed. The implementation adds helper functions to extract the `lifetimebound` attribute from a function type and to merge this attribute from an old method declaration to a new one when appropriate. --- Full diff: https://github.com/llvm/llvm-project/pull/172146.diff 3 Files Affected: - (modified) clang/lib/Sema/SemaDecl.cpp (+57-6) - (modified) clang/test/Sema/warn-lifetime-safety.cpp (+22) - (modified) clang/test/SemaCXX/attr-lifetimebound.cpp (+12) ``````````diff diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 47bd7295e93f6..3dd62c1428271 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4469,6 +4469,52 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S, return true; } +/// Check if a function has a lifetimebound attribute on its function type +/// (which represents the implicit 'this' parameter for methods). +/// Returns the attribute if found, nullptr otherwise. +static const LifetimeBoundAttr * +getLifetimeBoundAttrFromType(const TypeSourceInfo *TSI) { + if (!TSI) + return nullptr; + + for (TypeLoc TL = TSI->getTypeLoc();;) { + auto ATL = TL.getAsAdjusted<AttributedTypeLoc>(); + if (!ATL) + break; + if (auto *LBAttr = ATL.getAttrAs<LifetimeBoundAttr>()) + return LBAttr; + TL = ATL.getModifiedLoc(); + } + return nullptr; +} + +/// Merge lifetimebound attribute on function type (implicit 'this') +/// from Old to New method declaration. +static void mergeLifetimeBoundAttrOnMethod(Sema &S, CXXMethodDecl *New, + const CXXMethodDecl *Old) { + const TypeSourceInfo *OldTSI = Old->getTypeSourceInfo(); + const TypeSourceInfo *NewTSI = New->getTypeSourceInfo(); + + if (!OldTSI || !NewTSI) + return; + + const LifetimeBoundAttr *OldLBAttr = getLifetimeBoundAttrFromType(OldTSI); + const LifetimeBoundAttr *NewLBAttr = getLifetimeBoundAttrFromType(NewTSI); + + // If Old has lifetimebound but New doesn't, add it to New + if (OldLBAttr && !NewLBAttr) { + QualType NewMethodType = New->getType(); + QualType AttributedType = + S.Context.getAttributedType(OldLBAttr, NewMethodType, NewMethodType); + TypeLocBuilder TLB; + TLB.pushFullCopy(NewTSI->getTypeLoc()); + AttributedTypeLoc TyLoc = TLB.push<AttributedTypeLoc>(AttributedType); + TyLoc.setAttr(OldLBAttr); + New->setType(AttributedType); + New->setTypeSourceInfo(TLB.getTypeSourceInfo(S.Context, AttributedType)); + } +} + bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old, Scope *S, bool MergeTypeWithOld) { // Merge the attributes @@ -4485,12 +4531,17 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old, // Merge attributes from the parameters. These can mismatch with K&R // declarations. if (New->getNumParams() == Old->getNumParams()) - for (unsigned i = 0, e = New->getNumParams(); i != e; ++i) { - ParmVarDecl *NewParam = New->getParamDecl(i); - ParmVarDecl *OldParam = Old->getParamDecl(i); - mergeParamDeclAttributes(NewParam, OldParam, *this); - mergeParamDeclTypes(NewParam, OldParam, *this); - } + for (unsigned i = 0, e = New->getNumParams(); i != e; ++i) { + ParmVarDecl *NewParam = New->getParamDecl(i); + ParmVarDecl *OldParam = Old->getParamDecl(i); + mergeParamDeclAttributes(NewParam, OldParam, *this); + mergeParamDeclTypes(NewParam, OldParam, *this); + } + + // Merge function type attributes (e.g., lifetimebound on implicit 'this'). + if (auto *NewMethod = dyn_cast<CXXMethodDecl>(New)) + if (auto *OldMethod = dyn_cast<CXXMethodDecl>(Old)) + mergeLifetimeBoundAttrOnMethod(*this, NewMethod, OldMethod); if (getLangOpts().CPlusPlus) return MergeCXXFunctionDecl(New, Old, S); diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 1191469e23df1..658d68db44369 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -943,3 +943,25 @@ void parentheses(bool cond) { } // expected-note 4 {{destroyed here}} (void)*p; // expected-note 4 {{later used here}} } + +// Implicit this annotations with redecls. +namespace GH172013 { +// https://github.com/llvm/llvm-project/issues/62072 +// https://github.com/llvm/llvm-project/issues/172013 +struct S { + View x() const [[clang::lifetimebound]]; + MyObj i; +}; + +View S::x() const { return i; } + +void bar() { + View x; + { + S s; + x = s.x(); // expected-warning {{object whose reference is captured does not live long enough}} + View y = S().x(); // FIXME: Handle temporaries. + } // expected-note {{destroyed here}} + (void)x; // expected-note {{used here}} +} +} diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index 111bad65f7e1b..5b0aa0e518cb3 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -75,6 +75,18 @@ namespace usage_ok { r = A(1); // expected-warning {{object backing the pointer 'r' will be destroyed at the end of the full-expression}} } + // Test that lifetimebound on implicit 'this' is propagated across redeclarations + struct B { + int *method() [[clang::lifetimebound]]; + int i; + }; + int *B::method() { return &i; } + + void test_lifetimebound_on_implicit_this() { + int *t = B().method(); // expected-warning {{temporary whose address is used as value of local variable 't' will be destroyed at the end of the full-expression}} + t = {B().method()}; // expected-warning {{object backing the pointer 't' will be destroyed at the end of the full-expression}} + } + struct FieldCheck { struct Set { int a; `````````` </details> https://github.com/llvm/llvm-project/pull/172146 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
