Skip to content

[clang] Fix inaccurate wording of warn_second_arg_of_va_start_not_last_named_param #131238

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 1 commit into from
Mar 14, 2025
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
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -10613,8 +10613,8 @@ def err_va_start_used_in_wrong_abi_function : Error<
"'va_start' used in %select{System V|Win64}0 ABI function">;
def err_ms_va_start_used_in_sysv_function : Error<
"'__builtin_ms_va_start' used in System V ABI function">;
def warn_second_arg_of_va_start_not_last_named_param : Warning<
"second argument to 'va_start' is not the last named parameter">,
def warn_second_arg_of_va_start_not_last_non_variadic_param : Warning<
"second argument to 'va_start' is not the last non-variadic parameter">,
InGroup<Varargs>;
def warn_c17_compat_ellipsis_only_parameter : Warning<
"'...' as the only parameter of a function is incompatible with C standards "
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4893,7 +4893,7 @@ bool Sema::BuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {

if (!SecondArgIsLastNamedArgument)
Diag(TheCall->getArg(1)->getBeginLoc(),
diag::warn_second_arg_of_va_start_not_last_named_param);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would have preferred we change the name of the control variable (SecondArgIsLastNamedArg) as well/as a part of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the recommended action here? Is it worth it to open a new PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you'd like, a new PR would be greatly appreciated!

diag::warn_second_arg_of_va_start_not_last_non_variadic_param);
else if (IsCRegister || Type->isReferenceType() ||
Type->isSpecificBuiltinType(BuiltinType::Float) || [=] {
// Promotable integers are UB, but enumerations need a bit of
Expand Down
2 changes: 1 addition & 1 deletion clang/test/C/C23/n2975.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void diag(int a, int b, ...) {
va_start(list, a);
// However, the builtin itself is under no such constraints regarding
// expanding or evaluating the second argument, so it can still diagnose.
__builtin_va_start(list, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
__builtin_va_start(list, a); // expected-warning {{second argument to 'va_start' is not the last non-variadic parameter}}
va_end(list);
}

Expand Down
4 changes: 2 additions & 2 deletions clang/test/Sema/varargs-x86-64.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ void __attribute__((ms_abi)) g1(int a) {
void __attribute__((ms_abi)) g2(int a, int b, ...) {
__builtin_ms_va_list ap;

__builtin_ms_va_start(ap, 10); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
__builtin_ms_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
__builtin_ms_va_start(ap, 10); // expected-warning {{second argument to 'va_start' is not the last non-variadic parameter}}
__builtin_ms_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last non-variadic parameter}}
__builtin_ms_va_start(ap, b);
}

Expand Down
4 changes: 2 additions & 2 deletions clang/test/Sema/varargs.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ void f2(int a, int b, ...)
{
__builtin_va_list ap;

__builtin_va_start(ap, 10); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
__builtin_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
__builtin_va_start(ap, 10); // expected-warning {{second argument to 'va_start' is not the last non-variadic parameter}}
__builtin_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last non-variadic parameter}}
__builtin_va_start(ap, b);
}

Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/vararg-non-pod.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ void t6(Foo somearg, ... ) {
// it should behave the same
void t6b(Foo somearg, ... ) {
__builtin_va_list list;
__builtin_stdarg_start(list, somearg); // second argument to 'va_start' is not the last named parameter [-Wvarargs]
__builtin_stdarg_start(list, somearg); // second argument to 'va_start' is not the last non-variadic parameter [-Wvarargs]
}

void t7(int n, ...) {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/varargs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ void g(register int i, ...) { // expected-warning 0-1{{deprecated}}
// Don't crash when there is no last parameter.
void no_params(...) {
int a;
__builtin_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
__builtin_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last non-variadic parameter}}
}

// Reject this. The __builtin_va_start would execute in Foo's non-variadic
Expand Down