https://github.com/zyn0217 created https://github.com/llvm/llvm-project/pull/92385
We previously doubled the id-expression expansion, even when the pack was expanded to empty. The previous condition for determining whether we should expand couldn't distinguish between cases where 'the expansion was previously postponed' and 'the expansion occurred but resulted in emptiness.' In the latter scenario, we crash because we have not been examining the current lambda's parent local instantiation scope since [D98068](https://reviews.llvm.org/D98068): Any Decls instantiated in the parent scope are not visible to the generic lambda, and thus any attempt of looking for instantiated Decls in the lambda is capped to the current Lambda's LIS. Fixes https://github.com/llvm/llvm-project/issues/92230 >From 7acbb1dd89dbe266c3e53ab30178ac570722c759 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Thu, 16 May 2024 19:06:25 +0800 Subject: [PATCH] [Clang][Sema] Avoid pack expansion for expanded empty PackIndexingExprs We previously doubled the id-expression expansion, even when the pack was expanded to empty. The previous condition for determining whether we should expand couldn't distinguish between cases where 'the expansion was previously postponed' and 'the expansion occurred but resulted in emptiness.' In the latter scenario, we crash because we have not been examining the current lambda's parent local instantiation scope since D98068: Any Decls instantiated in the parent scope are not visible to the generic lambda. And thus any attempt of looking for instantiated Decls in the lambda is capped to the current Lambda's LIS. Fixes https://github.com/llvm/llvm-project/issues/92230 --- clang/include/clang/AST/ExprCXX.h | 14 ++++++++++---- clang/lib/AST/ExprCXX.cpp | 16 ++++++++-------- clang/lib/Sema/SemaTemplateVariadic.cpp | 2 +- clang/lib/Sema/TreeTransform.h | 6 ++---- clang/test/SemaCXX/cxx2c-pack-indexing.cpp | 8 ++++++-- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index fac65628ffede..2617cd36d0df9 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -4381,11 +4381,13 @@ class PackIndexingExpr final PackIndexingExpr(QualType Type, SourceLocation EllipsisLoc, SourceLocation RSquareLoc, Expr *PackIdExpr, Expr *IndexExpr, - ArrayRef<Expr *> SubstitutedExprs = {}) + ArrayRef<Expr *> SubstitutedExprs = {}, + bool EmptyPack = false) : Expr(PackIndexingExprClass, Type, VK_LValue, OK_Ordinary), EllipsisLoc(EllipsisLoc), RSquareLoc(RSquareLoc), SubExprs{PackIdExpr, IndexExpr}, - TransformedExpressions(SubstitutedExprs.size()) { + TransformedExpressions(EmptyPack ? size_t(-1) + : SubstitutedExprs.size()) { auto *Exprs = getTrailingObjects<Expr *>(); std::uninitialized_copy(SubstitutedExprs.begin(), SubstitutedExprs.end(), @@ -4408,10 +4410,13 @@ class PackIndexingExpr final SourceLocation EllipsisLoc, SourceLocation RSquareLoc, Expr *PackIdExpr, Expr *IndexExpr, std::optional<int64_t> Index, - ArrayRef<Expr *> SubstitutedExprs = {}); + ArrayRef<Expr *> SubstitutedExprs = {}, + bool EmptyPack = false); static PackIndexingExpr *CreateDeserialized(ASTContext &Context, unsigned NumTransformedExprs); + bool isEmptyPack() const { return TransformedExpressions == size_t(-1); } + /// Determine the location of the 'sizeof' keyword. SourceLocation getEllipsisLoc() const { return EllipsisLoc; } @@ -4446,7 +4451,8 @@ class PackIndexingExpr final } ArrayRef<Expr *> getExpressions() const { - return {getTrailingObjects<Expr *>(), TransformedExpressions}; + return {getTrailingObjects<Expr *>(), + isEmptyPack() ? 0 : TransformedExpressions}; } static bool classof(const Stmt *T) { diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp index 7e9343271ac3c..01cdd2709b472 100644 --- a/clang/lib/AST/ExprCXX.cpp +++ b/clang/lib/AST/ExprCXX.cpp @@ -1665,12 +1665,11 @@ NonTypeTemplateParmDecl *SubstNonTypeTemplateParmExpr::getParameter() const { getReplacedTemplateParameterList(getAssociatedDecl())->asArray()[Index]); } -PackIndexingExpr *PackIndexingExpr::Create(ASTContext &Context, - SourceLocation EllipsisLoc, - SourceLocation RSquareLoc, - Expr *PackIdExpr, Expr *IndexExpr, - std::optional<int64_t> Index, - ArrayRef<Expr *> SubstitutedExprs) { +PackIndexingExpr * +PackIndexingExpr::Create(ASTContext &Context, SourceLocation EllipsisLoc, + SourceLocation RSquareLoc, Expr *PackIdExpr, + Expr *IndexExpr, std::optional<int64_t> Index, + ArrayRef<Expr *> SubstitutedExprs, bool EmptyPack) { QualType Type; if (Index && !SubstitutedExprs.empty()) Type = SubstitutedExprs[*Index]->getType(); @@ -1679,8 +1678,9 @@ PackIndexingExpr *PackIndexingExpr::Create(ASTContext &Context, void *Storage = Context.Allocate(totalSizeToAlloc<Expr *>(SubstitutedExprs.size())); - return new (Storage) PackIndexingExpr( - Type, EllipsisLoc, RSquareLoc, PackIdExpr, IndexExpr, SubstitutedExprs); + return new (Storage) + PackIndexingExpr(Type, EllipsisLoc, RSquareLoc, PackIdExpr, IndexExpr, + SubstitutedExprs, EmptyPack); } NamedDecl *PackIndexingExpr::getPackDecl() const { diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp index a4b681ae4f008..0b20604665068 100644 --- a/clang/lib/Sema/SemaTemplateVariadic.cpp +++ b/clang/lib/Sema/SemaTemplateVariadic.cpp @@ -1128,7 +1128,7 @@ Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc, return PackIndexingExpr::Create(getASTContext(), EllipsisLoc, RSquareLoc, PackExpression, IndexExpr, Index, - ExpandedExprs); + ExpandedExprs, EmptyPack); } TemplateArgumentLoc Sema::getTemplateArgumentPackExpansionPattern( diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index c039b95293af2..7a560d1cbb32a 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -14975,7 +14975,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) { return ExprError(); SmallVector<Expr *, 5> ExpandedExprs; - if (E->getExpressions().empty()) { + if (!E->isEmptyPack() && E->getExpressions().empty()) { Expr *Pattern = E->getPackIdExpression(); SmallVector<UnexpandedParameterPack, 2> Unexpanded; getSema().collectUnexpandedParameterPacks(E->getPackIdExpression(), @@ -15029,9 +15029,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) { return true; ExpandedExprs.push_back(Out.get()); } - } - - else { + } else if (!E->isEmptyPack()) { if (getDerived().TransformExprs(E->getExpressions().data(), E->getExpressions().size(), false, ExpandedExprs)) diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp index 0ac85b5bcc14b..28b9765127f4e 100644 --- a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp +++ b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp @@ -206,13 +206,17 @@ void test(auto...args){ template<int... args> void test2(){ [&]<int idx>(){ - using R = decltype( args...[idx] ) ; - }.template operator()<0>(); + using R = decltype( args...[idx] ) ; // #test2-R + }.template operator()<0>(); // #test2-call } void f( ) { test(1); test2<1>(); + test2(); + // expected-error@#test2-R {{invalid index 0 for pack args of size 0}} + // expected-note@#test2-call {{requested here}} + // expected-note@-3 {{requested here}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
