-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[Clang] [C++26] Expansion Statements (Part 1: AST) #169680
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: main
Are you sure you want to change the base?
Conversation
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang-codegen Author: None (Sirraide) ChangesPatch is 98.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/169680.diff 43 Files Affected:
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h
index e74bb72571d64..fd64d86f83bfd 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -959,6 +959,12 @@ class ASTNodeTraverser
}
}
+ void VisitCXXExpansionStmtDecl(const CXXExpansionStmtDecl *Node) {
+ Visit(Node->getExpansionPattern());
+ if (Traversal != TK_IgnoreUnlessSpelledInSource)
+ Visit(Node->getInstantiations());
+ }
+
void VisitCallExpr(const CallExpr *Node) {
for (const auto *Child :
make_filter_range(Node->children(), [this](const Stmt *Child) {
diff --git a/clang/include/clang/AST/ComputeDependence.h b/clang/include/clang/AST/ComputeDependence.h
index c298f2620f211..792f45bea5aeb 100644
--- a/clang/include/clang/AST/ComputeDependence.h
+++ b/clang/include/clang/AST/ComputeDependence.h
@@ -94,6 +94,7 @@ class DesignatedInitExpr;
class ParenListExpr;
class PseudoObjectExpr;
class AtomicExpr;
+class CXXExpansionInitListExpr;
class ArraySectionExpr;
class OMPArrayShapingExpr;
class OMPIteratorExpr;
@@ -191,6 +192,8 @@ ExprDependence computeDependence(ParenListExpr *E);
ExprDependence computeDependence(PseudoObjectExpr *E);
ExprDependence computeDependence(AtomicExpr *E);
+ExprDependence computeDependence(CXXExpansionInitListExpr *E);
+
ExprDependence computeDependence(ArraySectionExpr *E);
ExprDependence computeDependence(OMPArrayShapingExpr *E);
ExprDependence computeDependence(OMPIteratorExpr *E);
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index ee2321dd158d4..b8f8e002ebcce 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -1254,7 +1254,9 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
if (getKind() != Decl::Var && getKind() != Decl::Decomposition)
return false;
if (const DeclContext *DC = getLexicalDeclContext())
- return DC->getRedeclContext()->isFunctionOrMethod();
+ return DC->getEnclosingNonExpansionStatementContext()
+ ->getRedeclContext()
+ ->isFunctionOrMethod();
return false;
}
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 5519787d71f88..71e6898f4c94d 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -2195,6 +2195,10 @@ class DeclContext {
return getDeclKind() == Decl::RequiresExprBody;
}
+ bool isExpansionStmt() const {
+ return getDeclKind() == Decl::CXXExpansionStmt;
+ }
+
bool isNamespace() const { return getDeclKind() == Decl::Namespace; }
bool isStdNamespace() const;
@@ -2292,6 +2296,15 @@ class DeclContext {
return const_cast<DeclContext *>(this)->getOuterLexicalRecordContext();
}
+ /// Retrieve the innermost enclosing context that doesn't belong to an
+ /// expansion statement. Returns 'this' if this context is not an expansion
+ /// statement.
+ DeclContext *getEnclosingNonExpansionStatementContext();
+ const DeclContext *getEnclosingNonExpansionStatementContext() const {
+ return const_cast<DeclContext *>(this)
+ ->getEnclosingNonExpansionStatementContext();
+ }
+
/// Test if this context is part of the enclosing namespace set of
/// the context NS, as defined in C++0x [namespace.def]p9. If either context
/// isn't a namespace, this is equivalent to Equals().
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index a4a1bb9c13c79..45a58b6589074 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -3343,6 +3343,120 @@ class TemplateParamObjectDecl : public ValueDecl,
static bool classofKind(Kind K) { return K == TemplateParamObject; }
};
+/// Represents a C++26 expansion statement declaration.
+///
+/// This is a bit of a hack, since expansion statements shouldn't really be
+/// 'declarations' per se (they don't declare anything). Nevertheless, we *do*
+/// need them to be declaration *contexts*, because the DeclContext is used to
+/// compute the 'template depth' of entities enclosed therein. In particular,
+/// the 'template depth' is used to find instantiations of parameter variables,
+/// and a lambda enclosed within an expansion statement cannot compute its
+/// template depth without a pointer to the enclosing expansion statement.
+///
+/// For the remainder of this comment, let 'expanding' an expansion statement
+/// refer to the process of performing template substitution on its body N
+/// times, where N is the expansion size (how this size is determined depends on
+/// the kind of expansion statement); by contrast we may sometimes 'instantiate'
+/// an expansion statement (because it happens to be in a template). This is
+/// just regular template instantiation.
+///
+/// Apart from a template parameter list that contains a template parameter used
+/// as the expansion index, this node contains a 'CXXExpansionStmtPattern' as
+/// well as a 'CXXExpansionStmtInstantiation'. These two members correspond to
+/// distinct representations of the expansion statement: the former is used
+/// prior to expansion and contains all the parts needed to perform expansion;
+/// the latter holds the expanded/desugared AST nodes that result from the
+/// expansion.
+///
+/// After expansion, the 'CXXExpansionStmtPattern' is no longer updated and left
+/// as-is; this also means that, if an already-expanded expansion statement is
+/// inside a template, and that template is then instantiated, the
+/// 'CXXExpansionStmtPattern' is *not* instantiated; only the
+/// 'CXXExpansionStmtInstantiation' is. The latter is also what's used for
+/// codegen and constant evaluation.
+///
+/// For example, if the user writes the following expansion statement:
+/// \verbatim
+/// std::tuple<int, int, int> a{1, 2, 3};
+/// template for (auto x : a) {
+/// // ...
+/// }
+/// \endverbatim
+///
+/// The 'CXXExpansionStmtPattern' of this particular 'CXXExpansionStmtDecl' is a
+/// 'CXXDestructuringExpansionStmtPattern', which stores, amongst other things,
+/// the declaration of the variable 'x' as well as the expansion-initializer
+/// 'a'.
+///
+/// After expansion, we end up with a 'CXXExpansionStmtInstantiation' that
+/// contains a DecompositionDecl and 3 CompoundStmts, one for each expansion:
+///
+/// \verbatim
+/// {
+/// auto [__u0, __u1, __u2] = a;
+/// {
+/// auto x = __u0;
+/// // ...
+/// }
+/// {
+/// auto x = __u1;
+/// // ...
+/// }
+/// {
+/// auto x = __u2;
+/// // ...
+/// }
+/// }
+/// \endverbatim
+///
+/// The outer braces shown above are implicit; we don't actually create another
+/// CompoundStmt wrapping everything.
+///
+/// \see CXXExpansionStmtPattern
+/// \see CXXExpansionStmtInstantiation
+class CXXExpansionStmtDecl : public Decl, public DeclContext {
+ CXXExpansionStmtPattern *Expansion = nullptr;
+ TemplateParameterList *TParams;
+ CXXExpansionStmtInstantiation *Instantiations = nullptr;
+
+ CXXExpansionStmtDecl(DeclContext *DC, SourceLocation Loc,
+ TemplateParameterList *TParams);
+
+public:
+ friend class ASTDeclReader;
+
+ static CXXExpansionStmtDecl *Create(ASTContext &C, DeclContext *DC,
+ SourceLocation Loc,
+ TemplateParameterList *TParams);
+ static CXXExpansionStmtDecl *CreateDeserialized(ASTContext &C,
+ GlobalDeclID ID);
+
+ CXXExpansionStmtPattern *getExpansionPattern() { return Expansion; }
+ const CXXExpansionStmtPattern *getExpansionPattern() const {
+ return Expansion;
+ }
+ void setExpansionPattern(CXXExpansionStmtPattern *S) { Expansion = S; }
+
+ CXXExpansionStmtInstantiation *getInstantiations() { return Instantiations; }
+ const CXXExpansionStmtInstantiation *getInstantiations() const {
+ return Instantiations;
+ }
+
+ void setInstantiations(CXXExpansionStmtInstantiation *S) {
+ Instantiations = S;
+ }
+
+ NonTypeTemplateParmDecl *getIndexTemplateParm() const {
+ return cast<NonTypeTemplateParmDecl>(TParams->getParam(0));
+ }
+ TemplateParameterList *getTemplateParameters() const { return TParams; }
+
+ SourceRange getSourceRange() const override LLVM_READONLY;
+
+ static bool classof(const Decl *D) { return classofKind(D->getKind()); }
+ static bool classofKind(Kind K) { return K == CXXExpansionStmt; }
+};
+
inline NamedDecl *getAsNamedDecl(TemplateParameter P) {
if (auto *PD = P.dyn_cast<TemplateTypeParmDecl *>())
return PD;
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 9435ab069a520..668a51dad0ae9 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -5499,6 +5499,165 @@ class BuiltinBitCastExpr final
}
};
+/// Represents an expansion-init-list of an enumerating expansion statement.
+///
+/// \see CXXEnumeratingExpansionStmtPattern
+class CXXExpansionInitListExpr final
+ : public Expr,
+ llvm::TrailingObjects<CXXExpansionInitListExpr, Expr *> {
+ friend class ASTStmtReader;
+ friend TrailingObjects;
+
+ const unsigned NumExprs;
+ SourceLocation LBraceLoc;
+ SourceLocation RBraceLoc;
+
+ CXXExpansionInitListExpr(EmptyShell ES, unsigned NumExprs);
+ CXXExpansionInitListExpr(ArrayRef<Expr *> Exprs, SourceLocation LBraceLoc,
+ SourceLocation RBraceLoc);
+
+public:
+ static CXXExpansionInitListExpr *Create(const ASTContext &C,
+ ArrayRef<Expr *> Exprs,
+ SourceLocation LBraceLoc,
+ SourceLocation RBraceLoc);
+
+ static CXXExpansionInitListExpr *
+ CreateEmpty(const ASTContext &C, EmptyShell Empty, unsigned NumExprs);
+
+ ArrayRef<Expr *> getExprs() const { return getTrailingObjects(NumExprs); }
+ MutableArrayRef<Expr *> getExprs() { return getTrailingObjects(NumExprs); }
+ unsigned getNumExprs() const { return NumExprs; }
+
+ bool containsPackExpansion() const;
+
+ SourceLocation getBeginLoc() const { return getLBraceLoc(); }
+ SourceLocation getEndLoc() const { return getRBraceLoc(); }
+
+ SourceLocation getLBraceLoc() const { return LBraceLoc; }
+ SourceLocation getRBraceLoc() const { return RBraceLoc; }
+
+ child_range children() {
+ const_child_range CCR =
+ const_cast<const CXXExpansionInitListExpr *>(this)->children();
+ return child_range(cast_away_const(CCR.begin()),
+ cast_away_const(CCR.end()));
+ }
+
+ const_child_range children() const {
+ Stmt **Stmts = getTrailingStmts();
+ return const_child_range(Stmts, Stmts + NumExprs);
+ }
+
+ static bool classof(const Stmt *T) {
+ return T->getStmtClass() == CXXExpansionInitListExprClass;
+ }
+
+private:
+ Stmt **getTrailingStmts() const {
+ return reinterpret_cast<Stmt **>(const_cast<Expr **>(getTrailingObjects()));
+ }
+};
+
+/// Helper that selects an expression from an expansion init list depending
+/// on the current expansion index.
+///
+/// \see CXXEnumeratingExpansionStmtPattern
+class CXXExpansionInitListSelectExpr : public Expr {
+ friend class ASTStmtReader;
+
+ enum SubExpr { RANGE, INDEX, COUNT };
+ Expr *SubExprs[COUNT];
+
+public:
+ CXXExpansionInitListSelectExpr(EmptyShell Empty);
+ CXXExpansionInitListSelectExpr(const ASTContext &C,
+ CXXExpansionInitListExpr *Range, Expr *Idx);
+
+ CXXExpansionInitListExpr *getRangeExpr() {
+ return cast<CXXExpansionInitListExpr>(SubExprs[RANGE]);
+ }
+
+ const CXXExpansionInitListExpr *getRangeExpr() const {
+ return cast<CXXExpansionInitListExpr>(SubExprs[RANGE]);
+ }
+
+ void setRangeExpr(CXXExpansionInitListExpr *E) { SubExprs[RANGE] = E; }
+
+ Expr *getIndexExpr() { return SubExprs[INDEX]; }
+ const Expr *getIndexExpr() const { return SubExprs[INDEX]; }
+ void setIndexExpr(Expr *E) { SubExprs[INDEX] = E; }
+
+ SourceLocation getBeginLoc() const { return getRangeExpr()->getBeginLoc(); }
+ SourceLocation getEndLoc() const { return getRangeExpr()->getEndLoc(); }
+
+ child_range children() {
+ return child_range(reinterpret_cast<Stmt **>(SubExprs),
+ reinterpret_cast<Stmt **>(SubExprs + COUNT));
+ }
+
+ const_child_range children() const {
+ return const_child_range(
+ reinterpret_cast<Stmt **>(const_cast<Expr **>(SubExprs)),
+ reinterpret_cast<Stmt **>(const_cast<Expr **>(SubExprs + COUNT)));
+ }
+
+ static bool classof(const Stmt *T) {
+ return T->getStmtClass() == CXXExpansionInitListSelectExprClass;
+ }
+};
+
+/// This class serves the same purpose as CXXExpansionInitListSelectExpr, but
+/// for destructuring expansion statements; that is, instead of selecting among
+/// a list of expressions, it selects from a list of 'BindingDecl's.
+///
+/// \see CXXEnumeratingExpansionStmtPattern
+/// \see CXXDestructuringExpansionStmtPattern
+class CXXDestructuringExpansionSelectExpr : public Expr {
+ friend class ASTStmtReader;
+
+ DecompositionDecl *Decomposition;
+ Expr *Index;
+
+public:
+ CXXDestructuringExpansionSelectExpr(EmptyShell Empty);
+ CXXDestructuringExpansionSelectExpr(const ASTContext &C,
+ DecompositionDecl *Decomposition,
+ Expr *Index);
+
+ DecompositionDecl *getDecompositionDecl() {
+ return cast<DecompositionDecl>(Decomposition);
+ }
+
+ const DecompositionDecl *getDecompositionDecl() const {
+ return cast<DecompositionDecl>(Decomposition);
+ }
+
+ void setDecompositionDecl(DecompositionDecl *E) { Decomposition = E; }
+
+ Expr *getIndexExpr() { return Index; }
+ const Expr *getIndexExpr() const { return Index; }
+ void setIndexExpr(Expr *E) { Index = E; }
+
+ SourceLocation getBeginLoc() const { return Decomposition->getBeginLoc(); }
+ SourceLocation getEndLoc() const { return Decomposition->getEndLoc(); }
+
+ child_range children() {
+ return child_range(reinterpret_cast<Stmt **>(&Index),
+ reinterpret_cast<Stmt **>(&Index + 1));
+ }
+
+ const_child_range children() const {
+ return const_child_range(
+ reinterpret_cast<Stmt **>(const_cast<Expr **>(&Index)),
+ reinterpret_cast<Stmt **>(const_cast<Expr **>(&Index + 1)));
+ }
+
+ static bool classof(const Stmt *T) {
+ return T->getStmtClass() == CXXDestructuringExpansionSelectExprClass;
+ }
+};
+
} // namespace clang
#endif // LLVM_CLANG_AST_EXPRCXX_H
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 8f427427d71ed..24052df70c7a8 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1881,6 +1881,14 @@ DEF_TRAVERSE_DECL(UsingShadowDecl, {})
DEF_TRAVERSE_DECL(ConstructorUsingShadowDecl, {})
+DEF_TRAVERSE_DECL(CXXExpansionStmtDecl, {
+ if (D->getInstantiations() &&
+ getDerived().shouldVisitTemplateInstantiations())
+ TRY_TO(TraverseStmt(D->getInstantiations()));
+
+ TRY_TO(TraverseStmt(D->getExpansionPattern()));
+})
+
DEF_TRAVERSE_DECL(OMPThreadPrivateDecl, {
for (auto *I : D->varlist()) {
TRY_TO(TraverseStmt(I));
@@ -3117,6 +3125,15 @@ DEF_TRAVERSE_STMT(RequiresExpr, {
TRY_TO(TraverseConceptRequirement(Req));
})
+DEF_TRAVERSE_STMT(CXXEnumeratingExpansionStmtPattern, {})
+DEF_TRAVERSE_STMT(CXXIteratingExpansionStmtPattern, {})
+DEF_TRAVERSE_STMT(CXXDestructuringExpansionStmtPattern, {})
+DEF_TRAVERSE_STMT(CXXDependentExpansionStmtPattern, {})
+DEF_TRAVERSE_STMT(CXXExpansionStmtInstantiation, {})
+DEF_TRAVERSE_STMT(CXXExpansionInitListExpr, {})
+DEF_TRAVERSE_STMT(CXXExpansionInitListSelectExpr, {})
+DEF_TRAVERSE_STMT(CXXDestructuringExpansionSelectExpr, {})
+
// These literals (all of them) do not need any action.
DEF_TRAVERSE_STMT(IntegerLiteral, {})
DEF_TRAVERSE_STMT(FixedPointLiteral, {})
diff --git a/clang/include/clang/AST/StmtCXX.h b/clang/include/clang/AST/StmtCXX.h
index 5d68d3ef64a20..96c3f912d6c3e 100644
--- a/clang/include/clang/AST/StmtCXX.h
+++ b/clang/include/clang/AST/StmtCXX.h
@@ -22,6 +22,7 @@
namespace clang {
class VarDecl;
+class CXXExpansionStmtDecl;
/// CXXCatchStmt - This represents a C++ catch block.
///
@@ -524,6 +525,483 @@ class CoreturnStmt : public Stmt {
}
};
+/// CXXExpansionStmtPattern - Base class for an unexpanded C++ expansion
+/// statement.
+///
+/// The main purpose for this class is to store the AST nodes common to all
+/// variants of expansion statements; it also provides storage for additional
+/// subexpressions required by its derived classes. This is to simplify the
+/// implementation of 'children()' and friends.
+///
+/// \see CXXExpansionStmtDecl
+/// \see CXXEnumeratingExpansionStmtPattern
+/// \see CXXIteratingExpansionStmtPattern
+/// \see CXXDestructuringExpansionStmtPattern
+/// \see CXXDependentExpansionStmtPattern
+class CXXExpansionStmtPattern : public Stmt {
+ friend class ASTStmtReader;
+
+ CXXExpansionStmtDecl *ParentDecl;
+ SourceLocation LParenLoc;
+ SourceLocation ColonLoc;
+ SourceLocation RParenLoc;
+
+protected:
+ enum SubStmt {
+ INIT,
+ VAR,
+ BODY,
+ FIRST_CHILD_STMT,
+
+ // CXXDependentExpansionStmtPattern
+ EXPANSION_INITIALIZER = FIRST_CHILD_STMT,
+ COUNT_CXXDependentExpansionStmtPattern,
+
+ // CXXDestructuringExpansionStmtPattern
+ DECOMP_DECL = FIRST_CHILD_STMT,
+ COUNT_CXXDestructuringExpansionStmtPattern,
+
+ // CXXIteratingExpansionStmtPattern
+ RANGE = FIRST_CHILD_STMT,
+ BEGIN,
+ END,
+ COUNT_CXXIteratingExpansionStmtPattern,
+
+ MAX_COUNT = COUNT_CXXIteratingExpansionStmtPattern,
+ };
+
+ // Managing the memory for this properly would be rather complicated, and
+ // expansion statements are fairly uncommon, so just allocate space for the
+ // maximum amount of substatements we could possibly have.
+ Stmt *SubStmts[MAX_COUNT];
+
+ CXXExpansionStmtPattern(StmtClass SC, EmptyShell Empty);
+ CXXExpansionStmtPattern(StmtClass SC, CXXExpansionStmtDecl *ESD, Stmt *Init,
+ DeclStmt *ExpansionVar, SourceLocation LParenLoc,
+ SourceLocation ColonLoc, SourceLocation RParenLoc);
+
+public:
+ SourceLocation getLParenLoc() const { return LParenLoc; }
+ SourceLocation getColonLoc() const { return ColonLoc; }
+ SourceLocation getRParenLoc() const { return RParenLoc; }
+
+ SourceLocation getBeginLoc() const;
+ SourceLocation getEndLoc() const {
+ return getBody() ? getBody()->getEndLoc() : RParenLoc;
+ }
+
+ bool hasDependentSize() const;
+
+ CXXExpansionStmtDecl *getDecl() { return ParentDecl; }
+ const CXXExpansionStmtDecl *getDecl() const { return ParentDecl; }
+
+ Stmt *getInit() { return SubStmts[INIT]; }
+ const Stmt *getInit() const { return SubStmts[INIT]; }
+ void setInit(Stmt *S) { SubStmts[INIT] = S; }
+
+ VarDecl *getExpansionVariable();
+ const VarDecl *getExpansionVariable() const {
+ return const_cast<CXXExpansionStmtPattern *>(this)->getExpansionVariable();
+ }
+
+ DeclStmt *getExpansionVarStmt() { return cast<DeclStmt>(SubStmts[VAR]); }
+ const DeclStmt *getExpansionVarStmt() const {
+ return cast<DeclStmt>(SubStmts[VAR]);
+ }
+
+ void setExpansionVarStmt(Stmt *S) { SubStmts[VAR] = S; }
+
+ Stmt *getBody() { return SubStmts[BODY]; }
+ const Stmt *getBody() const { return SubStmts[BODY]; }
+ void setBody(Stmt *S) { SubStmts[BODY] = S; }
+
+ static bool classof(const Stmt *T) {
+ return T->getStmtClass() >= firstCXXExpansionStmtPatternConstant &&
+ T->getStmtClass() <= lastCXXExpansionStmtPatternConstant;
+ }
+
+ child_range children() {
+ return child_range(SubStmts, SubStmts + FIRST_CHILD_STMT);
+ }
+
+ const_child_range children() const {
+ return const_child_range(SubStmts, SubStmts + FIRST_CHILD_STMT);
+ }
+};
+
+/// Represents an unexpanded enumerating expansion statement.
+///
+/// An 'enumerating' expansion statement is one whose expansion-initializer
+/// is a brace-enclosed expression-list; this list is syntactically similar to
+/// an initializer list, but it isn't actually an expression in and of itself
+/// (in that it is never evaluated or emitted) and instead is just treated as
+/// a group of expressions. The expansion initializer of this is always a
+/// 'CXXExpa...
[truncated]
|
|
Sorry for wreaking havoc on everyone’s inbox w/ these, but hopefully reviewing this is somewhat more manageable now... |
| /// an expansion statement (because it happens to be in a template). This is | ||
| /// just regular template instantiation. | ||
| /// | ||
| /// Apart from a template parameter list that contains a template parameter used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What else does the list contain? And why a list instead of a single template parameter if that is the only thing in the list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only contains one parameter; as to why we’re storing a template parameter list, er, the fork was using one, and I kind of assumed that we needed to have one because it feels a bit weird to have a template parameter just on its own, but if storing just the parameter works, then I’ll do that instead; I’ll take a look at that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to remove the template parameter list and only store the parameter instead
| /// the declaration of the variable 'x' as well as the expansion-initializer | ||
| /// 'a'. | ||
| /// | ||
| /// After expansion, we end up with a 'CXXExpansionStmtInstantiation' that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, huh.... so it is ALL of the statements wrapped in a compound stmt...
I rather wonder if we're better off NOT doing the compound statement and having code-gen just emit these scopes right. Are there any other advantages to the compound statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so that is explained in more detail in the CXXExpansionStmtInstantiation documentation I believe, but the outermost compound statement explicitly does not exist in the AST (only the inner ones shown here do); it pretty much is already as you’re describing (i.e. we handle the outer scope in codegen).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should update this comment to indicate that the outer compound statement is ‘not actually there’; I don’t remember why I didn’t explain that here (and perhaps some of the documentation needs to be deduplicated, and this section here should just say ‘see the comment on CXXExpansionStmtInstantiation for more information on the AST we produce for the expansions’).
Alternatively, as you (or Corentin, I don’t remember?) suggested, I can move all of the expansion statement documentation into one big comment here, and then all the other nodes just have a comment that says ‘see CXXExpansionStmtDecl for documentation on this’, which might be easier to work w/ than having it all fragmented across 6-odd AST nodes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of having it all in 1 place, but would also like each node to have a quick summary of its participation too.
I DO wonder if the 'big' comment has hit the "should be in the internals manual" though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of having it all in 1 place, but would also like each node to have a quick summary of its participation too.
I’ll see if I can figure out a way to make that work
I DO wonder if the 'big' comment has hit the "should be in the internals manual" though.
It is getting rather huge yeah... but at the same time, a large portion of the big comment(s) is just ‘this is how expansion statements work’, so not sure if that belongs in the internals manuall since those parts aren’t really Clang-specific
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we SHOULD be putting more of that sorta stuff into the internals manual. These comments get lost/stale/tough to figure out WHERE they are when you need them. So I think it makes sense to be there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided not to put everything in one place after all, but I’ve added a paragraph that explains the compount statement situation to both the decl and the instantiation stmt and overall added some more explanatory notes. Hopefully this makes everything a bit clearer.
| case Stmt::CXXDependentExpansionStmtPatternClass: | ||
| llvm_unreachable("unexpanded expansion statements should not be emitted"); | ||
| case Stmt::CXXExpansionStmtInstantiationClass: | ||
| llvm_unreachable("Todo"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have an error mechanism for this? Also, it should be TODO, capital makes these sorts of things greppable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That too is replaced later in this stack, so this is just temporary code that won’t ever make it to trunk
|
Because I was just asked about this, here’s a link that shows the entire diff for the patch series in case anyone prefers to look at all of it at once: llvm:llvm-project:main...llvm:llvm-project:users/Sirraide/expansion-stmts-11-final-touches-and-ast-tests |
|
So, the more I look at this, the less I like parts of it, which i think it's mostly a naming issue - or so I hope, because a lot of work clearly went into this! We know that the C++ committee is going to come up with explicit template regions Which is basically what we need the outer-scope to be. And sure, maybe we need to model it as a Decl (I'm pretty sure we are going to regret that one day), but we need a better name. Either I also do not understand why Can the |
Yeah, I’m not particularly good at naming.
That is currently impossible, because TableGen for
Iirc we can’t reuse the code that parses init lists, but just the node might be possible; I’ll take another look at that.
So the way those work is basically that we implicitly generate a variable declaration for the expansion variable and then use the select expr as its initialiser. Essentially, for e.g. template for (auto x : {1, 2, 3}) { ... }`;we generate a declaration whose initialiser is a select expr that wraps auto x = select-expr(__N, {1, 2, 3};)During template instantiation, if auto x = select-expr(1, {1, 2, 3});we instead replace that with auto x = 2;So the whole point of the select expr is that during expansion, we need to use something as the initialiser of Can you think of a better idea of implementing this maybe? I briefly considered and experimeted w/ doing something similar to what we do for pack expansion, i.e. using some global index in That would work for enumerating expansion statements, but destructuring expansions statements run into yet another issue: we need to select from a list of binding decls, and there is currently no way to represent those as an expression—actually, I guess we could just conjure up an On that note, can we just desugar destructuring expansion statements into enumerating ones? Sure, we need to create the So to summarise:
Anyway, sorry for all the rambling but these are my thoughts on the matter. Let me know if you think this approach is feasible. Figuring out an AST representation for all of this that isn’t horrible has been the hardest part of this entire patch for me by far... |
|
Ok, I’ve merged all of the Including the Decl, we’re now down to 4 AST nodes in total, which is a lot less horrible. |
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions h,cpp -- clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/AST/Decl.h clang/include/clang/AST/DeclBase.h clang/include/clang/AST/DeclTemplate.h clang/include/clang/AST/ExprCXX.h clang/include/clang/AST/RecursiveASTVisitor.h clang/include/clang/AST/StmtCXX.h clang/include/clang/AST/TextNodeDumper.h clang/include/clang/Serialization/ASTBitCodes.h clang/lib/AST/ASTImporter.cpp clang/lib/AST/DeclBase.cpp clang/lib/AST/DeclPrinter.cpp clang/lib/AST/DeclTemplate.cpp clang/lib/AST/Expr.cpp clang/lib/AST/ExprCXX.cpp clang/lib/AST/ExprClassification.cpp clang/lib/AST/ExprConstant.cpp clang/lib/AST/ItaniumMangle.cpp clang/lib/AST/StmtCXX.cpp clang/lib/AST/StmtPrinter.cpp clang/lib/AST/StmtProfile.cpp clang/lib/AST/TextNodeDumper.cpp clang/lib/CodeGen/CGDecl.cpp clang/lib/CodeGen/CGStmt.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaExceptionSpec.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/SemaLambda.cpp clang/lib/Sema/SemaLookup.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/lib/Sema/TreeTransform.h clang/lib/Serialization/ASTCommon.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTReaderStmt.cpp clang/lib/Serialization/ASTWriterDecl.cpp clang/lib/Serialization/ASTWriterStmt.cpp clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/tools/libclang/CIndex.cpp clang/tools/libclang/CXCursor.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 083576c53..c6380f9b1 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -5513,9 +5513,7 @@ public:
CXXExpansionSelectExpr(EmptyShell Empty);
CXXExpansionSelectExpr(const ASTContext &C, InitListExpr *Range, Expr *Idx);
- InitListExpr *getRangeExpr() {
- return cast<InitListExpr>(SubExprs[RANGE]);
- }
+ InitListExpr *getRangeExpr() { return cast<InitListExpr>(SubExprs[RANGE]); }
const InitListExpr *getRangeExpr() const {
return cast<InitListExpr>(SubExprs[RANGE]);
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 148a5b514..2b69262a0 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -7483,8 +7483,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
ToBody, ToForLoc, ToCoawaitLoc, ToColonLoc, ToRParenLoc);
}
-ExpectedStmt ASTNodeImporter::VisitCXXExpansionStmtPattern(
- CXXExpansionStmtPattern *S) {
+ExpectedStmt
+ASTNodeImporter::VisitCXXExpansionStmtPattern(CXXExpansionStmtPattern *S) {
Error Err = Error::success();
auto ToESD = importChecked(Err, S->getDecl());
auto ToInit = importChecked(Err, S->getInit());
@@ -9454,8 +9454,8 @@ ASTNodeImporter::VisitCXXParenListInitExpr(CXXParenListInitExpr *E) {
ToInitLoc, ToBeginLoc, ToEndLoc);
}
-ExpectedStmt ASTNodeImporter::VisitCXXExpansionSelectExpr(
- CXXExpansionSelectExpr *E) {
+ExpectedStmt
+ASTNodeImporter::VisitCXXExpansionSelectExpr(CXXExpansionSelectExpr *E) {
Error Err = Error::success();
auto ToRange = importChecked(Err, E->getRangeExpr());
auto ToIndex = importChecked(Err, E->getIndexExpr());
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 8898547e2..9ea8cd69f 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -2024,8 +2024,8 @@ CXXFoldExpr::CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
CXXExpansionSelectExpr::CXXExpansionSelectExpr(EmptyShell Empty)
: Expr(CXXExpansionSelectExprClass, Empty) {}
-CXXExpansionSelectExpr::CXXExpansionSelectExpr(
- const ASTContext &C, InitListExpr *Range, Expr *Idx)
+CXXExpansionSelectExpr::CXXExpansionSelectExpr(const ASTContext &C,
+ InitListExpr *Range, Expr *Idx)
: Expr(CXXExpansionSelectExprClass, C.DependentTy, VK_PRValue,
OK_Ordinary) {
setDependence(ExprDependence::TypeValueInstantiation);
diff --git a/clang/lib/AST/StmtCXX.cpp b/clang/lib/AST/StmtCXX.cpp
index 6f7cddd49..ffccd76d1 100644
--- a/clang/lib/AST/StmtCXX.cpp
+++ b/clang/lib/AST/StmtCXX.cpp
@@ -209,8 +209,7 @@ SourceLocation CXXExpansionStmtPattern::getBeginLoc() const {
return ParentDecl->getLocation();
}
-DecompositionDecl *
-CXXExpansionStmtPattern::getDecompositionDecl() {
+DecompositionDecl *CXXExpansionStmtPattern::getDecompositionDecl() {
assert(isDestructuring());
return cast<DecompositionDecl>(
cast<DeclStmt>(getDecompositionDeclStmt())->getSingleDecl());
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 6032e40ec..92785cf16 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -475,8 +475,7 @@ void StmtPrinter::VisitCXXExpansionStmtInstantiation(
llvm_unreachable("should never be printed");
}
-void StmtPrinter::VisitCXXExpansionSelectExpr(
- CXXExpansionSelectExpr *Node) {
+void StmtPrinter::VisitCXXExpansionSelectExpr(CXXExpansionSelectExpr *Node) {
PrintExpr(Node->getRangeExpr());
}
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 5c65be8c5..cdb1223f6 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1752,8 +1752,7 @@ void ASTStmtReader::VisitCXXExpansionStmtInstantiation(
S->setShouldApplyLifetimeExtensionToSharedStmts(Record.readBool());
}
-void ASTStmtReader::VisitCXXExpansionSelectExpr(
- CXXExpansionSelectExpr *E) {
+void ASTStmtReader::VisitCXXExpansionSelectExpr(CXXExpansionSelectExpr *E) {
VisitExpr(E);
E->setRangeExpr(cast<InitListExpr>(Record.readSubExpr()));
E->setIndexExpr(Record.readSubExpr());
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 2b223d1f1..d31750222 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1711,7 +1711,7 @@ void ASTStmtWriter::VisitCXXExpansionStmtPattern(CXXExpansionStmtPattern *S) {
Record.AddSourceLocation(S->getColonLoc());
Record.AddSourceLocation(S->getRParenLoc());
Record.AddDeclRef(S->getDecl());
- for (Stmt* SubStmt : S->children())
+ for (Stmt *SubStmt : S->children())
Record.AddStmt(SubStmt);
Code = serialization::STMT_CXX_EXPANSION_PATTERN;
}
@@ -1729,8 +1729,7 @@ void ASTStmtWriter::VisitCXXExpansionStmtInstantiation(
Code = serialization::STMT_CXX_EXPANSION_INSTANTIATION;
}
-void ASTStmtWriter::VisitCXXExpansionSelectExpr(
- CXXExpansionSelectExpr *E) {
+void ASTStmtWriter::VisitCXXExpansionSelectExpr(CXXExpansionSelectExpr *E) {
VisitExpr(E);
Record.AddStmt(E->getRangeExpr());
Record.AddStmt(E->getIndexExpr());
|
|
Oh yeah, and |

This is the first part of the expansion statements implementation. First things first, once everything has been approved, I’m going to merge the patches top-down into a single huge patch, and then merge that patch so we only end up with a single commit on trunk. Also, for the sake of avoiding too much rebasing, I’m only going to run
clang-formatafter everything has been merged into a single patch.This patch it introduces all of the AST nodes that are needed by various parts of the implementation. I know that this is still quite a bit of code... but it’s mostly just AST node boilerplate and splitting it up further wouldn’t have helped much I think.
In addition to the AST nodes, all of the decl-context specific code such as the many places where we need to ignore surrounding expansion statement contexts are also added here so we have all of that in one place.