[clang] Don't infer lifetime_capture-by for reference of raw pointer types.

[hokein]

When a vector is instantiated with a pointer type (T being const Foo*), the inferred annotation becomes push_back(const Foo*& value [[clang::lifetime_capture_by(this)]]).

For reference parameters, the lifetime_capture_by attribute treats the lifetime as referring to the referenced object -- in this case, the pointer itself, not the pointee object. In the push_back, we copy the pointer's value, which does not establish a reference to the pointer. This behavior is safe and does not capture the pointer's lifetime.

The annotation should not be inferred for cases where T is a pointer type, as the intended semantics do not align with the annotation.

Fixes #121391

[llvmbot]

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

When a vector is instantiated with a pointer type (T being const Foo*), the inferred annotation becomes push_back(const Foo*& value [[clang::lifetime_capture_by(this)]]).

For reference parameters, the lifetime_capture_by attribute treats the lifetime as referring to the referenced object -- in this case, the pointer itself, not the pointee object. In the push_back, we copy the pointer's value, which does not establish a reference to the pointer. This behavior is safe and does not capture the pointer's lifetime.

The annotation should not be inferred for cases where T is a pointer type, as the intended semantics do not align with the annotation.

Fixes #121391

---

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

5 Files Affected:

• (modified) clang/lib/Sema/CheckExprLifetime.cpp (+5-3)
• (modified) clang/lib/Sema/CheckExprLifetime.h (+2-3)
• (modified) clang/lib/Sema/SemaAttr.cpp (+1-1)
• (modified) clang/test/AST/attr-lifetime-capture-by.cpp (+3-3)
• (modified) clang/test/Sema/warn-lifetime-analysis-capture-by.cpp (+22-1)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 7109de03cadd12..837414c4840d75 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -281,9 +281,11 @@ template <typename T> static bool isRecordWithAttr(QualType Type) {
   return Result;
 }
 
-bool isPointerLikeType(QualType QT) {
-  return isRecordWithAttr<PointerAttr>(QT) || QT->isPointerType() ||
-         QT->isNullPtrType();
+// Tells whether the type is annotated with [[gsl::Pointer]].
+bool isGLSPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); }
+
+static bool isPointerLikeType(QualType QT) {
+  return isGLSPointerType(QT) || QT->isPointerType() || QT->isNullPtrType();
 }
 
 // Decl::isInStdNamespace will return false for iterators in some STL
diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h
index 95c249c6109774..6351e52a362f14 100644
--- a/clang/lib/Sema/CheckExprLifetime.h
+++ b/clang/lib/Sema/CheckExprLifetime.h
@@ -18,9 +18,8 @@
 
 namespace clang::sema {
 
-// Tells whether the type is annotated with [[gsl::Pointer]] or is a pointer
-// type.
-bool isPointerLikeType(QualType QT);
+// Tells whether the type is annotated with [[gsl::Pointer]].
+bool isGLSPointerType(QualType QT);
 
 /// Describes an entity that is being assigned.
 struct AssignedEntity {
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index 42aa68d2905c03..6907fa91e28c20 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -284,7 +284,7 @@ void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) {
       // We only apply the lifetime_capture_by attribute to parameters of
       // pointer-like reference types (`const T&`, `T&&`).
       if (PVD->getType()->isReferenceType() &&
-          sema::isPointerLikeType(PVD->getType().getNonReferenceType())) {
+          sema::isGLSPointerType(PVD->getType().getNonReferenceType())) {
         int CaptureByThis[] = {LifetimeCaptureByAttr::THIS};
         PVD->addAttr(
             LifetimeCaptureByAttr::CreateImplicit(Context, CaptureByThis, 1));
diff --git a/clang/test/AST/attr-lifetime-capture-by.cpp b/clang/test/AST/attr-lifetime-capture-by.cpp
index 71b348dac764bc..b453bef018c0bc 100644
--- a/clang/test/AST/attr-lifetime-capture-by.cpp
+++ b/clang/test/AST/attr-lifetime-capture-by.cpp
@@ -80,16 +80,16 @@ std::vector<int*> pointers;
 
 // CHECK:       CXXMethodDecl {{.*}} push_back 'void (int *const &)'
 // CHECK-NEXT:           ParmVarDecl {{.*}} 'int *const &'
-// CHECK-NEXT:               LifetimeCaptureByAttr {{.*}} Implicit
+// CHECK-NOT:               LifetimeCaptureByAttr
 
 // CHECK:       CXXMethodDecl {{.*}} push_back 'void (int *&&)'
 // CHECK-NEXT:           ParmVarDecl {{.*}} 'int *&&'
-// CHECK-NEXT:               LifetimeCaptureByAttr {{.*}} Implicit
+// CHECK-NOT:               LifetimeCaptureByAttr
 
 // CHECK:       CXXMethodDecl {{.*}} insert 'void (iterator, int *&&)'
 // CHECK-NEXT:           ParmVarDecl {{.*}} 'iterator'
 // CHECK-NEXT:           ParmVarDecl {{.*}} 'int *&&'
-// CHECK-NEXT:               LifetimeCaptureByAttr {{.*}} Implicit
+// CHECK-NOT:               LifetimeCaptureByAttr
 
 std::vector<int> ints;
 // CHECK:   ClassTemplateSpecializationDecl {{.*}} struct vector definition implicit_instantiation
diff --git a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
index 12b933e63edd7b..2877d8d6cd5f9c 100644
--- a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
@@ -406,7 +406,7 @@ void use() {
   strings.insert(strings.begin(), std::string());
 
   std::vector<const std::string*> pointers;
-  pointers.push_back(getLifetimeBoundPointer(std::string())); // expected-warning {{object whose reference is captured by 'pointers' will be destroyed at the end of the full-expression}}
+  pointers.push_back(getLifetimeBoundPointer(std::string()));
   pointers.push_back(&local);
 }
 
@@ -451,3 +451,24 @@ void test() {
   T2(1, a); // expected-warning {{object whose reference is captured by}}
 }
 } // namespace on_constructor
+
+namespace GH121391 {
+
+struct Foo {};
+
+template <typename T>
+struct Container {
+  const T& tt() [[clang::lifetimebound]];
+};
+template<typename T>
+struct StatusOr {
+   T* get() [[clang::lifetimebound]];
+};
+StatusOr<Container<const Foo*>> getContainer();
+
+void test() {
+  std::vector<const Foo*> vv;
+  vv.push_back(getContainer().get()->tt()); // OK
+}
+
+} // namespace GH121391

[Xazax-hun]

For a follow-up PR, but I wonder if we can actually get this diagnostic back in the future. I think in case the pointer originates from a gsl owner, it is a strong indication that we care about the lifetime of the pointee and not the pointer itself and the capture_by annotation is what we actually want.

But I am also OK never supporting this scenario as it starts to get a bit convoluted.

[hokein]

I completely agree that it would be ideal to restore the diagnostic without introducing false positives. However, I currently don’t have a simple solution for how to achieve this.