[Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr #125394
Conversation
This file contains hidden or 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 merges the functionality of ResolvedUnexpandedPackExpr into FunctionParmPackExpr. I also added a test to show that #125103 should be fixed with this. I put the removal of ResolvedUnexpandedPackExpr in its own commit. Let me know what you think. @cor3ntin Patch is 46.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125394.diff 29 Files Affected:
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 766821b4fb25cb5..1c630d616903550 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
/// decomposition declaration, and when the initializer is type-dependent.
Expr *getBinding() const { return Binding; }
- // Get the array of Exprs when the binding represents a pack.
- llvm::ArrayRef<Expr *> getBindingPackExprs() const;
+ // Get the array of nested BindingDecls when the binding represents a pack.
+ llvm::ArrayRef<BindingDecl *> getBindingPackDecls() const;
/// Get the decomposition declaration that this binding represents a
/// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
for (auto *B : Bindings) {
B->setDecomposedDecl(this);
if (B->isParameterPack() && B->getBinding()) {
- for (Expr *E : B->getBindingPackExprs()) {
- auto *DRE = cast<DeclRefExpr>(E);
- auto *NestedB = cast<BindingDecl>(DRE->getDecl());
- NestedB->setDecomposedDecl(this);
+ for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+ NestedBD->setDecomposedDecl(this);
}
}
}
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
// Provide a flattened range to visit each binding.
auto flat_bindings() const {
llvm::ArrayRef<BindingDecl *> Bindings = bindings();
- llvm::ArrayRef<Expr *> PackExprs;
+ llvm::ArrayRef<BindingDecl *> PackBindings;
// Split the bindings into subranges split by the pack.
- auto S1 = Bindings.take_until(
+ auto BeforePackBindings = Bindings.take_until(
[](BindingDecl *BD) { return BD->isParameterPack(); });
- Bindings = Bindings.drop_front(S1.size());
+ Bindings = Bindings.drop_front(BeforePackBindings.size());
if (!Bindings.empty()) {
- PackExprs = Bindings.front()->getBindingPackExprs();
+ PackBindings = Bindings.front()->getBindingPackDecls();
Bindings = Bindings.drop_front();
}
- auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
- auto *DRE = cast<DeclRefExpr>(E);
- return cast<BindingDecl>(DRE->getDecl());
- });
-
- return llvm::concat<BindingDecl *>(std::move(S1), std::move(S2),
- std::move(Bindings));
+ return llvm::concat<BindingDecl *const>(std::move(BeforePackBindings),
+ std::move(PackBindings),
+ std::move(Bindings));
}
void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 0b6c8cfb163c958..f6cf48cb66b801a 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
/// \endcode
class FunctionParmPackExpr final
: public Expr,
- private llvm::TrailingObjects<FunctionParmPackExpr, VarDecl *> {
+ private llvm::TrailingObjects<FunctionParmPackExpr, ValueDecl *> {
friend class ASTReader;
friend class ASTStmtReader;
friend TrailingObjects;
/// The function parameter pack which was referenced.
- VarDecl *ParamPack;
+ ValueDecl *ParamPack;
/// The location of the function parameter pack reference.
SourceLocation NameLoc;
@@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final
/// The number of expansions of this pack.
unsigned NumParameters;
- FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
- SourceLocation NameLoc, unsigned NumParams,
- VarDecl *const *Params);
+ FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation NameLoc,
+ unsigned NumParams, ValueDecl *const *Params);
public:
static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T,
- VarDecl *ParamPack,
+ ValueDecl *ParamPack,
SourceLocation NameLoc,
- ArrayRef<VarDecl *> Params);
+ ArrayRef<ValueDecl *> Params);
static FunctionParmPackExpr *CreateEmpty(const ASTContext &Context,
unsigned NumParams);
/// Get the parameter pack which this expression refers to.
- VarDecl *getParameterPack() const { return ParamPack; }
+ ValueDecl *getParameterPack() const { return ParamPack; }
/// Get the location of the parameter pack.
SourceLocation getParameterPackLocation() const { return NameLoc; }
/// Iterators over the parameters which the parameter pack expanded
/// into.
- using iterator = VarDecl * const *;
- iterator begin() const { return getTrailingObjects<VarDecl *>(); }
+ using iterator = ValueDecl *const *;
+ iterator begin() const { return getTrailingObjects<ValueDecl *>(); }
iterator end() const { return begin() + NumParameters; }
/// Get the number of parameters in this parameter pack.
unsigned getNumExpansions() const { return NumParameters; }
/// Get an expansion of the parameter pack by index.
- VarDecl *getExpansion(unsigned I) const { return begin()[I]; }
+ ValueDecl *getExpansion(unsigned I) const { return begin()[I]; }
SourceLocation getBeginLoc() const LLVM_READONLY { return NameLoc; }
SourceLocation getEndLoc() const LLVM_READONLY { return NameLoc; }
@@ -5319,59 +5318,6 @@ class BuiltinBitCastExpr final
}
};
-// Represents an unexpanded pack where the list of expressions are
-// known. These are used when structured bindings introduce a pack.
-class ResolvedUnexpandedPackExpr final
- : public Expr,
- private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Expr *> {
- friend class ASTStmtReader;
- friend class ASTStmtWriter;
- 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; }
-
- llvm::MutableArrayRef<Expr *> getExprs() {
- return {getTrailingObjects<Expr *>(), NumExprs};
- }
-
- llvm::ArrayRef<Expr *> getExprs() const {
- return {getTrailingObjects<Expr *>(), NumExprs};
- }
-
- Expr *getExpansion(unsigned Idx) { return getExprs()[Idx]; }
- Expr *getExpansion(unsigned Idx) const { return getExprs()[Idx]; }
-
- // Iterators
- child_range children() {
- return child_range((Stmt **)getTrailingObjects<Expr *>(),
- (Stmt **)getTrailingObjects<Expr *>() + 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 5f4c39b9cbdb75d..c4a1d03f1b3d107 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2950,7 +2950,6 @@ 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/StmtNodes.td b/clang/include/clang/Basic/StmtNodes.td
index 2fea05e322c7547..53fc77bbbcecc11 100644
--- a/clang/include/clang/Basic/StmtNodes.td
+++ b/clang/include/clang/Basic/StmtNodes.td
@@ -163,7 +163,6 @@ 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/Sema.h b/clang/include/clang/Sema/Sema.h
index 472a0e25adc9752..c6cc13eb13d441d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,8 +232,7 @@ 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 *,
- ResolvedUnexpandedPackExpr *>,
+typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
SourceLocation>
UnexpandedParameterPack;
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index 4206bd50b13dd6e..647c4cfa341e183 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -365,7 +365,7 @@ enum class TemplateSubstitutionKind : char {
class LocalInstantiationScope {
public:
/// A set of declarations.
- using DeclArgumentPack = SmallVector<VarDecl *, 4>;
+ using DeclArgumentPack = SmallVector<ValueDecl *, 4>;
private:
/// Reference to the semantic analysis that is performing
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 7656add0b6884cc..1b56ed2c9776b5e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1908,7 +1908,6 @@ 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/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index e394e0515e59938..91be7c8ccfaaea6 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -3492,10 +3492,13 @@ VarDecl *BindingDecl::getHoldingVar() const {
return VD;
}
-llvm::ArrayRef<Expr *> BindingDecl::getBindingPackExprs() const {
+llvm::ArrayRef<BindingDecl *> BindingDecl::getBindingPackDecls() const {
assert(Binding && "expecting a pack expr");
- auto *RP = cast<ResolvedUnexpandedPackExpr>(Binding);
- return RP->getExprs();
+ auto *FP = cast<FunctionParmPackExpr>(Binding);
+ ValueDecl *const *First = FP->getNumExpansions() > 0 ? FP->begin() : nullptr;
+ assert((!First || isa<BindingDecl>(*First)) && "expecting a BindingDecl");
+ return llvm::ArrayRef<BindingDecl *>((BindingDecl *const *)First,
+ FP->getNumExpansions());
}
void DecompositionDecl::anchor() {}
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 4fc62919fde94bf..06b04914426737c 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3659,7 +3659,6 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
case PackIndexingExprClass:
case HLSLOutArgExprClass:
case OpenACCAsteriskSizeExprClass:
- case ResolvedUnexpandedPackExprClass:
// These never have a side-effect.
return false;
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index d900af895b42a68..c8d61e2cf3f26cb 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1779,31 +1779,31 @@ TemplateArgument SubstNonTypeTemplateParmPackExpr::getArgumentPack() const {
return TemplateArgument(llvm::ArrayRef(Arguments, NumArguments));
}
-FunctionParmPackExpr::FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
+FunctionParmPackExpr::FunctionParmPackExpr(QualType T, ValueDecl *ParamPack,
SourceLocation NameLoc,
unsigned NumParams,
- VarDecl *const *Params)
+ ValueDecl *const *Params)
: Expr(FunctionParmPackExprClass, T, VK_LValue, OK_Ordinary),
ParamPack(ParamPack), NameLoc(NameLoc), NumParameters(NumParams) {
if (Params)
std::uninitialized_copy(Params, Params + NumParams,
- getTrailingObjects<VarDecl *>());
+ getTrailingObjects<ValueDecl *>());
setDependence(ExprDependence::TypeValueInstantiation |
ExprDependence::UnexpandedPack);
}
FunctionParmPackExpr *
FunctionParmPackExpr::Create(const ASTContext &Context, QualType T,
- VarDecl *ParamPack, SourceLocation NameLoc,
- ArrayRef<VarDecl *> Params) {
- return new (Context.Allocate(totalSizeToAlloc<VarDecl *>(Params.size())))
+ ValueDecl *ParamPack, SourceLocation NameLoc,
+ ArrayRef<ValueDecl *> Params) {
+ return new (Context.Allocate(totalSizeToAlloc<ValueDecl *>(Params.size())))
FunctionParmPackExpr(T, ParamPack, NameLoc, Params.size(), Params.data());
}
FunctionParmPackExpr *
FunctionParmPackExpr::CreateEmpty(const ASTContext &Context,
unsigned NumParams) {
- return new (Context.Allocate(totalSizeToAlloc<VarDecl *>(NumParams)))
+ return new (Context.Allocate(totalSizeToAlloc<ValueDecl *>(NumParams)))
FunctionParmPackExpr(QualType(), nullptr, SourceLocation(), 0, nullptr);
}
@@ -1965,52 +1965,3 @@ 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) {
- // C++ [temp.dep.expr]p3
- // An id-expression is type-dependent if it is
- // - associated by name lookup with a pack
- setDependence(ExprDependence::TypeValueInstantiation |
- ExprDependence::UnexpandedPack);
-}
-
-ResolvedUnexpandedPackExpr *
-ResolvedUnexpandedPackExpr::CreateDeserialized(ASTContext &Ctx,
- unsigned NumExprs) {
- void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(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<Expr *>(NumExprs),
- alignof(ResolvedUnexpandedPackExpr));
- ResolvedUnexpandedPackExpr *New =
- new (Mem) ResolvedUnexpandedPackExpr(BL, T, NumExprs);
-
- auto Exprs = New->getExprs();
- std::uninitialized_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::uninitialized_copy(Exprs.begin(), Exprs.end(), New->getExprs().begin());
- return New;
-}
-
-ResolvedUnexpandedPackExpr *ResolvedUnexpandedPackExpr::getFromDecl(Decl *D) {
- if (auto *BD = dyn_cast<BindingDecl>(D))
- return dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BD->getBinding());
- return nullptr;
-}
diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 5225c3ca773ad4f..3f37d06cc8f3a06 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -451,13 +451,6 @@ 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_LValue;
- }
-
case Expr::MaterializeTemporaryExprClass:
return cast<MaterializeTemporaryExpr>(E)->isBoundToLvalueReference()
? Cl::CL_LValue
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 37019b5235f5610..7d1074cc4d08e02 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -17253,7 +17253,6 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
case Expr::SYCLUniqueStableNameExprClass:
case Expr::CXXParenListInitExprClass:
case Expr::HLSLOutArgExprClass:
- case Expr::ResolvedUnexpandedPackExprClass:
return ICEDiag(IK_NotICE, E->getBeginLoc());
case Expr::InitListExprClass: {
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index e5eb22eae7dd139..4a090118c3d7b39 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -4932,8 +4932,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
case Expr::AtomicExprClass:
case Expr::SourceLocExprClass:
case Expr::EmbedExprClass:
- case Expr::BuiltinBitCastExprClass:
- case Expr::ResolvedUnexpandedPackExprClass: {
+ case Expr::BuiltinBitCastExprClass: {
NotPrimaryExpr();
if (!NullOut) {
// As bad as this diagnostic is, it's better than crashing.
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index d523abfe312848f..8c93596562663f5 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2595,15 +2595,6 @@ void StmtPrinter::VisitPackIndexingExpr(PackIndexingExpr *E) {
OS << "]";
}
-void StmtPrinter::VisitResolvedUnexpandedPackExpr(
- ResolvedUnexpandedPackExpr *E) {
- OS << "<<resolved pack(";
- llvm::interleave(
- E->getExprs().begin(), E->getExprs().end(),
- [this](auto *X) { PrintExpr(X); }, [this] { OS << ", "; });
- OS << ")>>";
-}
-
void StmtPrinter::VisitSubstNonTypeTemplateParmPackExpr(
SubstNonTypeTemplateParmPackExpr *Node) {
OS << *Node->getParameterPack();
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 84985fcb20ff9ee..5d1f370cac19f3a 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2280,10 +2280,6 @@ void StmtProfiler::VisitSizeOfPackExpr(const SizeOfPackExpr *S) {
ID.AddInteger(0);
}
}
-void StmtProfiler::VisitResolvedUnexpandedPackExpr(
- const ResolvedUnexpandedPackExpr *S) {
- VisitExpr(S);
-}
void StmtProfiler::VisitPackIndexingExpr(const PackIndexingExpr *E) {
VisitExpr(E);
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 0cf02fe6407c24c..c38a7ab97d70fd0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -980,24 +980,24 @@ static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
if (IsValid && HasPack) {
// Create the pack expr and assign it to the binding.
unsigned PackSize = MemberCount - Bindings.size() + 1;
- QualType PackType = S.Context.getPackExpansionType(
- S.Context.DependentTy, std::nullopt, /*ExpectsPackInType=*/false);
...
[truncated]
|
|
@llvm/pr-subscribers-clang-modules Author: Jason Rice (ricejasonf) ChangesThis merges the functionality of ResolvedUnexpandedPackExpr into FunctionParmPackExpr. I also added a test to show that #125103 should be fixed with this. I put the removal of ResolvedUnexpandedPackExpr in its own commit. Let me know what you think. @cor3ntin Patch is 46.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125394.diff 29 Files Affected:
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 766821b4fb25cb5..1c630d616903550 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
/// decomposition declaration, and when the initializer is type-dependent.
Expr *getBinding() const { return Binding; }
- // Get the array of Exprs when the binding represents a pack.
- llvm::ArrayRef<Expr *> getBindingPackExprs() const;
+ // Get the array of nested BindingDecls when the binding represents a pack.
+ llvm::ArrayRef<BindingDecl *> getBindingPackDecls() const;
/// Get the decomposition declaration that this binding represents a
/// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
for (auto *B : Bindings) {
B->setDecomposedDecl(this);
if (B->isParameterPack() && B->getBinding()) {
- for (Expr *E : B->getBindingPackExprs()) {
- auto *DRE = cast<DeclRefExpr>(E);
- auto *NestedB = cast<BindingDecl>(DRE->getDecl());
- NestedB->setDecomposedDecl(this);
+ for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+ NestedBD->setDecomposedDecl(this);
}
}
}
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
// Provide a flattened range to visit each binding.
auto flat_bindings() const {
llvm::ArrayRef<BindingDecl *> Bindings = bindings();
- llvm::ArrayRef<Expr *> PackExprs;
+ llvm::ArrayRef<BindingDecl *> PackBindings;
// Split the bindings into subranges split by the pack.
- auto S1 = Bindings.take_until(
+ auto BeforePackBindings = Bindings.take_until(
[](BindingDecl *BD) { return BD->isParameterPack(); });
- Bindings = Bindings.drop_front(S1.size());
+ Bindings = Bindings.drop_front(BeforePackBindings.size());
if (!Bindings.empty()) {
- PackExprs = Bindings.front()->getBindingPackExprs();
+ PackBindings = Bindings.front()->getBindingPackDecls();
Bindings = Bindings.drop_front();
}
- auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
- auto *DRE = cast<DeclRefExpr>(E);
- return cast<BindingDecl>(DRE->getDecl());
- });
-
- return llvm::concat<BindingDecl *>(std::move(S1), std::move(S2),
- std::move(Bindings));
+ return llvm::concat<BindingDecl *const>(std::move(BeforePackBindings),
+ std::move(PackBindings),
+ std::move(Bindings));
}
void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 0b6c8cfb163c958..f6cf48cb66b801a 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
/// \endcode
class FunctionParmPackExpr final
: public Expr,
- private llvm::TrailingObjects<FunctionParmPackExpr, VarDecl *> {
+ private llvm::TrailingObjects<FunctionParmPackExpr, ValueDecl *> {
friend class ASTReader;
friend class ASTStmtReader;
friend TrailingObjects;
/// The function parameter pack which was referenced.
- VarDecl *ParamPack;
+ ValueDecl *ParamPack;
/// The location of the function parameter pack reference.
SourceLocation NameLoc;
@@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final
/// The number of expansions of this pack.
unsigned NumParameters;
- FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
- SourceLocation NameLoc, unsigned NumParams,
- VarDecl *const *Params);
+ FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation NameLoc,
+ unsigned NumParams, ValueDecl *const *Params);
public:
static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T,
- VarDecl *ParamPack,
+ ValueDecl *ParamPack,
SourceLocation NameLoc,
- ArrayRef<VarDecl *> Params);
+ ArrayRef<ValueDecl *> Params);
static FunctionParmPackExpr *CreateEmpty(const ASTContext &Context,
unsigned NumParams);
/// Get the parameter pack which this expression refers to.
- VarDecl *getParameterPack() const { return ParamPack; }
+ ValueDecl *getParameterPack() const { return ParamPack; }
/// Get the location of the parameter pack.
SourceLocation getParameterPackLocation() const { return NameLoc; }
/// Iterators over the parameters which the parameter pack expanded
/// into.
- using iterator = VarDecl * const *;
- iterator begin() const { return getTrailingObjects<VarDecl *>(); }
+ using iterator = ValueDecl *const *;
+ iterator begin() const { return getTrailingObjects<ValueDecl *>(); }
iterator end() const { return begin() + NumParameters; }
/// Get the number of parameters in this parameter pack.
unsigned getNumExpansions() const { return NumParameters; }
/// Get an expansion of the parameter pack by index.
- VarDecl *getExpansion(unsigned I) const { return begin()[I]; }
+ ValueDecl *getExpansion(unsigned I) const { return begin()[I]; }
SourceLocation getBeginLoc() const LLVM_READONLY { return NameLoc; }
SourceLocation getEndLoc() const LLVM_READONLY { return NameLoc; }
@@ -5319,59 +5318,6 @@ class BuiltinBitCastExpr final
}
};
-// Represents an unexpanded pack where the list of expressions are
-// known. These are used when structured bindings introduce a pack.
-class ResolvedUnexpandedPackExpr final
- : public Expr,
- private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Expr *> {
- friend class ASTStmtReader;
- friend class ASTStmtWriter;
- 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; }
-
- llvm::MutableArrayRef<Expr *> getExprs() {
- return {getTrailingObjects<Expr *>(), NumExprs};
- }
-
- llvm::ArrayRef<Expr *> getExprs() const {
- return {getTrailingObjects<Expr *>(), NumExprs};
- }
-
- Expr *getExpansion(unsigned Idx) { return getExprs()[Idx]; }
- Expr *getExpansion(unsigned Idx) const { return getExprs()[Idx]; }
-
- // Iterators
- child_range children() {
- return child_range((Stmt **)getTrailingObjects<Expr *>(),
- (Stmt **)getTrailingObjects<Expr *>() + 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 5f4c39b9cbdb75d..c4a1d03f1b3d107 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2950,7 +2950,6 @@ 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/StmtNodes.td b/clang/include/clang/Basic/StmtNodes.td
index 2fea05e322c7547..53fc77bbbcecc11 100644
--- a/clang/include/clang/Basic/StmtNodes.td
+++ b/clang/include/clang/Basic/StmtNodes.td
@@ -163,7 +163,6 @@ 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/Sema.h b/clang/include/clang/Sema/Sema.h
index 472a0e25adc9752..c6cc13eb13d441d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,8 +232,7 @@ 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 *,
- ResolvedUnexpandedPackExpr *>,
+typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
SourceLocation>
UnexpandedParameterPack;
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index 4206bd50b13dd6e..647c4cfa341e183 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -365,7 +365,7 @@ enum class TemplateSubstitutionKind : char {
class LocalInstantiationScope {
public:
/// A set of declarations.
- using DeclArgumentPack = SmallVector<VarDecl *, 4>;
+ using DeclArgumentPack = SmallVector<ValueDecl *, 4>;
private:
/// Reference to the semantic analysis that is performing
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 7656add0b6884cc..1b56ed2c9776b5e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1908,7 +1908,6 @@ 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/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index e394e0515e59938..91be7c8ccfaaea6 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -3492,10 +3492,13 @@ VarDecl *BindingDecl::getHoldingVar() const {
return VD;
}
-llvm::ArrayRef<Expr *> BindingDecl::getBindingPackExprs() const {
+llvm::ArrayRef<BindingDecl *> BindingDecl::getBindingPackDecls() const {
assert(Binding && "expecting a pack expr");
- auto *RP = cast<ResolvedUnexpandedPackExpr>(Binding);
- return RP->getExprs();
+ auto *FP = cast<FunctionParmPackExpr>(Binding);
+ ValueDecl *const *First = FP->getNumExpansions() > 0 ? FP->begin() : nullptr;
+ assert((!First || isa<BindingDecl>(*First)) && "expecting a BindingDecl");
+ return llvm::ArrayRef<BindingDecl *>((BindingDecl *const *)First,
+ FP->getNumExpansions());
}
void DecompositionDecl::anchor() {}
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 4fc62919fde94bf..06b04914426737c 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3659,7 +3659,6 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
case PackIndexingExprClass:
case HLSLOutArgExprClass:
case OpenACCAsteriskSizeExprClass:
- case ResolvedUnexpandedPackExprClass:
// These never have a side-effect.
return false;
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index d900af895b42a68..c8d61e2cf3f26cb 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1779,31 +1779,31 @@ TemplateArgument SubstNonTypeTemplateParmPackExpr::getArgumentPack() const {
return TemplateArgument(llvm::ArrayRef(Arguments, NumArguments));
}
-FunctionParmPackExpr::FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
+FunctionParmPackExpr::FunctionParmPackExpr(QualType T, ValueDecl *ParamPack,
SourceLocation NameLoc,
unsigned NumParams,
- VarDecl *const *Params)
+ ValueDecl *const *Params)
: Expr(FunctionParmPackExprClass, T, VK_LValue, OK_Ordinary),
ParamPack(ParamPack), NameLoc(NameLoc), NumParameters(NumParams) {
if (Params)
std::uninitialized_copy(Params, Params + NumParams,
- getTrailingObjects<VarDecl *>());
+ getTrailingObjects<ValueDecl *>());
setDependence(ExprDependence::TypeValueInstantiation |
ExprDependence::UnexpandedPack);
}
FunctionParmPackExpr *
FunctionParmPackExpr::Create(const ASTContext &Context, QualType T,
- VarDecl *ParamPack, SourceLocation NameLoc,
- ArrayRef<VarDecl *> Params) {
- return new (Context.Allocate(totalSizeToAlloc<VarDecl *>(Params.size())))
+ ValueDecl *ParamPack, SourceLocation NameLoc,
+ ArrayRef<ValueDecl *> Params) {
+ return new (Context.Allocate(totalSizeToAlloc<ValueDecl *>(Params.size())))
FunctionParmPackExpr(T, ParamPack, NameLoc, Params.size(), Params.data());
}
FunctionParmPackExpr *
FunctionParmPackExpr::CreateEmpty(const ASTContext &Context,
unsigned NumParams) {
- return new (Context.Allocate(totalSizeToAlloc<VarDecl *>(NumParams)))
+ return new (Context.Allocate(totalSizeToAlloc<ValueDecl *>(NumParams)))
FunctionParmPackExpr(QualType(), nullptr, SourceLocation(), 0, nullptr);
}
@@ -1965,52 +1965,3 @@ 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) {
- // C++ [temp.dep.expr]p3
- // An id-expression is type-dependent if it is
- // - associated by name lookup with a pack
- setDependence(ExprDependence::TypeValueInstantiation |
- ExprDependence::UnexpandedPack);
-}
-
-ResolvedUnexpandedPackExpr *
-ResolvedUnexpandedPackExpr::CreateDeserialized(ASTContext &Ctx,
- unsigned NumExprs) {
- void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(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<Expr *>(NumExprs),
- alignof(ResolvedUnexpandedPackExpr));
- ResolvedUnexpandedPackExpr *New =
- new (Mem) ResolvedUnexpandedPackExpr(BL, T, NumExprs);
-
- auto Exprs = New->getExprs();
- std::uninitialized_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::uninitialized_copy(Exprs.begin(), Exprs.end(), New->getExprs().begin());
- return New;
-}
-
-ResolvedUnexpandedPackExpr *ResolvedUnexpandedPackExpr::getFromDecl(Decl *D) {
- if (auto *BD = dyn_cast<BindingDecl>(D))
- return dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BD->getBinding());
- return nullptr;
-}
diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 5225c3ca773ad4f..3f37d06cc8f3a06 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -451,13 +451,6 @@ 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_LValue;
- }
-
case Expr::MaterializeTemporaryExprClass:
return cast<MaterializeTemporaryExpr>(E)->isBoundToLvalueReference()
? Cl::CL_LValue
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 37019b5235f5610..7d1074cc4d08e02 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -17253,7 +17253,6 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
case Expr::SYCLUniqueStableNameExprClass:
case Expr::CXXParenListInitExprClass:
case Expr::HLSLOutArgExprClass:
- case Expr::ResolvedUnexpandedPackExprClass:
return ICEDiag(IK_NotICE, E->getBeginLoc());
case Expr::InitListExprClass: {
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index e5eb22eae7dd139..4a090118c3d7b39 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -4932,8 +4932,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
case Expr::AtomicExprClass:
case Expr::SourceLocExprClass:
case Expr::EmbedExprClass:
- case Expr::BuiltinBitCastExprClass:
- case Expr::ResolvedUnexpandedPackExprClass: {
+ case Expr::BuiltinBitCastExprClass: {
NotPrimaryExpr();
if (!NullOut) {
// As bad as this diagnostic is, it's better than crashing.
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index d523abfe312848f..8c93596562663f5 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2595,15 +2595,6 @@ void StmtPrinter::VisitPackIndexingExpr(PackIndexingExpr *E) {
OS << "]";
}
-void StmtPrinter::VisitResolvedUnexpandedPackExpr(
- ResolvedUnexpandedPackExpr *E) {
- OS << "<<resolved pack(";
- llvm::interleave(
- E->getExprs().begin(), E->getExprs().end(),
- [this](auto *X) { PrintExpr(X); }, [this] { OS << ", "; });
- OS << ")>>";
-}
-
void StmtPrinter::VisitSubstNonTypeTemplateParmPackExpr(
SubstNonTypeTemplateParmPackExpr *Node) {
OS << *Node->getParameterPack();
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 84985fcb20ff9ee..5d1f370cac19f3a 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2280,10 +2280,6 @@ void StmtProfiler::VisitSizeOfPackExpr(const SizeOfPackExpr *S) {
ID.AddInteger(0);
}
}
-void StmtProfiler::VisitResolvedUnexpandedPackExpr(
- const ResolvedUnexpandedPackExpr *S) {
- VisitExpr(S);
-}
void StmtProfiler::VisitPackIndexingExpr(const PackIndexingExpr *E) {
VisitExpr(E);
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 0cf02fe6407c24c..c38a7ab97d70fd0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -980,24 +980,24 @@ static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
if (IsValid && HasPack) {
// Create the pack expr and assign it to the binding.
unsigned PackSize = MemberCount - Bindings.size() + 1;
- QualType PackType = S.Context.getPackExpansionType(
- S.Context.DependentTy, std::nullopt, /*ExpectsPackInType=*/false);
...
[truncated]
|
cor3ntin
reviewed
Feb 2, 2025
|
Can you test if that also fixes #125165? |
|
@Sirraide I did try it and the bug persists. The |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ricejasonf
force-pushed
the
ricejasonf/pack_ice
branch
from
b2b85fd to
96b0096
Compare
|
@cor3ntin Sorry, I tried "Commit suggestion", but it didn't look like you can cast like that so I just reverted. |
|
@ricejasonf yeah... really not a fan but i played with it an i don't see a way to meaningfully improve it. Thanks for trying though |
cor3ntin
approved these changes
Feb 3, 2025
Yeah, that one did look like the problem is probably elsewhere; thanks for trying though! |
erichkeane
reviewed
Feb 3, 2025
clang/include/clang/AST/DeclCXX.h
Outdated
|
|
||
| // Split the bindings into subranges split by the pack. | ||
| auto S1 = Bindings.take_until( | ||
| auto BeforePackBindings = Bindings.take_until( |
There was a problem hiding this comment.
Suggested change
| auto BeforePackBindings = Bindings.take_until( | |
| llvm::ArrayRef<BindingDecl*> BeforePackBindings = Bindings.take_until( |
There was a problem hiding this comment.
I still very much dont' want the "C" cast, but otherwise i'm ok with this.
| SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldNestedBD, | ||
| NewNestedBD); | ||
| } | ||
| auto OldDecls = OldBindingPack->getBindingPackDecls(); |
There was a problem hiding this comment.
We shouldn't be using 'auto' on either of these. I realize they already were, but please change them anyway.,
2 tasks
cor3ntin
approved these changes
Feb 4, 2025
|
@ricejasonf Can i merge this for you? |
@cor3ntin Whenever you think it is ready, yes please. I know Erich did not want the C-style cast. |
Yep, I have a visceral reaction to C-style casts, whatever can be done to remove it, then I'm happy. |
shafik
reviewed
Feb 4, 2025
| int a, b, c; | ||
| }; | ||
|
|
||
| auto X = [] <typename = void> () { |
There was a problem hiding this comment.
I feel like there are a bunch of varieties on this case we can test as well: https://godbolt.org/z/r9zxxfoW5
There was a problem hiding this comment.
Oh, sorry, I added your test with clsss not realizing you were talking about modifying an existing test. 😅
(err.. maybe that is what you meant)
zyn0217
reviewed
Feb 6, 2025
Comment on lines
4650
to
+4652
| class FunctionParmPackExpr final | ||
| : public Expr, | ||
| private llvm::TrailingObjects<FunctionParmPackExpr, VarDecl *> { | ||
| private llvm::TrailingObjects<FunctionParmPackExpr, ValueDecl *> { |
There was a problem hiding this comment.
Can we also update the attached comments to reflect it now supports structure binding pack?
Comment on lines
+3500
to
+3501
| return llvm::ArrayRef<BindingDecl *>( | ||
| reinterpret_cast<BindingDecl *const *>(First), FP->getNumExpansions()); |
There was a problem hiding this comment.
can we use llvm::cast here?
There was a problem hiding this comment.
llvm::cast will not return a reference to the pointer so it cannot be used for the ArrayRef.
There was a problem hiding this comment.
Can we use static_cast here?
clang/lib/Sema/SemaDeclCXX.cpp
Outdated
| auto *NestedBD = BindingDecl::Create(S.Context, BPack->getDeclContext(), | ||
| BPack->getLocation(), | ||
| BPack->getIdentifier(), QualType()); | ||
| for (ValueDecl *&VD : NestedBDs) { |
There was a problem hiding this comment.
Maybe we can make it more straightforward like
for (unsigned I = 0; I < PackSize; ++I) {
...
NestedBDs[I] = ...
}I feel like *& makes it a little visually awkward :)
| dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BindingExpr)) { | ||
| assert(!OldResolvedPack && "no more than one pack is allowed"); | ||
| OldResolvedPack = RP; | ||
| if (isa_and_nonnull<FunctionParmPackExpr>(BindingExpr)) { |
Comment on lines
+1203
to
1204
| BindingDecl *NewBindingPack = *llvm::find_if( | ||
| Bindings, [](BindingDecl *D) -> bool { return D->isParameterPack(); }); |
There was a problem hiding this comment.
For sanity (we have lost the assertion of casting a non-null pointer), we'd better add an non-null assertion here
There was a problem hiding this comment.
What do you think about capturing the index of the old pack in the loop above and using that instead of searching?
wldfngrs
pushed a commit
to wldfngrs/llvm-project
that referenced
this pull request
Feb 19, 2025
…rmPackExpr (llvm#125394) This merges the functionality of ResolvedUnexpandedPackExpr into FunctionParmPackExpr. I also added a test to show that llvm#125103 should be fixed with this. I put the removal of ResolvedUnexpandedPackExpr in its own commit. Let me know what you think. Fixes llvm#125103
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 merges the functionality of ResolvedUnexpandedPackExpr into FunctionParmPackExpr. I also added a test to show that #125103 should be fixed with this. I put the removal of ResolvedUnexpandedPackExpr in its own commit. Let me know what you think.
Fixes #125103