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

via cfe-commits cfe-commits at lists.llvm.org
Thu May 16 04:46:48 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

<details>
<summary>Changes</summary>

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

---
Full diff: https://github.com/llvm/llvm-project/pull/92385.diff


5 Files Affected:

- (modified) clang/include/clang/AST/ExprCXX.h (+10-4) 
- (modified) clang/lib/AST/ExprCXX.cpp (+8-8) 
- (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+1-1) 
- (modified) clang/lib/Sema/TreeTransform.h (+2-4) 
- (modified) clang/test/SemaCXX/cxx2c-pack-indexing.cpp (+6-2) 


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

``````````

</details>


https://github.com/llvm/llvm-project/pull/92385


More information about the cfe-commits mailing list