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 all commits
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 @@ -256,6 +256,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<
"declaration of a variadic function without a comma before '...' is deprecated">,
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
54 changes: 54 additions & 0 deletions clang/test/Parser/cxx2c-oxford-variadic-comma.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// RUN: %clang_cc1 -std=c++2c -fsyntax-only -fblocks -verify %s

void a(...);

void b(auto...);
void c(auto, ...);

void d(auto......); // expected-warning {{declaration of a variadic function without a comma before '...' is 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 {{declaration of a variadic function without a comma before '...' is deprecated}}
void g(auto x, ...);

void h(auto... x...); // expected-warning {{declaration of a variadic function without a comma before '...' is 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 ...Ts>
void j(Ts... t...) {}; // expected-warning {{declaration of a variadic function without a comma before '...' is 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 ...Ts>
void k(Ts... t, ...) {}

void l(int...); // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}
void m(int, ...);

void n(int x...); // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}
void o(int x, ...);

struct S {
void p(this S...) {} // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}
};

template<class ...Ts>
void q(Ts......) {} // expected-warning {{declaration of a variadic function without a comma before '...' is 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 r(T...) {} // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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...){};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've added these test cases


auto type_specifier = (void (*)(int...)) nullptr; // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}

auto lambda = [](int...) {}; // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}

auto block = ^(int...){}; // expected-warning {{declaration of a variadic function without a comma before '...' is deprecated}}
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