================
@@ -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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
