[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/ilya-biryukov closed 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3250,26 +3285,17 @@ static void mergeParamDeclAttributes(ParmVarDecl *newDecl, diag::note_carries_dependency_missing_first_decl) << 1/*Param*/; } - if (!oldDecl->hasAttrs()) -return; - - bool foundAny = newDecl->hasAttrs(); - - // Ensure that any moving of objects within the allocated map is - // done before we process them. - if (!foundAny) newDecl->setAttrs(AttrVec()); - - for (const auto *I : oldDecl->specific_attrs()) { -if (!DeclHasAttr(newDecl, I)) { - InheritableAttr *newAttr = -cast(I->clone(S.Context)); - newAttr->setInherited(true); - newDecl->addAttr(newAttr); - foundAny = true; -} - } - - if (!foundAny) newDecl->dropAttrs(); + propagateAttributes( + newDecl, oldDecl, [&S](ParmVarDecl *toDecl, const ParmVarDecl *fromDecl) { +unsigned found = 0; +found += propagateAttribute(toDecl, fromDecl, S); +// Propagate the lifetimebound attribute from parameters to the +// most recent declaration. Note that this doesn't include the implicit +// 'this' parameter, as the attribute is applied to the function type in +// that case. +found += propagateAttribute(toDecl, fromDecl, S); ilya-biryukov wrote: > It doesn't seem like InheritableParamAttr works on types. In any case, other > attributes indicate they have this problem too, so let's worry about solving > it separately for all of them in the future? okay, that's something we can also do in a follow-up. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/ilya-biryukov approved this pull request. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
ilya-biryukov wrote: Thanks and sorry for the long review delays. LGTM, let's work on the details in follow-ups. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
higher-performance wrote: Okay, I believe this PR is ready. Could we merge it? 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 4dfad6c21220585a6a0f796f5699128ca7c4615b Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 20 ++--- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7109de03cadd12..4befd703beb422 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -645,9 +645,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -655,11 +655,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (const auto *CaptureAttr = - Callee->getParamDecl(I)->getAttr(); - CaptureAttr && isa(Callee) && + CanonCallee->getParamDecl(I)->getAttr(); + CaptureAttr && isa(CanonCallee) && llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { return ArgIdx == LifetimeCaptureByAttr::THIS; })) @@ -676,11 +676,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // `lifetimebound` and shares the same code path. This implies the emitted // diagnostics will be emitted under `-Wdangling`, not // `-Wdangling-capture`. - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 44485e71d57a01..3fd122b20d7b12 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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' p
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 4dfad6c21220585a6a0f796f5699128ca7c4615b Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 20 ++--- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7109de03cadd12..4befd703beb422 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -645,9 +645,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -655,11 +655,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (const auto *CaptureAttr = - Callee->getParamDecl(I)->getAttr(); - CaptureAttr && isa(Callee) && + CanonCallee->getParamDecl(I)->getAttr(); + CaptureAttr && isa(CanonCallee) && llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { return ArgIdx == LifetimeCaptureByAttr::THIS; })) @@ -676,11 +676,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // `lifetimebound` and shares the same code path. This implies the emitted // diagnostics will be emitted under `-Wdangling`, not // `-Wdangling-capture`. - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 44485e71d57a01..3fd122b20d7b12 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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' p
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 4dfad6c21220585a6a0f796f5699128ca7c4615b Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 20 ++--- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7109de03cadd12..4befd703beb422 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -645,9 +645,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -655,11 +655,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (const auto *CaptureAttr = - Callee->getParamDecl(I)->getAttr(); - CaptureAttr && isa(Callee) && + CanonCallee->getParamDecl(I)->getAttr(); + CaptureAttr && isa(CanonCallee) && llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { return ArgIdx == LifetimeCaptureByAttr::THIS; })) @@ -676,11 +676,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // `lifetimebound` and shares the same code path. This implies the emitted // diagnostics will be emitted under `-Wdangling`, not // `-Wdangling-capture`. - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 44485e71d57a01..3fd122b20d7b12 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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' p
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 4dfad6c21220585a6a0f796f5699128ca7c4615b Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 20 ++--- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7109de03cadd12..4befd703beb422 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -645,9 +645,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -655,11 +655,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (const auto *CaptureAttr = - Callee->getParamDecl(I)->getAttr(); - CaptureAttr && isa(Callee) && + CanonCallee->getParamDecl(I)->getAttr(); + CaptureAttr && isa(CanonCallee) && llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { return ArgIdx == LifetimeCaptureByAttr::THIS; })) @@ -676,11 +676,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // `lifetimebound` and shares the same code path. This implies the emitted // diagnostics will be emitted under `-Wdangling`, not // `-Wdangling-capture`. - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 44485e71d57a01..3fd122b20d7b12 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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' p
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
higher-performance wrote: It seems the canonicalization is unnecessary -- we already always examine the most recent declaration, from what I can tell. So I'll strip out that code. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 4dfad6c21220585a6a0f796f5699128ca7c4615b Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 20 ++--- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7109de03cadd12..4befd703beb422 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -645,9 +645,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -655,11 +655,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (const auto *CaptureAttr = - Callee->getParamDecl(I)->getAttr(); - CaptureAttr && isa(Callee) && + CanonCallee->getParamDecl(I)->getAttr(); + CaptureAttr && isa(CanonCallee) && llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { return ArgIdx == LifetimeCaptureByAttr::THIS; })) @@ -676,11 +676,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // `lifetimebound` and shares the same code path. This implies the emitted // diagnostics will be emitted under `-Wdangling`, not // `-Wdangling-capture`. - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 44485e71d57a01..3fd122b20d7b12 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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' p
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 4dfad6c21220585a6a0f796f5699128ca7c4615b Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 20 ++--- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7109de03cadd12..4befd703beb422 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -645,9 +645,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -655,11 +655,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (const auto *CaptureAttr = - Callee->getParamDecl(I)->getAttr(); - CaptureAttr && isa(Callee) && + CanonCallee->getParamDecl(I)->getAttr(); + CaptureAttr && isa(CanonCallee) && llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { return ArgIdx == LifetimeCaptureByAttr::THIS; })) @@ -676,11 +676,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // `lifetimebound` and shares the same code path. This implies the emitted // diagnostics will be emitted under `-Wdangling`, not // `-Wdangling-capture`. - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 44485e71d57a01..3fd122b20d7b12 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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' p
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3227,6 +3227,41 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old, if (!foundAny) New->dropAttrs(); } +template +static unsigned propagateAttribute(ParmVarDecl *toDecl, higher-performance wrote: Done. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -499,6 +499,7 @@ static bool isNormalAssignmentOperator(const FunctionDecl *FD) { } bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { + FD = FD->getMostRecentDecl(); higher-performance wrote: Done. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3227,6 +3227,41 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old, if (!foundAny) New->dropAttrs(); } +template +static unsigned propagateAttribute(ParmVarDecl *toDecl, higher-performance wrote: Done. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 3764d3e1062c6b748cea1faa9e4bd9628a1a5dea Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 20 ++--- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 843fdb4a65cd73..8ba59dfde3d69b 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -636,9 +636,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -646,11 +646,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (const auto *CaptureAttr = - Callee->getParamDecl(I)->getAttr(); - CaptureAttr && isa(Callee) && + CanonCallee->getParamDecl(I)->getAttr(); + CaptureAttr && isa(CanonCallee) && llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { return ArgIdx == LifetimeCaptureByAttr::THIS; })) @@ -667,11 +667,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // `lifetimebound` and shares the same code path. This implies the emitted // diagnostics will be emitted under `-Wdangling`, not // `-Wdangling-capture`. - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 44485e71d57a01..3fd122b20d7b12 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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' p
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3250,26 +3285,17 @@ static void mergeParamDeclAttributes(ParmVarDecl *newDecl, diag::note_carries_dependency_missing_first_decl) << 1/*Param*/; } - if (!oldDecl->hasAttrs()) -return; - - bool foundAny = newDecl->hasAttrs(); - - // Ensure that any moving of objects within the allocated map is - // done before we process them. - if (!foundAny) newDecl->setAttrs(AttrVec()); - - for (const auto *I : oldDecl->specific_attrs()) { -if (!DeclHasAttr(newDecl, I)) { - InheritableAttr *newAttr = -cast(I->clone(S.Context)); - newAttr->setInherited(true); - newDecl->addAttr(newAttr); - foundAny = true; -} - } - - if (!foundAny) newDecl->dropAttrs(); + propagateAttributes( + newDecl, oldDecl, [&S](ParmVarDecl *toDecl, const ParmVarDecl *fromDecl) { +unsigned found = 0; +found += propagateAttribute(toDecl, fromDecl, S); +// Propagate the lifetimebound attribute from parameters to the +// most recent declaration. Note that this doesn't include the implicit +// 'this' parameter, as the attribute is applied to the function type in +// that case. +found += propagateAttribute(toDecl, fromDecl, S); higher-performance wrote: It doesn't seem like `InheritableParamAttr` works on types. In any case, other attributes indicate they have this problem too, so let's worry about solving it separately for all of them in the future? 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -238,18 +239,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { default: break; } -return; higher-performance wrote: Done. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3250,26 +3285,17 @@ static void mergeParamDeclAttributes(ParmVarDecl *newDecl, diag::note_carries_dependency_missing_first_decl) << 1/*Param*/; } - if (!oldDecl->hasAttrs()) -return; - - bool foundAny = newDecl->hasAttrs(); - - // Ensure that any moving of objects within the allocated map is - // done before we process them. - if (!foundAny) newDecl->setAttrs(AttrVec()); - - for (const auto *I : oldDecl->specific_attrs()) { -if (!DeclHasAttr(newDecl, I)) { - InheritableAttr *newAttr = -cast(I->clone(S.Context)); - newAttr->setInherited(true); - newDecl->addAttr(newAttr); - foundAny = true; -} - } - - if (!foundAny) newDecl->dropAttrs(); + propagateAttributes( + newDecl, oldDecl, [&S](ParmVarDecl *toDecl, const ParmVarDecl *fromDecl) { +unsigned found = 0; +found += propagateAttribute(toDecl, fromDecl, S); +// Propagate the lifetimebound attribute from parameters to the +// most recent declaration. Note that this doesn't include the implicit +// 'this' parameter, as the attribute is applied to the function type in higher-performance wrote: Yeah. It didn't look trivial the last time I looked into it, and I've lost a lot of context by now. I don't have the bandwidth to look into that unfortunately. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3227,6 +3227,41 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old, if (!foundAny) New->dropAttrs(); } +template +static unsigned propagateAttribute(ParmVarDecl *toDecl, ilya-biryukov wrote: NIT: maybe document that it returns a number of added attributes? 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/ilya-biryukov commented: I really like the current implementation, but have one major suggestion about using`InheritableParamAttr`. Also, could we add a release note for it? 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3250,26 +3285,17 @@ static void mergeParamDeclAttributes(ParmVarDecl *newDecl, diag::note_carries_dependency_missing_first_decl) << 1/*Param*/; } - if (!oldDecl->hasAttrs()) -return; - - bool foundAny = newDecl->hasAttrs(); - - // Ensure that any moving of objects within the allocated map is - // done before we process them. - if (!foundAny) newDecl->setAttrs(AttrVec()); - - for (const auto *I : oldDecl->specific_attrs()) { -if (!DeclHasAttr(newDecl, I)) { - InheritableAttr *newAttr = -cast(I->clone(S.Context)); - newAttr->setInherited(true); - newDecl->addAttr(newAttr); - foundAny = true; -} - } - - if (!foundAny) newDecl->dropAttrs(); + propagateAttributes( + newDecl, oldDecl, [&S](ParmVarDecl *toDecl, const ParmVarDecl *fromDecl) { +unsigned found = 0; +found += propagateAttribute(toDecl, fromDecl, S); +// Propagate the lifetimebound attribute from parameters to the +// most recent declaration. Note that this doesn't include the implicit +// 'this' parameter, as the attribute is applied to the function type in +// that case. +found += propagateAttribute(toDecl, fromDecl, S); ilya-biryukov wrote: It feels that LifetimeBoundAttr might need to be an `InheritableParamAttr` itself, which would allow to avoid having a template altogether. Are there any downsides to that? (I'm assuming that `InheritableParamAttr` represents the attributes that should be added from redeclarations, not just from the base methods of overridden members) 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3227,6 +3227,41 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old, if (!foundAny) New->dropAttrs(); } +template +static unsigned propagateAttribute(ParmVarDecl *toDecl, ilya-biryukov wrote: NIT: rename to `ToDecl` to match LLVM naming rules. I'd also just call them `To` and `From`, the types are spelled out explicitly anyway, `Decl` suffix seems redundant. But up to you. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -238,18 +239,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { default: break; } -return; ilya-biryukov wrote: NIT: LLVM codifies a preference to use `return` in the Style Guide, see examples in [this section](https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return). Could you add return to the `CXXMethodDecl` case and avoid the `else`? 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -3250,26 +3285,17 @@ static void mergeParamDeclAttributes(ParmVarDecl *newDecl, diag::note_carries_dependency_missing_first_decl) << 1/*Param*/; } - if (!oldDecl->hasAttrs()) -return; - - bool foundAny = newDecl->hasAttrs(); - - // Ensure that any moving of objects within the allocated map is - // done before we process them. - if (!foundAny) newDecl->setAttrs(AttrVec()); - - for (const auto *I : oldDecl->specific_attrs()) { -if (!DeclHasAttr(newDecl, I)) { - InheritableAttr *newAttr = -cast(I->clone(S.Context)); - newAttr->setInherited(true); - newDecl->addAttr(newAttr); - foundAny = true; -} - } - - if (!foundAny) newDecl->dropAttrs(); + propagateAttributes( + newDecl, oldDecl, [&S](ParmVarDecl *toDecl, const ParmVarDecl *fromDecl) { +unsigned found = 0; +found += propagateAttribute(toDecl, fromDecl, S); +// Propagate the lifetimebound attribute from parameters to the +// most recent declaration. Note that this doesn't include the implicit +// 'this' parameter, as the attribute is applied to the function type in ilya-biryukov wrote: Are you saying we don't handle the `implicit this` at all? Do you think we could try doing that in `MergeCompatibleFunctionDecls` (particularly, in `mergeDeclAttributes`)? It's okay to do that in a follow-up. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -499,6 +499,7 @@ static bool isNormalAssignmentOperator(const FunctionDecl *FD) { } bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { + FD = FD->getMostRecentDecl(); ilya-biryukov wrote: It would be great to explain the need for `getMostRecentDecl()`, either: - (my preferred option) with a function with descriptive name that calls `getMostRecentDecl`, e.g. `getDeclWithLifetimeBound(...)` - with a comment at each call site. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/ilya-biryukov edited 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From e9af4e3d8f629fff790f2f573ceccdf01fcb9495 Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 15 +- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 6cdd4dc629e50a..d3423b50f3b051 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -611,9 +611,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -621,12 +621,13 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 716d8ed1fae4f8..754f36fa03f1d1 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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 NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit( +Context, CanonParam->getLocation())); + } +} } } diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index f89b556f5bba08..c4a4e5415252e7 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -19,6 +19,10 @@ namespace usage_invalid { namespace usage_ok { struct IntRef
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -266,6 +262,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { higher-performance wrote: Okay sure, let's see if `getMostRecentDecl` works. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From dffc6abf0a1e4b1e8977bf9476bb65d808b46986 Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/3] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 15 +- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 8886e5e307ddf86..e7c8a7591804efb 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -611,9 +611,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -621,12 +621,13 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 9fbad7ed67ccbe2..85274aee785a58f 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -216,7 +216,8 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) { } void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { - if (FD->getNumParams() == 0) + unsigned NumParams = FD->getNumParams(); + if (NumParams == 0) return; if (unsigned BuiltinID = FD->getBuiltinID()) { @@ -238,18 +239,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { default: break; } -return; - } - if (auto *CMD = dyn_cast(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + CRD->getName() == "basic_string_view" && Param->getType()->isPointerType()) { // construct from a char array pointed by a pointer. // basic_string_view(const CharT* s); @@ -265,6 +261,20 @@ 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 NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit( +Context, CanonParam->getLocation())); + } +} } } diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index f89b556f5bba088..c4a4e5415252e7f 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -19,6 +19,10 @@ namespace usage_invalid { namespace usage_ok { struct I
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -266,6 +262,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { ilya-biryukov wrote: Should we be looking at the attributes of the most recent decl instead, then? How do other mergable attributes handle that? I think it's fairly important to have a single place in the AST that handles the merging logic for the attributes, even if that means changing some mechanism of how other places work. Looking at that test, I guess what happens is: - the attributes get propagated to the **new** redeclaration, but never to the original declaration; - the canonical declaration is the old one that does not have the attribute; - when checking for the lifetimebound attribute, we look at canonical declaration and don't notice the attribute. I will look into the code, but could you check if it makes sense to put something like `getMostRecentDecl()` calls when checking for presence of the attributes? PS the meta-suggestion here is to not add new ways to merge attributes. I am not sure if it there is a new way that would make more sense, e.g. maybe we can explore merging all the attributes into a canonical declaration instead or ensuring there is a helper function that calls `getMostRecentDecl` before checking for an attribute being present. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -266,6 +262,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { higher-performance wrote: I reverted that rewrite due to the bug. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 581264c9ec198e98ff5f0f2c6e46073d79959a0b Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 15 +- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index a1a402b4a2b530..3d9858097f07dc 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -607,9 +607,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); if (auto *DAE = dyn_cast(Arg)) { @@ -617,12 +617,13 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 9fbad7ed67ccbe..85274aee785a58 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -216,7 +216,8 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) { } void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { - if (FD->getNumParams() == 0) + unsigned NumParams = FD->getNumParams(); + if (NumParams == 0) return; if (unsigned BuiltinID = FD->getBuiltinID()) { @@ -238,18 +239,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { default: break; } -return; - } - if (auto *CMD = dyn_cast(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + CRD->getName() == "basic_string_view" && Param->getType()->isPointerType()) { // construct from a char array pointed by a pointer. // basic_string_view(const CharT* s); @@ -265,6 +261,20 @@ 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 NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit( +Context, CanonParam->getLocation())); + } +} } } diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index 81e9193cf76a04..fb5a68f2497013 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -19,6 +19,10 @@ namespace usage_invalid { namespace usage_ok { struct IntRef { in
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 9bdc5fde9c1ba711297d1580b3d581db16da4ee2 Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 14 ++ clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 5 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index a1a402b4a2b530..64dcd212c451de 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -607,8 +607,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); @@ -617,12 +618,13 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 9fbad7ed67ccbe..85274aee785a58 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -216,7 +216,8 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) { } void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { - if (FD->getNumParams() == 0) + unsigned NumParams = FD->getNumParams(); + if (NumParams == 0) return; if (unsigned BuiltinID = FD->getBuiltinID()) { @@ -238,18 +239,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { default: break; } -return; - } - if (auto *CMD = dyn_cast(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + CRD->getName() == "basic_string_view" && Param->getType()->isPointerType()) { // construct from a char array pointed by a pointer. // basic_string_view(const CharT* s); @@ -265,6 +261,20 @@ 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 NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit( +Context, CanonParam->getLocation())); + } +} } } diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index 81e9193cf76a04..fb5a68f2497013 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -19,6 +19,10 @@ namespace usage_invalid { namespace usage_ok { struct IntRef { int *target; }; + const int &crefparam(const int ¶m)
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 5edfc3a8558b42e143d58041f0f77a8bfc39b847 Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 14 ++ clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index a1a402b4a2b530..64dcd212c451de 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -607,8 +607,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { Expr *Arg = Args[I]; RevertToOldSizeRAII RAII(Path); @@ -617,12 +618,13 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, {IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()}); Arg = DAE->getExpr(); } -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg); +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(Callee)) { -VisitGSLPointerArg(Callee, Arg); + if (shouldTrackFirstArgument(CanonCallee)) { +VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { VisitGSLPointerArg(Ctor->getConstructor(), Arg); diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 9fbad7ed67ccbe..85274aee785a58 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -216,7 +216,8 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) { } void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { - if (FD->getNumParams() == 0) + unsigned NumParams = FD->getNumParams(); + if (NumParams == 0) return; if (unsigned BuiltinID = FD->getBuiltinID()) { @@ -238,18 +239,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { default: break; } -return; - } - if (auto *CMD = dyn_cast(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + CRD->getName() == "basic_string_view" && Param->getType()->isPointerType()) { // construct from a char array pointed by a pointer. // basic_string_view(const CharT* s); @@ -265,6 +261,20 @@ 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 NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit( +Context, CanonParam->getLocation())); + } +} } } diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index 81e9193cf76a04..e3ba17df46fd6c 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);
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -266,6 +262,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { higher-performance wrote: See the failing test. I'm inclined to revert to the old code since this approach ends up buggy, but let me know how you'd approach it if it would be different: ``` error: 'expected-warning' diagnostics expected but not seen: File clang/test/SemaCXX/attr-lifetimebound.cpp Line 48: temporary bound to local reference 's' will be destroyed at the end of the full-expression ``` Failing test case: ``` const int &crefparam(const int ¶m); // Omitted in first decl const int &crefparam(const int ¶m); // Omitted in second decl const int &crefparam(const int ¶m [[clang::lifetimebound]]); // Add LB const int &crefparam(const int ¶m) { return param; } // Omit in impl const int& s = crefparam(2); // expected-warning {{temporary bound to local reference 's' will be destroyed at the end of the full-expression}} ``` 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 2d9cc933aa8dc6819c3b0a08f5457bad98e8e6df Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/2] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 15 +- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++ 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f62e18543851c1..1b1220456ded85 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -462,14 +462,15 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + 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(Call); CCE && CCE->getConstructor()->getParent()->hasAttr()) { diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index fb594e6f13c0b3..acdfe190d9f19b 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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 NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -266,6 +262,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { higher-performance wrote: I just tried this but I realized it doesn't work because we want backward propagation to the canonical declaration (since that's what we'll use), rather than forward propagation to the latest one. Unless you're suggesting I change that logic? 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance deleted 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -266,6 +262,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { higher-performance wrote: Done, but note that this is a backward propagation rather than forward, so I had to do a bit of local refactoring to allow that. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -462,14 +462,16 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + const unsigned int NP = higher-performance wrote: Done. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 2d9cc933aa8dc6819c3b0a08f5457bad98e8e6df Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH 1/2] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 15 +- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++ 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f62e18543851c1..1b1220456ded85 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -462,14 +462,15 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + 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(Call); CCE && CCE->getConstructor()->getParent()->hasAttr()) { diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index fb594e6f13c0b3..acdfe190d9f19b 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) + unsigned 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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 NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -266,6 +262,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { ilya-biryukov wrote: I'll probably have more comments, but just wanted to point out that a much more appropriate place for this logic would be `Sema::mergeDeclAttributes` and the `mergeParamDeclAttributes` helper function, in particular. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
@@ -462,14 +462,16 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + const unsigned int NP = ilya-biryukov wrote: NIT: LLVM uses `unsigned`, could you remove `int` to be consistent with the rest of the code? Also, using `const` for local variables is not very common. I'm not sure if the LLVM Style Guide has a position on it, but it's at least inconsistent with the code around the change, so we should probably lean on the side of **not** adding const. 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From edadbdc5844b1d99ce624e3c64dc3d519e2a1e51 Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 16 ++- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++ 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f1507ebb9a5068..03e91f2628b290 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -462,14 +462,16 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + const unsigned int NP = + std::min(Callee->getNumParams(), CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + 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(Call); CCE && CCE->getConstructor()->getParent()->hasAttr()) { diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index fb594e6f13c0b3..37909c822b7588 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +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,
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff c014db47ca298ad7a0f52a57c3bfc2a9914371b2 4c0561d79eedc7060597aa33bfb6e41a64df340c --extensions cpp -- clang/lib/Sema/CheckExprLifetime.cpp clang/lib/Sema/SemaAttr.cpp clang/test/SemaCXX/attr-lifetimebound.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 3224469c6c..03e91f2628 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -463,8 +463,8 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); - const unsigned int NP = std::min(Callee->getNumParams(), - CanonCallee->getNumParams()); + const unsigned int NP = + std::min(Callee->getNumParams(), CanonCallee->getNumParams()); for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr()) `` 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627 >From 4c0561d79eedc7060597aa33bfb6e41a64df340c Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 16 ++- clang/lib/Sema/SemaAttr.cpp | 34 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++ 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f1507ebb9a5068..3224469c6ceda1 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -462,14 +462,16 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, } } - for (unsigned I = 0, -N = std::min(Callee->getNumParams(), Args.size()); - I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); + const FunctionDecl *CanonCallee = Callee->getCanonicalDecl(); + const unsigned int NP = std::min(Callee->getNumParams(), + CanonCallee->getNumParams()); + for (unsigned I = 0, N = std::min(NP, Args.size()); I != N; ++I) { +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + 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(Call); CCE && CCE->getConstructor()->getParent()->hasAttr()) { diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index fb594e6f13c0b3..37909c822b7588 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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,20 @@ 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. +const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams()); +for (unsigned int I = 0; I < NP; ++I) { + auto *CanonParam = CanonDecl->getParamDecl(I); + if (!CanonParam->hasAttr() && + FD->getParamDecl(I)->hasAttr()) { +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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (higher-performance) Changes 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(Callee->getNumParams(), Args.size()); +N = std::min(std::min(Callee->getNumParams(), +CanonCallee->getNumParams()), + Args.size()); I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + 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(Call); CCE && CCE->getConstructor()->getParent()->hasAttr()) { 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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() && + FD->getParamDecl(I)->hasAttr()) { +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().clas
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
higher-performance wrote: @ilya-biryukov 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
[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
https://github.com/higher-performance created https://github.com/llvm/llvm-project/pull/107627 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. >From f2915e19a3d772942b4088b0a7df2d364fb032a3 Mon Sep 17 00:00:00 2001 From: higher-performance Date: Fri, 6 Sep 2024 14:16:15 -0400 Subject: [PATCH] Propagate lifetimebound from formal parameters to those in the canonical declaration, then use the canonical declaration for analysis Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit. --- clang/lib/Sema/CheckExprLifetime.cpp | 14 + clang/lib/Sema/SemaAttr.cpp | 35 +++ clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++ 3 files changed, 36 insertions(+), 17 deletions(-) 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(Callee->getNumParams(), Args.size()); +N = std::min(std::min(Callee->getNumParams(), +CanonCallee->getNumParams()), + Args.size()); I != N; ++I) { -if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) - VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); +if (CheckCoroCall || +CanonCallee->getParamDecl(I)->hasAttr()) + 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(Call); CCE && CCE->getConstructor()->getParent()->hasAttr()) { 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(FD)) { -const auto *CRD = CMD->getParent(); -if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) - return; - -if (isa(CMD)) { + } else if (auto *CMD = dyn_cast(FD)) { +const CXXRecordDecl *CRD = CMD->getParent(); +if (CRD->isInStdNamespace() && CRD->getIdentifier() && +isa(CMD)) { auto *Param = CMD->getParamDecl(0); - if (Param->hasAttr()) -return; - if (CRD->getName() == "basic_string_view" && + if (!Param->hasAttr() && + 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() && + FD->getParamDecl(I)->hasAttr()) { +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 &crefpar