[clang] 6e720df - [clang] Improve the lifetime_capture_by diagnostic on the constructor. (#117792)

[via cfe-commits]

Author: Haojian Wu
Date: 2024-11-28T12:35:15+01:00
New Revision: 6e720df1ae23c715aefc36476ab46284a96e9371

URL: https://github.com/llvm/llvm-project/commit/6e720df1ae23c715aefc36476ab46284a96e9371
DIFF: https://github.com/llvm/llvm-project/commit/6e720df1ae23c715aefc36476ab46284a96e9371.diff

LOG: [clang] Improve the lifetime_capture_by diagnostic on the constructor. (#117792)

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

Added: 
    

Modified: 
    clang/include/clang/Basic/AttrDocs.td
    clang/lib/Sema/CheckExprLifetime.cpp
    clang/lib/Sema/SemaChecking.cpp
    clang/test/Sema/warn-lifetime-analysis-capture-by.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 9617687ac69caf..5de39be4805600 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3985,6 +3985,8 @@ The capturing entity ``X`` can be one of the following:
       std::set<std::string_view> s;
     };
 
+  Note: When applied to a constructor parameter, `[[clang::lifetime_capture_by(this)]]` is just an alias of `[[clang::lifetimebound]]`.
+
 - `global`, `unknown`.
 
   .. code-block:: c++

diff  --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 6cdd4dc629e50a..607b7daf878e17 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -623,6 +623,26 @@ 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>();
+             CaptureAttr && isa<CXXConstructorDecl>(Callee) &&
+             llvm::any_of(CaptureAttr->params(), [](int ArgIdx) {
+               return ArgIdx == LifetimeCaptureByAttr::THIS;
+             }))
+      // `lifetime_capture_by(this)` in a class constructor has the same
+      // semantics as `lifetimebound`:
+      //
+      // struct Foo {
+      //   const int& a;
+      //   // Equivalent to Foo(const int& t [[clang::lifetimebound]])
+      //   Foo(const int& t [[clang::lifetime_capture_by(this)]]) : a(t) {}
+      // };
+      //
+      // In the implementation, `lifetime_capture_by` is treated as an alias for
+      // `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);
     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 e071e2b7f33500..a248a6b53b0d06 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 e867296d6d8ea7..12b933e63edd7b 100644
--- a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
@@ -425,3 +425,29 @@ 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);
+};
+struct T3 {
+  T3(const T& t [[clang::lifetime_capture_by(this)]]);
+};
+
+int foo(const T& t);
+int bar(const T& t[[clang::lifetimebound]]);
+
+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}}
+  auto y = bar(T(1)); // expected-warning {{temporary whose address is used}}
+  T3 t3(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