Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -235,7 +236,8 @@ def Deprecated : DiagGroup<"deprecated", [DeprecatedAnonEnumEnumConversion,
DeprecatedType,
DeprecatedVolatile,
DeprecatedWritableStr,
DeprecatedRedundantConstexprStaticDef
DeprecatedRedundantConstexprStaticDef,
DeprecatedMissingCommaVariadicParam
]>,
DiagCategory<"Deprecations">;

Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

InGroup<DeprecatedMissingCommaVariadicParam>;
def err_unexpected_typedef_ident : Error<
"unexpected type name %0: expected identifier">;
def warn_cxx98_compat_decltype : Warning<
Expand Down
8 changes: 8 additions & 0 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, ", ");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, nah, lets leave it like that

if (!getLangOpts().CPlusPlus) {
// We have ellipsis without a preceding ',', which is ill-formed
// in C. Complain and provide the fix.
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CXX/drs/cwg722.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions clang/test/Parser/cxx2c-oxford-variadic-comma.cpp
Original file line number Diff line number Diff line change
@@ -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, ...);

2 changes: 1 addition & 1 deletion clang/www/cxx_status.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading