-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[Clang] [C++26] Expansion Statements (Part 5: Iterating Expansion Statements) #169684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: users/Sirraide/expansion-stmts-4-nfc-for-range-refactor
Are you sure you want to change the base?
Conversation
🐧 Linux x64 Test Results
|
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: None (Sirraide) ChangesFull diff: https://github.com/llvm/llvm-project/pull/169684.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d4783c1c9677d..96292d0a4e306 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -165,6 +165,10 @@ def err_ice_too_large : Error<
def err_expr_not_string_literal : Error<"expression is not a string literal">;
def note_constexpr_assert_failed : Note<
"assertion failed during evaluation of constant expression">;
+def err_expansion_size_expr_not_ice : Error<
+ "expansion size is not a constant expression">;
+def err_expansion_size_negative : Error<
+ "expansion size must not be negative (was %0)">;
// Semantic analysis of constant literals.
def ext_predef_outside_function : Warning<
@@ -3698,6 +3702,9 @@ def err_conflicting_codeseg_attribute : Error<
def warn_duplicate_codeseg_attribute : Warning<
"duplicate code segment specifiers">, InGroup<Section>;
+def err_expansion_stmt_lambda : Error<
+ "cannot expand lambda closure type">;
+
def err_attribute_patchable_function_entry_invalid_section
: Error<"section argument to 'patchable_function_entry' attribute is not "
"valid for this target: %0">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1101ee9e10778..f001851b36ff7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -15693,6 +15693,12 @@ class Sema final : public SemaBase {
SourceLocation ColonLoc,
SourceLocation RParenLoc);
+ StmtResult BuildNonEnumeratingCXXExpansionStmtPattern(
+ CXXExpansionStmtDecl *ESD, Stmt *Init, DeclStmt *ExpansionVarStmt,
+ Expr *ExpansionInitializer, SourceLocation LParenLoc,
+ SourceLocation ColonLoc, SourceLocation RParenLoc,
+ ArrayRef<MaterializeTemporaryExpr *> LifetimeExtendTemps = {});
+
ExprResult
BuildCXXExpansionInitListSelectExpr(CXXExpansionInitListExpr *Range,
Expr *Idx);
diff --git a/clang/lib/Sema/SemaExpand.cpp b/clang/lib/Sema/SemaExpand.cpp
index acb28d6bbdcfb..228551c27d2d8 100644
--- a/clang/lib/Sema/SemaExpand.cpp
+++ b/clang/lib/Sema/SemaExpand.cpp
@@ -24,6 +24,25 @@
using namespace clang;
using namespace sema;
+namespace {
+struct IterableExpansionStmtData {
+ enum class State {
+ NotIterable,
+ Error,
+ Ok,
+ };
+
+ DeclStmt *RangeDecl = nullptr;
+ DeclStmt *BeginDecl = nullptr;
+ DeclStmt *EndDecl = nullptr;
+ Expr *Initializer = nullptr;
+ State TheState = State::NotIterable;
+
+ bool isIterable() const { return TheState == State::Ok; }
+ bool hasError() { return TheState == State::Error; }
+};
+} // namespace
+
// Build a 'DeclRefExpr' designating the template parameter '__N'.
static DeclRefExpr *BuildIndexDRE(Sema &S, CXXExpansionStmtDecl *ESD) {
return S.BuildDeclRefExpr(ESD->getIndexTemplateParm(),
@@ -42,6 +61,118 @@ static bool FinaliseExpansionVar(Sema &S, VarDecl *ExpansionVar,
return ExpansionVar->isInvalidDecl();
}
+static IterableExpansionStmtData
+TryBuildIterableExpansionStmtInitializer(Sema &S, Expr *ExpansionInitializer,
+ Expr *Index, SourceLocation ColonLoc,
+ bool VarIsConstexpr) {
+ IterableExpansionStmtData Data;
+
+ // C++26 [stmt.expand]p3: An expression is expansion-iterable if it does not
+ // have array type [...]
+ QualType Ty = ExpansionInitializer->getType().getNonReferenceType();
+ if (Ty->isArrayType())
+ return Data;
+
+ // Lookup member and ADL 'begin()'/'end()'. Only check if they exist; even if
+ // they're deleted, inaccessible, etc., this is still an iterating expansion
+ // statement, albeit an ill-formed one.
+ DeclarationNameInfo BeginName(&S.PP.getIdentifierTable().get("begin"),
+ ColonLoc);
+ DeclarationNameInfo EndName(&S.PP.getIdentifierTable().get("end"), ColonLoc);
+
+ // Try member lookup first.
+ bool FoundBeginEnd = false;
+ if (auto *Record = Ty->getAsCXXRecordDecl()) {
+ LookupResult BeginLR(S, BeginName, Sema::LookupMemberName);
+ LookupResult EndLR(S, EndName, Sema::LookupMemberName);
+ FoundBeginEnd = S.LookupQualifiedName(BeginLR, Record) &&
+ S.LookupQualifiedName(EndLR, Record);
+ }
+
+ // Try ADL.
+ //
+ // If overload resolution for 'begin()' *and* 'end()' succeeds (irrespective
+ // of whether it results in a usable candidate), then assume this is an
+ // iterating expansion statement.
+ auto HasADLCandidate = [&](DeclarationName Name) {
+ OverloadCandidateSet Candidates(ColonLoc, OverloadCandidateSet::CSK_Normal);
+ OverloadCandidateSet::iterator Best;
+
+ S.AddArgumentDependentLookupCandidates(Name, ColonLoc, ExpansionInitializer,
+ /*ExplicitTemplateArgs=*/nullptr,
+ Candidates);
+
+ return Candidates.BestViableFunction(S, ColonLoc, Best) !=
+ OR_No_Viable_Function;
+ };
+
+ if (!FoundBeginEnd && (!HasADLCandidate(BeginName.getName()) ||
+ !HasADLCandidate(EndName.getName())))
+ return Data;
+
+ auto Ctx = Sema::ExpressionEvaluationContext::PotentiallyEvaluated;
+ if (VarIsConstexpr)
+ Ctx = Sema::ExpressionEvaluationContext::ImmediateFunctionContext;
+ EnterExpressionEvaluationContext ExprEvalCtx(S, Ctx);
+
+ // The declarations should be attached to the parent decl context.
+ Sema::ContextRAII CtxGuard(
+ S, S.CurContext->getEnclosingNonExpansionStatementContext(),
+ /*NewThis=*/false);
+
+ // Ok, we know that this is supposed to be an iterable expansion statement;
+ // delegate to the for-range code to build the range/begin/end variables.
+ //
+ // Any failure at this point is a hard error.
+ Data.TheState = IterableExpansionStmtData::State::Error;
+ Scope *Scope = S.getCurScope();
+
+ // TODO: CWG 3131 changes how this range is declared.
+ StmtResult Var = S.BuildCXXForRangeRangeVar(Scope, ExpansionInitializer,
+ /*ForExpansionStmt=*/true);
+ if (Var.isInvalid())
+ return Data;
+
+ auto *RangeVar = cast<DeclStmt>(Var.get());
+ Sema::ForRangeBeginEndInfo Info = S.BuildCXXForRangeBeginEndVars(
+ Scope, cast<VarDecl>(RangeVar->getSingleDecl()), ColonLoc,
+ /*CoawaitLoc=*/{},
+ /*LifetimeExtendTemps=*/{}, Sema::BFRK_Build, /*ForExpansionStmt=*/true);
+
+ if (!Info.isValid())
+ return Data;
+
+ StmtResult BeginStmt = S.ActOnDeclStmt(
+ S.ConvertDeclToDeclGroup(Info.BeginVar), ColonLoc, ColonLoc);
+ StmtResult EndStmt = S.ActOnDeclStmt(S.ConvertDeclToDeclGroup(Info.EndVar),
+ ColonLoc, ColonLoc);
+ if (BeginStmt.isInvalid() || EndStmt.isInvalid())
+ return Data;
+
+ // Build '*(begin + i)'.
+ DeclRefExpr *Begin = S.BuildDeclRefExpr(
+ Info.BeginVar, Info.BeginVar->getType().getNonReferenceType(), VK_LValue,
+ ColonLoc);
+
+ ExprResult BeginPlusI =
+ S.ActOnBinOp(Scope, ColonLoc, tok::plus, Begin, Index);
+ if (BeginPlusI.isInvalid())
+ return Data;
+
+ ExprResult Deref =
+ S.ActOnUnaryOp(Scope, ColonLoc, tok::star, BeginPlusI.get());
+ if (Deref.isInvalid())
+ return Data;
+
+ Deref = S.MaybeCreateExprWithCleanups(Deref.get());
+ Data.BeginDecl = BeginStmt.getAs<DeclStmt>();
+ Data.EndDecl = EndStmt.getAs<DeclStmt>();
+ Data.RangeDecl = RangeVar;
+ Data.Initializer = Deref.get();
+ Data.TheState = IterableExpansionStmtData::State::Ok;
+ return Data;
+}
+
CXXExpansionStmtDecl *
Sema::ActOnCXXExpansionStmtDecl(unsigned TemplateDepth,
SourceLocation TemplateKWLoc) {
@@ -115,8 +246,26 @@ StmtResult Sema::ActOnCXXExpansionStmtPattern(
ColonLoc, RParenLoc);
}
- Diag(ESD->getLocation(), diag::err_expansion_statements_todo);
- return StmtError();
+ if (ExpansionInitializer->hasPlaceholderType()) {
+ ExprResult R = CheckPlaceholderExpr(ExpansionInitializer);
+ if (R.isInvalid())
+ return StmtError();
+ ExpansionInitializer = R.get();
+ }
+
+ if (DiagnoseUnexpandedParameterPack(ExpansionInitializer))
+ return StmtError();
+
+ // Reject lambdas early.
+ if (auto *RD = ExpansionInitializer->getType()->getAsCXXRecordDecl();
+ RD && RD->isLambda()) {
+ Diag(ExpansionInitializer->getBeginLoc(), diag::err_expansion_stmt_lambda);
+ return StmtError();
+ }
+
+ return BuildNonEnumeratingCXXExpansionStmtPattern(
+ ESD, Init, DS, ExpansionInitializer, LParenLoc, ColonLoc, RParenLoc,
+ LifetimeExtendTemps);
}
StmtResult Sema::BuildCXXEnumeratingExpansionStmtPattern(
@@ -127,6 +276,43 @@ StmtResult Sema::BuildCXXEnumeratingExpansionStmtPattern(
LParenLoc, ColonLoc, RParenLoc);
}
+StmtResult Sema::BuildNonEnumeratingCXXExpansionStmtPattern(
+ CXXExpansionStmtDecl *ESD, Stmt *Init, DeclStmt *ExpansionVarStmt,
+ Expr *ExpansionInitializer, SourceLocation LParenLoc,
+ SourceLocation ColonLoc, SourceLocation RParenLoc,
+ ArrayRef<MaterializeTemporaryExpr *> LifetimeExtendTemps) {
+ VarDecl *ExpansionVar = cast<VarDecl>(ExpansionVarStmt->getSingleDecl());
+
+ if (ExpansionInitializer->isTypeDependent()) {
+ ActOnDependentForRangeInitializer(ExpansionVar, BFRK_Build);
+ return new (Context) CXXDependentExpansionStmtPattern(
+ ESD, Init, ExpansionVarStmt, ExpansionInitializer, LParenLoc, ColonLoc,
+ RParenLoc);
+ }
+
+ // Otherwise, if it can be an iterating expansion statement, it is one.
+ DeclRefExpr *Index = BuildIndexDRE(*this, ESD);
+ IterableExpansionStmtData Data = TryBuildIterableExpansionStmtInitializer(
+ *this, ExpansionInitializer, Index, ColonLoc,
+ ExpansionVar->isConstexpr());
+ if (Data.hasError()) {
+ ActOnInitializerError(ExpansionVar);
+ return StmtError();
+ }
+
+ if (Data.isIterable()) {
+ if (FinaliseExpansionVar(*this, ExpansionVar, Data.Initializer))
+ return StmtError();
+
+ return new (Context) CXXIteratingExpansionStmtPattern(
+ ESD, Init, ExpansionVarStmt, Data.RangeDecl, Data.BeginDecl,
+ Data.EndDecl, LParenLoc, ColonLoc, RParenLoc);
+ }
+
+ Diag(ESD->getLocation(), diag::err_expansion_statements_todo);
+ return StmtError();
+}
+
StmtResult Sema::FinishCXXExpansionStmt(Stmt *Exp, Stmt *Body) {
if (!Exp || !Body)
return StmtError();
@@ -149,7 +335,13 @@ StmtResult Sema::FinishCXXExpansionStmt(Stmt *Exp, Stmt *Body) {
if (Expansion->getInit())
Shared.push_back(Expansion->getInit());
- assert(isa<CXXEnumeratingExpansionStmtPattern>(Expansion) && "TODO");
+ if (auto *Iter = dyn_cast<CXXIteratingExpansionStmtPattern>(Expansion)) {
+ Shared.push_back(Iter->getRangeVarStmt());
+ Shared.push_back(Iter->getBeginVarStmt());
+ Shared.push_back(Iter->getEndVarStmt());
+ } else {
+ assert(isa<CXXEnumeratingExpansionStmtPattern>(Expansion) && "TODO");
+ }
// Return an empty statement if the range is empty.
if (*NumInstantiations == 0) {
@@ -228,5 +420,53 @@ Sema::ComputeExpansionSize(CXXExpansionStmtPattern *Expansion) {
->getExprs()
.size();
+ // By [stmt.expand]5.2, N is the result of evaluating the expression
+ //
+ // [] consteval {
+ // std::ptrdiff_t result = 0;
+ // for (auto i = begin; i != end; ++i) ++result;
+ // return result;
+ // }()
+ // TODO: CWG 3131 changes this lambda a bit.
+ if (auto *Iterating = dyn_cast<CXXIteratingExpansionStmtPattern>(Expansion)) {
+ EnterExpressionEvaluationContext ExprEvalCtx(
+ *this, ExpressionEvaluationContext::ConstantEvaluated);
+
+ // FIXME: Actually do that; unfortunately, conjuring a lambda out of thin
+ // air in Sema is a massive pain, so for now just cheat by computing
+ // 'end - begin'.
+ SourceLocation Loc = Iterating->getColonLoc();
+ DeclRefExpr *Begin = BuildDeclRefExpr(
+ Iterating->getBeginVar(),
+ Iterating->getBeginVar()->getType().getNonReferenceType(), VK_LValue,
+ Loc);
+
+ DeclRefExpr *End = BuildDeclRefExpr(
+ Iterating->getEndVar(),
+ Iterating->getEndVar()->getType().getNonReferenceType(), VK_LValue,
+ Loc);
+
+ ExprResult N = ActOnBinOp(getCurScope(), Loc, tok::minus, End, Begin);
+ if (N.isInvalid())
+ return std::nullopt;
+
+ Expr::EvalResult ER;
+ SmallVector<PartialDiagnosticAt, 4> Notes;
+ ER.Diag = &Notes;
+ if (!N.get()->EvaluateAsInt(ER, Context)) {
+ Diag(Loc, diag::err_expansion_size_expr_not_ice);
+ for (const auto &[Location, PDiag] : Notes)
+ Diag(Location, PDiag);
+ return std::nullopt;
+ }
+
+ if (ER.Val.getInt().isNegative()) {
+ Diag(Loc, diag::err_expansion_size_negative) << ER.Val.getInt();
+ return std::nullopt;
+ }
+
+ return ER.Val.getInt().getZExtValue();
+ }
+
llvm_unreachable("TODO");
}
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index f948f516b5136..47c8f9ab6725c 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2411,6 +2411,11 @@ void NoteForRangeBeginEndFunction(Sema &SemaRef, Expr *E,
/// Build a variable declaration for a for-range statement.
VarDecl *BuildForRangeVarDecl(Sema &SemaRef, SourceLocation Loc, QualType Type,
StringRef Name, bool ForExpansionStmt) {
+ // Making the variable constexpr doesn't automatically add 'const' to the
+ // type, so do that now.
+ if (ForExpansionStmt && !Type->isReferenceType())
+ Type = Type.withConst();
+
DeclContext *DC = SemaRef.CurContext;
IdentifierInfo *II = &SemaRef.PP.getIdentifierTable().get(Name);
TypeSourceInfo *TInfo = SemaRef.Context.getTrivialTypeSourceInfo(Type, Loc);
@@ -2418,6 +2423,9 @@ VarDecl *BuildForRangeVarDecl(Sema &SemaRef, SourceLocation Loc, QualType Type,
TInfo, SC_None);
Decl->setImplicit();
Decl->setCXXForRangeImplicitVar(true);
+ if (ForExpansionStmt)
+ // CWG 3044: Do not make the variable 'static'.
+ Decl->setConstexpr(true);
return Decl;
}
}
@@ -2731,7 +2739,10 @@ Sema::ForRangeBeginEndInfo Sema::BuildCXXForRangeBeginEndVars(
return {};
// P2718R0 - Lifetime extension in range-based for loops.
- if (getLangOpts().CPlusPlus23)
+ //
+ // CWG 3043 – Do not apply lifetime extension to iterating
+ // expansion statements.
+ if (getLangOpts().CPlusPlus23 && !ForExpansionStmt)
ApplyForRangeOrExpansionStatementLifetimeExtension(RangeVar,
LifetimeExtendTemps);
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 825660fe8174e..d471b106065fb 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -9352,13 +9352,59 @@ StmtResult TreeTransform<Derived>::TransformCXXEnumeratingExpansionStmtPattern(
template <typename Derived>
StmtResult TreeTransform<Derived>::TransformCXXIteratingExpansionStmtPattern(
CXXIteratingExpansionStmtPattern *S) {
- llvm_unreachable("TOOD");
+ TransformCXXExpansionStmtPatternResult Common =
+ TransformCXXExpansionStmtPatternCommonParts(S);
+ if (!Common.isValid())
+ return StmtError();
+
+ StmtResult Range = getDerived().TransformStmt(S->getRangeVarStmt());
+ if (Range.isInvalid())
+ return StmtError();
+
+ StmtResult Begin = getDerived().TransformStmt(S->getBeginVarStmt());
+ StmtResult End = getDerived().TransformStmt(S->getEndVarStmt());
+ if (Begin.isInvalid() || End.isInvalid())
+ return StmtError();
+
+ auto *Expansion = new (SemaRef.Context) CXXIteratingExpansionStmtPattern(
+ Common.NewESD, Common.NewInit, Common.NewExpansionVarDecl,
+ Range.getAs<DeclStmt>(), Begin.getAs<DeclStmt>(), End.getAs<DeclStmt>(),
+ S->getLParenLoc(), S->getColonLoc(), S->getRParenLoc());
+
+ StmtResult Body = getDerived().TransformStmt(S->getBody());
+ if (Body.isInvalid())
+ return StmtError();
+
+ return SemaRef.FinishCXXExpansionStmt(Expansion, Body.get());
}
template <typename Derived>
StmtResult TreeTransform<Derived>::TransformCXXDependentExpansionStmtPattern(
CXXDependentExpansionStmtPattern *S) {
- llvm_unreachable("TOOD");
+ TransformCXXExpansionStmtPatternResult Common;
+ ExprResult ExpansionInitializer;
+
+ Common = TransformCXXExpansionStmtPatternCommonParts(S);
+ if (!Common.isValid())
+ return StmtError();
+
+ ExpansionInitializer =
+ getDerived().TransformExpr(S->getExpansionInitializer());
+ if (ExpansionInitializer.isInvalid())
+ return StmtError();
+
+ StmtResult Expansion = SemaRef.BuildNonEnumeratingCXXExpansionStmtPattern(
+ Common.NewESD, Common.NewInit, Common.NewExpansionVarDecl,
+ ExpansionInitializer.get(), S->getLParenLoc(), S->getColonLoc(),
+ S->getRParenLoc(), /*LifetimeExtendTemps=*/{});
+ if (Expansion.isInvalid())
+ return StmtError();
+
+ StmtResult Body = getDerived().TransformStmt(S->getBody());
+ if (Body.isInvalid())
+ return StmtError();
+
+ return SemaRef.FinishCXXExpansionStmt(Expansion.get(), Body.get());
}
template <typename Derived>
|
|
As it is a US holiday tomorrow and I'm wrapping stuff up, I likely wont get a chance to see this until mid-next-week. Please feel free to ping me a week from today and I'll start working through these. |
d36d97f to
bf106a6
Compare
2ec657b to
1c80c09
Compare
Sure, will do; thanks! |
Essentially, the expression we need to evaluate is this (CWG 3131 may end up changing this a bit, but not in ways that matter for the main problem here): [] consteval {
std::ptrdiff_t result = 0;
for (auto i = begin; i != end; ++i) ++result;
return result;
}()I’ve been looking into this a bit more, and I can think of a few approaches, but none of them seem ‘good’ to me:
So if anyone has any better ideas or an opinion as to which of these approaches seems the least horrible (I’m currently leaning towards approach 1), please let me know. |
I agree with that. |

This implements Sema and template instantiation for iterating expansion statements, as well as expansion statements whose initialiser is dependent (and which could either be iterating or destructuring).
One thing that I still need to figure out is how to construct and evaluate a lambda in Sema, because that’s required for computing the expansion size.