[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)

2025-01-14 Thread Ilya Biryukov via cfe-commits

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)

2025-01-14 Thread Ilya Biryukov via cfe-commits


@@ -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)

2025-01-14 Thread Ilya Biryukov via cfe-commits

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)

2025-01-14 Thread Ilya Biryukov via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-30 Thread via cfe-commits

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)

2024-12-13 Thread via cfe-commits


@@ -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)

2024-12-13 Thread via cfe-commits


@@ -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)

2024-12-13 Thread via cfe-commits


@@ -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)

2024-12-13 Thread via cfe-commits

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)

2024-12-13 Thread via cfe-commits


@@ -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)

2024-12-13 Thread via cfe-commits


@@ -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)

2024-12-13 Thread via cfe-commits


@@ -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)

2024-12-10 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-12-10 Thread Ilya Biryukov via cfe-commits

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)

2024-12-10 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-12-10 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-12-10 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-12-10 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-12-10 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-12-10 Thread Ilya Biryukov via cfe-commits

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)

2024-11-26 Thread via cfe-commits

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)

2024-11-26 Thread via cfe-commits


@@ -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)

2024-11-26 Thread via cfe-commits

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)

2024-11-18 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-11-18 Thread via cfe-commits


@@ -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)

2024-11-18 Thread via cfe-commits

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)

2024-11-18 Thread via cfe-commits

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)

2024-11-18 Thread via cfe-commits

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)

2024-09-20 Thread via cfe-commits


@@ -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)

2024-09-13 Thread via cfe-commits

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)

2024-09-13 Thread via cfe-commits


@@ -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)

2024-09-13 Thread via cfe-commits

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)

2024-09-13 Thread via cfe-commits


@@ -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)

2024-09-13 Thread via cfe-commits


@@ -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)

2024-09-13 Thread via cfe-commits

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)

2024-09-13 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-09-13 Thread Ilya Biryukov via cfe-commits


@@ -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)

2024-09-06 Thread via cfe-commits

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)

2024-09-06 Thread via cfe-commits

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)

2024-09-06 Thread via cfe-commits

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)

2024-09-06 Thread via cfe-commits

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)

2024-09-06 Thread via cfe-commits

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)

2024-09-06 Thread via cfe-commits

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