Skip to content

Commit 51e1552

Browse files
SirraideSirraide
committed
CWG 3131
1 parent 058e272 commit 51e1552

File tree

2 files changed

+78
-47
lines changed

2 files changed

+78
-47
lines changed

‎clang/lib/Sema/SemaExpand.cpp‎

Lines changed: 64 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -128,17 +128,34 @@ TryBuildIterableExpansionStmtInitializer(Sema &S, Expr *ExpansionInitializer,
128128
Data.TheState = IterableExpansionStmtData::State::Error;
129129
Scope *Scope = S.getCurScope();
130130

131-
// TODO: CWG 3131 changes how this range is declared.
132-
StmtResult Var = S.BuildCXXForRangeRangeVar(Scope, ExpansionInitializer,
133-
/*ForExpansionStmt=*/true);
131+
// CWG 3131: The declaration of 'range' is of the form
132+
//
133+
// constexpr[opt] decltype(auto) range = (expansion-initializer);
134+
//
135+
// where 'constexpr' is present iff the for-range-declaration is 'constexpr'.
136+
StmtResult Var = S.BuildCXXForRangeRangeVar(
137+
Scope, S.ActOnParenExpr(ColonLoc, ColonLoc, ExpansionInitializer).get(),
138+
S.Context.getAutoType(QualType(), AutoTypeKeyword::DecltypeAuto,
139+
/*IsDependent*/ false),
140+
VarIsConstexpr);
134141
if (Var.isInvalid())
135142
return Data;
136143

144+
// CWG 3131: Discussion around this core issue (though as of the time of
145+
// writing not the resolution itself) suggests that the other variables we
146+
// create here should likewise be 'constexpr' iff the range variable is
147+
// declared 'constexpr'.
148+
//
149+
// FIXME: As of CWG 3131, 'end' is no longer used outside the lambda that
150+
// performs the size calculation (despite that, CWG 3131 currently still
151+
// lists it in the generated code, but this is likely an oversight). Ideally,
152+
// we should only create 'begin' here instead, but that requires another
153+
// substantial refactor of the for-range code.
137154
auto *RangeVar = cast<DeclStmt>(Var.get());
138155
Sema::ForRangeBeginEndInfo Info = S.BuildCXXForRangeBeginEndVars(
139156
Scope, cast<VarDecl>(RangeVar->getSingleDecl()), ColonLoc,
140157
/*CoawaitLoc=*/{},
141-
/*LifetimeExtendTemps=*/{}, Sema::BFRK_Build, /*ForExpansionStmt=*/true);
158+
/*LifetimeExtendTemps=*/{}, Sema::BFRK_Build, VarIsConstexpr);
142159

143160
if (!Info.isValid())
144161
return Data;
@@ -240,6 +257,10 @@ StmtResult Sema::ActOnCXXExpansionStmtPattern(
240257
// Note that lifetime extension only applies to destructuring expansion
241258
// statements, so we just ignore 'LifetimeExtendedTemps' entirely for other
242259
// types of expansion statements (this is CWG 3043).
260+
//
261+
// TODO: CWG 3131 makes it so the 'range' variable of an iterating
262+
// expansion statement need no longer be 'constexpr'... so do we want
263+
// lifetime extension for iterating expansion statements after all?
243264
return BuildCXXEnumeratingExpansionStmtPattern(ESD, Init, DS, LParenLoc,
244265
ColonLoc, RParenLoc);
245266
}
@@ -436,14 +457,15 @@ Sema::ComputeExpansionSize(CXXExpansionStmtPattern *Expansion) {
436457
->getExprs()
437458
.size();
438459

439-
// By [stmt.expand]5.2, N is the result of evaluating the expression
460+
// CWG 3131: N is the result of evaluating the expression
440461
//
441-
// [] consteval {
462+
// [&] consteval {
442463
// std::ptrdiff_t result = 0;
443-
// for (auto i = begin; i != end; ++i) ++result;
464+
// auto b = begin-expr;
465+
// auto e = end-expr;
466+
// for (; b != e; ++b) ++result;
444467
// return result;
445468
// }()
446-
// TODO: CWG 3131 changes this lambda a bit.
447469
if (auto *Iterating = dyn_cast<CXXIteratingExpansionStmtPattern>(Expansion)) {
448470
SourceLocation Loc = Expansion->getColonLoc();
449471
EnterExpressionEvaluationContext ExprEvalCtx(
@@ -506,32 +528,32 @@ Sema::ComputeExpansionSize(CXXExpansionStmtPattern *Expansion) {
506528
// Start the for loop.
507529
ParseScope ForScope(*this, Scope::DeclScope | Scope::ControlScope);
508530

509-
// auto i = begin;
510-
VarDecl *IterationVar = VarDecl::Create(
511-
Context, CurContext, Loc, Loc, &PP.getIdentifierTable().get("__i"),
512-
Context.getAutoDeductType(),
513-
Context.getTrivialTypeSourceInfo(Context.getAutoDeductType(), Loc),
514-
SC_None);
515-
DeclRefExpr *Begin = BuildDeclRefExpr(
516-
Iterating->getBeginVar(),
517-
Iterating->getBeginVar()->getType().getNonReferenceType(), VK_LValue,
518-
Loc);
519-
AddInitializerToDecl(IterationVar, Begin, false);
520-
StmtResult IterationVarStmt =
521-
ActOnDeclStmt(ConvertDeclToDeclGroup(IterationVar), Loc, Loc);
522-
if (IterationVarStmt.isInvalid() || IterationVar->isInvalidDecl())
531+
// auto b = begin-expr;
532+
// auto e = end-expr;
533+
ForRangeBeginEndInfo Info = BuildCXXForRangeBeginEndVars(
534+
getCurScope(), Iterating->getRangeVar(), Loc,
535+
/*CoawaitLoc=*/{},
536+
/*LifetimeExtendTemps=*/{}, BFRK_Build, /*Constexpr=*/false);
537+
if (!Info.isValid())
523538
return std::nullopt;
524539

525-
// i != end
526-
DeclRefExpr *IterationVarDeclRef = BuildDeclRefExpr(
527-
IterationVar, IterationVar->getType().getNonReferenceType(), VK_LValue,
528-
Loc);
529-
DeclRefExpr *End = BuildDeclRefExpr(
530-
Iterating->getEndVar(),
531-
Iterating->getEndVar()->getType().getNonReferenceType(), VK_LValue,
532-
Loc);
533-
ExprResult NotEqual = ActOnBinOp(getCurScope(), Loc, tok::exclaimequal,
534-
IterationVarDeclRef, End);
540+
StmtResult BeginStmt =
541+
ActOnDeclStmt(ConvertDeclToDeclGroup(Info.BeginVar), Loc, Loc);
542+
StmtResult EndStmt =
543+
ActOnDeclStmt(ConvertDeclToDeclGroup(Info.EndVar), Loc, Loc);
544+
if (BeginStmt.isInvalid() || EndStmt.isInvalid())
545+
return std::nullopt;
546+
547+
// b != e
548+
auto GetDeclRef = [&](VarDecl *VD) -> DeclRefExpr * {
549+
return BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(),
550+
VK_LValue, Loc);
551+
};
552+
553+
DeclRefExpr *Begin = GetDeclRef(Info.BeginVar);
554+
DeclRefExpr *End = GetDeclRef(Info.EndVar);
555+
ExprResult NotEqual =
556+
ActOnBinOp(getCurScope(), Loc, tok::exclaimequal, Begin, End);
535557
if (NotEqual.isInvalid())
536558
return std::nullopt;
537559
ConditionResult Condition = ActOnCondition(
@@ -540,12 +562,10 @@ Sema::ComputeExpansionSize(CXXExpansionStmtPattern *Expansion) {
540562
if (Condition.isInvalid())
541563
return std::nullopt;
542564

543-
// ++i
544-
IterationVarDeclRef = BuildDeclRefExpr(
545-
IterationVar, IterationVar->getType().getNonReferenceType(), VK_LValue,
546-
Loc);
565+
// ++b
566+
Begin = GetDeclRef(Info.BeginVar);
547567
ExprResult Increment =
548-
ActOnUnaryOp(getCurScope(), Loc, tok::plusplus, IterationVarDeclRef);
568+
ActOnUnaryOp(getCurScope(), Loc, tok::plusplus, Begin);
549569
if (Increment.isInvalid())
550570
return std::nullopt;
551571
FullExprArg ThirdPart = MakeFullDiscardedValueExpr(Increment.get());
@@ -568,9 +588,8 @@ Sema::ComputeExpansionSize(CXXExpansionStmtPattern *Expansion) {
568588
// Exit the for loop.
569589
InnerScope.Exit();
570590
ForScope.Exit();
571-
StmtResult ForLoop =
572-
ActOnForStmt(Loc, Loc, IterationVarStmt.get(), Condition, ThirdPart,
573-
Loc, IncrementStmt.get());
591+
StmtResult ForLoop = ActOnForStmt(Loc, Loc, /*First=*/nullptr, Condition,
592+
ThirdPart, Loc, IncrementStmt.get());
574593
if (ForLoop.isInvalid())
575594
return std::nullopt;
576595

@@ -582,9 +601,11 @@ Sema::ComputeExpansionSize(CXXExpansionStmtPattern *Expansion) {
582601
return std::nullopt;
583602

584603
// Finally, we can build the compound statement that is the lambda body.
585-
StmtResult LambdaBody = ActOnCompoundStmt(
586-
Loc, Loc, {ResultVarStmt.get(), ForLoop.get(), Return.get()},
587-
/*isStmtExpr=*/false);
604+
StmtResult LambdaBody =
605+
ActOnCompoundStmt(Loc, Loc,
606+
{ResultVarStmt.get(), BeginStmt.get(), EndStmt.get(),
607+
ForLoop.get(), Return.get()},
608+
/*isStmtExpr=*/false);
588609
if (LambdaBody.isInvalid())
589610
return std::nullopt;
590611

‎clang/lib/Sema/SemaStmt.cpp‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2410,10 +2410,10 @@ void NoteForRangeBeginEndFunction(Sema &SemaRef, Expr *E,
24102410

24112411
/// Build a variable declaration for a for-range statement.
24122412
VarDecl *BuildForRangeVarDecl(Sema &SemaRef, SourceLocation Loc, QualType Type,
2413-
StringRef Name, bool ForExpansionStmt) {
2413+
StringRef Name, bool Constexpr) {
24142414
// Making the variable constexpr doesn't automatically add 'const' to the
24152415
// type, so do that now.
2416-
if (ForExpansionStmt && !Type->isReferenceType())
2416+
if (Constexpr && !Type->isReferenceType())
24172417
Type = Type.withConst();
24182418

24192419
DeclContext *DC = SemaRef.CurContext;
@@ -2423,7 +2423,7 @@ VarDecl *BuildForRangeVarDecl(Sema &SemaRef, SourceLocation Loc, QualType Type,
24232423
TInfo, SC_None);
24242424
Decl->setImplicit();
24252425
Decl->setCXXForRangeImplicitVar(true);
2426-
if (ForExpansionStmt)
2426+
if (Constexpr)
24272427
// CWG 3044: Do not make the variable 'static'.
24282428
Decl->setConstexpr(true);
24292429
return Decl;
@@ -2742,7 +2742,17 @@ Sema::ForRangeBeginEndInfo Sema::BuildCXXForRangeBeginEndVars(
27422742
//
27432743
// CWG 3043 – Do not apply lifetime extension to iterating
27442744
// expansion statements.
2745-
if (getLangOpts().CPlusPlus23 && !ForExpansionStmt)
2745+
//
2746+
// Note: CWG 3131 makes it so the 'range' variable need not be
2747+
// constexpr anymore, which means that we probably *do* want
2748+
// lifetime extension in that case after all, contrary to what
2749+
// CWG 3043 currently states. This just works out naturally with
2750+
// this implementation at the moment, but wg21 insist on no lifetime
2751+
// extension for iterating expansion statements, then this instead
2752+
// needs to check whether we're building this for an expansion statement
2753+
// instead of just applying lifetime extension if the variable isn't
2754+
// constexpr (or we could pass in an empty range for 'LifetimeExtendTemps').
2755+
if (getLangOpts().CPlusPlus23 && !Constexpr)
27462756
ApplyForRangeOrExpansionStatementLifetimeExtension(RangeVar,
27472757
LifetimeExtendTemps);
27482758

0 commit comments

Comments
 (0)