[clang] Fix inaccurate wording of warn_second_arg_of_va_start_not_last_named_param #131238
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t_named_param Rename warn_second_arg_of_va_start_not_last_named_param to the more accurately wording warn_second_arg_of_va_start_not_last_non_variadic_param
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Imad Aldij (imdj) ChangesRename and update the wording of As per issue: Full diff: https://github.com/llvm/llvm-project/pull/131238.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e6e6e892cdd7..135d3bd161356 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -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 "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 9bcb014ab9bfc..e99e30d75df94 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -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);
+ 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
diff --git a/clang/test/C/C23/n2975.c b/clang/test/C/C23/n2975.c
index 2269400fe47c3..338f987f6a65d 100644
--- a/clang/test/C/C23/n2975.c
+++ b/clang/test/C/C23/n2975.c
@@ -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);
}
diff --git a/clang/test/Sema/varargs-x86-64.c b/clang/test/Sema/varargs-x86-64.c
index f3164400d8479..ff7fdd533ab0b 100644
--- a/clang/test/Sema/varargs-x86-64.c
+++ b/clang/test/Sema/varargs-x86-64.c
@@ -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);
}
diff --git a/clang/test/Sema/varargs.c b/clang/test/Sema/varargs.c
index bec41dda65d57..315589ff3fd16 100644
--- a/clang/test/Sema/varargs.c
+++ b/clang/test/Sema/varargs.c
@@ -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);
}
diff --git a/clang/test/SemaCXX/vararg-non-pod.cpp b/clang/test/SemaCXX/vararg-non-pod.cpp
index 0c24ced40f7aa..0744004fdeaef 100644
--- a/clang/test/SemaCXX/vararg-non-pod.cpp
+++ b/clang/test/SemaCXX/vararg-non-pod.cpp
@@ -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, ...) {
diff --git a/clang/test/SemaCXX/varargs.cpp b/clang/test/SemaCXX/varargs.cpp
index bc2fe89a6ff8d..1e5920a8728d3 100644
--- a/clang/test/SemaCXX/varargs.cpp
+++ b/clang/test/SemaCXX/varargs.cpp
@@ -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
|
cor3ntin
approved these changes
Mar 14, 2025
|
@imdj Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
erichkeane
reviewed
Mar 14, 2025
| @@ -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); | |||
There was a problem hiding this comment.
I would have preferred we change the name of the control variable (SecondArgIsLastNamedArg) as well/as a part of this.
There was a problem hiding this comment.
What is the recommended action here? Is it worth it to open a new PR?
There was a problem hiding this comment.
If you'd like, a new PR would be greatly appreciated!
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/8427 Here is the relevant piece of the build log for the reference |
AaronBallman
pushed a commit
that referenced
this pull request
Mar 17, 2025
…istency (#131346) Change the name of the control variable `SecondArgIsLastNamedArgument` to `SecondArgIsLastNonVariadicArgument` for clarity and consistency. Following feedback on earlier PR that was merged: - #131238 (comment)
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Mar 17, 2025
…ty and consistency (#131346) Change the name of the control variable `SecondArgIsLastNamedArgument` to `SecondArgIsLastNonVariadicArgument` for clarity and consistency. Following feedback on earlier PR that was merged: - llvm/llvm-project#131238 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rename and update the wording of
warn_second_arg_of_va_start_not_last_named_paramto best indicate that it's actually the last non-variadic parameter instead.As per issue:
warn_second_arg_of_va_start_not_last_named_paramis inaccurate- #131171