[clang] Implement P3176R1: The Oxford variadic comma #117524

antangelo · 2024-11-25T07:20:25Z

Emit a deprecation warning when a variadic parameter in a parameter-declaration-clause is not preceded by a comma for C++26.

Emit a warning when a variadic parameter in a parameter-declaration-clause is not preceded by a comma for C++26.

llvmbot · 2024-11-25T07:20:59Z

@llvm/pr-subscribers-clang

Author: None (antangelo)

Changes

Emit a deprecation warning when a variadic parameter in a parameter-declaration-clause is not preceded by a comma for C++26.

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

7 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2)
(modified) clang/include/clang/Basic/DiagnosticGroups.td (+3-1)
(modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+3)
(modified) clang/lib/Parse/ParseDecl.cpp (+8)
(modified) clang/test/CXX/drs/cwg722.cpp (+1-1)
(added) clang/test/Parser/cxx2c-oxford-variadic-comma.cpp (+36)
(modified) clang/www/cxx_status.html (+1-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8bd06fadfdc984..b8d5c4201aeaef 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -227,6 +227,8 @@ C++2c Feature Support
 - Added the ``__builtin_is_within_lifetime`` builtin, which supports
   `P2641R4 Checking if a union alternative is active <https://wg21.link/p2641r4>`_
 
+- Implemented `P3176R1 The Oxford variadic comma <https://wg21.link/P3176R1>`_
+
 C++23 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 - Removed the restriction to literal types in constexpr functions in C++23 mode.
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..7ad8ac0e7a2e46 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -211,6 +211,7 @@ def DeprecatedWritableStr : DiagGroup<"deprecated-writable-strings",
                                       [CXX11CompatDeprecatedWritableStr]>;
 def DeprecatedPragma : DiagGroup<"deprecated-pragma">;
 def DeprecatedType : DiagGroup<"deprecated-type">;
+def DeprecatedMissingCommaVariadicParam : DiagGroup<"deprecated-missing-comma-variadic-parameter">;
 // FIXME: Why is DeprecatedImplementations not in this group?
 def Deprecated : DiagGroup<"deprecated", [DeprecatedAnonEnumEnumConversion,
                                           DeprecatedArrayCompare,
@@ -235,7 +236,8 @@ def Deprecated : DiagGroup<"deprecated", [DeprecatedAnonEnumEnumConversion,
                                           DeprecatedType,
                                           DeprecatedVolatile,
                                           DeprecatedWritableStr,
-                                          DeprecatedRedundantConstexprStaticDef
+                                          DeprecatedRedundantConstexprStaticDef,
+                                          DeprecatedMissingCommaVariadicParam
                                           ]>,
                  DiagCategory<"Deprecations">;
 
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 77bf08453dea51..7c3961059d1d0d 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -413,6 +413,9 @@ def err_function_scope_depth_exceeded : Error<
   "function scope depth exceeded maximum of %0">, DefaultFatal;
 def err_missing_comma_before_ellipsis : Error<
   "C requires a comma prior to the ellipsis in a variadic function type">;
+def warn_deprecated_missing_comma_before_ellipsis : Warning<
+  "variadic parameters that are not preceded by a comma are deprecated">,
+  InGroup<DeprecatedMissingCommaVariadicParam>;
 def err_unexpected_typedef_ident : Error<
   "unexpected type name %0: expected identifier">;
 def warn_cxx98_compat_decltype : Warning<
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index aa5c2d28d429ac..d28a74662aefe0 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -8119,6 +8119,14 @@ void Parser::ParseParameterDeclarationClause(
     }
 
     if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
+      if (getLangOpts().CPlusPlus26) {
+        // C++26 [dcl.dcl.fct]p3:
+        //   A parameter-declaration-clause of the form
+        //   parameter-list '...' is deprecated
+        Diag(EllipsisLoc, diag::warn_deprecated_missing_comma_before_ellipsis)
+            << FixItHint::CreateInsertion(EllipsisLoc, ", ");
+      }
+
       if (!getLangOpts().CPlusPlus) {
         // We have ellipsis without a preceding ',', which is ill-formed
         // in C. Complain and provide the fix.
diff --git a/clang/test/CXX/drs/cwg722.cpp b/clang/test/CXX/drs/cwg722.cpp
index e90d9af360d9d0..6e7d4569c55381 100644
--- a/clang/test/CXX/drs/cwg722.cpp
+++ b/clang/test/CXX/drs/cwg722.cpp
@@ -14,7 +14,7 @@ namespace std {
   using nullptr_t = decltype(nullptr);
 }
 
-void f(std::nullptr_t...);
+void f(std::nullptr_t, ...);
 std::nullptr_t g();
 void h() {
   std::nullptr_t np;
diff --git a/clang/test/Parser/cxx2c-oxford-variadic-comma.cpp b/clang/test/Parser/cxx2c-oxford-variadic-comma.cpp
new file mode 100644
index 00000000000000..dc78dcbb6b074f
--- /dev/null
+++ b/clang/test/Parser/cxx2c-oxford-variadic-comma.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -std=c++2c -fsyntax-only -verify %s
+
+void a(...);
+
+void b(auto...);
+void c(auto, ...);
+
+void d(auto......); // expected-warning {{variadic parameters that are not preceded by a comma are deprecated}} \
+                    // expected-warning {{'...' in this location creates a C-style varargs function}} \
+                    // expected-note {{preceding '...' declares a function parameter pack}} \
+                    // expected-note {{insert ',' before '...' to silence this warning}}
+void e(auto..., ...);
+
+void f(auto x...); // expected-warning {{variadic parameters that are not preceded by a comma are deprecated}}
+void g(auto x, ...);
+
+void h(auto... x...); // expected-warning {{variadic parameters that are not preceded by a comma are deprecated}} \
+                      // expected-warning {{'...' in this location creates a C-style varargs function}} \
+                      // expected-note {{preceding '...' declares a function parameter pack}} \
+                      // expected-note {{insert ',' before '...' to silence this warning}}
+void i(auto... x, ...);
+
+template<class ...T>
+void j(T... t...); // expected-warning {{variadic parameters that are not preceded by a comma are deprecated}} \
+                   // expected-warning {{'...' in this location creates a C-style varargs function}} \
+                   // expected-note {{preceding '...' declares a function parameter pack}} \
+                   // expected-note {{insert ',' before '...' to silence this warning}}
+template<class ...T>
+void k(T... t, ...);
+
+void l(int...); // expected-warning {{variadic parameters that are not preceded by a comma are deprecated}}
+void m(int, ...);
+
+void n(int x...); // expected-warning {{variadic parameters that are not preceded by a comma are deprecated}}
+void o(int x, ...);
+
diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index da01cf6ceab59b..fdb9807b1168c7 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -249,7 +249,7 @@ <h2 id="cxx26">C++2c implementation status</h2>
  <tr>
   <td>The Oxford variadic comma</td>
   <td><a href="https://wg21.link/P3176R1">P3176R1</a></td>
-  <td class="none" align="center">No</td>
+  <td class="unreleased" align="center">Clang 20</td>
  </tr>
 </table>
 </details>

Sirraide

I managed to think of a few more test cases, but other than that, this looks fine imo.

clang/test/Parser/cxx2c-oxford-variadic-comma.cpp

cor3ntin · 2024-11-26T08:36:10Z

clang/lib/Parse/ParseDecl.cpp

+        Diag(EllipsisLoc, diag::warn_deprecated_missing_comma_before_ellipsis)
+            << FixItHint::CreateInsertion(EllipsisLoc, ", ");
+      }
+


below: else if (!getLangOpts().CPlusPlus) {

I had intended to fallthrough to those if blocks so that the else if (ParmDeclarator.getEllipsisLoc().isValid() || Actions.containsUnexpandedParameterPacks(ParmDeclarator)) block would be evaluated to also emit the parameter pack diagnostics. Do you think those should be omitted in this case?

Hum, nah, lets leave it like that

cor3ntin · 2024-11-26T08:42:06Z

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -413,6 +413,9 @@ def err_function_scope_depth_exceeded : Error<
  "function scope depth exceeded maximum of %0">, DefaultFatal;
 def err_missing_comma_before_ellipsis : Error<
  "C requires a comma prior to the ellipsis in a variadic function type">;
+def warn_deprecated_missing_comma_before_ellipsis : Warning<
+  "variadic parameters that are not preceded by a comma are deprecated">,


Maybe "declaration of a variadic function without a comma before '...' is deprecated"

("variadic parameter" is not defined and somewhat confusing)

I've updated the diagnostic to use your suggested wording.

cor3ntin

LGTM, thanks

cor3ntin · 2024-11-27T05:26:41Z

clang/lib/Parse/ParseDecl.cpp

+        Diag(EllipsisLoc, diag::warn_deprecated_missing_comma_before_ellipsis)
+            << FixItHint::CreateInsertion(EllipsisLoc, ", ");
+      }
+


Hum, nah, lets leave it like that

AaronBallman · 2024-11-27T15:11:18Z

clang/test/Parser/cxx2c-oxford-variadic-comma.cpp

+                    // expected-note {{insert ',' before '...' to silence this warning}}
+
+template<class T>
+void r(T...) {} // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}


Other test cases:

// Testing that a type specifier is also diagnosed auto a = (void (*)(int...))nullptr; // Testing that a lambda parameter list is also diagnosed (void)[](int...){}; // Testing that a block parameter list is also diagnosed (void)^(int...){};

Thanks, I've added these test cases

cor3ntin · 2024-12-02T08:52:15Z

@antangelo Thanks! Feel free to merge

[clang] Implement P3176R1: The Oxford variadic comma

ffcae3a

Emit a warning when a variadic parameter in a parameter-declaration-clause is not preceded by a comma for C++26.

antangelo requested review from AaronBallman and shafik November 25, 2024 07:20

antangelo requested a review from Endilll as a code owner November 25, 2024 07:20

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 25, 2024

Sirraide reviewed Nov 25, 2024

View reviewed changes

clang/test/Parser/cxx2c-oxford-variadic-comma.cpp Show resolved Hide resolved

cor3ntin reviewed Nov 26, 2024

View reviewed changes

antangelo added 4 commits November 26, 2024 20:02

Change diagnostic warning wording

98d249c

Add additional test cases

66e7804

Merge remote-tracking branch 'upstream/main' into p3176r1

ca3d970

Update comment to add full-stop

425d560

cor3ntin approved these changes Nov 27, 2024

View reviewed changes

AaronBallman reviewed Nov 27, 2024

View reviewed changes

antangelo added 2 commits December 1, 2024 09:50

Merge remote-tracking branch 'upstream/main' into p3176r1

e4ef5af

Add test cases for type specifiers, lambdas and blocks

b0a33c7

antangelo merged commit 54fcf3e into llvm:main Dec 2, 2024
9 checks passed

antangelo deleted the p3176r1 branch December 2, 2024 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] Implement P3176R1: The Oxford variadic comma #117524

[clang] Implement P3176R1: The Oxford variadic comma #117524

Uh oh!

antangelo commented Nov 25, 2024

Uh oh!

llvmbot commented Nov 25, 2024

Uh oh!

Sirraide left a comment

Uh oh!

Uh oh!

cor3ntin Nov 26, 2024

Uh oh!

antangelo Nov 27, 2024

Uh oh!

cor3ntin Nov 27, 2024

Uh oh!

cor3ntin Nov 26, 2024

Uh oh!

antangelo Nov 27, 2024

Uh oh!

cor3ntin left a comment

Uh oh!

cor3ntin Nov 27, 2024

Uh oh!

AaronBallman Nov 27, 2024

Uh oh!

antangelo Dec 1, 2024

Uh oh!

cor3ntin commented Dec 2, 2024

Uh oh!

Uh oh!

Uh oh!

[clang] Implement P3176R1: The Oxford variadic comma #117524

[clang] Implement P3176R1: The Oxford variadic comma #117524

Uh oh!

Conversation

antangelo commented Nov 25, 2024

Uh oh!

llvmbot commented Nov 25, 2024

Uh oh!

Sirraide left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cor3ntin commented Dec 2, 2024

Uh oh!

Uh oh!

Uh oh!