[clang] [Clang][Sema] Avoid pack expansion for expanded empty PackIndexingExprs (PR #92385)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Fri May 17 01:00:15 PDT 2024
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/92385
>From 7acbb1dd89dbe266c3e53ab30178ac570722c759 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 16 May 2024 19:06:25 +0800
Subject: [PATCH 1/2] [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 at -3 {{requested here}}
}
>From afa1de4d01ef4b8471e192a2e20dec2fd9a0bb5e Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 17 May 2024 15:58:43 +0800
Subject: [PATCH 2/2] Address feedback. Bit fields & some correction
---
clang/include/clang/AST/ExprCXX.h | 16 ++++++++++------
clang/lib/Serialization/ASTReaderStmt.cpp | 1 +
clang/lib/Serialization/ASTWriterStmt.cpp | 2 +-
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 2617cd36d0df9..18c00dfbf5a34 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4377,7 +4377,11 @@ class PackIndexingExpr final
// The pack being indexed, followed by the index
Stmt *SubExprs[2];
- size_t TransformedExpressions;
+ // The size of the trailing expressions.
+ unsigned TransformedExpressions : 31;
+
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned EmptyPack : 1;
PackIndexingExpr(QualType Type, SourceLocation EllipsisLoc,
SourceLocation RSquareLoc, Expr *PackIdExpr, Expr *IndexExpr,
@@ -4386,8 +4390,7 @@ class PackIndexingExpr final
: Expr(PackIndexingExprClass, Type, VK_LValue, OK_Ordinary),
EllipsisLoc(EllipsisLoc), RSquareLoc(RSquareLoc),
SubExprs{PackIdExpr, IndexExpr},
- TransformedExpressions(EmptyPack ? size_t(-1)
- : SubstitutedExprs.size()) {
+ TransformedExpressions(SubstitutedExprs.size()), EmptyPack(EmptyPack) {
auto *Exprs = getTrailingObjects<Expr *>();
std::uninitialized_copy(SubstitutedExprs.begin(), SubstitutedExprs.end(),
@@ -4415,7 +4418,8 @@ class PackIndexingExpr final
static PackIndexingExpr *CreateDeserialized(ASTContext &Context,
unsigned NumTransformedExprs);
- bool isEmptyPack() const { return TransformedExpressions == size_t(-1); }
+ /// Determine if the expression was expanded to empty.
+ bool isEmptyPack() const { return EmptyPack; }
/// Determine the location of the 'sizeof' keyword.
SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
@@ -4450,9 +4454,9 @@ class PackIndexingExpr final
return getTrailingObjects<Expr *>()[*Index];
}
+ /// Return the trailing expressions, regardless of the expansion.
ArrayRef<Expr *> getExpressions() const {
- return {getTrailingObjects<Expr *>(),
- isEmptyPack() ? 0 : TransformedExpressions};
+ return {getTrailingObjects<Expr *>(), TransformedExpressions};
}
static bool classof(const Stmt *T) {
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 7d3930022a69c..bf3ad830c986b 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2177,6 +2177,7 @@ void ASTStmtReader::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
void ASTStmtReader::VisitPackIndexingExpr(PackIndexingExpr *E) {
VisitExpr(E);
E->TransformedExpressions = Record.readInt();
+ E->EmptyPack = Record.readInt();
E->EllipsisLoc = readSourceLocation();
E->RSquareLoc = readSourceLocation();
E->SubExprs[0] = Record.readStmt();
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 39aec31b6d879..bdeaed0cef4f0 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -2157,11 +2157,11 @@ void ASTStmtWriter::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
void ASTStmtWriter::VisitPackIndexingExpr(PackIndexingExpr *E) {
VisitExpr(E);
Record.push_back(E->TransformedExpressions);
+ Record.push_back(E->EmptyPack);
Record.AddSourceLocation(E->getEllipsisLoc());
Record.AddSourceLocation(E->getRSquareLoc());
Record.AddStmt(E->getPackIdExpression());
Record.AddStmt(E->getIndexExpr());
- Record.push_back(E->TransformedExpressions);
for (Expr *Sub : E->getExpressions())
Record.AddStmt(Sub);
Code = serialization::EXPR_PACK_INDEXING;
More information about the cfe-commits
mailing list