llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (higher-performance) <details> <summary>Changes</summary> This partially fixes #<!-- -->62072 by making sure that re-declarations of a function do not have the effect of removing lifetimebound from the canonical declaration. It doesn't handle the implicit 'this' parameter, but that can be addressed in a separate fix. --- Full diff: https://github.com/llvm/llvm-project/pull/107627.diff 3 Files Affected: - (modified) clang/lib/Sema/CheckExprLifetime.cpp (+9-5) - (modified) clang/lib/Sema/SemaAttr.cpp (+23-12) - (modified) clang/test/SemaCXX/attr-lifetimebound.cpp (+4) ``````````diff diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f1507ebb9a5068..b484117cecd3f4 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -462,14 +462,18 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); for (unsigned I = 0, - N = std::min<unsigned>(Callee->getNumParams(), Args.size()); + N = std::min<unsigned>(std::min(Callee->getNumParams(), + CanonCallee->getNumParams()), + Args.size()); I != N; ++I) { - if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); + if (CheckCoroCall || + CanonCallee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Args[I]); else if (EnableGSLAnalysis && I == 0) { - if (shouldTrackFirstArgument(Callee)) { - VisitGSLPointerArg(Callee, Args[0]); + if (shouldTrackFirstArgument(CanonCallee)) { + VisitGSLPointerArg(CanonCallee, Args[0]); } else if (auto *CCE = dyn_cast<CXXConstructExpr>(Call); CCE && CCE->getConstructor()->getParent()->hasAttr<PointerAttr>()) { diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index fb594e6f13c0b3..e546d031229a77 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -217,7 +217,8 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) { } void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { - if (FD->getNumParams() == 0) + const unsigned int NumParams = FD->getNumParams(); + if (NumParams == 0) return; if (unsigned BuiltinID = FD->getBuiltinID()) { @@ -239,18 +240,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { default: break; } - return; - } - if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) { - const auto *CRD = CMD->getParent(); - if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - - if (isa<CXXConstructorDecl>(CMD)) { + } else if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) { + const CXXRecordDecl *CRD = CMD->getParent(); + if (CRD->isInStdNamespace() && CRD->getIdentifier() && + isa<CXXConstructorDecl>(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr<LifetimeBoundAttr>()) - return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr<LifetimeBoundAttr>() && + CRD->getName() == "basic_string_view" && Param->getType()->isPointerType()) { // construct from a char array pointed by a pointer. // basic_string_view(const CharT* s); @@ -266,6 +262,21 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation())); } } + } else if (auto *CanonDecl = FD->getCanonicalDecl(); FD != CanonDecl) { + // Propagate the lifetimebound attribute from parameters to the canonical + // declaration. + // Note that this doesn't include the implicit 'this' parameter, as the + // attribute is applied to the function type in that case. + unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); + assert(NP == NumParams); + for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr<LifetimeBoundAttr>() && + FD->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) { + CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit( + Context, CanonParam->getLocation())); + } + } } } diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index 0fb997a5671085..c0255e9a9f7280 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -19,6 +19,9 @@ namespace usage_invalid { namespace usage_ok { struct IntRef { int *target; }; + const int &crefparam(const int ¶m); // Omitted in first decl + const int &crefparam(const int ¶m [[clang::lifetimebound]]); // Add LB + const int &crefparam(const int ¶m) { return param; } // Omit in impl int &refparam(int ¶m [[clang::lifetimebound]]); int &classparam(IntRef param [[clang::lifetimebound]]); @@ -41,6 +44,7 @@ namespace usage_ok { int *p = A().class_member(); // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}} int *q = A(); // expected-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}} int *r = A(1); // expected-warning {{temporary whose address is used as value of local variable 'r' will be destroyed at the end of the full-expression}} + const int& s = crefparam(2); // expected-warning {{temporary bound to local reference 's' will be destroyed at the end of the full-expression}} void test_assignment() { p = A().class_member(); // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}} `````````` </details> https://github.com/llvm/llvm-project/pull/107627 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
