[clang] 8ce2045 - [Clang][Sema] Avoid pack expansion for expanded empty PackIndexingExprs (#92385)

via cfe-commits cfe-commits at lists.llvm.org
Mon May 20 18:47:09 PDT 2024


Author: Younan Zhang
Date: 2024-05-21T09:47:05+08:00
New Revision: 8ce2045be0ce708af0bfce5dc14632fa15dc743a

URL: https://github.com/llvm/llvm-project/commit/8ce2045be0ce708af0bfce5dc14632fa15dc743a
DIFF: https://github.com/llvm/llvm-project/commit/8ce2045be0ce708af0bfce5dc14632fa15dc743a.diff

LOG: [Clang][Sema] Avoid pack expansion for expanded empty PackIndexingExprs (#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

Added: 
    

Modified: 
    clang/include/clang/AST/ExprCXX.h
    clang/lib/AST/ExprCXX.cpp
    clang/lib/Sema/SemaTemplateVariadic.cpp
    clang/lib/Sema/TreeTransform.h
    clang/lib/Serialization/ASTReaderStmt.cpp
    clang/lib/Serialization/ASTWriterStmt.cpp
    clang/test/PCH/pack_indexing.cpp
    clang/test/SemaCXX/cxx2c-pack-indexing.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index fac65628ffede..dbf693611a7fa 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4377,15 +4377,21 @@ 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 ExpandedToEmptyPack : 1;
 
   PackIndexingExpr(QualType Type, SourceLocation EllipsisLoc,
                    SourceLocation RSquareLoc, Expr *PackIdExpr, Expr *IndexExpr,
-                   ArrayRef<Expr *> SubstitutedExprs = {})
+                   ArrayRef<Expr *> SubstitutedExprs = {},
+                   bool ExpandedToEmptyPack = false)
       : Expr(PackIndexingExprClass, Type, VK_LValue, OK_Ordinary),
         EllipsisLoc(EllipsisLoc), RSquareLoc(RSquareLoc),
         SubExprs{PackIdExpr, IndexExpr},
-        TransformedExpressions(SubstitutedExprs.size()) {
+        TransformedExpressions(SubstitutedExprs.size()),
+        ExpandedToEmptyPack(ExpandedToEmptyPack) {
 
     auto *Exprs = getTrailingObjects<Expr *>();
     std::uninitialized_copy(SubstitutedExprs.begin(), SubstitutedExprs.end(),
@@ -4408,10 +4414,14 @@ class PackIndexingExpr final
                                   SourceLocation EllipsisLoc,
                                   SourceLocation RSquareLoc, Expr *PackIdExpr,
                                   Expr *IndexExpr, std::optional<int64_t> Index,
-                                  ArrayRef<Expr *> SubstitutedExprs = {});
+                                  ArrayRef<Expr *> SubstitutedExprs = {},
+                                  bool ExpandedToEmptyPack = false);
   static PackIndexingExpr *CreateDeserialized(ASTContext &Context,
                                               unsigned NumTransformedExprs);
 
+  /// Determine if the expression was expanded to empty.
+  bool expandsToEmptyPack() const { return ExpandedToEmptyPack; }
+
   /// Determine the location of the 'sizeof' keyword.
   SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
 
@@ -4445,6 +4455,7 @@ class PackIndexingExpr final
     return getTrailingObjects<Expr *>()[*Index];
   }
 
+  /// Return the trailing expressions, regardless of the expansion.
   ArrayRef<Expr *> getExpressions() const {
     return {getTrailingObjects<Expr *>(), TransformedExpressions};
   }

diff  --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 7e9343271ac3c..2abc0acbfde3b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1665,12 +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) {
+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();
@@ -1679,8 +1677,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, ExpandedToEmptyPack);
 }
 
 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 d99bb20320604..06ed0843ef504 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14967,7 +14967,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
     return ExprError();
 
   SmallVector<Expr *, 5> ExpandedExprs;
-  if (E->getExpressions().empty()) {
+  if (!E->expandsToEmptyPack() && E->getExpressions().empty()) {
     Expr *Pattern = E->getPackIdExpression();
     SmallVector<UnexpandedParameterPack, 2> Unexpanded;
     getSema().collectUnexpandedParameterPacks(E->getPackIdExpression(),
@@ -15021,9 +15021,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
         return true;
       ExpandedExprs.push_back(Out.get());
     }
-  }
-
-  else {
+  } 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 7d3930022a69c..eac4faff28549 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->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 6f7c368ce9ca4..a44852af97bea 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->ExpandedToEmptyPack);
   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;

diff  --git a/clang/test/PCH/pack_indexing.cpp b/clang/test/PCH/pack_indexing.cpp
index cf8124617b3c6..1c4dac0fd9a30 100644
--- a/clang/test/PCH/pack_indexing.cpp
+++ b/clang/test/PCH/pack_indexing.cpp
@@ -10,7 +10,11 @@ using Type = U...[I];
 template <int I, auto...V>
 constexpr auto Var = V...[I];
 
+template <int I, auto...V>
+decltype(V...[I]) foo() { return V...[I]; }
+
 void fn1() {
   using A = Type<1, int, long, double>;
   constexpr auto V = Var<2, 0, 1, 42>;
+  foo<2, 0, 1, 42>();
 }

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}}
 }
 
 


        


More information about the cfe-commits mailing list