[clang] [Clang][Sema] Avoid pack expansion for expanded empty PackIndexingExprs (PR #92385)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Mon May 20 07:39:25 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/3] [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/3] 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;
>From e95fe54c31fdb871588bca726363c961b46d22d3 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Mon, 20 May 2024 22:38:20 +0800
Subject: [PATCH 3/3] Say 'ExpandedToEmptyPack'
---
clang/include/clang/AST/ExprCXX.h | 11 ++++++-----
clang/lib/AST/ExprCXX.cpp | 11 +++++------
clang/lib/Sema/TreeTransform.h | 4 ++--
clang/lib/Serialization/ASTReaderStmt.cpp | 2 +-
clang/lib/Serialization/ASTWriterStmt.cpp | 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 18c00dfbf5a34..dbf693611a7fa 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4381,16 +4381,17 @@ class PackIndexingExpr final
unsigned TransformedExpressions : 31;
LLVM_PREFERRED_TYPE(bool)
- unsigned EmptyPack : 1;
+ unsigned ExpandedToEmptyPack : 1;
PackIndexingExpr(QualType Type, SourceLocation EllipsisLoc,
SourceLocation RSquareLoc, Expr *PackIdExpr, Expr *IndexExpr,
ArrayRef<Expr *> SubstitutedExprs = {},
- bool EmptyPack = false)
+ bool ExpandedToEmptyPack = false)
: Expr(PackIndexingExprClass, Type, VK_LValue, OK_Ordinary),
EllipsisLoc(EllipsisLoc), RSquareLoc(RSquareLoc),
SubExprs{PackIdExpr, IndexExpr},
- TransformedExpressions(SubstitutedExprs.size()), EmptyPack(EmptyPack) {
+ TransformedExpressions(SubstitutedExprs.size()),
+ ExpandedToEmptyPack(ExpandedToEmptyPack) {
auto *Exprs = getTrailingObjects<Expr *>();
std::uninitialized_copy(SubstitutedExprs.begin(), SubstitutedExprs.end(),
@@ -4414,12 +4415,12 @@ class PackIndexingExpr final
SourceLocation RSquareLoc, Expr *PackIdExpr,
Expr *IndexExpr, std::optional<int64_t> Index,
ArrayRef<Expr *> SubstitutedExprs = {},
- bool EmptyPack = false);
+ bool ExpandedToEmptyPack = false);
static PackIndexingExpr *CreateDeserialized(ASTContext &Context,
unsigned NumTransformedExprs);
/// Determine if the expression was expanded to empty.
- bool isEmptyPack() const { return EmptyPack; }
+ bool expandsToEmptyPack() const { return ExpandedToEmptyPack; }
/// Determine the location of the 'sizeof' keyword.
SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 01cdd2709b472..2abc0acbfde3b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1665,11 +1665,10 @@ 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, bool EmptyPack) {
+PackIndexingExpr *PackIndexingExpr::Create(
+ ASTContext &Context, SourceLocation EllipsisLoc, SourceLocation RSquareLoc,
+ Expr *PackIdExpr, Expr *IndexExpr, std::optional<int64_t> Index,
+ ArrayRef<Expr *> SubstitutedExprs, bool ExpandedToEmptyPack) {
QualType Type;
if (Index && !SubstitutedExprs.empty())
Type = SubstitutedExprs[*Index]->getType();
@@ -1680,7 +1679,7 @@ PackIndexingExpr::Create(ASTContext &Context, SourceLocation EllipsisLoc,
Context.Allocate(totalSizeToAlloc<Expr *>(SubstitutedExprs.size()));
return new (Storage)
PackIndexingExpr(Type, EllipsisLoc, RSquareLoc, PackIdExpr, IndexExpr,
- SubstitutedExprs, EmptyPack);
+ SubstitutedExprs, ExpandedToEmptyPack);
}
NamedDecl *PackIndexingExpr::getPackDecl() const {
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7a560d1cbb32a..a0fa4fba311cf 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->isEmptyPack() && E->getExpressions().empty()) {
+ if (!E->expandsToEmptyPack() && E->getExpressions().empty()) {
Expr *Pattern = E->getPackIdExpression();
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
getSema().collectUnexpandedParameterPacks(E->getPackIdExpression(),
@@ -15029,7 +15029,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
return true;
ExpandedExprs.push_back(Out.get());
}
- } else if (!E->isEmptyPack()) {
+ } else if (!E->expandsToEmptyPack()) {
if (getDerived().TransformExprs(E->getExpressions().data(),
E->getExpressions().size(), false,
ExpandedExprs))
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index bf3ad830c986b..eac4faff28549 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2177,7 +2177,7 @@ void ASTStmtReader::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
void ASTStmtReader::VisitPackIndexingExpr(PackIndexingExpr *E) {
VisitExpr(E);
E->TransformedExpressions = Record.readInt();
- E->EmptyPack = Record.readInt();
+ E->ExpandedToEmptyPack = 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 bdeaed0cef4f0..ef6cef40d87e3 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -2157,7 +2157,7 @@ void ASTStmtWriter::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
void ASTStmtWriter::VisitPackIndexingExpr(PackIndexingExpr *E) {
VisitExpr(E);
Record.push_back(E->TransformedExpressions);
- Record.push_back(E->EmptyPack);
+ Record.push_back(E->ExpandedToEmptyPack);
Record.AddSourceLocation(E->getEllipsisLoc());
Record.AddSourceLocation(E->getRSquareLoc());
Record.AddStmt(E->getPackIdExpression());
More information about the cfe-commits
mailing list