[clang] [Clang][Parser] Add a warning to ambiguous uses of T...[N] types (PR #116332)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 27 03:37:43 PST 2024


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

>From 5973de1d4c368a26fd179954a13de94595f35575 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 15 Nov 2024 14:09:14 +0800
Subject: [PATCH 1/4] [Clang][Parser] Make 'T...[N]' within a function
 parameter a valid pack expansion prior to C++2c

---
 .../clang/Basic/DiagnosticParseKinds.td       |  5 ++++
 clang/lib/Parse/ParseExprCXX.cpp              | 18 ++++++++++++++
 .../CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp  |  5 ++--
 clang/test/Parser/cxx0x-decl.cpp              |  8 ++-----
 .../SemaCXX/cxx2c-pack-indexing-ext-diags.cpp | 24 +++++++++++++++++++
 5 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 0da509280068ad..7b408f3eed5ba5 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -718,6 +718,11 @@ def warn_empty_init_statement : Warning<
   "has no effect">, InGroup<EmptyInitStatement>, DefaultIgnore;
 def err_keyword_as_parameter : Error <
   "invalid parameter name: '%0' is a keyword">;
+def warn_pre_cxx26_ambiguous_pack_indexing_type : Warning<
+  "parameter packs without specifying a name would become a pack indexing "
+  "declaration in C++2c onwards">, InGroup<CXXPre26Compat>;
+def note_add_a_name_to_pre_cxx26_parameter_packs : Note<
+  "add a name to disambiguate">;
 
 // C++ derived classes
 def err_dup_virtual : Error<"duplicate 'virtual' in base specifier">;
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index ce3624f366a2a1..c596785b267b5b 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -242,7 +242,25 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
     SourceLocation Start = Tok.getLocation();
     DeclSpec DS(AttrFactory);
     SourceLocation CCLoc;
+    TentativeParsingAction MaybePackIndexing(*this, /*Unannotated=*/true);
     SourceLocation EndLoc = ParsePackIndexingType(DS);
+    // C++ [cpp23.dcl.dcl-2]:
+    //   Previously, T...[n] would declare a pack of function parameters.
+    //   T...[n] is now a pack-index-specifier. [...] Valid C++ 2023 code that
+    //   declares a pack of parameters without specifying a declarator-id
+    //   becomes ill-formed.
+    if (!Tok.is(tok::coloncolon) && !getLangOpts().CPlusPlus26 &&
+        getCurScope()->isFunctionDeclarationScope()) {
+      Diag(DS.getEllipsisLoc(),
+           diag::warn_pre_cxx26_ambiguous_pack_indexing_type);
+      Diag(DS.getEllipsisLoc(),
+           diag::note_add_a_name_to_pre_cxx26_parameter_packs);
+      MaybePackIndexing.Revert();
+      return false;
+    }
+
+    MaybePackIndexing.Commit();
+
     if (DS.getTypeSpecType() == DeclSpec::TST_error)
       return false;
 
diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp
index dbb6e60d9b93d7..0ae08da280b152 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp
@@ -58,8 +58,9 @@ void b(T[] ...);
 
 template<typename T>
 void c(T ... []); // expected-error {{expected expression}} \
-                  // expected-error {{'T' does not refer to the name of a parameter pack}} \
-                  // expected-warning {{pack indexing is a C++2c extension}}
+                  // expected-error {{type 'T[]' of function parameter pack does not contain any unexpanded parameter packs}} \
+                  // expected-warning {{would become a pack indexing declaration in C++2c onwards}} \
+                  // expected-note {{add a name to disambiguate}}
 
 template<typename T>
 void d(T ... x[]); // expected-error{{type 'T[]' of function parameter pack does not contain any unexpanded parameter packs}}
diff --git a/clang/test/Parser/cxx0x-decl.cpp b/clang/test/Parser/cxx0x-decl.cpp
index a0b3266c738ff5..91b97df459df92 100644
--- a/clang/test/Parser/cxx0x-decl.cpp
+++ b/clang/test/Parser/cxx0x-decl.cpp
@@ -214,12 +214,8 @@ struct MemberComponentOrder : Base {
 void NoMissingSemicolonHere(struct S
                             [3]);
 template<int ...N> void NoMissingSemicolonHereEither(struct S... [N]);
-// expected-error at -1 {{'S' does not refer to the name of a parameter pack}} \
-// expected-error at -1 {{declaration of anonymous struct must be a definition}} \
-// expected-error at -1 {{expected parameter declarator}} \
-// expected-error at -1 {{pack indexing is a C++2c extension}} \
-
-
+// expected-warning at -1 {{parameter packs without specifying a name would become a pack indexing declaration in C++2c onwards}} \
+// expected-note at -1 {{add a name to disambiguate}}
 
 // This must be at the end of the file; we used to look ahead past the EOF token here.
 // expected-error at +1 {{expected unqualified-id}} expected-error at +1{{expected ';'}}
diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
index 80dfeb9b6a8bf5..f4dbd5ca539635 100644
--- a/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
+++ b/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
@@ -19,3 +19,27 @@ void f(T... t) {
     // cxx11-warning at +1 {{pack indexing is a C++2c extension}}
     T...[0] c;
 }
+
+template <typename... T>
+void g(T... [1]); // cxx11-warning {{parameter packs without specifying a name would become a pack indexing declaration in C++2c onwards}} \
+                  // cxx11-note {{add a name to disambiguate}} \
+                  // cxx26-warning {{pack indexing is incompatible with C++ standards before C++2c}} \
+                  // cxx26-note {{candidate function template not viable}}
+
+template <typename... T>
+void h(T... param[1]);
+
+template <class T>
+struct S {
+  using type = T;
+};
+
+template <typename... T>
+void h(typename T... [1]::type); // cxx11-warning {{pack indexing is a C++2c extension}} \
+                                 // cxx26-warning {{pack indexing is incompatible with C++ standards before C++2c}}
+
+void call() {
+  g<int, double>(nullptr, nullptr); // cxx26-error {{no matching function for call to 'g'}}
+  h<int, double>(nullptr, nullptr);
+  h<S<int>, S<const char *>>("hello");
+}

>From c48988ae40d6a6934a75b527cb9d3d4a42de1faa Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 16 Nov 2024 21:24:41 +0800
Subject: [PATCH 2/4] Accept 'T...[]' within a function parameter scope

---
 clang/lib/Parse/ParseExprCXX.cpp                    | 3 ++-
 clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp | 9 +++++----
 clang/test/Parser/cxx2c-pack-indexing.cpp           | 5 ++---
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index c596785b267b5b..66ba5e2cd074c3 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -238,7 +238,8 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
 
   else if (!HasScopeSpecifier && Tok.is(tok::identifier) &&
            GetLookAheadToken(1).is(tok::ellipsis) &&
-           GetLookAheadToken(2).is(tok::l_square)) {
+           GetLookAheadToken(2).is(tok::l_square) &&
+           !GetLookAheadToken(3).is(tok::r_square)) {
     SourceLocation Start = Tok.getLocation();
     DeclSpec DS(AttrFactory);
     SourceLocation CCLoc;
diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp
index 0ae08da280b152..a633c108bc22cd 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p13.cpp
@@ -57,10 +57,11 @@ template<typename T>
 void b(T[] ...);
 
 template<typename T>
-void c(T ... []); // expected-error {{expected expression}} \
-                  // expected-error {{type 'T[]' of function parameter pack does not contain any unexpanded parameter packs}} \
-                  // expected-warning {{would become a pack indexing declaration in C++2c onwards}} \
-                  // expected-note {{add a name to disambiguate}}
+void c(T ... []); // expected-error {{type 'T[]' of function parameter pack does not contain any unexpanded parameter packs}}
+
+// A function that takes pointers to each type T as arguments, after any decay.
+template <typename ...T>
+void g(T...[]);
 
 template<typename T>
 void d(T ... x[]); // expected-error{{type 'T[]' of function parameter pack does not contain any unexpanded parameter packs}}
diff --git a/clang/test/Parser/cxx2c-pack-indexing.cpp b/clang/test/Parser/cxx2c-pack-indexing.cpp
index c279bdd7af8c44..45a0e047221e1f 100644
--- a/clang/test/Parser/cxx2c-pack-indexing.cpp
+++ b/clang/test/Parser/cxx2c-pack-indexing.cpp
@@ -12,8 +12,7 @@ struct S {
             // expected-note {{to match this '['}} \
            // expected-warning{{declaration does not declare anything}}
 
-    T...[]; // expected-error{{expected expression}} \
-           // expected-warning{{declaration does not declare anything}}
+    T...[]; // expected-error{{expected member name or ';' after declaration specifiers}}
 
     void f(auto... v) {
         decltype(v...[1]) a = v...[1];
@@ -65,7 +64,7 @@ int main() {
 }
 
 
-namespace GH11460 {
+namespace GH111460 {
 template <typename... T>
 requires( ); // expected-error {{expected expression}}
 struct SS {

>From 0ba59a76b14c879aa3b102e4c1c9d0e6b94b00d2 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 27 Nov 2024 19:02:08 +0800
Subject: [PATCH 3/4] Address the feedback

---
 .../clang/Basic/DiagnosticParseKinds.td       |  6 ++--
 clang/lib/Parse/ParseExprCXX.cpp              | 31 ++++++++-----------
 clang/test/Parser/cxx0x-decl.cpp              |  8 +++--
 .../SemaCXX/cxx2c-pack-indexing-ext-diags.cpp | 13 ++++++--
 4 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 7b408f3eed5ba5..2c23ad5b5c4915 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -719,10 +719,8 @@ def warn_empty_init_statement : Warning<
 def err_keyword_as_parameter : Error <
   "invalid parameter name: '%0' is a keyword">;
 def warn_pre_cxx26_ambiguous_pack_indexing_type : Warning<
-  "parameter packs without specifying a name would become a pack indexing "
-  "declaration in C++2c onwards">, InGroup<CXXPre26Compat>;
-def note_add_a_name_to_pre_cxx26_parameter_packs : Note<
-  "add a name to disambiguate">;
+  "%0 is no longer a pack expansion but a pack "
+  "indexing type; add a name to specify a pack expansion">, InGroup<CXXPre26Compat>;
 
 // C++ derived classes
 def err_dup_virtual : Error<"duplicate 'virtual' in base specifier">;
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 66ba5e2cd074c3..074a75fc9f8c06 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -243,25 +243,7 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
     SourceLocation Start = Tok.getLocation();
     DeclSpec DS(AttrFactory);
     SourceLocation CCLoc;
-    TentativeParsingAction MaybePackIndexing(*this, /*Unannotated=*/true);
     SourceLocation EndLoc = ParsePackIndexingType(DS);
-    // C++ [cpp23.dcl.dcl-2]:
-    //   Previously, T...[n] would declare a pack of function parameters.
-    //   T...[n] is now a pack-index-specifier. [...] Valid C++ 2023 code that
-    //   declares a pack of parameters without specifying a declarator-id
-    //   becomes ill-formed.
-    if (!Tok.is(tok::coloncolon) && !getLangOpts().CPlusPlus26 &&
-        getCurScope()->isFunctionDeclarationScope()) {
-      Diag(DS.getEllipsisLoc(),
-           diag::warn_pre_cxx26_ambiguous_pack_indexing_type);
-      Diag(DS.getEllipsisLoc(),
-           diag::note_add_a_name_to_pre_cxx26_parameter_packs);
-      MaybePackIndexing.Revert();
-      return false;
-    }
-
-    MaybePackIndexing.Commit();
-
     if (DS.getTypeSpecType() == DeclSpec::TST_error)
       return false;
 
@@ -272,6 +254,19 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
     if (Type.isNull())
       return false;
 
+    // C++ [cpp23.dcl.dcl-2]:
+    //   Previously, T...[n] would declare a pack of function parameters.
+    //   T...[n] is now a pack-index-specifier. [...] Valid C++ 2023 code that
+    //   declares a pack of parameters without specifying a declarator-id
+    //   becomes ill-formed.
+    //
+    // However, we still avoid parsing them as pack expansions because this is a
+    // rare use case of packs, despite being partway non-conforming, to ensure
+    // semantic consistency given that we have backported this feature.
+    if (!Tok.is(tok::coloncolon) && !getLangOpts().CPlusPlus26 &&
+        getCurScope()->isFunctionDeclarationScope())
+      Diag(Start, diag::warn_pre_cxx26_ambiguous_pack_indexing_type) << Type;
+
     if (!TryConsumeToken(tok::coloncolon, CCLoc)) {
       AnnotateExistingIndexedTypeNamePack(ParsedType::make(Type), Start,
                                           EndLoc);
diff --git a/clang/test/Parser/cxx0x-decl.cpp b/clang/test/Parser/cxx0x-decl.cpp
index 91b97df459df92..caa7f84e667655 100644
--- a/clang/test/Parser/cxx0x-decl.cpp
+++ b/clang/test/Parser/cxx0x-decl.cpp
@@ -214,8 +214,12 @@ struct MemberComponentOrder : Base {
 void NoMissingSemicolonHere(struct S
                             [3]);
 template<int ...N> void NoMissingSemicolonHereEither(struct S... [N]);
-// expected-warning at -1 {{parameter packs without specifying a name would become a pack indexing declaration in C++2c onwards}} \
-// expected-note at -1 {{add a name to disambiguate}}
+// expected-warning at -1 {{'S...[N]' is no longer a pack expansion but a pack indexing type; add a name to specify a pack expansion}} \
+// expected-error at -1 {{'S' does not refer to the name of a parameter pack}} \
+// expected-error at -1 {{declaration of anonymous struct must be a definition}} \
+// expected-error at -1 {{expected parameter declarator}} \
+// expected-error at -1 {{pack indexing is a C++2c extension}} \
+
 
 // This must be at the end of the file; we used to look ahead past the EOF token here.
 // expected-error at +1 {{expected unqualified-id}} expected-error at +1{{expected ';'}}
diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
index f4dbd5ca539635..6c2b9e9d175d63 100644
--- a/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
+++ b/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
@@ -21,8 +21,9 @@ void f(T... t) {
 }
 
 template <typename... T>
-void g(T... [1]); // cxx11-warning {{parameter packs without specifying a name would become a pack indexing declaration in C++2c onwards}} \
-                  // cxx11-note {{add a name to disambiguate}} \
+void g(T... [1]); // cxx11-warning {{'T...[1]' is no longer a pack expansion but a pack indexing type; add a name to specify a pack expansion}} \
+                  // cxx11-warning {{pack indexing is a C++2c extension}} \
+                  // cxx11-note {{candidate function template not viable}} \
                   // cxx26-warning {{pack indexing is incompatible with C++ standards before C++2c}} \
                   // cxx26-note {{candidate function template not viable}}
 
@@ -38,8 +39,14 @@ template <typename... T>
 void h(typename T... [1]::type); // cxx11-warning {{pack indexing is a C++2c extension}} \
                                  // cxx26-warning {{pack indexing is incompatible with C++ standards before C++2c}}
 
+template <typename... T>
+void x(T... [0]); // cxx11-warning {{'T...[0]' is no longer a pack expansion but a pack indexing type; add a name to specify a pack expansion}} \
+                  // cxx26-warning {{pack indexing is incompatible with C++ standards before C++2c}}
+
 void call() {
-  g<int, double>(nullptr, nullptr); // cxx26-error {{no matching function for call to 'g'}}
+  g<int, double>(nullptr, nullptr); // cxx26-error {{no matching function for call to 'g'}} \
+                                    // cxx11-error {{no matching function for call to 'g'}}
   h<int, double>(nullptr, nullptr);
   h<S<int>, S<const char *>>("hello");
+  x<int*>(nullptr);
 }

>From 3ac80d0ead34b46e7a0d6304274c106ca620b748 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 27 Nov 2024 19:37:25 +0800
Subject: [PATCH 4/4] Fix CI

---
 clang/test/Parser/cxx0x-decl.cpp                     | 1 +
 clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp | 1 +
 2 files changed, 2 insertions(+)

diff --git a/clang/test/Parser/cxx0x-decl.cpp b/clang/test/Parser/cxx0x-decl.cpp
index caa7f84e667655..6d4f16c6f533e8 100644
--- a/clang/test/Parser/cxx0x-decl.cpp
+++ b/clang/test/Parser/cxx0x-decl.cpp
@@ -221,6 +221,7 @@ template<int ...N> void NoMissingSemicolonHereEither(struct S... [N]);
 // expected-error at -1 {{pack indexing is a C++2c extension}} \
 
 
+
 // This must be at the end of the file; we used to look ahead past the EOF token here.
 // expected-error at +1 {{expected unqualified-id}} expected-error at +1{{expected ';'}}
 using
diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
index 6c2b9e9d175d63..118af8e867f5f6 100644
--- a/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
+++ b/clang/test/SemaCXX/cxx2c-pack-indexing-ext-diags.cpp
@@ -41,6 +41,7 @@ void h(typename T... [1]::type); // cxx11-warning {{pack indexing is a C++2c ext
 
 template <typename... T>
 void x(T... [0]); // cxx11-warning {{'T...[0]' is no longer a pack expansion but a pack indexing type; add a name to specify a pack expansion}} \
+                  // cxx11-warning {{pack indexing is a C++2c extension}} \
                   // cxx26-warning {{pack indexing is incompatible with C++ standards before C++2c}}
 
 void call() {



More information about the cfe-commits mailing list