[Clang][P1061] Add stuctured binding packs #121417
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Jason Rice (ricejasonf) ChangesThis is an implementation of P1061 Structure Bindings Introduce a Pack without the ability to use packs outside of templates. There is a couple of ways the AST could have been sliced so let me know what you think. The only part of this change that I am unsure of is the serialization/deserialization stuff. I followed the implementation of other Exprs, but I do not really know how it is tested. Thank you for your time considering this. Patch is 65.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/121417.diff 45 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
return const_cast<ValueDecl *>(this)->getPotentiallyDecomposedVarDecl();
}
+ /// Determine whether this value is actually a function parameter pack,
+ /// init-capture pack, or structured binding pack
+ bool isParameterPack() const;
+
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
NonParmVarDeclBits.IsInitCapture = IC;
}
- /// Determine whether this variable is actually a function parameter pack or
- /// init-capture pack.
- bool isParameterPack() const;
-
/// Whether this local extern variable declaration's previous declaration
/// was declared in the same block scope. Only correct in C++.
bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
/// binding).
Expr *Binding = nullptr;
- BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
- : ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
+ BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T)
+ : ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}
void anchor() override;
@@ -4140,7 +4141,8 @@ class BindingDecl : public ValueDecl {
friend class ASTDeclReader;
static BindingDecl *Create(ASTContext &C, DeclContext *DC,
- SourceLocation IdLoc, IdentifierInfo *Id);
+ SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T);
static BindingDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);
/// Get the expression to which this declaration is bound. This may be null
@@ -4152,10 +4154,6 @@ class BindingDecl : public ValueDecl {
/// decomposition of.
ValueDecl *getDecomposedDecl() const { return Decomp; }
- /// Get the variable (if any) that holds the value of evaluating the binding.
- /// Only present for user-defined bindings for tuple-like types.
- VarDecl *getHoldingVar() const;
-
/// Set the binding for this BindingDecl, along with its declared type (which
/// should be a possibly-cv-qualified form of the type of the binding, or a
/// reference to such a type).
@@ -4167,6 +4165,9 @@ class BindingDecl : public ValueDecl {
/// Set the decomposed variable for this BindingDecl.
void setDecomposedDecl(ValueDecl *Decomposed) { Decomp = Decomposed; }
+ VarDecl *getHoldingVar() const;
+ static VarDecl *getHoldingVar(Expr *E);
+
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decl::Binding; }
};
@@ -4219,6 +4220,13 @@ class DecompositionDecl final
void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
+ /// Visit the variables (if any) that hold the values of evaluating the
+ /// binding. Only present for user-defined bindings for tuple-like types.
+ void VisitHoldingVars(llvm::function_ref<void(VarDecl *)> F) const;
+
+ // Visit the concrete bindings. (workaround)
+ void VisitBindings(llvm::function_ref<void(BindingDecl *)> F) const;
+
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decomposition; }
};
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 4cec89c979f775..6162c712c90dc3 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -5321,6 +5321,54 @@ class BuiltinBitCastExpr final
}
};
+class ResolvedUnexpandedPackExpr final
+ : public Expr,
+ private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Stmt *> {
+ friend TrailingObjects;
+
+ SourceLocation BeginLoc;
+ unsigned NumExprs;
+
+ ResolvedUnexpandedPackExpr(SourceLocation BL, QualType QT, unsigned NumExprs);
+
+public:
+ static ResolvedUnexpandedPackExpr *CreateDeserialized(ASTContext &C,
+ unsigned NumExprs);
+ static ResolvedUnexpandedPackExpr *
+ Create(ASTContext &C, SourceLocation BeginLoc, QualType T, unsigned NumExprs);
+ static ResolvedUnexpandedPackExpr *Create(ASTContext &C,
+ SourceLocation BeginLoc, QualType T,
+ llvm::ArrayRef<Expr *> Exprs);
+
+ unsigned getNumExprs() const { return NumExprs; }
+
+ Expr **getExprs() {
+ return reinterpret_cast<Expr **>(getTrailingObjects<Stmt *>());
+ }
+ Expr *const *getExprs() const {
+ return reinterpret_cast<Expr *const *>(getTrailingObjects<Stmt *>());
+ }
+
+ Expr *getExpansion(unsigned Idx) { return getExprs()[Idx]; }
+ Expr *getExpansion(unsigned Idx) const { return getExprs()[Idx]; }
+
+ // Iterators
+ child_range children() {
+ return child_range(getTrailingObjects<Stmt *>(),
+ getTrailingObjects<Stmt *>() + getNumExprs());
+ }
+
+ SourceLocation getBeginLoc() const LLVM_READONLY { return BeginLoc; }
+ SourceLocation getEndLoc() const LLVM_READONLY { return BeginLoc; }
+
+ // Returns the resolved pack of a decl or nullptr
+ static ResolvedUnexpandedPackExpr *getFromDecl(Decl *);
+
+ static bool classof(const Stmt *T) {
+ return T->getStmtClass() == ResolvedUnexpandedPackExprClass;
+ }
+};
+
} // 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 f5b32ed51698e0..9f8a8f2a8348f2 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2936,6 +2936,7 @@ DEF_TRAVERSE_STMT(FunctionParmPackExpr, {})
DEF_TRAVERSE_STMT(CXXFoldExpr, {})
DEF_TRAVERSE_STMT(AtomicExpr, {})
DEF_TRAVERSE_STMT(CXXParenListInitExpr, {})
+DEF_TRAVERSE_STMT(ResolvedUnexpandedPackExpr, {})
DEF_TRAVERSE_STMT(MaterializeTemporaryExpr, {
if (S->getLifetimeExtendedTemporaryDecl()) {
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 86fcae209c40db..5ee6f9ff28c4c8 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1099,6 +1099,11 @@ def err_lambda_capture_misplaced_ellipsis : Error<
"the name of the capture">;
def err_lambda_capture_multiple_ellipses : Error<
"multiple ellipses in pack capture">;
+def err_binding_multiple_ellipses : Error<
+ "multiple ellipses in structured binding declaration">;
+def warn_cxx2c_binding_pack : Warning<
+ "structured binding pack is incompatible with C++ standards before C++2c">,
+ DefaultIgnore, InGroup<CXXPre26Compat>;
def err_capture_default_first : Error<
"capture default must be first">;
def ext_decl_attrs_on_lambda : ExtWarn<
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 330ae045616aba..22012fb2663534 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5906,6 +5906,9 @@ def warn_cxx23_pack_indexing : Warning<
"pack indexing is incompatible with C++ standards before C++2c">,
DefaultIgnore, InGroup<CXXPre26Compat>;
+def err_pack_outside_template : Error<
+ "pack declaration outside of template">;
+
def err_fold_expression_packs_both_sides : Error<
"binary fold expression has unexpanded parameter packs in both operands">;
def err_fold_expression_empty : Error<
diff --git a/clang/include/clang/Basic/StmtNodes.td b/clang/include/clang/Basic/StmtNodes.td
index 31280df93e4c6e..a5ac8eba371f2f 100644
--- a/clang/include/clang/Basic/StmtNodes.td
+++ b/clang/include/clang/Basic/StmtNodes.td
@@ -162,6 +162,7 @@ def MaterializeTemporaryExpr : StmtNode<Expr>;
def LambdaExpr : StmtNode<Expr>;
def CXXFoldExpr : StmtNode<Expr>;
def CXXParenListInitExpr: StmtNode<Expr>;
+def ResolvedUnexpandedPackExpr : StmtNode<Expr>;
// C++ Coroutines expressions
def CoroutineSuspendExpr : StmtNode<Expr, 1>;
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 06243f2624876f..5f5df3a45d41dc 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1795,6 +1795,7 @@ class DecompositionDeclarator {
IdentifierInfo *Name;
SourceLocation NameLoc;
std::optional<ParsedAttributes> Attrs;
+ SourceLocation EllipsisLoc;
};
private:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5ee7ea48cc983c..24e13278cb2a89 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -230,7 +230,8 @@ void threadSafetyCleanup(BeforeSet *Cache);
// FIXME: No way to easily map from TemplateTypeParmTypes to
// TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
+typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *,
+ ResolvedUnexpandedPackExpr *>,
SourceLocation>
UnexpandedParameterPack;
@@ -6012,6 +6013,7 @@ class Sema final : public SemaBase {
RecordDecl *ClassDecl,
const IdentifierInfo *Name);
+ unsigned GetDecompositionElementCount(QualType DecompType);
void CheckCompleteDecompositionDeclaration(DecompositionDecl *DD);
/// Stack containing information needed when in C++2a an 'auto' is encountered
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index dfd82afad40070..bab63be73e58e8 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1890,6 +1890,7 @@ enum StmtCode {
EXPR_PACK_EXPANSION, // PackExpansionExpr
EXPR_PACK_INDEXING, // PackIndexingExpr
EXPR_SIZEOF_PACK, // SizeOfPackExpr
+ EXPR_RESOLVED_UNEXPANDED_PACK, // ResolvedUnexpandedPackExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM, // SubstNonTypeTemplateParmExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM_PACK, // SubstNonTypeTemplateParmPackExpr
EXPR_FUNCTION_PARM_PACK, // FunctionParmPackExpr
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 8b4ae58e8427a9..6216d896a88ac3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12726,11 +12726,15 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
// Likewise, variables with tuple-like bindings are required if their
// bindings have side-effects.
- if (const auto *DD = dyn_cast<DecompositionDecl>(VD))
- for (const auto *BD : DD->bindings())
- if (const auto *BindingVD = BD->getHoldingVar())
- if (DeclMustBeEmitted(BindingVD))
- return true;
+ if (const auto *DD = dyn_cast<DecompositionDecl>(VD)) {
+ bool BindingResult = false;
+ DD->VisitHoldingVars([&](VarDecl *BindingVD) {
+ if (DeclMustBeEmitted(BindingVD))
+ BindingResult = true;
+ });
+ if (BindingResult)
+ return true;
+ }
return false;
}
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 26d33b0d94795f..33b4fe0b8fecc9 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2552,7 +2552,7 @@ ExpectedDecl ASTNodeImporter::VisitBindingDecl(BindingDecl *D) {
BindingDecl *ToD;
if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
- Name.getAsIdentifierInfo()))
+ Name.getAsIdentifierInfo(), D->getType()))
return ToD;
Error Err = Error::success();
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 741e908cf9bc56..76c208bef6031c 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2659,10 +2659,6 @@ bool VarDecl::checkForConstantInitialization(
return Eval->HasConstantInitialization;
}
-bool VarDecl::isParameterPack() const {
- return isa<PackExpansionType>(getType());
-}
-
template<typename DeclT>
static DeclT *getDefinitionOrSelf(DeclT *D) {
assert(D);
@@ -5397,6 +5393,13 @@ bool ValueDecl::isInitCapture() const {
return false;
}
+bool ValueDecl::isParameterPack() const {
+ if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(this))
+ return NTTP->isParameterPack();
+
+ return isa_and_nonnull<PackExpansionType>(getType().getTypePtrOrNull());
+}
+
void ImplicitParamDecl::anchor() {}
ImplicitParamDecl *ImplicitParamDecl::Create(ASTContext &C, DeclContext *DC,
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index fb701f76231bcd..6a5662f9d074e4 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -237,10 +237,12 @@ bool Decl::isTemplateParameterPack() const {
}
bool Decl::isParameterPack() const {
- if (const auto *Var = dyn_cast<VarDecl>(this))
+ if (isTemplateParameterPack())
+ return true;
+ if (const auto *Var = dyn_cast<ValueDecl>(this))
return Var->isParameterPack();
- return isTemplateParameterPack();
+ return false;
}
FunctionDecl *Decl::getAsFunction() {
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index af73c658d6a0c5..371bf5dcf02206 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -3395,26 +3395,37 @@ VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
if (auto *Var = llvm::dyn_cast<VarDecl>(this))
return Var;
if (auto *BD = llvm::dyn_cast<BindingDecl>(this))
- return llvm::dyn_cast<VarDecl>(BD->getDecomposedDecl());
+ return llvm::dyn_cast_or_null<VarDecl>(BD->getDecomposedDecl());
return nullptr;
}
void BindingDecl::anchor() {}
BindingDecl *BindingDecl::Create(ASTContext &C, DeclContext *DC,
- SourceLocation IdLoc, IdentifierInfo *Id) {
- return new (C, DC) BindingDecl(DC, IdLoc, Id);
+ SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T) {
+ return new (C, DC) BindingDecl(DC, IdLoc, Id, T);
}
BindingDecl *BindingDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID) {
- return new (C, ID) BindingDecl(nullptr, SourceLocation(), nullptr);
+ return new (C, ID)
+ BindingDecl(nullptr, SourceLocation(), nullptr, QualType());
}
VarDecl *BindingDecl::getHoldingVar() const {
Expr *B = getBinding();
if (!B)
return nullptr;
- auto *DRE = dyn_cast<DeclRefExpr>(B->IgnoreImplicit());
+ return getHoldingVar(B);
+}
+
+VarDecl *BindingDecl::getHoldingVar(Expr *E) {
+ auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreImplicit());
+ if (!DRE)
+ return nullptr;
+ if (auto *BD = dyn_cast<BindingDecl>(DRE->getDecl())) {
+ DRE = dyn_cast<DeclRefExpr>(BD->getBinding());
+ }
if (!DRE)
return nullptr;
@@ -3423,6 +3434,45 @@ VarDecl *BindingDecl::getHoldingVar() const {
return VD;
}
+void DecompositionDecl::VisitHoldingVars(
+ llvm::function_ref<void(VarDecl *)> F) const {
+ for (BindingDecl *B : bindings()) {
+ Expr *BE = B->getBinding();
+ // All BindingDecls will contain holding vars or none will
+ if (!BE)
+ return;
+
+ llvm::ArrayRef<Expr *> Exprs;
+ if (auto *RP = dyn_cast<ResolvedUnexpandedPackExpr>(BE))
+ Exprs = llvm::ArrayRef(RP->getExprs(), RP->getNumExprs());
+ else
+ Exprs = BE;
+
+ for (Expr *E : Exprs) {
+ VarDecl *VD = BindingDecl::getHoldingVar(E);
+ if (!VD)
+ return;
+ F(VD);
+ }
+ }
+}
+
+void DecompositionDecl::VisitBindings(
+ llvm::function_ref<void(BindingDecl *)> F) const {
+ for (BindingDecl *B : bindings()) {
+ llvm::ArrayRef<Expr *> Exprs;
+ if (B->isParameterPack()) {
+ auto *RP = cast<ResolvedUnexpandedPackExpr>(B->getBinding());
+ Exprs = llvm::ArrayRef(RP->getExprs(), RP->getNumExprs());
+ for (Expr *E : Exprs) {
+ auto *DRE = cast<DeclRefExpr>(E);
+ F(cast<BindingDecl>(DRE->getDecl()));
+ }
+ } else
+ F(B);
+ }
+}
+
void DecompositionDecl::anchor() {}
DecompositionDecl *DecompositionDecl::Create(ASTContext &C, DeclContext *DC,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 8c8ccdb61dc01c..39f02ebf85b2ce 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3656,6 +3656,11 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
// These never have a side-effect.
return false;
+ // ResolvedUnexpandedPackExpr is currently only used for
+ // structed bindings which have no side effects
+ case ResolvedUnexpandedPackExprClass:
+ return false;
+
case ConstantExprClass:
// FIXME: Move this into the "return false;" block above.
return cast<ConstantExpr>(this)->getSubExpr()->HasSideEffects(
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index fc09d24fc30cb4..190af789d306ed 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1965,3 +1965,51 @@ CXXFoldExpr::CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
SubExprs[SubExpr::RHS] = RHS;
setDependence(computeDependence(this));
}
+
+ResolvedUnexpandedPackExpr::ResolvedUnexpandedPackExpr(SourceLocation BL,
+ QualType QT,
+ unsigned NumExprs)
+ : Expr(ResolvedUnexpandedPackExprClass, QT, VK_PRValue, OK_Ordinary),
+ BeginLoc(BL), NumExprs(NumExprs) {
+ setDependence(ExprDependence::TypeValueInstantiation |
+ ExprDependence::UnexpandedPack);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::CreateDeserialized(ASTContext &Ctx,
+ unsigned NumExprs) {
+ void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs),
+ alignof(ResolvedUnexpandedPackExpr));
+ return new (Mem)
+ ResolvedUnexpandedPackExpr(SourceLocation(), QualType(), NumExprs);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+ QualType T, unsigned NumExprs) {
+ void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs),
+ alignof(ResolvedUnexpandedPackExpr));
+ ResolvedUnexpandedPackExpr *New =
+ new (Mem) ResolvedUnexpandedPackExpr(BL, T, NumExprs);
+
+ auto Exprs = llvm::MutableArrayRef(New->getExprs(), New->getNumExprs());
+ std::fill(Exprs.begin(), Exprs.end(), nullptr);
+
+ return New;
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+ QualType T, ArrayRef<Expr *> Exprs) {
+ auto *New = Create(Ctx, BL, T, Exprs.size());
+ std::copy(Exprs.begin(), Exprs.end(), New->getExprs());
+ return New;
+}
+
+ResolvedUnexpandedPackExpr *ResolvedUnexpandedPackExpr::getFromDecl(Decl *D) {
+ // TODO P1858: Extend to VarDecls for P1858
+ if (auto *BD = dyn_cast<BindingDecl>(D)) {
+ return dyn_cast_or_null<ResolvedUnexpandedPackExpr>(BD->getBinding());
+ }
+ return nullptr;
+}
diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 3f37d06cc8f3a0..29a869bd76ca37 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -451,6 +451,13 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
case Expr::PackExpansionExprClass:
return ClassifyInternal(Ctx, cast<PackExpansionExpr>(E)->getPattern());
+ case Expr::ResolvedUnexpandedPackExprClass: {
+ if (cast<ResolvedUnexpandedPackExpr>(E)->getNumExprs() > 0)
+ return ClassifyInternal(
+ Ctx, cast<ResolvedUnexpandedPackExpr>(E)->getExpansion(0));
+ return Cl::CL_PRVa...
[truncated]
|
|
@llvm/pr-subscribers-clang-codegen Author: Jason Rice (ricejasonf) ChangesThis is an implementation of P1061 Structure Bindings Introduce a Pack without the ability to use packs outside of templates. There is a couple of ways the AST could have been sliced so let me know what you think. The only part of this change that I am unsure of is the serialization/deserialization stuff. I followed the implementation of other Exprs, but I do not really know how it is tested. Thank you for your time considering this. Patch is 65.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/121417.diff 45 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
return const_cast<ValueDecl *>(this)->getPotentiallyDecomposedVarDecl();
}
+ /// Determine whether this value is actually a function parameter pack,
+ /// init-capture pack, or structured binding pack
+ bool isParameterPack() const;
+
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
NonParmVarDeclBits.IsInitCapture = IC;
}
- /// Determine whether this variable is actually a function parameter pack or
- /// init-capture pack.
- bool isParameterPack() const;
-
/// Whether this local extern variable declaration's previous declaration
/// was declared in the same block scope. Only correct in C++.
bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
/// binding).
Expr *Binding = nullptr;
- BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
- : ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
+ BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T)
+ : ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}
void anchor() override;
@@ -4140,7 +4141,8 @@ class BindingDecl : public ValueDecl {
friend class ASTDeclReader;
static BindingDecl *Create(ASTContext &C, DeclContext *DC,
- SourceLocation IdLoc, IdentifierInfo *Id);
+ SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T);
static BindingDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);
/// Get the expression to which this declaration is bound. This may be null
@@ -4152,10 +4154,6 @@ class BindingDecl : public ValueDecl {
/// decomposition of.
ValueDecl *getDecomposedDecl() const { return Decomp; }
- /// Get the variable (if any) that holds the value of evaluating the binding.
- /// Only present for user-defined bindings for tuple-like types.
- VarDecl *getHoldingVar() const;
-
/// Set the binding for this BindingDecl, along with its declared type (which
/// should be a possibly-cv-qualified form of the type of the binding, or a
/// reference to such a type).
@@ -4167,6 +4165,9 @@ class BindingDecl : public ValueDecl {
/// Set the decomposed variable for this BindingDecl.
void setDecomposedDecl(ValueDecl *Decomposed) { Decomp = Decomposed; }
+ VarDecl *getHoldingVar() const;
+ static VarDecl *getHoldingVar(Expr *E);
+
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decl::Binding; }
};
@@ -4219,6 +4220,13 @@ class DecompositionDecl final
void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
+ /// Visit the variables (if any) that hold the values of evaluating the
+ /// binding. Only present for user-defined bindings for tuple-like types.
+ void VisitHoldingVars(llvm::function_ref<void(VarDecl *)> F) const;
+
+ // Visit the concrete bindings. (workaround)
+ void VisitBindings(llvm::function_ref<void(BindingDecl *)> F) const;
+
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decomposition; }
};
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 4cec89c979f775..6162c712c90dc3 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -5321,6 +5321,54 @@ class BuiltinBitCastExpr final
}
};
+class ResolvedUnexpandedPackExpr final
+ : public Expr,
+ private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Stmt *> {
+ friend TrailingObjects;
+
+ SourceLocation BeginLoc;
+ unsigned NumExprs;
+
+ ResolvedUnexpandedPackExpr(SourceLocation BL, QualType QT, unsigned NumExprs);
+
+public:
+ static ResolvedUnexpandedPackExpr *CreateDeserialized(ASTContext &C,
+ unsigned NumExprs);
+ static ResolvedUnexpandedPackExpr *
+ Create(ASTContext &C, SourceLocation BeginLoc, QualType T, unsigned NumExprs);
+ static ResolvedUnexpandedPackExpr *Create(ASTContext &C,
+ SourceLocation BeginLoc, QualType T,
+ llvm::ArrayRef<Expr *> Exprs);
+
+ unsigned getNumExprs() const { return NumExprs; }
+
+ Expr **getExprs() {
+ return reinterpret_cast<Expr **>(getTrailingObjects<Stmt *>());
+ }
+ Expr *const *getExprs() const {
+ return reinterpret_cast<Expr *const *>(getTrailingObjects<Stmt *>());
+ }
+
+ Expr *getExpansion(unsigned Idx) { return getExprs()[Idx]; }
+ Expr *getExpansion(unsigned Idx) const { return getExprs()[Idx]; }
+
+ // Iterators
+ child_range children() {
+ return child_range(getTrailingObjects<Stmt *>(),
+ getTrailingObjects<Stmt *>() + getNumExprs());
+ }
+
+ SourceLocation getBeginLoc() const LLVM_READONLY { return BeginLoc; }
+ SourceLocation getEndLoc() const LLVM_READONLY { return BeginLoc; }
+
+ // Returns the resolved pack of a decl or nullptr
+ static ResolvedUnexpandedPackExpr *getFromDecl(Decl *);
+
+ static bool classof(const Stmt *T) {
+ return T->getStmtClass() == ResolvedUnexpandedPackExprClass;
+ }
+};
+
} // 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 f5b32ed51698e0..9f8a8f2a8348f2 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2936,6 +2936,7 @@ DEF_TRAVERSE_STMT(FunctionParmPackExpr, {})
DEF_TRAVERSE_STMT(CXXFoldExpr, {})
DEF_TRAVERSE_STMT(AtomicExpr, {})
DEF_TRAVERSE_STMT(CXXParenListInitExpr, {})
+DEF_TRAVERSE_STMT(ResolvedUnexpandedPackExpr, {})
DEF_TRAVERSE_STMT(MaterializeTemporaryExpr, {
if (S->getLifetimeExtendedTemporaryDecl()) {
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 86fcae209c40db..5ee6f9ff28c4c8 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1099,6 +1099,11 @@ def err_lambda_capture_misplaced_ellipsis : Error<
"the name of the capture">;
def err_lambda_capture_multiple_ellipses : Error<
"multiple ellipses in pack capture">;
+def err_binding_multiple_ellipses : Error<
+ "multiple ellipses in structured binding declaration">;
+def warn_cxx2c_binding_pack : Warning<
+ "structured binding pack is incompatible with C++ standards before C++2c">,
+ DefaultIgnore, InGroup<CXXPre26Compat>;
def err_capture_default_first : Error<
"capture default must be first">;
def ext_decl_attrs_on_lambda : ExtWarn<
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 330ae045616aba..22012fb2663534 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5906,6 +5906,9 @@ def warn_cxx23_pack_indexing : Warning<
"pack indexing is incompatible with C++ standards before C++2c">,
DefaultIgnore, InGroup<CXXPre26Compat>;
+def err_pack_outside_template : Error<
+ "pack declaration outside of template">;
+
def err_fold_expression_packs_both_sides : Error<
"binary fold expression has unexpanded parameter packs in both operands">;
def err_fold_expression_empty : Error<
diff --git a/clang/include/clang/Basic/StmtNodes.td b/clang/include/clang/Basic/StmtNodes.td
index 31280df93e4c6e..a5ac8eba371f2f 100644
--- a/clang/include/clang/Basic/StmtNodes.td
+++ b/clang/include/clang/Basic/StmtNodes.td
@@ -162,6 +162,7 @@ def MaterializeTemporaryExpr : StmtNode<Expr>;
def LambdaExpr : StmtNode<Expr>;
def CXXFoldExpr : StmtNode<Expr>;
def CXXParenListInitExpr: StmtNode<Expr>;
+def ResolvedUnexpandedPackExpr : StmtNode<Expr>;
// C++ Coroutines expressions
def CoroutineSuspendExpr : StmtNode<Expr, 1>;
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 06243f2624876f..5f5df3a45d41dc 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1795,6 +1795,7 @@ class DecompositionDeclarator {
IdentifierInfo *Name;
SourceLocation NameLoc;
std::optional<ParsedAttributes> Attrs;
+ SourceLocation EllipsisLoc;
};
private:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5ee7ea48cc983c..24e13278cb2a89 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -230,7 +230,8 @@ void threadSafetyCleanup(BeforeSet *Cache);
// FIXME: No way to easily map from TemplateTypeParmTypes to
// TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
+typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *,
+ ResolvedUnexpandedPackExpr *>,
SourceLocation>
UnexpandedParameterPack;
@@ -6012,6 +6013,7 @@ class Sema final : public SemaBase {
RecordDecl *ClassDecl,
const IdentifierInfo *Name);
+ unsigned GetDecompositionElementCount(QualType DecompType);
void CheckCompleteDecompositionDeclaration(DecompositionDecl *DD);
/// Stack containing information needed when in C++2a an 'auto' is encountered
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index dfd82afad40070..bab63be73e58e8 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1890,6 +1890,7 @@ enum StmtCode {
EXPR_PACK_EXPANSION, // PackExpansionExpr
EXPR_PACK_INDEXING, // PackIndexingExpr
EXPR_SIZEOF_PACK, // SizeOfPackExpr
+ EXPR_RESOLVED_UNEXPANDED_PACK, // ResolvedUnexpandedPackExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM, // SubstNonTypeTemplateParmExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM_PACK, // SubstNonTypeTemplateParmPackExpr
EXPR_FUNCTION_PARM_PACK, // FunctionParmPackExpr
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 8b4ae58e8427a9..6216d896a88ac3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12726,11 +12726,15 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
// Likewise, variables with tuple-like bindings are required if their
// bindings have side-effects.
- if (const auto *DD = dyn_cast<DecompositionDecl>(VD))
- for (const auto *BD : DD->bindings())
- if (const auto *BindingVD = BD->getHoldingVar())
- if (DeclMustBeEmitted(BindingVD))
- return true;
+ if (const auto *DD = dyn_cast<DecompositionDecl>(VD)) {
+ bool BindingResult = false;
+ DD->VisitHoldingVars([&](VarDecl *BindingVD) {
+ if (DeclMustBeEmitted(BindingVD))
+ BindingResult = true;
+ });
+ if (BindingResult)
+ return true;
+ }
return false;
}
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 26d33b0d94795f..33b4fe0b8fecc9 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2552,7 +2552,7 @@ ExpectedDecl ASTNodeImporter::VisitBindingDecl(BindingDecl *D) {
BindingDecl *ToD;
if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
- Name.getAsIdentifierInfo()))
+ Name.getAsIdentifierInfo(), D->getType()))
return ToD;
Error Err = Error::success();
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 741e908cf9bc56..76c208bef6031c 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2659,10 +2659,6 @@ bool VarDecl::checkForConstantInitialization(
return Eval->HasConstantInitialization;
}
-bool VarDecl::isParameterPack() const {
- return isa<PackExpansionType>(getType());
-}
-
template<typename DeclT>
static DeclT *getDefinitionOrSelf(DeclT *D) {
assert(D);
@@ -5397,6 +5393,13 @@ bool ValueDecl::isInitCapture() const {
return false;
}
+bool ValueDecl::isParameterPack() const {
+ if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(this))
+ return NTTP->isParameterPack();
+
+ return isa_and_nonnull<PackExpansionType>(getType().getTypePtrOrNull());
+}
+
void ImplicitParamDecl::anchor() {}
ImplicitParamDecl *ImplicitParamDecl::Create(ASTContext &C, DeclContext *DC,
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index fb701f76231bcd..6a5662f9d074e4 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -237,10 +237,12 @@ bool Decl::isTemplateParameterPack() const {
}
bool Decl::isParameterPack() const {
- if (const auto *Var = dyn_cast<VarDecl>(this))
+ if (isTemplateParameterPack())
+ return true;
+ if (const auto *Var = dyn_cast<ValueDecl>(this))
return Var->isParameterPack();
- return isTemplateParameterPack();
+ return false;
}
FunctionDecl *Decl::getAsFunction() {
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index af73c658d6a0c5..371bf5dcf02206 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -3395,26 +3395,37 @@ VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
if (auto *Var = llvm::dyn_cast<VarDecl>(this))
return Var;
if (auto *BD = llvm::dyn_cast<BindingDecl>(this))
- return llvm::dyn_cast<VarDecl>(BD->getDecomposedDecl());
+ return llvm::dyn_cast_or_null<VarDecl>(BD->getDecomposedDecl());
return nullptr;
}
void BindingDecl::anchor() {}
BindingDecl *BindingDecl::Create(ASTContext &C, DeclContext *DC,
- SourceLocation IdLoc, IdentifierInfo *Id) {
- return new (C, DC) BindingDecl(DC, IdLoc, Id);
+ SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T) {
+ return new (C, DC) BindingDecl(DC, IdLoc, Id, T);
}
BindingDecl *BindingDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID) {
- return new (C, ID) BindingDecl(nullptr, SourceLocation(), nullptr);
+ return new (C, ID)
+ BindingDecl(nullptr, SourceLocation(), nullptr, QualType());
}
VarDecl *BindingDecl::getHoldingVar() const {
Expr *B = getBinding();
if (!B)
return nullptr;
- auto *DRE = dyn_cast<DeclRefExpr>(B->IgnoreImplicit());
+ return getHoldingVar(B);
+}
+
+VarDecl *BindingDecl::getHoldingVar(Expr *E) {
+ auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreImplicit());
+ if (!DRE)
+ return nullptr;
+ if (auto *BD = dyn_cast<BindingDecl>(DRE->getDecl())) {
+ DRE = dyn_cast<DeclRefExpr>(BD->getBinding());
+ }
if (!DRE)
return nullptr;
@@ -3423,6 +3434,45 @@ VarDecl *BindingDecl::getHoldingVar() const {
return VD;
}
+void DecompositionDecl::VisitHoldingVars(
+ llvm::function_ref<void(VarDecl *)> F) const {
+ for (BindingDecl *B : bindings()) {
+ Expr *BE = B->getBinding();
+ // All BindingDecls will contain holding vars or none will
+ if (!BE)
+ return;
+
+ llvm::ArrayRef<Expr *> Exprs;
+ if (auto *RP = dyn_cast<ResolvedUnexpandedPackExpr>(BE))
+ Exprs = llvm::ArrayRef(RP->getExprs(), RP->getNumExprs());
+ else
+ Exprs = BE;
+
+ for (Expr *E : Exprs) {
+ VarDecl *VD = BindingDecl::getHoldingVar(E);
+ if (!VD)
+ return;
+ F(VD);
+ }
+ }
+}
+
+void DecompositionDecl::VisitBindings(
+ llvm::function_ref<void(BindingDecl *)> F) const {
+ for (BindingDecl *B : bindings()) {
+ llvm::ArrayRef<Expr *> Exprs;
+ if (B->isParameterPack()) {
+ auto *RP = cast<ResolvedUnexpandedPackExpr>(B->getBinding());
+ Exprs = llvm::ArrayRef(RP->getExprs(), RP->getNumExprs());
+ for (Expr *E : Exprs) {
+ auto *DRE = cast<DeclRefExpr>(E);
+ F(cast<BindingDecl>(DRE->getDecl()));
+ }
+ } else
+ F(B);
+ }
+}
+
void DecompositionDecl::anchor() {}
DecompositionDecl *DecompositionDecl::Create(ASTContext &C, DeclContext *DC,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 8c8ccdb61dc01c..39f02ebf85b2ce 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3656,6 +3656,11 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
// These never have a side-effect.
return false;
+ // ResolvedUnexpandedPackExpr is currently only used for
+ // structed bindings which have no side effects
+ case ResolvedUnexpandedPackExprClass:
+ return false;
+
case ConstantExprClass:
// FIXME: Move this into the "return false;" block above.
return cast<ConstantExpr>(this)->getSubExpr()->HasSideEffects(
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index fc09d24fc30cb4..190af789d306ed 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1965,3 +1965,51 @@ CXXFoldExpr::CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
SubExprs[SubExpr::RHS] = RHS;
setDependence(computeDependence(this));
}
+
+ResolvedUnexpandedPackExpr::ResolvedUnexpandedPackExpr(SourceLocation BL,
+ QualType QT,
+ unsigned NumExprs)
+ : Expr(ResolvedUnexpandedPackExprClass, QT, VK_PRValue, OK_Ordinary),
+ BeginLoc(BL), NumExprs(NumExprs) {
+ setDependence(ExprDependence::TypeValueInstantiation |
+ ExprDependence::UnexpandedPack);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::CreateDeserialized(ASTContext &Ctx,
+ unsigned NumExprs) {
+ void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs),
+ alignof(ResolvedUnexpandedPackExpr));
+ return new (Mem)
+ ResolvedUnexpandedPackExpr(SourceLocation(), QualType(), NumExprs);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+ QualType T, unsigned NumExprs) {
+ void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs),
+ alignof(ResolvedUnexpandedPackExpr));
+ ResolvedUnexpandedPackExpr *New =
+ new (Mem) ResolvedUnexpandedPackExpr(BL, T, NumExprs);
+
+ auto Exprs = llvm::MutableArrayRef(New->getExprs(), New->getNumExprs());
+ std::fill(Exprs.begin(), Exprs.end(), nullptr);
+
+ return New;
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+ QualType T, ArrayRef<Expr *> Exprs) {
+ auto *New = Create(Ctx, BL, T, Exprs.size());
+ std::copy(Exprs.begin(), Exprs.end(), New->getExprs());
+ return New;
+}
+
+ResolvedUnexpandedPackExpr *ResolvedUnexpandedPackExpr::getFromDecl(Decl *D) {
+ // TODO P1858: Extend to VarDecls for P1858
+ if (auto *BD = dyn_cast<BindingDecl>(D)) {
+ return dyn_cast_or_null<ResolvedUnexpandedPackExpr>(BD->getBinding());
+ }
+ return nullptr;
+}
diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 3f37d06cc8f3a0..29a869bd76ca37 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -451,6 +451,13 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
case Expr::PackExpansionExprClass:
return ClassifyInternal(Ctx, cast<PackExpansionExpr>(E)->getPattern());
+ case Expr::ResolvedUnexpandedPackExprClass: {
+ if (cast<ResolvedUnexpandedPackExpr>(E)->getNumExprs() > 0)
+ return ClassifyInternal(
+ Ctx, cast<ResolvedUnexpandedPackExpr>(E)->getExpansion(0));
+ return Cl::CL_PRVa...
[truncated]
|
|
@llvm/pr-subscribers-clang-modules Author: Jason Rice (ricejasonf) ChangesThis is an implementation of P1061 Structure Bindings Introduce a Pack without the ability to use packs outside of templates. There is a couple of ways the AST could have been sliced so let me know what you think. The only part of this change that I am unsure of is the serialization/deserialization stuff. I followed the implementation of other Exprs, but I do not really know how it is tested. Thank you for your time considering this. Patch is 65.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/121417.diff 45 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
return const_cast<ValueDecl *>(this)->getPotentiallyDecomposedVarDecl();
}
+ /// Determine whether this value is actually a function parameter pack,
+ /// init-capture pack, or structured binding pack
+ bool isParameterPack() const;
+
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
NonParmVarDeclBits.IsInitCapture = IC;
}
- /// Determine whether this variable is actually a function parameter pack or
- /// init-capture pack.
- bool isParameterPack() const;
-
/// Whether this local extern variable declaration's previous declaration
/// was declared in the same block scope. Only correct in C++.
bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
/// binding).
Expr *Binding = nullptr;
- BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
- : ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
+ BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T)
+ : ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}
void anchor() override;
@@ -4140,7 +4141,8 @@ class BindingDecl : public ValueDecl {
friend class ASTDeclReader;
static BindingDecl *Create(ASTContext &C, DeclContext *DC,
- SourceLocation IdLoc, IdentifierInfo *Id);
+ SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T);
static BindingDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);
/// Get the expression to which this declaration is bound. This may be null
@@ -4152,10 +4154,6 @@ class BindingDecl : public ValueDecl {
/// decomposition of.
ValueDecl *getDecomposedDecl() const { return Decomp; }
- /// Get the variable (if any) that holds the value of evaluating the binding.
- /// Only present for user-defined bindings for tuple-like types.
- VarDecl *getHoldingVar() const;
-
/// Set the binding for this BindingDecl, along with its declared type (which
/// should be a possibly-cv-qualified form of the type of the binding, or a
/// reference to such a type).
@@ -4167,6 +4165,9 @@ class BindingDecl : public ValueDecl {
/// Set the decomposed variable for this BindingDecl.
void setDecomposedDecl(ValueDecl *Decomposed) { Decomp = Decomposed; }
+ VarDecl *getHoldingVar() const;
+ static VarDecl *getHoldingVar(Expr *E);
+
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decl::Binding; }
};
@@ -4219,6 +4220,13 @@ class DecompositionDecl final
void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
+ /// Visit the variables (if any) that hold the values of evaluating the
+ /// binding. Only present for user-defined bindings for tuple-like types.
+ void VisitHoldingVars(llvm::function_ref<void(VarDecl *)> F) const;
+
+ // Visit the concrete bindings. (workaround)
+ void VisitBindings(llvm::function_ref<void(BindingDecl *)> F) const;
+
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decomposition; }
};
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 4cec89c979f775..6162c712c90dc3 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -5321,6 +5321,54 @@ class BuiltinBitCastExpr final
}
};
+class ResolvedUnexpandedPackExpr final
+ : public Expr,
+ private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Stmt *> {
+ friend TrailingObjects;
+
+ SourceLocation BeginLoc;
+ unsigned NumExprs;
+
+ ResolvedUnexpandedPackExpr(SourceLocation BL, QualType QT, unsigned NumExprs);
+
+public:
+ static ResolvedUnexpandedPackExpr *CreateDeserialized(ASTContext &C,
+ unsigned NumExprs);
+ static ResolvedUnexpandedPackExpr *
+ Create(ASTContext &C, SourceLocation BeginLoc, QualType T, unsigned NumExprs);
+ static ResolvedUnexpandedPackExpr *Create(ASTContext &C,
+ SourceLocation BeginLoc, QualType T,
+ llvm::ArrayRef<Expr *> Exprs);
+
+ unsigned getNumExprs() const { return NumExprs; }
+
+ Expr **getExprs() {
+ return reinterpret_cast<Expr **>(getTrailingObjects<Stmt *>());
+ }
+ Expr *const *getExprs() const {
+ return reinterpret_cast<Expr *const *>(getTrailingObjects<Stmt *>());
+ }
+
+ Expr *getExpansion(unsigned Idx) { return getExprs()[Idx]; }
+ Expr *getExpansion(unsigned Idx) const { return getExprs()[Idx]; }
+
+ // Iterators
+ child_range children() {
+ return child_range(getTrailingObjects<Stmt *>(),
+ getTrailingObjects<Stmt *>() + getNumExprs());
+ }
+
+ SourceLocation getBeginLoc() const LLVM_READONLY { return BeginLoc; }
+ SourceLocation getEndLoc() const LLVM_READONLY { return BeginLoc; }
+
+ // Returns the resolved pack of a decl or nullptr
+ static ResolvedUnexpandedPackExpr *getFromDecl(Decl *);
+
+ static bool classof(const Stmt *T) {
+ return T->getStmtClass() == ResolvedUnexpandedPackExprClass;
+ }
+};
+
} // 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 f5b32ed51698e0..9f8a8f2a8348f2 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2936,6 +2936,7 @@ DEF_TRAVERSE_STMT(FunctionParmPackExpr, {})
DEF_TRAVERSE_STMT(CXXFoldExpr, {})
DEF_TRAVERSE_STMT(AtomicExpr, {})
DEF_TRAVERSE_STMT(CXXParenListInitExpr, {})
+DEF_TRAVERSE_STMT(ResolvedUnexpandedPackExpr, {})
DEF_TRAVERSE_STMT(MaterializeTemporaryExpr, {
if (S->getLifetimeExtendedTemporaryDecl()) {
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 86fcae209c40db..5ee6f9ff28c4c8 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1099,6 +1099,11 @@ def err_lambda_capture_misplaced_ellipsis : Error<
"the name of the capture">;
def err_lambda_capture_multiple_ellipses : Error<
"multiple ellipses in pack capture">;
+def err_binding_multiple_ellipses : Error<
+ "multiple ellipses in structured binding declaration">;
+def warn_cxx2c_binding_pack : Warning<
+ "structured binding pack is incompatible with C++ standards before C++2c">,
+ DefaultIgnore, InGroup<CXXPre26Compat>;
def err_capture_default_first : Error<
"capture default must be first">;
def ext_decl_attrs_on_lambda : ExtWarn<
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 330ae045616aba..22012fb2663534 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5906,6 +5906,9 @@ def warn_cxx23_pack_indexing : Warning<
"pack indexing is incompatible with C++ standards before C++2c">,
DefaultIgnore, InGroup<CXXPre26Compat>;
+def err_pack_outside_template : Error<
+ "pack declaration outside of template">;
+
def err_fold_expression_packs_both_sides : Error<
"binary fold expression has unexpanded parameter packs in both operands">;
def err_fold_expression_empty : Error<
diff --git a/clang/include/clang/Basic/StmtNodes.td b/clang/include/clang/Basic/StmtNodes.td
index 31280df93e4c6e..a5ac8eba371f2f 100644
--- a/clang/include/clang/Basic/StmtNodes.td
+++ b/clang/include/clang/Basic/StmtNodes.td
@@ -162,6 +162,7 @@ def MaterializeTemporaryExpr : StmtNode<Expr>;
def LambdaExpr : StmtNode<Expr>;
def CXXFoldExpr : StmtNode<Expr>;
def CXXParenListInitExpr: StmtNode<Expr>;
+def ResolvedUnexpandedPackExpr : StmtNode<Expr>;
// C++ Coroutines expressions
def CoroutineSuspendExpr : StmtNode<Expr, 1>;
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 06243f2624876f..5f5df3a45d41dc 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1795,6 +1795,7 @@ class DecompositionDeclarator {
IdentifierInfo *Name;
SourceLocation NameLoc;
std::optional<ParsedAttributes> Attrs;
+ SourceLocation EllipsisLoc;
};
private:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5ee7ea48cc983c..24e13278cb2a89 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -230,7 +230,8 @@ void threadSafetyCleanup(BeforeSet *Cache);
// FIXME: No way to easily map from TemplateTypeParmTypes to
// TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
+typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *,
+ ResolvedUnexpandedPackExpr *>,
SourceLocation>
UnexpandedParameterPack;
@@ -6012,6 +6013,7 @@ class Sema final : public SemaBase {
RecordDecl *ClassDecl,
const IdentifierInfo *Name);
+ unsigned GetDecompositionElementCount(QualType DecompType);
void CheckCompleteDecompositionDeclaration(DecompositionDecl *DD);
/// Stack containing information needed when in C++2a an 'auto' is encountered
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index dfd82afad40070..bab63be73e58e8 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1890,6 +1890,7 @@ enum StmtCode {
EXPR_PACK_EXPANSION, // PackExpansionExpr
EXPR_PACK_INDEXING, // PackIndexingExpr
EXPR_SIZEOF_PACK, // SizeOfPackExpr
+ EXPR_RESOLVED_UNEXPANDED_PACK, // ResolvedUnexpandedPackExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM, // SubstNonTypeTemplateParmExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM_PACK, // SubstNonTypeTemplateParmPackExpr
EXPR_FUNCTION_PARM_PACK, // FunctionParmPackExpr
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 8b4ae58e8427a9..6216d896a88ac3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12726,11 +12726,15 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
// Likewise, variables with tuple-like bindings are required if their
// bindings have side-effects.
- if (const auto *DD = dyn_cast<DecompositionDecl>(VD))
- for (const auto *BD : DD->bindings())
- if (const auto *BindingVD = BD->getHoldingVar())
- if (DeclMustBeEmitted(BindingVD))
- return true;
+ if (const auto *DD = dyn_cast<DecompositionDecl>(VD)) {
+ bool BindingResult = false;
+ DD->VisitHoldingVars([&](VarDecl *BindingVD) {
+ if (DeclMustBeEmitted(BindingVD))
+ BindingResult = true;
+ });
+ if (BindingResult)
+ return true;
+ }
return false;
}
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 26d33b0d94795f..33b4fe0b8fecc9 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2552,7 +2552,7 @@ ExpectedDecl ASTNodeImporter::VisitBindingDecl(BindingDecl *D) {
BindingDecl *ToD;
if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
- Name.getAsIdentifierInfo()))
+ Name.getAsIdentifierInfo(), D->getType()))
return ToD;
Error Err = Error::success();
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 741e908cf9bc56..76c208bef6031c 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2659,10 +2659,6 @@ bool VarDecl::checkForConstantInitialization(
return Eval->HasConstantInitialization;
}
-bool VarDecl::isParameterPack() const {
- return isa<PackExpansionType>(getType());
-}
-
template<typename DeclT>
static DeclT *getDefinitionOrSelf(DeclT *D) {
assert(D);
@@ -5397,6 +5393,13 @@ bool ValueDecl::isInitCapture() const {
return false;
}
+bool ValueDecl::isParameterPack() const {
+ if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(this))
+ return NTTP->isParameterPack();
+
+ return isa_and_nonnull<PackExpansionType>(getType().getTypePtrOrNull());
+}
+
void ImplicitParamDecl::anchor() {}
ImplicitParamDecl *ImplicitParamDecl::Create(ASTContext &C, DeclContext *DC,
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index fb701f76231bcd..6a5662f9d074e4 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -237,10 +237,12 @@ bool Decl::isTemplateParameterPack() const {
}
bool Decl::isParameterPack() const {
- if (const auto *Var = dyn_cast<VarDecl>(this))
+ if (isTemplateParameterPack())
+ return true;
+ if (const auto *Var = dyn_cast<ValueDecl>(this))
return Var->isParameterPack();
- return isTemplateParameterPack();
+ return false;
}
FunctionDecl *Decl::getAsFunction() {
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index af73c658d6a0c5..371bf5dcf02206 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -3395,26 +3395,37 @@ VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
if (auto *Var = llvm::dyn_cast<VarDecl>(this))
return Var;
if (auto *BD = llvm::dyn_cast<BindingDecl>(this))
- return llvm::dyn_cast<VarDecl>(BD->getDecomposedDecl());
+ return llvm::dyn_cast_or_null<VarDecl>(BD->getDecomposedDecl());
return nullptr;
}
void BindingDecl::anchor() {}
BindingDecl *BindingDecl::Create(ASTContext &C, DeclContext *DC,
- SourceLocation IdLoc, IdentifierInfo *Id) {
- return new (C, DC) BindingDecl(DC, IdLoc, Id);
+ SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T) {
+ return new (C, DC) BindingDecl(DC, IdLoc, Id, T);
}
BindingDecl *BindingDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID) {
- return new (C, ID) BindingDecl(nullptr, SourceLocation(), nullptr);
+ return new (C, ID)
+ BindingDecl(nullptr, SourceLocation(), nullptr, QualType());
}
VarDecl *BindingDecl::getHoldingVar() const {
Expr *B = getBinding();
if (!B)
return nullptr;
- auto *DRE = dyn_cast<DeclRefExpr>(B->IgnoreImplicit());
+ return getHoldingVar(B);
+}
+
+VarDecl *BindingDecl::getHoldingVar(Expr *E) {
+ auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreImplicit());
+ if (!DRE)
+ return nullptr;
+ if (auto *BD = dyn_cast<BindingDecl>(DRE->getDecl())) {
+ DRE = dyn_cast<DeclRefExpr>(BD->getBinding());
+ }
if (!DRE)
return nullptr;
@@ -3423,6 +3434,45 @@ VarDecl *BindingDecl::getHoldingVar() const {
return VD;
}
+void DecompositionDecl::VisitHoldingVars(
+ llvm::function_ref<void(VarDecl *)> F) const {
+ for (BindingDecl *B : bindings()) {
+ Expr *BE = B->getBinding();
+ // All BindingDecls will contain holding vars or none will
+ if (!BE)
+ return;
+
+ llvm::ArrayRef<Expr *> Exprs;
+ if (auto *RP = dyn_cast<ResolvedUnexpandedPackExpr>(BE))
+ Exprs = llvm::ArrayRef(RP->getExprs(), RP->getNumExprs());
+ else
+ Exprs = BE;
+
+ for (Expr *E : Exprs) {
+ VarDecl *VD = BindingDecl::getHoldingVar(E);
+ if (!VD)
+ return;
+ F(VD);
+ }
+ }
+}
+
+void DecompositionDecl::VisitBindings(
+ llvm::function_ref<void(BindingDecl *)> F) const {
+ for (BindingDecl *B : bindings()) {
+ llvm::ArrayRef<Expr *> Exprs;
+ if (B->isParameterPack()) {
+ auto *RP = cast<ResolvedUnexpandedPackExpr>(B->getBinding());
+ Exprs = llvm::ArrayRef(RP->getExprs(), RP->getNumExprs());
+ for (Expr *E : Exprs) {
+ auto *DRE = cast<DeclRefExpr>(E);
+ F(cast<BindingDecl>(DRE->getDecl()));
+ }
+ } else
+ F(B);
+ }
+}
+
void DecompositionDecl::anchor() {}
DecompositionDecl *DecompositionDecl::Create(ASTContext &C, DeclContext *DC,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 8c8ccdb61dc01c..39f02ebf85b2ce 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3656,6 +3656,11 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
// These never have a side-effect.
return false;
+ // ResolvedUnexpandedPackExpr is currently only used for
+ // structed bindings which have no side effects
+ case ResolvedUnexpandedPackExprClass:
+ return false;
+
case ConstantExprClass:
// FIXME: Move this into the "return false;" block above.
return cast<ConstantExpr>(this)->getSubExpr()->HasSideEffects(
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index fc09d24fc30cb4..190af789d306ed 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1965,3 +1965,51 @@ CXXFoldExpr::CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
SubExprs[SubExpr::RHS] = RHS;
setDependence(computeDependence(this));
}
+
+ResolvedUnexpandedPackExpr::ResolvedUnexpandedPackExpr(SourceLocation BL,
+ QualType QT,
+ unsigned NumExprs)
+ : Expr(ResolvedUnexpandedPackExprClass, QT, VK_PRValue, OK_Ordinary),
+ BeginLoc(BL), NumExprs(NumExprs) {
+ setDependence(ExprDependence::TypeValueInstantiation |
+ ExprDependence::UnexpandedPack);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::CreateDeserialized(ASTContext &Ctx,
+ unsigned NumExprs) {
+ void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs),
+ alignof(ResolvedUnexpandedPackExpr));
+ return new (Mem)
+ ResolvedUnexpandedPackExpr(SourceLocation(), QualType(), NumExprs);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+ QualType T, unsigned NumExprs) {
+ void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs),
+ alignof(ResolvedUnexpandedPackExpr));
+ ResolvedUnexpandedPackExpr *New =
+ new (Mem) ResolvedUnexpandedPackExpr(BL, T, NumExprs);
+
+ auto Exprs = llvm::MutableArrayRef(New->getExprs(), New->getNumExprs());
+ std::fill(Exprs.begin(), Exprs.end(), nullptr);
+
+ return New;
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+ QualType T, ArrayRef<Expr *> Exprs) {
+ auto *New = Create(Ctx, BL, T, Exprs.size());
+ std::copy(Exprs.begin(), Exprs.end(), New->getExprs());
+ return New;
+}
+
+ResolvedUnexpandedPackExpr *ResolvedUnexpandedPackExpr::getFromDecl(Decl *D) {
+ // TODO P1858: Extend to VarDecls for P1858
+ if (auto *BD = dyn_cast<BindingDecl>(D)) {
+ return dyn_cast_or_null<ResolvedUnexpandedPackExpr>(BD->getBinding());
+ }
+ return nullptr;
+}
diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 3f37d06cc8f3a0..29a869bd76ca37 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -451,6 +451,13 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
case Expr::PackExpansionExprClass:
return ClassifyInternal(Ctx, cast<PackExpansionExpr>(E)->getPattern());
+ case Expr::ResolvedUnexpandedPackExprClass: {
+ if (cast<ResolvedUnexpandedPackExpr>(E)->getNumExprs() > 0)
+ return ClassifyInternal(
+ Ctx, cast<ResolvedUnexpandedPackExpr>(E)->getExpansion(0));
+ return Cl::CL_PRVa...
[truncated]
|
cor3ntin
requested review from
shafik,
AaronBallman,
erichkeane,
Sirraide,
zyn0217 and
cor3ntin
cor3ntin
reviewed
Jan 1, 2025
There was a problem hiding this comment.
Thanks a lot for this PR
First review pass.
I think the changes generally.
I'd like to see more tests. Notably
- Check it works with pack indexing
- BitField tests
- Codegen tests
- PCH/Module tests
There are unimplemented bits (ASTDumper/JSONDumper) - you should have warnings for unhandled enumerator in switch
| void addUnexpanded(const TemplateTypeParmType *T, | ||
| SourceLocation Loc = SourceLocation()) { | ||
| if (T->getDepth() < DepthLimit) | ||
| Unexpanded.push_back({T, Loc}); | ||
| } | ||
| void addUnexpanded(ResolvedUnexpandedPackExpr *E) { |
There was a problem hiding this comment.
Suggested change
| void addUnexpanded(ResolvedUnexpandedPackExpr *E) { | |
| void addUnexpanded(ResolvedUnexpandedPackExpr *E) { |
zyn0217
reviewed
Jan 2, 2025
There was a problem hiding this comment.
Thanks for working on this. I left some comments, and I think we need more tests for contexts e.g. templated member functions/nested dependent lambdas to test the state machine in TreeTransform, apart from @cor3ntin's request.
Moreover, you may need to build LLDB locally and check if we're missing anything else (e.g. handling newly added expressions/decls) there.
Comment on lines
-425
to
+449
| else | ||
| Name = cast<NamedDecl *>(Unexpanded[I].first)->getIdentifier(); | ||
| else if (NamedDecl *ND = Unexpanded[I].first.dyn_cast<NamedDecl *>()) | ||
| Name = ND->getIdentifier(); |
There was a problem hiding this comment.
Don't we have to handle ResolvedUnexpandedPackExpr here?
There was a problem hiding this comment.
ResolvedUnexpandedPackExpr is not directly associated with a name. My original implementation was not for structured bindings so it is a little more general.
There was a problem hiding this comment.
But that also makes it confusing as to this implementation - I suggest avoiding unrelated changes for better clarity and safety. WDYT?
There was a problem hiding this comment.
The change is related. We do not set Name for ResolvedUnexpandedPackExprs. It was previously assuming the remaining case must be NamedDecl. (from my email)
There was a problem hiding this comment.
It is possible there is a completely different approach that uses BindingDecl more directly as the parameter pack. I think it would still run into issues with BindingDecl not being a VarDecl so the ResolvedUnexpandedPackExpr worked out, but it was part of a more general approach to dealing with packs.
| template <typename T> | ||
| void decompose_array() { | ||
| // previously unable to use non-dependent array here | ||
| // Fixes https://bugs.llvm.org/show_bug.cgi?id=45964 |
There was a problem hiding this comment.
https://llvm.org/PR45964 was marked as fixed at somepoint between 13~14.
#45964 had been closed too
Are you referring to something else?
There was a problem hiding this comment.
It was me that submitted that fix which I broke out from this. I can remove the comment if you want.
There was a problem hiding this comment.
Oh sorry I missed that
So if it directly links to code that comes next, feel free to preserve it; otherwise it looks misleading
|
As a quick note, the best way to check serialization/deserialization is a PCH /restore test. I did a bunch in |
zyn0217
reviewed
Jan 3, 2025
There was a problem hiding this comment.
Here is an interesting case involving lambda captures:
https://godbolt.org/z/Y7EhE7Gvq
(Everything would become scary when lambda comes into play)
|
That crash looks like a result of my call to setDecomposedDecl(nullptr) in
the BindingsInitWalker stuff. I don't remember why I don't set that. I will
check it out when I try to clean that up.
But that also makes it confusing as to this implementation - I suggest
avoiding unrelated changes for better clarity and safety. WDYT?
The change is related. We do not set `Name` for
ResolvedUnexpandedPackExprs. It was previously assuming the remaining case
must be NamedDecl.
…On Thu, Jan 2, 2025 at 10:06 PM Younan Zhang ***@***.***> wrote:
***@***.**** commented on this pull request.
Here is an interesting case involving lambda captures:
https://godbolt.org/z/Y7EhE7Gvq
(Everything would become scary when lambda comes into play)
—
Reply to this email directly, view it on GitHub
<#121417 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARHBFFR5PST2GGKPWMISRD2IYSH3AVCNFSM6AAAAABUN63362VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMRYGU4DSNRWGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
cor3ntin
reviewed
Jan 3, 2025
zygoloid
reviewed
Jan 18, 2025
Comment on lines
+5324
to
+5326
| // Represents an unexpanded pack where the list of expressions are | ||
| // known. These are used when structured bindings introduce a pack. | ||
| class ResolvedUnexpandedPackExpr final |
There was a problem hiding this comment.
This seems to be doing the same job as SubstNonTypeTemplateParmPackExpr and FunctionParmPackExpr. Do we need all three? If so, mentioning the relationship between the three in this comment might be nice -- but I do wonder if FunctionParmPackExpr could be generalized to cover the case of a binding declaration too.
There was a problem hiding this comment.
I definitely think they could be generalized which is what I had in mind with ResolvedUnexpandedPackExpr. This is originally what I had for parmexprs (P1221).
There was a problem hiding this comment.
Should we try to address that?
Comment on lines
1102
to
1105
| def err_binding_multiple_ellipses : Error< | ||
| "multiple ellipses in structured binding declaration">; | ||
| def note_previous_ellipsis : Note< | ||
| "previous ellipsis specified here">; |
There was a problem hiding this comment.
It seems a little strange to talk about ellipses rather than packs here. Maybe "multiple binding packs in [...]" instead?
clang/lib/Sema/SemaDeclCXX.cpp
Outdated
Comment on lines
987
to
990
| BindingDecl *BD = (*BindingWithPackItr); | ||
| BD->setBinding(PackType, | ||
| ResolvedUnexpandedPackExpr::Create( | ||
| S.Context, DD->getBeginLoc(), DecompType, PackSize)); |
There was a problem hiding this comment.
Instead of creating a single binding with a pack of values, I think template instantiation should be creating N BindingDecls each with a single value, like we do when we instantiate a function parameter pack or a template parameter pack. I think that would simplify a lot of what you're doing here.
(During the initial parse, we should treat a structured binding declaration with a binding pack as being dependent, just like we treat a case where the initializer has an unexpanded pack as dependent.)
There was a problem hiding this comment.
The problem is that we do not know the length of the pack until the initializer expression is created. This happens in various locations depending on context such as range based for loops. I am not saying it isn't possible, but I think it would require creating the init expression up front which I do not know if that would be problematic with how variables are checked. We could also do Bindings in a separate allocation, but that would require keeping the DecompositionDeclarator around somehow, and there would need to be that extra information in the DecompositionDecl which would have a cost for Decomps that do not have a pack. (I don't know if that is an issue.)
There was a problem hiding this comment.
(During the initial parse, we should treat a structured binding declaration with a binding pack as being dependent, just like we treat a case where the initializer has an unexpanded pack as dependent.)
The BindingDecls are created even when the initializer is dependent, and the init expression is created after the DecompositionDecl even in template instantiation. (I remember trying this, but template instantation does not like instantiation of the same local decl twice.)
There was a problem hiding this comment.
Hum. Did you try to transform the initializer first?
I don't think there is anything stopping us from trying as the following is IF
struct S { int i; };
auto [a] = S{a};
There was a problem hiding this comment.
What is IF? (initializer first?)
I have gone down this rabbit hole a couple of times. The initializer for a variable is built differently based on the context. For instance, range based for loops defer building the initializer (for whatever reason idk,) but it all happens in the call to VisitVarDecl which builds the new DecompositionDecl. I do not know what it would take to rip all of that out of there.
See https://github.com/ricejasonf/llvm-project/blob/ricejasonf/p1061r9/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L1193
and
https://github.com/ricejasonf/llvm-project/blob/ricejasonf/p1061r9/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L1224
The above attempt was building the initializer just to get the number of bindings and discarding it which felt like a waste.
Forgive me, IF I am misunderstanding what you are saying here. 🤣
There was a problem hiding this comment.
can we do at least do something like
auto* DD = cast<DecompositionDecl>(Src);
if (CheckBindingsCount(S, DD, DecompType, Bindings,
NumElems))
return true;
if(ResolvePackInDecomposition(DD, /*TotalSize=*/NumElems)) // create the nested bindings but does not call `setBindings`
return true;
for (auto *BD : DD->flat_bindings()) {
/*...*/
BD->setBinding( /*...*/)
}
IE, my question is can we reuse flat_bindings() and not have BindingInitWalker
There was a problem hiding this comment.
Oh yeah, using flat_bindings there looks doable if we put the ResolvedUnexpandedPack in there ahead of it like that.
There was a problem hiding this comment.
Here is a PR to this branch so you can see what would have to change to remove BindingInitWalker like that. Let me know, and I will add the commit to this branch.
https://github.com/ricejasonf/llvm-project/pull/1/files
There was a problem hiding this comment.
I think I like that much better, yes. Thanks!
ricejasonf
force-pushed
the
ricejasonf/p1061
branch
from
e0cb748 to
8cf8205
Compare
cor3ntin
reviewed
Jan 18, 2025
clang/lib/Sema/SemaDeclCXX.cpp
Outdated
Comment on lines
987
to
990
| BindingDecl *BD = (*BindingWithPackItr); | ||
| BD->setBinding(PackType, | ||
| ResolvedUnexpandedPackExpr::Create( | ||
| S.Context, DD->getBeginLoc(), DecompType, PackSize)); |
There was a problem hiding this comment.
Hum. Did you try to transform the initializer first?
I don't think there is anything stopping us from trying as the following is IF
struct S { int i; };
auto [a] = S{a};
zwuis
reviewed
Jan 21, 2025
| // known. These are used when structured bindings introduce a pack. | ||
| class ResolvedUnexpandedPackExpr final | ||
| : public Expr, | ||
| private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Expr *> { |
There was a problem hiding this comment.
Can we store BindingDecl * in trailing objects like FunctionParmPackExpr?
There was a problem hiding this comment.
Probably. That would simplify the DeclRefExpr stuff and a few other things. BindingPackExpr?
There was a problem hiding this comment.
...
BindingPackExpr?
This is what I think: using BindingPackExpr and modifying it when implementing other features (member packs etc).
There was a problem hiding this comment.
Personally, I am content to leave it as is, but storing Decls instead of Exprs does have the benefit of delaying building the DeclRefExprs until expansion which creates less garbage and would simplify flat_bindings. If anything, I would leave the name ResolvedUnexpandedPackExpr and make the trailing objects ValueDecl to support future consolidation. That is premised on never wanting to create a pack of expressions which was my original intent as this was made for P1221 (which I have abandoned). I will defer to @cor3ntin.
There was a problem hiding this comment.
That is premised on never wanting to create a pack of expressions
A use case would be unpacking a tuple for example (or data member pack but in that case maybe we want to store decl for that too)
Either way, I don't think we should predicament the design on future evolution. If we need to change it later, we can
|
I found this branch cann't handle explicitly capturing binding packs correctly. I send generated patch file here because GitHub doesn't support creating PR between Forks. The patch fileFrom d9232860da764c4e745726d6740900182e81bda6 Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <[email protected]>
Date: Wed, 22 Jan 2025 14:29:17 +0800
Subject: [PATCH] [Clang][P1061] Handle explicitly capturing binding packs
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 10 +++++++++-
clang/lib/Sema/TreeTransform.h | 7 +++----
clang/test/SemaCXX/cxx2c-binding-pack.cpp | 12 ++++++++++++
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 7e278981176e..686912ef7b69 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6232,8 +6232,16 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
// declarations to their instantiations.
if (CurrentInstantiationScope) {
if (auto Found = CurrentInstantiationScope->findInstantiationOf(D)) {
- if (Decl *FD = Found->dyn_cast<Decl *>())
+ if (Decl *FD = Found->dyn_cast<Decl *>()) {
+ if (auto *BD = dyn_cast<BindingDecl>(FD);
+ BD && BD->isParameterPack() &&
+ ArgumentPackSubstitutionIndex != -1) {
+ auto *DRE = cast<DeclRefExpr>(
+ BD->getBindingPackExprs()[ArgumentPackSubstitutionIndex]);
+ return cast<NamedDecl>(DRE->getDecl());
+ }
return cast<NamedDecl>(FD);
+ }
int PackIdx = ArgumentPackSubstitutionIndex;
assert(PackIdx != -1 &&
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 8bc4eaf60f85..f59b4fdb5e60 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -15298,12 +15298,11 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
// The transform has determined that we should perform an expansion;
// transform and capture each of the arguments.
// expansion of the pattern. Do so.
- auto *Pack = cast<VarDecl>(C->getCapturedVar());
+ auto *Pack = cast<ValueDecl>(C->getCapturedVar());
for (unsigned I = 0; I != *NumExpansions; ++I) {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
- VarDecl *CapturedVar
- = cast_or_null<VarDecl>(getDerived().TransformDecl(C->getLocation(),
- Pack));
+ ValueDecl *CapturedVar = cast_if_present<ValueDecl>(
+ getDerived().TransformDecl(C->getLocation(), Pack));
if (!CapturedVar) {
Invalid = true;
continue;
diff --git a/clang/test/SemaCXX/cxx2c-binding-pack.cpp b/clang/test/SemaCXX/cxx2c-binding-pack.cpp
index 675c0465123b..cb905a879f45 100644
--- a/clang/test/SemaCXX/cxx2c-binding-pack.cpp
+++ b/clang/test/SemaCXX/cxx2c-binding-pack.cpp
@@ -108,10 +108,22 @@ void decompose_bit_field() {
int d = x...[3];
}
+template <typename T>
+void lambda_capture() {
+ auto [...x] = T{};
+ [=] { (void)sum(x...); }();
+ [&] { (void)sum(x...); }();
+ [x...] { (void)sum(x...); }();
+ [&x...] { (void)sum(x...); }();
+}
+
int main() {
decompose_array<int>();
decompose_tuple<fake_tuple>();
decompose_struct<my_struct>();
S<1, 2, 3, 4>::N<5, 6>().foo();
decompose_bit_field<bit_fields>();
+ lambda_capture<int[5]>();
+ lambda_capture<fake_tuple>();
+ lambda_capture<my_struct>();
}
--
2.34.1
|
|
I wonder if we should rename |
I do not have a strong preference, but I kept |
Lets not then I'm inclined to say we should merge that once @zygoloid and @zwuis are happy their concerns are addressed We also need
We should hold off updating the feature test macro though. WDYT? |
zygoloid
reviewed
Jan 24, 2025
clang/include/clang/AST/DeclCXX.h
Outdated
Comment on lines
4254
to
4255
| // llvm::concat must take temporaries or it will capture | ||
| // references. |
There was a problem hiding this comment.
I found this comment a bit confusing, because move produces a reference rather than a temporary. How about something like "Pass xvalues to llvm::concat to request that it makes a copy."
There was a problem hiding this comment.
Ah, right. I just removed the comment.
clang/lib/Sema/SemaDeclCXX.cpp
Outdated
| @@ -951,28 +959,124 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D, | |||
| return New; | |||
| } | |||
|
|
|||
| // CheckBindingsCount | |||
There was a problem hiding this comment.
We don't start function comments with the function name. (We used to but that's an old style we're phasing out.)
clang/lib/Sema/SemaDeclCXX.cpp
Outdated
Comment on lines
987
to
990
| BindingDecl *BD = (*BindingWithPackItr); | ||
| BD->setBinding(PackType, | ||
| ResolvedUnexpandedPackExpr::Create( | ||
| S.Context, DD->getBeginLoc(), DecompType, PackSize)); |
shafik
reviewed
Jan 24, 2025
cor3ntin
reviewed
Jan 26, 2025
clang/lib/AST/DeclCXX.cpp
Outdated
Comment on lines
3429
to
3430
| if (!Binding) | ||
| return {}; |
There was a problem hiding this comment.
Looking at how this is used, i think we can assert here instead
There was a problem hiding this comment.
Added. The assert was being triggered in one spot that I realized was unnecessary (when I tried removing the whole block of code that was going back and adding types to the DeclRefExprs.)
clang/lib/Sema/SemaStmt.cpp
Outdated
Comment on lines
2674
to
2676
| if (!Binding->isParameterPack()) { | ||
| Binding->setType(Context.DependentTy); | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (!Binding->isParameterPack()) { | |
| Binding->setType(Context.DependentTy); | |
| } | |
| if (!Binding->isParameterPack()) | |
| Binding->setType(Context.DependentTy); |
Comment on lines
2496
to
2501
| if (auto *RP = | ||
| dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BD->getBinding())) { | ||
| return TransformResolvedUnexpandedPackExpr(RP); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Suggested change
| if (auto *RP = | |
| dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BD->getBinding())) { | |
| return TransformResolvedUnexpandedPackExpr(RP); | |
| } | |
| } | |
| if (auto *RP = | |
| dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BD->getBinding())) | |
| return TransformResolvedUnexpandedPackExpr(RP); | |
| } | |
zwuis
approved these changes
Jan 28, 2025
cor3ntin
reviewed
Jan 28, 2025
There was a problem hiding this comment.
The code and test coverage looks good.
There are a couple of comments that have not been addressed.
And there is @zygoloid suggestion to generalize SubstNonTypeTemplateParmPackExpr - do we want to do that now or address it after the fact?
Maybe trying to land that today is not the brightest idea.
However, we should target to have it landed before rc2 or something like that
We'd make a lot of folks happy having that in 20, I think. And while the PR is not tiny, it's seems also fairly low risk (famous last words!)
Comment on lines
+5324
to
+5326
| // Represents an unexpanded pack where the list of expressions are | ||
| // known. These are used when structured bindings introduce a pack. | ||
| class ResolvedUnexpandedPackExpr final |
There was a problem hiding this comment.
Should we try to address that?
cor3ntin
approved these changes
Jan 29, 2025
There was a problem hiding this comment.
Now that the Clang 20 branch has happened, I think we should go ahead with that PR
-
It would be great to explore merging ResolvedUnexpandedPackExpr and FunctionParmPackExpr in a separate PR. Is that something you would be interested in exploring?
-
If there isn't any issue reported in the next ~ 2 weeks or so we should backport that to the Clang 20 branch
Thank you Jason for contributing this feature and for being reactive on this rather involved review :)
|
@ricejasonf let us know if you want us to merge on your behalf |
Does this mean we should adjust the cxx_status page to not say Clang 20? I also see there is a conflict with the ReleaseNotes.md where it appears all of the C++2c items no longer exist. (I guess that is because it is for the next release.)
I can look into this. I will consider
Likewise and thank you and the others for your contributions and diligence while reviewing this! |
Meh, let's assume 20 and we can edit it later if we end up not merging, I think. |
@cor3ntin When you think its ready, yes please. I think it requires write access. I fixed the conflict in the ReleaseNotes through Github's UI so that only my feature is under the C++2c features added. I assume other ones were removed as they are for the "previous release". |
Edit: On second thought, lets revert cxx_status.html too to avoid confusion |
ricejasonf
force-pushed
the
ricejasonf/p1061
branch
from
35086e7 to
36f8029
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/14879 Here is the relevant piece of the build log for the reference |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an implementation of P1061 Structure Bindings Introduce a Pack without the ability to use packs outside of templates. There is a couple of ways the AST could have been sliced so let me know what you think. The only part of this change that I am unsure of is the serialization/deserialization stuff. I followed the implementation of other Exprs, but I do not really know how it is tested. Thank you for your time considering this.