[clang] 2177e45 - PR47861: Expand dangling reference warning to look through copy
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 30 10:20:53 PDT 2020
Author: Richard Smith
Date: 2020-10-30T10:19:50-07:00
New Revision: 2177e4555ab84771c611a3295ab1149af7f79c29
URL: https://github.com/llvm/llvm-project/commit/2177e4555ab84771c611a3295ab1149af7f79c29
DIFF: https://github.com/llvm/llvm-project/commit/2177e4555ab84771c611a3295ab1149af7f79c29.diff
LOG: PR47861: Expand dangling reference warning to look through copy
construction, and to assume that assignment operators return *this.
Added:
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/OperatorKinds.h
clang/lib/Sema/SemaInit.cpp
clang/test/CXX/expr/expr.prim/expr.prim.lambda/p11-1y.cpp
clang/test/SemaCXX/conditional-expr.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
clang/test/SemaCXX/return-stack-addr.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 47becc959fb2..fe6b88321f6b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9077,6 +9077,10 @@ def err_ret_local_block : Error<
def note_local_var_initializer : Note<
"%select{via initialization of|binding reference}0 variable "
"%select{%2 |}1here">;
+def note_lambda_capture_initializer : Note<
+ "%select{implicitly |}2captured%select{| by reference}3"
+ "%select{%select{ due to use|}2 here|"
+ " via initialization of lambda capture %0}1">;
def note_init_with_default_member_initalizer : Note<
"initializing field %0 with default member initializer">;
diff --git a/clang/include/clang/Basic/OperatorKinds.h b/clang/include/clang/Basic/OperatorKinds.h
index d66189144511..3315df246981 100644
--- a/clang/include/clang/Basic/OperatorKinds.h
+++ b/clang/include/clang/Basic/OperatorKinds.h
@@ -49,6 +49,11 @@ getRewrittenOverloadedOperator(OverloadedOperatorKind Kind) {
}
}
+/// Determine if this is a compound assignment operator.
+inline bool isCompoundAssignmentOperator(OverloadedOperatorKind Kind) {
+ return Kind >= OO_PlusEqual && Kind <= OO_PipeEqual;
+}
+
} // end namespace clang
#endif
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 419b25d278f5..5131ce446d04 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -6710,15 +6710,22 @@ struct IndirectLocalPathEntry {
VarInit,
LValToRVal,
LifetimeBoundCall,
+ TemporaryCopy,
+ LambdaCaptureInit,
GslReferenceInit,
GslPointerInit
} Kind;
Expr *E;
- const Decl *D = nullptr;
+ union {
+ const Decl *D = nullptr;
+ const LambdaCapture *Capture;
+ };
IndirectLocalPathEntry() {}
IndirectLocalPathEntry(EntryKind K, Expr *E) : Kind(K), E(E) {}
IndirectLocalPathEntry(EntryKind K, Expr *E, const Decl *D)
: Kind(K), E(E), D(D) {}
+ IndirectLocalPathEntry(EntryKind K, Expr *E, const LambdaCapture *Capture)
+ : Kind(K), E(E), Capture(Capture) {}
};
using IndirectLocalPath = llvm::SmallVectorImpl<IndirectLocalPathEntry>;
@@ -6912,6 +6919,26 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
if (ATL.getAttrAs<LifetimeBoundAttr>())
return true;
}
+
+ // Assume that all assignment operators with a "normal" return type return
+ // *this, that is, an lvalue reference that is the same type as the implicit
+ // object parameter (or the LHS for a non-member operator$=).
+ OverloadedOperatorKind OO = FD->getDeclName().getCXXOverloadedOperator();
+ if (OO == OO_Equal || isCompoundAssignmentOperator(OO)) {
+ QualType RetT = FD->getReturnType();
+ if (RetT->isLValueReferenceType()) {
+ ASTContext &Ctx = FD->getASTContext();
+ QualType LHST;
+ auto *MD = dyn_cast<CXXMethodDecl>(FD);
+ if (MD && MD->isCXXInstanceMember())
+ LHST = Ctx.getLValueReferenceType(MD->getThisObjectType());
+ else
+ LHST = MD->getParamDecl(0)->getType();
+ if (Ctx.hasSameType(RetT, LHST))
+ return true;
+ }
+ }
+
return false;
}
@@ -7257,15 +7284,37 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
// The lifetime of an init-capture is that of the closure object constructed
// by a lambda-expression.
if (auto *LE = dyn_cast<LambdaExpr>(Init)) {
+ LambdaExpr::capture_iterator CapI = LE->capture_begin();
for (Expr *E : LE->capture_inits()) {
+ assert(CapI != LE->capture_end());
+ const LambdaCapture &Cap = *CapI++;
if (!E)
continue;
+ if (Cap.capturesVariable())
+ Path.push_back({IndirectLocalPathEntry::LambdaCaptureInit, E, &Cap});
if (E->isGLValue())
visitLocalsRetainedByReferenceBinding(Path, E, RK_ReferenceBinding,
Visit, EnableLifetimeWarnings);
else
visitLocalsRetainedByInitializer(Path, E, Visit, true,
EnableLifetimeWarnings);
+ if (Cap.capturesVariable())
+ Path.pop_back();
+ }
+ }
+
+ // Assume that a copy or move from a temporary references the same objects
+ // that the temporary does.
+ if (auto *CCE = dyn_cast<CXXConstructExpr>(Init)) {
+ if (CCE->getConstructor()->isCopyOrMoveConstructor()) {
+ if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(CCE->getArg(0))) {
+ Expr *Arg = MTE->getSubExpr();
+ Path.push_back({IndirectLocalPathEntry::TemporaryCopy, Arg,
+ CCE->getConstructor()});
+ visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
+ /*EnableLifetimeWarnings*/false);
+ Path.pop_back();
+ }
}
}
@@ -7342,14 +7391,31 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
}
}
+/// Whether a path to an object supports lifetime extension.
+enum PathLifetimeKind {
+ /// Lifetime-extend along this path.
+ Extend,
+ /// We should lifetime-extend, but we don't because (due to technical
+ /// limitations) we can't. This happens for default member initializers,
+ /// which we don't clone for every use, so we don't have a unique
+ /// MaterializeTemporaryExpr to update.
+ ShouldExtend,
+ /// Do not lifetime extend along this path.
+ NoExtend
+};
+
/// Determine whether this is an indirect path to a temporary that we are
-/// supposed to lifetime-extend along (but don't).
-static bool shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
+/// supposed to lifetime-extend along.
+static PathLifetimeKind
+shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
+ PathLifetimeKind Kind = PathLifetimeKind::Extend;
for (auto Elem : Path) {
- if (Elem.Kind != IndirectLocalPathEntry::DefaultInit)
- return false;
+ if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
+ Kind = PathLifetimeKind::ShouldExtend;
+ else if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
+ return PathLifetimeKind::NoExtend;
}
- return true;
+ return Kind;
}
/// Find the range for the first interesting entry in the path at or after I.
@@ -7360,6 +7426,7 @@ static SourceRange nextPathEntryRange(const IndirectLocalPath &Path, unsigned I,
case IndirectLocalPathEntry::AddressOf:
case IndirectLocalPathEntry::LValToRVal:
case IndirectLocalPathEntry::LifetimeBoundCall:
+ case IndirectLocalPathEntry::TemporaryCopy:
case IndirectLocalPathEntry::GslReferenceInit:
case IndirectLocalPathEntry::GslPointerInit:
// These exist primarily to mark the path as not permitting or
@@ -7372,6 +7439,11 @@ static SourceRange nextPathEntryRange(const IndirectLocalPath &Path, unsigned I,
LLVM_FALLTHROUGH;
case IndirectLocalPathEntry::DefaultInit:
return Path[I].E->getSourceRange();
+
+ case IndirectLocalPathEntry::LambdaCaptureInit:
+ if (!Path[I].Capture->capturesVariable())
+ continue;
+ return Path[I].E->getSourceRange();
}
}
return E->getSourceRange();
@@ -7449,17 +7521,16 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
return false;
}
- // Lifetime-extend the temporary.
- if (Path.empty()) {
+ switch (shouldLifetimeExtendThroughPath(Path)) {
+ case PathLifetimeKind::Extend:
// Update the storage duration of the materialized temporary.
// FIXME: Rebuild the expression instead of mutating it.
MTE->setExtendingDecl(ExtendingEntity->getDecl(),
ExtendingEntity->allocateManglingNumber());
// Also visit the temporaries lifetime-extended by this initializer.
return true;
- }
- if (shouldLifetimeExtendThroughPath(Path)) {
+ case PathLifetimeKind::ShouldExtend:
// We're supposed to lifetime-extend the temporary along this path (per
// the resolution of DR1815), but we don't support that yet.
//
@@ -7468,7 +7539,9 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
// lifetime extend its temporaries.
Diag(DiagLoc, diag::warn_unsupported_lifetime_extension)
<< RK << DiagRange;
- } else {
+ break;
+
+ case PathLifetimeKind::NoExtend:
// If the path goes through the initialization of a variable or field,
// it can't possibly reach a temporary created in this full-expression.
// We will have already diagnosed any problems with the initializer.
@@ -7479,6 +7552,7 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
<< RK << !Entity.getParent()
<< ExtendingEntity->getDecl()->isImplicit()
<< ExtendingEntity->getDecl() << Init->isGLValue() << DiagRange;
+ break;
}
break;
}
@@ -7499,7 +7573,8 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
return false;
}
bool IsSubobjectMember = ExtendingEntity != &Entity;
- Diag(DiagLoc, shouldLifetimeExtendThroughPath(Path)
+ Diag(DiagLoc, shouldLifetimeExtendThroughPath(Path) !=
+ PathLifetimeKind::NoExtend
? diag::err_dangling_member
: diag::warn_dangling_member)
<< ExtendingDecl << IsSubobjectMember << RK << DiagRange;
@@ -7606,6 +7681,7 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
break;
case IndirectLocalPathEntry::LifetimeBoundCall:
+ case IndirectLocalPathEntry::TemporaryCopy:
case IndirectLocalPathEntry::GslPointerInit:
case IndirectLocalPathEntry::GslReferenceInit:
// FIXME: Consider adding a note for these.
@@ -7618,7 +7694,7 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
break;
}
- case IndirectLocalPathEntry::VarInit:
+ case IndirectLocalPathEntry::VarInit: {
const VarDecl *VD = cast<VarDecl>(Elem.D);
Diag(VD->getLocation(), diag::note_local_var_initializer)
<< VD->getType()->isReferenceType()
@@ -7626,6 +7702,19 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
<< nextPathEntryRange(Path, I + 1, L);
break;
}
+
+ case IndirectLocalPathEntry::LambdaCaptureInit:
+ if (!Elem.Capture->capturesVariable())
+ break;
+ // FIXME: We can't easily tell apart an init-capture from a nested
+ // capture of an init-capture.
+ const VarDecl *VD = Elem.Capture->getCapturedVar();
+ Diag(Elem.Capture->getLocation(), diag::note_lambda_capture_initializer)
+ << VD << VD->isInitCapture() << Elem.Capture->isExplicit()
+ << (Elem.Capture->getCaptureKind() == LCK_ByRef) << VD
+ << nextPathEntryRange(Path, I + 1, L);
+ break;
+ }
}
// We didn't lifetime-extend, so don't go any further; we don't need more
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/p11-1y.cpp b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/p11-1y.cpp
index 7fd94b658f46..4a0cf39bc56b 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/p11-1y.cpp
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/p11-1y.cpp
@@ -54,9 +54,11 @@ auto bad_init_7 = [a{{1}}] {}; // expected-error {{cannot deduce type for lambda
template<typename...T> void pack_1(T...t) { (void)[a(t...)] {}; } // expected-error {{initializer missing for lambda capture 'a'}}
template void pack_1<>(); // expected-note {{instantiation of}}
-// FIXME: Might need lifetime extension for the temporary here.
-// See DR1695.
-auto a = [a(4), b = 5, &c = static_cast<const int&&>(0)] {
+// No lifetime-extension of the temporary here.
+auto a_copy = [&c = static_cast<const int&&>(0)] {}; // expected-warning {{temporary whose address is used as value of local variable 'a_copy' will be destroyed at the end of the full-expression}} expected-note {{via initialization of lambda capture 'c'}}
+
+// But there is lifetime extension here.
+auto &&a = [a(4), b = 5, &c = static_cast<const int&&>(0)] {
static_assert(sizeof(a) == sizeof(int), "");
static_assert(sizeof(b) == sizeof(int), "");
using T = decltype(c);
diff --git a/clang/test/SemaCXX/conditional-expr.cpp b/clang/test/SemaCXX/conditional-expr.cpp
index 68e23a1f5727..9a5e2bac4341 100644
--- a/clang/test/SemaCXX/conditional-expr.cpp
+++ b/clang/test/SemaCXX/conditional-expr.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 -Wsign-conversion %s
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++17 -Wsign-conversion %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify=expected,expected-cxx11 -std=c++11 -Wsign-conversion %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify=expected,expected-cxx17 -std=c++17 -Wsign-conversion %s
// C++ rules for ?: are a lot stricter than C rules, and have to take into
// account more conversion options.
@@ -406,7 +406,8 @@ namespace lifetime_extension {
struct D { A &&a; };
void f_indirect(bool b) {
- D d = b ? D{B()} : D{C()};
+ D d = b ? D{B()} // expected-cxx11-warning {{temporary whose address is used as value of local variable 'd' will be destroyed at the end of the full-expression}}
+ : D{C()}; // expected-cxx11-warning {{temporary whose address is used as value of local variable 'd' will be destroyed at the end of the full-expression}}
}
}
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 0e97798cc197..8b80f5438d8e 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -387,6 +387,7 @@ constexpr B &&b4 = ((1, 2), 3, 4, B { {10}, {{20}} });
static_assert(&b4 != &b2, "");
// Proposed DR: copy-elision doesn't trigger lifetime extension.
+// expected-warning at +1 2{{temporary whose address is used as value of local variable 'b5' will be destroyed at the end of the full-expression}}
constexpr B b5 = B{ {0}, {0} }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}}
namespace NestedNonStatic {
@@ -396,6 +397,7 @@ namespace NestedNonStatic {
struct A { int &&r; };
struct B { A &&a; };
constexpr B a = { A{0} }; // ok
+ // expected-warning at +1 {{temporary bound to reference member of local variable 'b' will be destroyed at the end of the full-expression}}
constexpr B b = { A(A{0}) }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}}
}
diff --git a/clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp b/clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
index 9380330b41f3..24500b684d58 100644
--- a/clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
+++ b/clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
@@ -376,3 +376,13 @@ namespace weird_initlist {
// ... but we do in constant evaluation.
constexpr auto y = {weird{}, weird{}, weird{}, weird{}, weird{}}; // expected-error {{constant}} expected-note {{type 'const std::initializer_list<weird_initlist::weird>' has unexpected layout}}
}
+
+auto v = std::initializer_list<int>{1,2,3}; // expected-warning {{array backing local initializer list 'v' will be destroyed at the end of the full-expression}}
+
+std::initializer_list<int> get(int cond) {
+ if (cond == 0)
+ return {};
+ if (cond == 1)
+ return {1, 2, 3}; // expected-warning {{returning address of local temporary object}}
+ return std::initializer_list<int>{1, 2, 3}; // expected-warning {{returning address of local temporary object}}
+}
diff --git a/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp b/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
index a98366c8794a..8d7845dc607d 100644
--- a/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
+++ b/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
@@ -177,19 +177,15 @@ void doit() {
sample::X cx{5};
auto L = [=](auto a) {
const int z = 3;
- // FIXME: The warning below is correct but for some reason doesn't show
- // up in C++17 mode.
return [&,a](auto b) {
-#if __cplusplus > 201702L
- // expected-warning at -2 {{address of stack memory associated with local variable 'z' returned}}
+ // expected-warning at -1 {{address of stack memory associated with local variable 'z' returned}}
// expected-note@#call {{in instantiation of}}
-#endif
- const int y = 5;
- return [=](auto c) {
+ const int y = 5;
+ return [=](auto c) {
int d[sizeof(a) == sizeof(c) || sizeof(c) == sizeof(b) ? 2 : 1];
f(x, d);
f(y, d);
- f(z, d);
+ f(z, d); // expected-note {{implicitly captured by reference due to use here}}
decltype(a) A = a;
decltype(b) B = b;
const int &i = cx.i;
diff --git a/clang/test/SemaCXX/return-stack-addr.cpp b/clang/test/SemaCXX/return-stack-addr.cpp
index 08aaa749b4e3..2fa2de10f0d6 100644
--- a/clang/test/SemaCXX/return-stack-addr.cpp
+++ b/clang/test/SemaCXX/return-stack-addr.cpp
@@ -156,6 +156,54 @@ void ret_from_lambda() {
(void) [=]() -> int& { int a; return a; }; // expected-warning {{reference to stack}}
(void) [&]() -> int& { int &a = b; return a; };
(void) [=]() mutable -> int& { int &a = b; return a; };
+
+ (void) [] {
+ int a;
+ return [&] { // expected-warning {{address of stack memory associated with local variable 'a' returned}}
+ return a; // expected-note {{implicitly captured by reference due to use here}}
+ };
+ };
+ (void) [] {
+ int a;
+ return [&a] {}; // expected-warning {{address of stack memory associated with local variable 'a' returned}} expected-note {{captured by reference here}}
+ };
+ (void) [] {
+ int a;
+ return [=] {
+ return a;
+ };
+ };
+ (void) [] {
+ int a;
+ return [a] {};
+ };
+ (void) [] {
+ int a;
+ // expected-warning at +1 {{C++14}}
+ return [&b = a] {}; // expected-warning {{address of stack memory associated with local variable 'a' returned}} expected-note {{captured by reference via initialization of lambda capture 'b'}}
+ };
+ (void) [] {
+ int a;
+ // expected-warning at +1 {{C++14}}
+ return [b = &a] {}; // expected-warning {{address of stack memory associated with local variable 'a' returned}} expected-note {{captured via initialization of lambda capture 'b'}}
+ };
+}
+
+struct HoldsPointer { int *p; };
+
+HoldsPointer ret_via_member_1() {
+ int n;
+ return {&n}; // expected-warning {{address of stack memory associated with local variable 'n' returned}}
+}
+HoldsPointer ret_via_member_2() {
+ int n;
+ return HoldsPointer(HoldsPointer{&n}); // expected-warning {{address of stack memory associated with local variable 'n' returned}}
+}
+// FIXME: We could diagnose this too.
+HoldsPointer ret_via_member_3() {
+ int n;
+ const HoldsPointer hp = HoldsPointer{&n};
+ return hp;
}
namespace mem_ptr {
@@ -163,3 +211,11 @@ namespace mem_ptr {
int X::*f();
int &r(X *p) { return p->*f(); }
}
+
+namespace PR47861 {
+ struct A {
+ A(int i);
+ A &operator+=(int i);
+ };
+ A const &b = A(5) += 5; // expected-warning {{temporary bound to local reference 'b' will be destroyed at the end of the full-expression}}
+}
More information about the cfe-commits
mailing list