Skip to content

Commit 10e20a2

Browse files
committed
Reapply "[Clang][CWG1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer"
Signed-off-by: yronglin <[email protected]>
1 parent d31603e commit 10e20a2

15 files changed

+189
-61
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10138,13 +10138,6 @@ def warn_dangling_pointer_assignment : Warning<
1013810138
"will be destroyed at the end of the full-expression">,
1013910139
InGroup<DanglingAssignment>;
1014010140

10141-
def warn_unsupported_lifetime_extension : Warning<
10142-
"lifetime extension of "
10143-
"%select{temporary|backing array of initializer list}0 created "
10144-
"by aggregate initialization using a default member initializer "
10145-
"is not yet supported; lifetime of %select{temporary|backing array}0 "
10146-
"will end at the end of the full-expression">, InGroup<Dangling>;
10147-
1014810141
// For non-floating point, expressions of the form x == x or x != x
1014910142
// should result in a warning, since these always evaluate to a constant.
1015010143
// Array comparisons have similar warnings

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -905,11 +905,6 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
905905
enum PathLifetimeKind {
906906
/// Lifetime-extend along this path.
907907
Extend,
908-
/// We should lifetime-extend, but we don't because (due to technical
909-
/// limitations) we can't. This happens for default member initializers,
910-
/// which we don't clone for every use, so we don't have a unique
911-
/// MaterializeTemporaryExpr to update.
912-
ShouldExtend,
913908
/// Do not lifetime extend along this path.
914909
NoExtend
915910
};
@@ -921,7 +916,7 @@ shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
921916
PathLifetimeKind Kind = PathLifetimeKind::Extend;
922917
for (auto Elem : Path) {
923918
if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
924-
Kind = PathLifetimeKind::ShouldExtend;
919+
return PathLifetimeKind::Extend;
925920
else if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
926921
return PathLifetimeKind::NoExtend;
927922
}
@@ -1053,17 +1048,6 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
10531048
// Also visit the temporaries lifetime-extended by this initializer.
10541049
return true;
10551050

1056-
case PathLifetimeKind::ShouldExtend:
1057-
// We're supposed to lifetime-extend the temporary along this path (per
1058-
// the resolution of DR1815), but we don't support that yet.
1059-
//
1060-
// FIXME: Properly handle this situation. Perhaps the easiest approach
1061-
// would be to clone the initializer expression on each use that would
1062-
// lifetime extend its temporaries.
1063-
SemaRef.Diag(DiagLoc, diag::warn_unsupported_lifetime_extension)
1064-
<< RK << DiagRange;
1065-
break;
1066-
10671051
case PathLifetimeKind::NoExtend:
10681052
// If the path goes through the initialization of a variable or field,
10691053
// it can't possibly reach a temporary created in this full-expression.

clang/lib/Sema/SemaExpr.cpp

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5397,6 +5397,8 @@ struct EnsureImmediateInvocationInDefaultArgs
53975397
EnsureImmediateInvocationInDefaultArgs(Sema &SemaRef)
53985398
: TreeTransform(SemaRef) {}
53995399

5400+
bool AlwaysRebuild() { return true; }
5401+
54005402
// Lambda can only have immediate invocations in the default
54015403
// args of their parameters, which is transformed upon calling the closure.
54025404
// The body is not a subexpression, so we have nothing to do.
@@ -5474,10 +5476,9 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
54745476
Res = Immediate.TransformInitializer(Param->getInit(),
54755477
/*NotCopy=*/false);
54765478
});
5477-
if (Res.isInvalid())
5478-
return ExprError();
5479-
Res = ConvertParamDefaultArgument(Param, Res.get(),
5480-
Res.get()->getBeginLoc());
5479+
if (Res.isUsable())
5480+
Res = ConvertParamDefaultArgument(Param, Res.get(),
5481+
Res.get()->getBeginLoc());
54815482
if (Res.isInvalid())
54825483
return ExprError();
54835484
Init = Res.get();
@@ -5513,7 +5514,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
55135514
Expr *Init = nullptr;
55145515

55155516
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
5516-
5517+
bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
55175518
EnterExpressionEvaluationContext EvalContext(
55185519
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
55195520

@@ -5548,19 +5549,33 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
55485549
ImmediateCallVisitor V(getASTContext());
55495550
if (!NestedDefaultChecking)
55505551
V.TraverseDecl(Field);
5551-
if (V.HasImmediateCalls) {
5552+
5553+
// CWG1815
5554+
// Support lifetime extension of temporary created by aggregate
5555+
// initialization using a default member initializer. We should always rebuild
5556+
// the initializer if it contains any temporaries (if the initializer
5557+
// expression is an ExprWithCleanups). Then make sure the normal lifetime
5558+
// extension code recurses into the default initializer and does lifetime
5559+
// extension when warranted.
5560+
bool ContainsAnyTemporaries =
5561+
isa_and_present<ExprWithCleanups>(Field->getInClassInitializer());
5562+
if (V.HasImmediateCalls || InLifetimeExtendingContext ||
5563+
ContainsAnyTemporaries) {
55525564
ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
55535565
CurContext};
55545566
ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
55555567
NestedDefaultChecking;
5556-
5568+
// Pass down lifetime extending flag, and collect temporaries in
5569+
// CreateMaterializeTemporaryExpr when we rewrite the call argument.
5570+
keepInLifetimeExtendingContext();
55575571
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
55585572
ExprResult Res;
5573+
SFINAETrap Trap(*this);
55595574
runWithSufficientStackSpace(Loc, [&] {
55605575
Res = Immediate.TransformInitializer(Field->getInClassInitializer(),
55615576
/*CXXDirectInit=*/false);
55625577
});
5563-
if (!Res.isInvalid())
5578+
if (Res.isUsable())
55645579
Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);
55655580
if (Res.isInvalid()) {
55665581
Field->setInvalidDecl();

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,9 +1539,6 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
15391539
bool ListInitialization) {
15401540
QualType Ty = TInfo->getType();
15411541
SourceLocation TyBeginLoc = TInfo->getTypeLoc().getBeginLoc();
1542-
1543-
assert((!ListInitialization || Exprs.size() == 1) &&
1544-
"List initialization must have exactly one expression.");
15451542
SourceRange FullRange = SourceRange(TyBeginLoc, RParenOrBraceLoc);
15461543

15471544
InitializedEntity Entity =

clang/lib/Sema/TreeTransform.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14257,6 +14257,13 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
1425714257
if (TransformExprs(E->getArgs(), E->getNumArgs(), true, Args,
1425814258
&ArgumentChanged))
1425914259
return ExprError();
14260+
14261+
if (E->isListInitialization() && !E->isStdInitListInitialization()) {
14262+
ExprResult Res = RebuildInitList(E->getBeginLoc(), Args, E->getEndLoc());
14263+
if (Res.isInvalid())
14264+
return ExprError();
14265+
Args = {Res.get()};
14266+
}
1426014267
}
1426114268

1426214269
if (!getDerived().AlwaysRebuild() &&
@@ -14268,12 +14275,9 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
1426814275
return SemaRef.MaybeBindToTemporary(E);
1426914276
}
1427014277

14271-
// FIXME: We should just pass E->isListInitialization(), but we're not
14272-
// prepared to handle list-initialization without a child InitListExpr.
1427314278
SourceLocation LParenLoc = T->getTypeLoc().getEndLoc();
1427414279
return getDerived().RebuildCXXTemporaryObjectExpr(
14275-
T, LParenLoc, Args, E->getEndLoc(),
14276-
/*ListInitialization=*/LParenLoc.isInvalid());
14280+
T, LParenLoc, Args, E->getEndLoc(), E->isListInitialization());
1427714281
}
1427814282

1427914283
template<typename Derived>

clang/test/AST/ast-dump-default-init-json.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -789,10 +789,10 @@ void test() {
789789
// CHECK-NEXT: "valueCategory": "lvalue",
790790
// CHECK-NEXT: "extendingDecl": {
791791
// CHECK-NEXT: "id": "0x{{.*}}",
792-
// CHECK-NEXT: "kind": "FieldDecl",
793-
// CHECK-NEXT: "name": "a",
792+
// CHECK-NEXT: "kind": "VarDecl",
793+
// CHECK-NEXT: "name": "b",
794794
// CHECK-NEXT: "type": {
795-
// CHECK-NEXT: "qualType": "const A &"
795+
// CHECK-NEXT: "qualType": "B"
796796
// CHECK-NEXT: }
797797
// CHECK-NEXT: },
798798
// CHECK-NEXT: "storageDuration": "automatic",

clang/test/AST/ast-dump-default-init.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ void test() {
1313
}
1414
// CHECK: -CXXDefaultInitExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue has rewritten init
1515
// CHECK-NEXT: `-ExprWithCleanups 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue
16-
// CHECK-NEXT: `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Field 0x{{[^ ]*}} 'a' 'const A &'
16+
// CHECK-NEXT: `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Var 0x{{[^ ]*}} 'b' 'B'
1717
// CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <{{.*}}> 'const A' <NoOp>
1818
// CHECK-NEXT: `-CXXFunctionalCastExpr 0x{{[^ ]*}} <{{.*}}> 'A' functional cast to A <NoOp>
1919
// CHECK-NEXT: `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'A'

clang/test/Analysis/lifetime-extended-regions.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,11 @@ void aggregateWithReferences() {
120120
clang_analyzer_dump(viaReference); // expected-warning-re {{&lifetime_extended_object{RefAggregate, viaReference, S{{[0-9]+}}} }}
121121
clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
122122
clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
123-
124-
// clang does not currently implement extending lifetime of object bound to reference members of aggregates,
125-
// that are created from default member initializer (see `warn_unsupported_lifetime_extension` from `-Wdangling`)
126-
RefAggregate defaultInitExtended{i}; // clang-bug does not extend `Composite`
123+
124+
// FIXME: clang currently support extending lifetime of object bound to reference members of aggregates,
125+
// that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change.
126+
// The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
127+
RefAggregate defaultInitExtended{i};
127128
clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
128129
}
129130

clang/test/CXX/drs/cwg16xx.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,6 @@ namespace cwg1696 { // cwg1696: 7
483483
const A &a = A(); // #cwg1696-D1-a
484484
};
485485
D1 d1 = {}; // #cwg1696-d1
486-
// since-cxx14-warning@-1 {{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported; lifetime of temporary will end at the end of the full-expression}}
487-
// since-cxx14-note@#cwg1696-D1-a {{initializing field 'a' with default member initializer}}
488486

489487
struct D2 {
490488
const A &a = A(); // #cwg1696-D2-a

clang/test/CXX/drs/cwg18xx.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,19 +206,28 @@ namespace cwg1814 { // cwg1814: yes
206206
#endif
207207
}
208208

209-
namespace cwg1815 { // cwg1815: no
209+
namespace cwg1815 { // cwg1815: 19
210210
#if __cplusplus >= 201402L
211-
// FIXME: needs codegen test
212-
struct A { int &&r = 0; }; // #cwg1815-A
211+
struct A { int &&r = 0; };
213212
A a = {};
214-
// since-cxx14-warning@-1 {{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported; lifetime of temporary will end at the end of the full-expression}} FIXME
215-
// since-cxx14-note@#cwg1815-A {{initializing field 'r' with default member initializer}}
216213

217214
struct B { int &&r = 0; }; // #cwg1815-B
218215
// since-cxx14-error@-1 {{reference member 'r' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object}}
219216
// since-cxx14-note@#cwg1815-B {{initializing field 'r' with default member initializer}}
220217
// since-cxx14-note@#cwg1815-b {{in implicit default constructor for 'cwg1815::B' first required here}}
221218
B b; // #cwg1815-b
219+
220+
#if __cplusplus >= 201703L
221+
struct C { const int &r = 0; };
222+
constexpr C c = {}; // OK, since cwg1815
223+
static_assert(c.r == 0);
224+
225+
constexpr int f() {
226+
A a = {}; // OK, since cwg1815
227+
return a.r;
228+
}
229+
static_assert(f() == 0);
230+
#endif
222231
#endif
223232
}
224233

0 commit comments

Comments
 (0)