Skip to content

Commit 39d21fc

Browse files
yronglinyronglin
committed
Address review comments and fix bugs in dependent context
Signed-off-by: yronglin <[email protected]>
1 parent 89eeeae commit 39d21fc

File tree

8 files changed

+391
-253
lines changed

8 files changed

+391
-253
lines changed

‎clang/include/clang/Sema/Sema.h

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1343,7 +1343,7 @@ class Sema final {
13431343
llvm::SmallPtrSet<DeclRefExpr *, 4> ReferenceToConsteval;
13441344

13451345
/// P2718R0 - Lifetime extension in range-based for loops.
1346-
/// MaterializeTemporaryExprs in for-range-init expression which need to
1346+
/// MaterializeTemporaryExprs in for-range-init expressions which need to
13471347
/// extend lifetime. Add MaterializeTemporaryExpr* if the value of
13481348
/// IsInLifetimeExtendingContext is true.
13491349
SmallVector<MaterializeTemporaryExpr *, 8> ForRangeLifetimeExtendTemps;
@@ -1367,8 +1367,9 @@ class Sema final {
13671367
// VLAs).
13681368
bool InConditionallyConstantEvaluateContext = false;
13691369

1370-
/// Whether we are currently in a context in which temporaries must be
1371-
/// lifetime-extended (Eg. in a for-range initializer).
1370+
/// Whether we are currently in a context in which all temporaries must be
1371+
/// lifetime-extended, even if they're not bound to a reference (for example,
1372+
/// in a for-range initializer).
13721373
bool IsInLifetimeExtendingContext = false;
13731374

13741375
/// Whether we should materialize temporaries in discarded expressions.
@@ -5277,7 +5278,8 @@ class Sema final {
52775278
Expr *Cond, Expr *Inc,
52785279
Stmt *LoopVarDecl,
52795280
SourceLocation RParenLoc,
5280-
BuildForRangeKind Kind);
5281+
BuildForRangeKind Kind,
5282+
ArrayRef<MaterializeTemporaryExpr *> LifetimeExtendTemps = {});
52815283
StmtResult FinishCXXForRangeStmt(Stmt *ForRange, Stmt *Body);
52825284

52835285
StmtResult ActOnGotoStmt(SourceLocation GotoLoc,
@@ -10008,7 +10010,7 @@ class Sema final {
1000810010
return ExprEvalContexts.back().IsInLifetimeExtendingContext;
1000910011
}
1001010012

10011-
bool ShouldMaterializePRValueInDiscardedExpression() const {
10013+
bool shouldMaterializePRValueInDiscardedExpression() const {
1001210014
assert(!ExprEvalContexts.empty() &&
1001310015
"Must be in an expression evaluation context");
1001410016
return ExprEvalContexts.back().MaterializePRValueInDiscardedExpression;
@@ -10053,6 +10055,32 @@ class Sema final {
1005310055
return Res;
1005410056
}
1005510057

10058+
/// keepInLifetimeExtendingContext - Pull down IsInLifetimeExtendingContext
10059+
/// flag from previous context.
10060+
void keepInLifetimeExtendingContext() {
10061+
if (ExprEvalContexts.size() > 2 &&
10062+
ExprEvalContexts[ExprEvalContexts.size() - 2]
10063+
.IsInLifetimeExtendingContext) {
10064+
auto &LastRecord = ExprEvalContexts.back();
10065+
auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
10066+
LastRecord.IsInLifetimeExtendingContext =
10067+
PrevRecord.IsInLifetimeExtendingContext;
10068+
}
10069+
}
10070+
10071+
/// keepInLifetimeExtendingContext - Pull down
10072+
/// MaterializePRValueInDiscardedExpression flag from previous context.
10073+
void keepMaterializePRValueInDiscardedExpression() {
10074+
if (ExprEvalContexts.size() > 2 &&
10075+
ExprEvalContexts[ExprEvalContexts.size() - 2]
10076+
.MaterializePRValueInDiscardedExpression) {
10077+
auto &LastRecord = ExprEvalContexts.back();
10078+
auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
10079+
LastRecord.MaterializePRValueInDiscardedExpression =
10080+
PrevRecord.MaterializePRValueInDiscardedExpression;
10081+
}
10082+
}
10083+
1005610084
/// RAII class used to determine whether SFINAE has
1005710085
/// trapped any errors that occur during template argument
1005810086
/// deduction.

‎clang/lib/Parse/ParseDecl.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2334,9 +2334,16 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
23342334
FRI->RangeExpr = ParseBraceInitializer();
23352335
else
23362336
FRI->RangeExpr = ParseExpression();
2337-
if (getLangOpts().CPlusPlus23)
2338-
FRI->LifetimeExtendTemps = std::move(
2339-
Actions.ExprEvalContexts.back().ForRangeLifetimeExtendTemps);
2337+
2338+
// Before c++23, ForRangeLifetimeExtendTemps should be empty.
2339+
assert(
2340+
getLangOpts().CPlusPlus23 ||
2341+
Actions.ExprEvalContexts.back().ForRangeLifetimeExtendTemps.empty());
2342+
2343+
// Move the collected materialized temporaries into ForRangeInit before
2344+
// ForRangeInitContext exit.
2345+
FRI->LifetimeExtendTemps = std::move(
2346+
Actions.ExprEvalContexts.back().ForRangeLifetimeExtendTemps);
23402347
}
23412348

23422349
Decl *ThisDecl = Actions.ActOnDeclarator(getCurScope(), D);

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6298,10 +6298,8 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
62986298
CallLoc, Param, CurContext};
62996299
// Pass down lifetime extending flag, and collect temporaries in
63006300
// CreateMaterializeTemporaryExpr when we rewrite the call argument.
6301-
auto &LastRecord = ExprEvalContexts.back();
6302-
auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
6303-
LastRecord.IsInLifetimeExtendingContext = IsInLifetimeExtendingContext;
6304-
6301+
keepInLifetimeExtendingContext();
6302+
keepMaterializePRValueInDiscardedExpression();
63056303
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
63066304
ExprResult Res;
63076305
runWithSufficientStackSpace(CallLoc, [&] {
@@ -6315,10 +6313,6 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
63156313
if (Res.isInvalid())
63166314
return ExprError();
63176315
Init = Res.get();
6318-
6319-
// Collect MaterializeTemporaryExprs in the rewritten CXXDefaultArgExpr.
6320-
PrevRecord.ForRangeLifetimeExtendTemps.append(
6321-
LastRecord.ForRangeLifetimeExtendTemps);
63226316
}
63236317
}
63246318

@@ -6351,7 +6345,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
63516345
Expr *Init = nullptr;
63526346

63536347
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
6354-
6348+
bool IsInLifetimeExtendingContext = isInLifetimeExtendingContext();
63556349
EnterExpressionEvaluationContext EvalContext(
63566350
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
63576351

@@ -6386,12 +6380,16 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
63866380
ImmediateCallVisitor V(getASTContext());
63876381
if (!NestedDefaultChecking)
63886382
V.TraverseDecl(Field);
6389-
if (V.HasImmediateCalls) {
6383+
if (V.HasImmediateCalls || IsInLifetimeExtendingContext) {
63906384
ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
63916385
CurContext};
63926386
ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
63936387
NestedDefaultChecking;
63946388

6389+
// Pass down lifetime extending flag, and collect temporaries in
6390+
// CreateMaterializeTemporaryExpr when we rewrite the call argument.
6391+
keepInLifetimeExtendingContext();
6392+
keepMaterializePRValueInDiscardedExpression();
63956393
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
63966394
ExprResult Res;
63976395
runWithSufficientStackSpace(Loc, [&] {
@@ -18664,6 +18662,16 @@ void Sema::PopExpressionEvaluationContext() {
1866418662
}
1866518663
}
1866618664

18665+
// Append the collected materialized temporaries into previous context before
18666+
// exit if the previous also is a lifetime extending context.
18667+
auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
18668+
if (getLangOpts().CPlusPlus23 && isInLifetimeExtendingContext() &&
18669+
PrevRecord.IsInLifetimeExtendingContext && !ExprEvalContexts.empty()) {
18670+
auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
18671+
PrevRecord.ForRangeLifetimeExtendTemps.append(
18672+
Rec.ForRangeLifetimeExtendTemps);
18673+
}
18674+
1866718675
WarnOnPendingNoDerefs(Rec);
1866818676
HandleImmediateInvocations(*this, Rec);
1866918677

‎clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8237,7 +8237,7 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
82378237
// unnecessary temporary objects. If we skip this step, IR generation is
82388238
// able to synthesize the storage for itself in the aggregate case, and
82398239
// adding the extra node to the AST is just clutter.
8240-
if (ShouldMaterializePRValueInDiscardedExpression() &&
8240+
if (shouldMaterializePRValueInDiscardedExpression() &&
82418241
getLangOpts().CPlusPlus17 && E->isPRValue() &&
82428242
!E->getType()->isVoidType()) {
82438243
ExprResult Res = TemporaryMaterializationConversion(E);

‎clang/lib/Sema/SemaStmt.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "clang/AST/TypeOrdering.h"
2828
#include "clang/Basic/TargetInfo.h"
2929
#include "clang/Lex/Preprocessor.h"
30+
#include "clang/Sema/EnterExpressionEvaluationContext.h"
3031
#include "clang/Sema/Initialization.h"
3132
#include "clang/Sema/Lookup.h"
3233
#include "clang/Sema/Ownership.h"
@@ -2545,13 +2546,6 @@ StmtResult Sema::ActOnCXXForRangeStmt(
25452546
return StmtError();
25462547
}
25472548

2548-
// P2718R0 - Lifetime extension in range-based for loops.
2549-
if (getLangOpts().CPlusPlus23 && !LifetimeExtendTemps.empty()) {
2550-
InitializedEntity Entity = InitializedEntity::InitializeVariable(RangeVar);
2551-
for (auto *MTE : LifetimeExtendTemps)
2552-
MTE->setExtendingDecl(RangeVar, Entity.allocateManglingNumber());
2553-
}
2554-
25552549
// Claim the type doesn't contain auto: we've already done the checking.
25562550
DeclGroupPtrTy RangeGroup =
25572551
BuildDeclaratorGroup(MutableArrayRef<Decl *>((Decl **)&RangeVar, 1));
@@ -2564,7 +2558,7 @@ StmtResult Sema::ActOnCXXForRangeStmt(
25642558
StmtResult R = BuildCXXForRangeStmt(
25652559
ForLoc, CoawaitLoc, InitStmt, ColonLoc, RangeDecl.get(),
25662560
/*BeginStmt=*/nullptr, /*EndStmt=*/nullptr,
2567-
/*Cond=*/nullptr, /*Inc=*/nullptr, DS, RParenLoc, Kind);
2561+
/*Cond=*/nullptr, /*Inc=*/nullptr, DS, RParenLoc, Kind, LifetimeExtendTemps);
25682562
if (R.isInvalid()) {
25692563
ActOnInitializerError(LoopVar);
25702564
return StmtError();
@@ -2760,7 +2754,7 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
27602754
Stmt *Begin, Stmt *End, Expr *Cond,
27612755
Expr *Inc, Stmt *LoopVarDecl,
27622756
SourceLocation RParenLoc,
2763-
BuildForRangeKind Kind) {
2757+
BuildForRangeKind Kind, ArrayRef<MaterializeTemporaryExpr *> LifetimeExtendTemps) {
27642758
// FIXME: This should not be used during template instantiation. We should
27652759
// pick up the set of unqualified lookup results for the != and + operators
27662760
// in the initial parse.
@@ -2820,6 +2814,14 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
28202814
diag::err_for_range_incomplete_type))
28212815
return StmtError();
28222816

2817+
// P2718R0 - Lifetime extension in range-based for loops.
2818+
if (getLangOpts().CPlusPlus23 && !LifetimeExtendTemps.empty()) {
2819+
InitializedEntity Entity =
2820+
InitializedEntity::InitializeVariable(RangeVar);
2821+
for (auto *MTE : LifetimeExtendTemps)
2822+
MTE->setExtendingDecl(RangeVar, Entity.allocateManglingNumber());
2823+
}
2824+
28232825
// Build auto __begin = begin-expr, __end = end-expr.
28242826
// Divide by 2, since the variables are in the inner scope (loop body).
28252827
const auto DepthStr = std::to_string(S->getDepth() / 2);

‎clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5440,6 +5440,8 @@ void Sema::InstantiateVariableInitializer(
54405440
EnterExpressionEvaluationContext Evaluated(
54415441
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
54425442

5443+
keepInLifetimeExtendingContext();
5444+
keepMaterializePRValueInDiscardedExpression();
54435445
// Instantiate the initializer.
54445446
ExprResult Init;
54455447

‎clang/lib/Sema/TreeTransform.h

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2549,7 +2549,7 @@ class TreeTransform {
25492549
SourceLocation ColonLoc, Stmt *Range,
25502550
Stmt *Begin, Stmt *End, Expr *Cond,
25512551
Expr *Inc, Stmt *LoopVar,
2552-
SourceLocation RParenLoc) {
2552+
SourceLocation RParenLoc, ArrayRef<MaterializeTemporaryExpr *> LifetimeExtendTemps) {
25532553
// If we've just learned that the range is actually an Objective-C
25542554
// collection, treat this as an Objective-C fast enumeration loop.
25552555
if (DeclStmt *RangeStmt = dyn_cast<DeclStmt>(Range)) {
@@ -2577,7 +2577,7 @@ class TreeTransform {
25772577

25782578
return getSema().BuildCXXForRangeStmt(ForLoc, CoawaitLoc, Init, ColonLoc,
25792579
Range, Begin, End, Cond, Inc, LoopVar,
2580-
RParenLoc, Sema::BFRK_Rebuild);
2580+
RParenLoc, Sema::BFRK_Rebuild, LifetimeExtendTemps);
25812581
}
25822582

25832583
/// Build a new C++0x range-based for statement.
@@ -4134,6 +4134,8 @@ ExprResult TreeTransform<Derived>::TransformInitializer(Expr *Init,
41344134
getSema(), EnterExpressionEvaluationContext::InitList,
41354135
Construct->isListInitialization());
41364136

4137+
getSema().keepInLifetimeExtendingContext();
4138+
getSema().keepInLifetimeExtendingContext();
41374139
SmallVector<Expr*, 8> NewArgs;
41384140
bool ArgChanged = false;
41394141
if (getDerived().TransformExprs(Construct->getArgs(), Construct->getNumArgs(),
@@ -8537,6 +8539,21 @@ StmtResult TreeTransform<Derived>::TransformCXXTryStmt(CXXTryStmt *S) {
85378539
template<typename Derived>
85388540
StmtResult
85398541
TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
8542+
EnterExpressionEvaluationContext ForRangeInitContext(
8543+
getSema(), Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
8544+
/*LambdaContextDecl=*/nullptr,
8545+
Sema::ExpressionEvaluationContextRecord::EK_Other,
8546+
getSema().getLangOpts().CPlusPlus23);
8547+
8548+
// P2718R0 - Lifetime extension in range-based for loops.
8549+
if (getSema().getLangOpts().CPlusPlus23) {
8550+
auto &LastRecord = getSema().ExprEvalContexts.back();
8551+
LastRecord.IsInLifetimeExtendingContext = true;
8552+
8553+
// Materialize non-`cv void` prvalue temporaries in discarded
8554+
// expressions. These materialized temporaries may be lifetime-extented.
8555+
LastRecord.MaterializePRValueInDiscardedExpression = true;
8556+
}
85408557
StmtResult Init =
85418558
S->getInit() ? getDerived().TransformStmt(S->getInit()) : StmtResult();
85428559
if (Init.isInvalid())
@@ -8546,6 +8563,11 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
85468563
if (Range.isInvalid())
85478564
return StmtError();
85488565

8566+
// Before c++23, ForRangeLifetimeExtendTemps should be empty.
8567+
assert(getSema().getLangOpts().CPlusPlus23 ||
8568+
getSema().ExprEvalContexts.back().ForRangeLifetimeExtendTemps.empty());
8569+
auto ForRangeLifetimeExtendTemps = getSema().ExprEvalContexts.back().ForRangeLifetimeExtendTemps;
8570+
85498571
StmtResult Begin = getDerived().TransformStmt(S->getBeginStmt());
85508572
if (Begin.isInvalid())
85518573
return StmtError();
@@ -8582,13 +8604,10 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
85828604
Cond.get() != S->getCond() ||
85838605
Inc.get() != S->getInc() ||
85848606
LoopVar.get() != S->getLoopVarStmt()) {
8585-
NewStmt = getDerived().RebuildCXXForRangeStmt(S->getForLoc(),
8586-
S->getCoawaitLoc(), Init.get(),
8587-
S->getColonLoc(), Range.get(),
8588-
Begin.get(), End.get(),
8589-
Cond.get(),
8590-
Inc.get(), LoopVar.get(),
8591-
S->getRParenLoc());
8607+
NewStmt = getDerived().RebuildCXXForRangeStmt(
8608+
S->getForLoc(), S->getCoawaitLoc(), Init.get(), S->getColonLoc(),
8609+
Range.get(), Begin.get(), End.get(), Cond.get(), Inc.get(),
8610+
LoopVar.get(), S->getRParenLoc(), ForRangeLifetimeExtendTemps);
85928611
if (NewStmt.isInvalid() && LoopVar.get() != S->getLoopVarStmt()) {
85938612
// Might not have attached any initializer to the loop variable.
85948613
getSema().ActOnInitializerError(
@@ -8604,13 +8623,10 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
86048623
// Body has changed but we didn't rebuild the for-range statement. Rebuild
86058624
// it now so we have a new statement to attach the body to.
86068625
if (Body.get() != S->getBody() && NewStmt.get() == S) {
8607-
NewStmt = getDerived().RebuildCXXForRangeStmt(S->getForLoc(),
8608-
S->getCoawaitLoc(), Init.get(),
8609-
S->getColonLoc(), Range.get(),
8610-
Begin.get(), End.get(),
8611-
Cond.get(),
8612-
Inc.get(), LoopVar.get(),
8613-
S->getRParenLoc());
8626+
NewStmt = getDerived().RebuildCXXForRangeStmt(
8627+
S->getForLoc(), S->getCoawaitLoc(), Init.get(), S->getColonLoc(),
8628+
Range.get(), Begin.get(), End.get(), Cond.get(), Inc.get(),
8629+
LoopVar.get(), S->getRParenLoc(), ForRangeLifetimeExtendTemps);
86148630
if (NewStmt.isInvalid())
86158631
return StmtError();
86168632
}

0 commit comments

Comments
 (0)