[clang] [Sema] Avoid an undesired pack expansion while transforming PackIndexingType (PR #90195)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 30 20:49:00 PDT 2024


https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/90195

>From f708694fc2686684589dca7b8f3738a117fc047e Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 26 Apr 2024 19:06:57 +0800
Subject: [PATCH 1/6] [Sema] Avoid an undesired pack expansion while
 transforming PackIndexingType

---
 clang/lib/Sema/TreeTransform.h             |  3 +++
 clang/test/SemaCXX/cxx2c-pack-indexing.cpp | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 9404be5a46f3f7..abc4a16c004a9f 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -6649,6 +6649,9 @@ TreeTransform<Derived>::TransformPackIndexingType(TypeLocBuilder &TLB,
     }
   }
 
+  // We may be doing this in the context of expanding the Pattern. Forget that
+  // because it has been handled above.
+  Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
   QualType Result = getDerived().TransformType(TLB, TL.getPatternLoc());
 
   QualType Out = getDerived().RebuildPackIndexingType(
diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
index 606715e6aacffd..2fd0dbfed294a5 100644
--- a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
+++ b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
@@ -160,3 +160,22 @@ namespace GH88929 {
     using E = P...[0]; // expected-error {{unknown type name 'P'}} \
                        // expected-error {{expected ';' after alias declaration}}
 }
+
+namespace GH88925 {
+template <typename...> struct S {};
+
+template <int...> struct sequence {};
+
+template <typename... Args, int... indices> auto f(sequence<indices...>) {
+  return S<Args...[indices]...>(); // #use
+}
+
+void g() {
+  static_assert(__is_same(decltype(f<int>(sequence<0, 0>())), S<int, int>));
+  static_assert(__is_same(decltype(f<int, long>(sequence<0, 0>())), S<int, int>));
+  static_assert(__is_same(decltype(f<int, long>(sequence<0, 1>())), S<int, long>));
+  f<int, long>(sequence<3>());
+  // expected-error@#use {{invalid index 3 for pack 'Args' of size 2}}}
+  // expected-note-re at -2 {{function template specialization '{{.*}}' requested here}}
+}
+}

>From 7b0ae16b6777c6e98df64cd2366434972fc68164 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 27 Apr 2024 12:07:30 +0800
Subject: [PATCH 2/6] Clarify comments

---
 clang/lib/Sema/TreeTransform.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index abc4a16c004a9f..b50fdab8bfd05e 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -6649,8 +6649,9 @@ TreeTransform<Derived>::TransformPackIndexingType(TypeLocBuilder &TLB,
     }
   }
 
-  // We may be doing this in the context of expanding the Pattern. Forget that
-  // because it has been handled above.
+  // A pack indexing type can appear in a larger pack expansion,
+  // e.g. `Pack...[pack_of_indexes]...`
+  // so we need to temporarily disable substitution of pack elements
   Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
   QualType Result = getDerived().TransformType(TLB, TL.getPatternLoc());
 

>From 1dbfe522a1b4efd92023f0ff3c41cc296bf53299 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sun, 28 Apr 2024 13:29:37 +0800
Subject: [PATCH 3/6] Make dependent PackIndexingExpr always an LValue

---
 clang/lib/AST/ExprClassification.cpp       |  8 +++++++-
 clang/test/SemaCXX/cxx2c-pack-indexing.cpp | 23 ++++++++++++++++++----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 7026fca8554ce9..abfc3a51f95a8d 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -216,8 +216,14 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
     return ClassifyInternal(Ctx,
                  cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement());
 
-  case Expr::PackIndexingExprClass:
+  case Expr::PackIndexingExprClass: {
+    // A dependent pack-index-expression is now supposed to denote a function
+    // parameter pack, an NTTP pack, or the pack introduced by a structured
+    // binding. Consider it as an LValue expression.
+    if (cast<PackIndexingExpr>(E)->isInstantiationDependent())
+      return Cl::CL_LValue;
     return ClassifyInternal(Ctx, cast<PackIndexingExpr>(E)->getSelectedExpr());
+  }
 
     // C, C++98 [expr.sub]p1: The result is an lvalue of type "T".
     // C++11 (DR1213): in the case of an array operand, the result is an lvalue
diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
index 2fd0dbfed294a5..a3e5a0931491b5 100644
--- a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
+++ b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
@@ -164,18 +164,33 @@ namespace GH88929 {
 namespace GH88925 {
 template <typename...> struct S {};
 
+template <auto...> struct W {};
+
 template <int...> struct sequence {};
 
-template <typename... Args, int... indices> auto f(sequence<indices...>) {
-  return S<Args...[indices]...>(); // #use
+template <typename... args, int... indices> auto f(sequence<indices...>) {
+  return S<args...[indices]...>(); // #use
 }
 
-void g() {
+template <auto... args, int... indices> auto g(sequence<indices...>) {
+  return W<args...[indices]...>(); // #nttp-use
+}
+
+void h() {
   static_assert(__is_same(decltype(f<int>(sequence<0, 0>())), S<int, int>));
   static_assert(__is_same(decltype(f<int, long>(sequence<0, 0>())), S<int, int>));
   static_assert(__is_same(decltype(f<int, long>(sequence<0, 1>())), S<int, long>));
   f<int, long>(sequence<3>());
-  // expected-error@#use {{invalid index 3 for pack 'Args' of size 2}}}
+  // expected-error@#use {{invalid index 3 for pack 'args' of size 2}}}
+  // expected-note-re at -2 {{function template specialization '{{.*}}' requested here}}
+
+  struct foo {};
+  struct bar {};
+  struct baz {};
+
+  static_assert(__is_same(decltype(g<foo{}, bar{}, baz{}>(sequence<0, 2, 1>())), W<foo{}, baz{}, bar{}>));
+  g<foo{}>(sequence<4>());
+  // expected-error@#nttp-use {{invalid index 4 for pack args of size 1}}
   // expected-note-re at -2 {{function template specialization '{{.*}}' requested here}}
 }
 }

>From 7a087f06240ce09a64fe679df14380e2fa538701 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 1 May 2024 11:44:27 +0800
Subject: [PATCH 4/6] Apply changes from Corentin

Co-authored-by: cor3ntin <corentinjabot at gmail.com>
---
 clang/lib/AST/ExprClassification.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index abfc3a51f95a8d..f943a6ba0df04d 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -217,9 +217,8 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
                  cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement());
 
   case Expr::PackIndexingExprClass: {
-    // A dependent pack-index-expression is now supposed to denote a function
-    // parameter pack, an NTTP pack, or the pack introduced by a structured
-    // binding. Consider it as an LValue expression.
+    // A pack-index-expression always expand to an id-expression. 
+    // Consider it as an LValue expression.
     if (cast<PackIndexingExpr>(E)->isInstantiationDependent())
       return Cl::CL_LValue;
     return ClassifyInternal(Ctx, cast<PackIndexingExpr>(E)->getSelectedExpr());

>From 5e85c87a682fba46e1783cab5fbf96baa5e0c456 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 1 May 2024 11:46:19 +0800
Subject: [PATCH 5/6] Expand -> expands

---
 clang/lib/AST/ExprClassification.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index f943a6ba0df04d..41ecabc3001b01 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -217,7 +217,7 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
                  cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement());
 
   case Expr::PackIndexingExprClass: {
-    // A pack-index-expression always expand to an id-expression. 
+    // A pack-index-expression always expands to an id-expression. 
     // Consider it as an LValue expression.
     if (cast<PackIndexingExpr>(E)->isInstantiationDependent())
       return Cl::CL_LValue;

>From b23d40da37079e6263c9aca34d6faa13bc955aa6 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 1 May 2024 11:48:34 +0800
Subject: [PATCH 6/6] Run clang-format

---
 clang/lib/AST/ExprClassification.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 41ecabc3001b01..79b7b2bd48aeff 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -217,7 +217,7 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
                  cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement());
 
   case Expr::PackIndexingExprClass: {
-    // A pack-index-expression always expands to an id-expression. 
+    // A pack-index-expression always expands to an id-expression.
     // Consider it as an LValue expression.
     if (cast<PackIndexingExpr>(E)->isInstantiationDependent())
       return Cl::CL_LValue;



More information about the cfe-commits mailing list