Re-land "[clang] Merge lifetimebound and GSL code paths for li… #105753
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…fetime a…" This reverts commit 1df1504.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Vitaly Buka (vitalybuka) Changes... just for visibility Patch is 25.79 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105753.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 04cc4cc22964a8..34f6680c85e87e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -239,6 +239,8 @@ Improvements to Clang's diagnostics
- Clang now diagnoses when the result of a [[nodiscard]] function is discarded after being cast in C. Fixes #GH104391.
+- Don't emit duplicated dangling diagnostics. (#GH93386).
+
- Improved diagnostic when trying to befriend a concept. (#GH45182).
Improvements to Clang's time-trace
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 7389046eaddde1..7e23c08cc79ffb 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -237,13 +237,11 @@ static bool pathContainsInit(IndirectLocalPath &Path) {
static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
Expr *Init, LocalVisitor Visit,
- bool RevisitSubinits,
- bool EnableLifetimeWarnings);
+ bool RevisitSubinits);
static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
Expr *Init, ReferenceKind RK,
- LocalVisitor Visit,
- bool EnableLifetimeWarnings);
+ LocalVisitor Visit);
template <typename T> static bool isRecordWithAttr(QualType Type) {
if (auto *RD = Type->getAsCXXRecordDecl())
@@ -326,66 +324,6 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
return false;
}
-static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
- LocalVisitor Visit) {
- auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
- // We are not interested in the temporary base objects of gsl Pointers:
- // Temp().ptr; // Here ptr might not dangle.
- if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
- return;
- // Once we initialized a value with a reference, it can no longer dangle.
- if (!Value) {
- for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
- if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit)
- continue;
- if (PE.Kind == IndirectLocalPathEntry::GslPointerInit ||
- PE.Kind == IndirectLocalPathEntry::GslPointerAssignment)
- return;
- break;
- }
- }
- Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit
- : IndirectLocalPathEntry::GslReferenceInit,
- Arg, D});
- if (Arg->isGLValue())
- visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
- Visit,
- /*EnableLifetimeWarnings=*/true);
- else
- visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
- /*EnableLifetimeWarnings=*/true);
- Path.pop_back();
- };
-
- if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
- const auto *MD = cast_or_null<CXXMethodDecl>(MCE->getDirectCallee());
- if (MD && shouldTrackImplicitObjectArg(MD))
- VisitPointerArg(MD, MCE->getImplicitObjectArgument(),
- !MD->getReturnType()->isReferenceType());
- return;
- } else if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(Call)) {
- FunctionDecl *Callee = OCE->getDirectCallee();
- if (Callee && Callee->isCXXInstanceMember() &&
- shouldTrackImplicitObjectArg(cast<CXXMethodDecl>(Callee)))
- VisitPointerArg(Callee, OCE->getArg(0),
- !Callee->getReturnType()->isReferenceType());
- return;
- } else if (auto *CE = dyn_cast<CallExpr>(Call)) {
- FunctionDecl *Callee = CE->getDirectCallee();
- if (Callee && shouldTrackFirstArgument(Callee))
- VisitPointerArg(Callee, CE->getArg(0),
- !Callee->getReturnType()->isReferenceType());
- return;
- }
-
- if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) {
- const auto *Ctor = CCE->getConstructor();
- const CXXRecordDecl *RD = Ctor->getParent();
- if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>())
- VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0], true);
- }
-}
-
static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
if (!TSI)
@@ -423,8 +361,9 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
return false;
}
-static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
- LocalVisitor Visit) {
+// Visit lifetimebound or gsl-pointer arguments.
+static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
+ LocalVisitor Visit) {
const FunctionDecl *Callee;
ArrayRef<Expr *> Args;
@@ -439,6 +378,8 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
if (!Callee)
return;
+ bool EnableGSLAnalysis = !Callee->getASTContext().getDiagnostics().isIgnored(
+ diag::warn_dangling_lifetime_pointer, SourceLocation());
Expr *ObjectArg = nullptr;
if (isa<CXXOperatorCallExpr>(Call) && Callee->isCXXInstanceMember()) {
ObjectArg = Args[0];
@@ -451,11 +392,35 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
Path.push_back({IndirectLocalPathEntry::LifetimeBoundCall, Arg, D});
if (Arg->isGLValue())
visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
- Visit,
- /*EnableLifetimeWarnings=*/false);
+ Visit);
else
- visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
- /*EnableLifetimeWarnings=*/false);
+ visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+ Path.pop_back();
+ };
+ auto VisitGSLPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
+ // We are not interested in the temporary base objects of gsl Pointers:
+ // Temp().ptr; // Here ptr might not dangle.
+ if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
+ return;
+ // Once we initialized a value with a reference, it can no longer dangle.
+ if (!Value) {
+ for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
+ if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit)
+ continue;
+ if (PE.Kind == IndirectLocalPathEntry::GslPointerInit ||
+ PE.Kind == IndirectLocalPathEntry::GslPointerAssignment)
+ return;
+ break;
+ }
+ }
+ Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit
+ : IndirectLocalPathEntry::GslReferenceInit,
+ Arg, D});
+ if (Arg->isGLValue())
+ visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
+ Visit);
+ else
+ visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
Path.pop_back();
};
@@ -478,6 +443,12 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
CheckCoroObjArg = false;
if (implicitObjectParamIsLifetimeBound(Callee) || CheckCoroObjArg)
VisitLifetimeBoundArg(Callee, ObjectArg);
+ else if (EnableGSLAnalysis) {
+ if (auto *CME = dyn_cast<CXXMethodDecl>(Callee);
+ CME && shouldTrackImplicitObjectArg(CME))
+ VisitGSLPointerArg(Callee, ObjectArg,
+ !Callee->getReturnType()->isReferenceType());
+ }
}
for (unsigned I = 0,
@@ -485,6 +456,17 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
I != N; ++I) {
if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]);
+ else if (EnableGSLAnalysis && I == 0) { // GSL
+ if (shouldTrackFirstArgument(Callee)) {
+ VisitGSLPointerArg(Callee, Args[0],
+ !Callee->getReturnType()->isReferenceType());
+ } else if (auto *CCE = dyn_cast<CXXConstructExpr>(Call);
+ CCE &&
+ CCE->getConstructor()->getParent()->hasAttr<PointerAttr>()) {
+ VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0],
+ true);
+ }
+ }
}
}
@@ -492,8 +474,7 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
/// glvalue expression \c Init.
static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
Expr *Init, ReferenceKind RK,
- LocalVisitor Visit,
- bool EnableLifetimeWarnings) {
+ LocalVisitor Visit) {
RevertToOldSizeRAII RAII(Path);
// Walk past any constructs which we can lifetime-extend across.
@@ -530,8 +511,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
else
// We can't lifetime extend through this but we might still find some
// retained temporaries.
- return visitLocalsRetainedByInitializer(Path, Init, Visit, true,
- EnableLifetimeWarnings);
+ return visitLocalsRetainedByInitializer(Path, Init, Visit, true);
}
// Step into CXXDefaultInitExprs so we can diagnose cases where a
@@ -545,23 +525,18 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init)) {
if (Visit(Path, Local(MTE), RK))
- visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit, true,
- EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit, true);
}
if (auto *M = dyn_cast<MemberExpr>(Init)) {
// Lifetime of a non-reference type field is same as base object.
if (auto *F = dyn_cast<FieldDecl>(M->getMemberDecl());
F && !F->getType()->isReferenceType())
- visitLocalsRetainedByInitializer(Path, M->getBase(), Visit, true,
- EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(Path, M->getBase(), Visit, true);
}
- if (isa<CallExpr>(Init)) {
- if (EnableLifetimeWarnings)
- handleGslAnnotatedTypes(Path, Init, Visit);
- return visitLifetimeBoundArguments(Path, Init, Visit);
- }
+ if (isa<CallExpr>(Init))
+ return visitFunctionCallArguments(Path, Init, Visit);
switch (Init->getStmtClass()) {
case Stmt::DeclRefExprClass: {
@@ -580,8 +555,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
} else if (VD->getInit() && !isVarOnPath(Path, VD)) {
Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD});
visitLocalsRetainedByReferenceBinding(Path, VD->getInit(),
- RK_ReferenceBinding, Visit,
- EnableLifetimeWarnings);
+ RK_ReferenceBinding, Visit);
}
}
break;
@@ -593,15 +567,13 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
// handling all sorts of rvalues passed to a unary operator.
const UnaryOperator *U = cast<UnaryOperator>(Init);
if (U->getOpcode() == UO_Deref)
- visitLocalsRetainedByInitializer(Path, U->getSubExpr(), Visit, true,
- EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(Path, U->getSubExpr(), Visit, true);
break;
}
case Stmt::ArraySectionExprClass: {
- visitLocalsRetainedByInitializer(Path,
- cast<ArraySectionExpr>(Init)->getBase(),
- Visit, true, EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(
+ Path, cast<ArraySectionExpr>(Init)->getBase(), Visit, true);
break;
}
@@ -609,11 +581,9 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
case Stmt::BinaryConditionalOperatorClass: {
auto *C = cast<AbstractConditionalOperator>(Init);
if (!C->getTrueExpr()->getType()->isVoidType())
- visitLocalsRetainedByReferenceBinding(Path, C->getTrueExpr(), RK, Visit,
- EnableLifetimeWarnings);
+ visitLocalsRetainedByReferenceBinding(Path, C->getTrueExpr(), RK, Visit);
if (!C->getFalseExpr()->getType()->isVoidType())
- visitLocalsRetainedByReferenceBinding(Path, C->getFalseExpr(), RK, Visit,
- EnableLifetimeWarnings);
+ visitLocalsRetainedByReferenceBinding(Path, C->getFalseExpr(), RK, Visit);
break;
}
@@ -636,8 +606,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
/// the prvalue expression \c Init.
static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
Expr *Init, LocalVisitor Visit,
- bool RevisitSubinits,
- bool EnableLifetimeWarnings) {
+ bool RevisitSubinits) {
RevertToOldSizeRAII RAII(Path);
Expr *Old;
@@ -678,18 +647,16 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
if (VD && VD->getType().isConstQualified() && VD->getInit() &&
!isVarOnPath(Path, VD)) {
Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD});
- visitLocalsRetainedByInitializer(
- Path, VD->getInit(), Visit, true, EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(Path, VD->getInit(), Visit,
+ true);
}
} else if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L)) {
if (MTE->getType().isConstQualified())
visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(),
- Visit, true,
- EnableLifetimeWarnings);
+ Visit, true);
}
return false;
- },
- EnableLifetimeWarnings);
+ });
// We assume that objects can be retained by pointers cast to integers,
// but not if the integer is cast to floating-point type or to _Complex.
@@ -718,9 +685,8 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
// Model array-to-pointer decay as taking the address of the array
// lvalue.
Path.push_back({IndirectLocalPathEntry::AddressOf, CE});
- return visitLocalsRetainedByReferenceBinding(Path, CE->getSubExpr(),
- RK_ReferenceBinding, Visit,
- EnableLifetimeWarnings);
+ return visitLocalsRetainedByReferenceBinding(
+ Path, CE->getSubExpr(), RK_ReferenceBinding, Visit);
default:
return;
@@ -735,8 +701,7 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
// lifetime of the array exactly like binding a reference to a temporary.
if (auto *ILE = dyn_cast<CXXStdInitializerListExpr>(Init))
return visitLocalsRetainedByReferenceBinding(Path, ILE->getSubExpr(),
- RK_StdInitializerList, Visit,
- EnableLifetimeWarnings);
+ RK_StdInitializerList, Visit);
if (InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) {
// We already visited the elements of this initializer list while
@@ -747,14 +712,12 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
if (ILE->isTransparent())
return visitLocalsRetainedByInitializer(Path, ILE->getInit(0), Visit,
- RevisitSubinits,
- EnableLifetimeWarnings);
+ RevisitSubinits);
if (ILE->getType()->isArrayType()) {
for (unsigned I = 0, N = ILE->getNumInits(); I != N; ++I)
visitLocalsRetainedByInitializer(Path, ILE->getInit(I), Visit,
- RevisitSubinits,
- EnableLifetimeWarnings);
+ RevisitSubinits);
return;
}
@@ -767,14 +730,12 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
if (RD->isUnion() && ILE->getInitializedFieldInUnion() &&
ILE->getInitializedFieldInUnion()->getType()->isReferenceType())
visitLocalsRetainedByReferenceBinding(Path, ILE->getInit(0),
- RK_ReferenceBinding, Visit,
- EnableLifetimeWarnings);
+ RK_ReferenceBinding, Visit);
else {
unsigned Index = 0;
for (; Index < RD->getNumBases() && Index < ILE->getNumInits(); ++Index)
visitLocalsRetainedByInitializer(Path, ILE->getInit(Index), Visit,
- RevisitSubinits,
- EnableLifetimeWarnings);
+ RevisitSubinits);
for (const auto *I : RD->fields()) {
if (Index >= ILE->getNumInits())
break;
@@ -783,14 +744,13 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
Expr *SubInit = ILE->getInit(Index);
if (I->getType()->isReferenceType())
visitLocalsRetainedByReferenceBinding(Path, SubInit,
- RK_ReferenceBinding, Visit,
- EnableLifetimeWarnings);
+ RK_ReferenceBinding, Visit);
else
// This might be either aggregate-initialization of a member or
// initialization of a std::initializer_list object. Regardless,
// we should recursively lifetime-extend that initializer.
- visitLocalsRetainedByInitializer(
- Path, SubInit, Visit, RevisitSubinits, EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(Path, SubInit, Visit,
+ RevisitSubinits);
++Index;
}
}
@@ -811,10 +771,9 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
Path.push_back({IndirectLocalPathEntry::LambdaCaptureInit, E, &Cap});
if (E->isGLValue())
visitLocalsRetainedByReferenceBinding(Path, E, RK_ReferenceBinding,
- Visit, EnableLifetimeWarnings);
+ Visit);
else
- visitLocalsRetainedByInitializer(Path, E, Visit, true,
- EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(Path, E, Visit, true);
if (Cap.capturesVariable())
Path.pop_back();
}
@@ -828,18 +787,14 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
Expr *Arg = MTE->getSubExpr();
Path.push_back({IndirectLocalPathEntry::TemporaryCopy, Arg,
CCE->getConstructor()});
- visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
- /*EnableLifetimeWarnings*/ false);
+ visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
Path.pop_back();
}
...
[truncated]
|
|
Thanks, acked. Closing it now. I will reland it. |
nunoplopes
deleted the
revert-105752-users/vitalybuka/spr/revert-clang-merge-lifetimebound-and-gsl-code-paths-for-lifetime-analysis-104906
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... just for visibility of revert #105752