[clang] Improve the lifetime_capture_by diagnostic on the constructor.

[hokein]

With this change, the lifetime_capture_by code path will not handle the constructor decl to avoid bogus diagnostics (see the testcase).

Instead, we reuse the lifetimebound code as the lifetime_capture_by(this) has the same semantic as lifetimebound in constructor. The downside is that the lifetimebound diagnostic is reused for the capture case (I think it is not a big issue).

Fixes #117680

[llvmbot]

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

With this change, the lifetime_capture_by code path will not handle the constructor decl to avoid bogus diagnostics (see the testcase).

Instead, we reuse the lifetimebound code as the lifetime_capture_by(this) has the same semantic as lifetimebound in constructor. The downside is that the lifetimebound diagnostic is reused for the capture case (I think it is not a big issue).

Fixes #117680

---

Full diff: https://github.com/llvm/llvm-project/pull/117792.diff

3 Files Affected:

• (modified) clang/lib/Sema/CheckExprLifetime.cpp (+11)
• (modified) clang/lib/Sema/SemaChecking.cpp (+6)
• (modified) clang/test/Sema/warn-lifetime-analysis-capture-by.cpp (+19)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 6cdd4dc629e50a..c4fa73127410b5 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -535,6 +535,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
 
   bool EnableGSLAnalysis = !Callee->getASTContext().getDiagnostics().isIgnored(
       diag::warn_dangling_lifetime_pointer, SourceLocation());
+  bool EnableDanglingCapture =
+      !Callee->getASTContext().getDiagnostics().isIgnored(
+          diag::warn_dangling_reference_captured, SourceLocation());
   Expr *ObjectArg = nullptr;
   if (isa<CXXOperatorCallExpr>(Call) && Callee->isCXXInstanceMember()) {
     ObjectArg = Args[0];
@@ -623,6 +626,14 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
     }
     if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
       VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
+    else if (const auto *CaptureAttr =
+                 Callee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
+             EnableDanglingCapture && CaptureAttr &&
+             isa<CXXConstructorDecl>(Callee) &&
+             llvm::any_of(CaptureAttr->params(), [](int ArgIdx) {
+               return ArgIdx == LifetimeCaptureByAttr::THIS;
+             }))
+      VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
     else if (EnableGSLAnalysis && I == 0) {
       // Perform GSL analysis for the first argument
       if (shouldTrackFirstArgument(Callee)) {
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a49605e4867651..1605523097a6b1 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3240,8 +3240,14 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
                                  unsigned ArgIdx) {
     if (!Attr)
       return;
+
     Expr *Captured = const_cast<Expr *>(GetArgAt(ArgIdx));
     for (int CapturingParamIdx : Attr->params()) {
+      // lifetime_capture_by(this) case is handled in the lifetimebound expr
+      // initialization codepath.
+      if (CapturingParamIdx == LifetimeCaptureByAttr::THIS &&
+          isa<CXXConstructorDecl>(FD))
+        continue;
       Expr *Capturing = const_cast<Expr *>(GetArgAt(CapturingParamIdx));
       CapturingEntity CE{Capturing};
       // Ensure that 'Captured' outlives the 'Capturing' entity.
diff --git a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
index 4d562bac1e305b..77523210e50203 100644
--- a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
@@ -411,3 +411,22 @@ void use() {
 }
 } // namespace with_span
 } // namespace inferred_capture_by
+
+namespace on_constructor {
+struct T {
+  T(const int& t [[clang::lifetime_capture_by(this)]]);
+};
+struct T2 {
+  T2(const int& t [[clang::lifetime_capture_by(x)]], int& x);
+};
+int foo(const T& t);
+
+void test() {
+  auto x = foo(T(1)); // OK. no diagnosic
+  T(1); // OK. no diagnostic
+  T t(1); // expected-warning {{temporary whose address is used}}
+    
+  int a;
+  T2(1, a); // expected-warning {{object whose reference is captured by}}
+}
+} // namespace on_constructor

[usx95]

Thanks. Can you also add a one line description to the capture by docs as well.

[hokein]

Done.