[clang] [llvm] [clang] Implement lifetime analysis for lifetime_capture_by(X) (PR #115921)

[Haojian Wu via llvm-commits]

================
@@ -1412,17 +1438,34 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
     return false;
   };
 
+  bool HasReferenceBinding = Init->isGLValue();
   llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
-  if (LK == LK_Assignment &&
-      shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) {
-    Path.push_back(
-        {isAssignmentOperatorLifetimeBound(AEntity->AssignmentOperator)
-             ? IndirectLocalPathEntry::LifetimeBoundCall
-             : IndirectLocalPathEntry::GslPointerAssignment,
-         Init});
+  switch (LK) {
+  case LK_Assignment: {
+    if (shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity))
+      Path.push_back(
+          {isAssignmentOperatorLifetimeBound(AEntity->AssignmentOperator)
+               ? IndirectLocalPathEntry::LifetimeBoundCall
+               : IndirectLocalPathEntry::GslPointerAssignment,
+           Init});
+    break;
+  }
+  case LK_LifetimeCapture: {
+    Path.push_back({IndirectLocalPathEntry::LifetimeCapture, Init});
+    if (isRecordWithAttr<PointerAttr>(Init->getType()))
+      HasReferenceBinding = false;
+    // Skip the top MTE if it is a temporary object of the pointer-like type
+    // itself.
+    if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init);
+        MTE && isPointerLikeType(Init->getType()))
+      Init = MTE->getSubExpr();
----------------
hokein wrote:

I know that this code originated from my patch that added assignment support for "container<pointer>." That patch has some issues: although it passed the unit tests, it failed to diagnose cases in real STL headers.

I'm not sure I fully understand the implementation here (my initial idea was to leverage the existing code paths for `lifetimebound` and `gsl::Pointer`, so the need for `IndirectLocalPathEntry::LifetimeCapture` isn’t clear). I experimented this patch, and made some tweaks to simplify the logic, and here’s the diff I came up with:

```cpp
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1212,9 +1212,9 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
       // and the capturing entity does too, so don't warn.
       if (!MTE)
         return false;
-      assert(shouldLifetimeExtendThroughPath(Path) ==
-                 PathLifetimeKind::NoExtend &&
-             "No lifetime extension in function calls");
+      // assert(shouldLifetimeExtendThroughPath(Path) ==
+      //            PathLifetimeKind::NoExtend &&
+      //        "No lifetime extension in function calls");
       if (CapEntity->Entity)
         SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured)
             << CapEntity->Entity << DiagRange;
@@ -1451,14 +1451,8 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
     break;
   }
   case LK_LifetimeCapture: {
-    Path.push_back({IndirectLocalPathEntry::LifetimeCapture, Init});
-    if (isRecordWithAttr<PointerAttr>(Init->getType()))
-      HasReferenceBinding = false;
-    // Skip the top MTE if it is a temporary object of the pointer-like type
-    // itself.
-    if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init);
-        MTE && isPointerLikeType(Init->getType()))
-      Init = MTE->getSubExpr();
+    if (isPointerLikeType(Init->getType()))
+      Path.push_back({IndirectLocalPathEntry::GslPointerInit, Init});
     break;
   }
   default:
```

The tests passed, and it also fixes the known false positives:

```
error: 'expected-warning' diagnostics expected but not seen: 
  File /workspace/llvm-project/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp Line 313: captured
  File /workspace/llvm-project/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp Line 314: captured
  File /workspace/llvm-project/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp Line 318: captured
```

Would you mind taking it further?

https://github.com/llvm/llvm-project/pull/115921