Thanks Aaron (and Hubert).
I've attached an updated patch that now includes new test cases alongside some
existing "too few / too many" test cases in test/Sema/exprs.c. This splits the
function declaration over two lines so it can use -verify to validate the
source location's line (but not column). If you'd prefer a FileCheck approach
to get the column too, I'm happy to do that but please advise whether it would
be best to create a new test/Sema/foo.c file for these tests or to add to one
of the existing test files.
Verified that without the patch, the notes are on the "MY_EXPORT void" line and
the test cases fail. All tests still pass after this, after adjusting one
existing FileCheck-based test case that also happens to exercise the patch's
change.
John
On 7 Feb 2020, at 15:40, Aaron Ballman wrote:
> Thank you for the patch -- I think the changes look reasonable, but it
> should come with some test cases as well. Source location stuff is a
> bit onerous to try to test, but I think the best approach would be to
> add a new test that uses FileCheck instead of -verify so that you can
> validate the source location's line and column are as expected in the
> note. Once you have such a test (and have verified that no other tests
> fail with your changes), I'm happy to commit on your behalf.
>
> ~Aaron
>
> On Fri, Feb 7, 2020 at 10:23 AM Hubert Tong
> <hubert.reinterpretc...@gmail.com> wrote:
>>
>> I think this looks okay. I think Richard or Aaron might be able to provide a
>> more informed opinion.
>>
>> -- HT
commit cbd4a4a155b40dc77c2ed82f397fe303dfc10837
Author: John Marshall <john.w.marsh...@glasgow.ac.uk>
AuthorDate: Mon Jan 20 14:58:14 2020 +0000
Commit: John Marshall <john.w.marsh...@glasgow.ac.uk>
CommitDate: Mon Feb 10 14:30:58 2020 +0000
Use getLocation() in "too few/too many arguments" diagnostic
Use the more accurate location when emitting the location of the
function being called's prototype in diagnostics emitted when calling
a function with an incorrect number of arguments.
In particular, avoids showing a trace of irrelevant macro expansions
for "MY_EXPORT static int AwesomeFunction(int, int);". Fixes PR#23564.
Add test cases alongside other "too few/too many arguments" tests.
Adjust column position in incidentally related FileCheck-based test.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ffe72c98356..b9d7024f083 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5194,7 +5194,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
// Emit the location of the prototype.
if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
- Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
+ Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
return true;
}
@@ -5239,7 +5239,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
// Emit the location of the prototype.
if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
- Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
+ Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
// This deletes the extra arguments.
Call->shrinkNumArgs(NumParams);
diff --git a/clang/test/Misc/serialized-diags.c
b/clang/test/Misc/serialized-diags.c
index e401477a2eb..2f4b86fb42f 100644
--- a/clang/test/Misc/serialized-diags.c
+++ b/clang/test/Misc/serialized-diags.c
@@ -56,7 +56,7 @@ void rdar11040133() {
// CHECK: Range: {{.*[/\\]}}serialized-diags.c:22:13
{{.*[/\\]}}serialized-diags.c:22:18
// CHECK: +-{{.*[/\\]}}serialized-diags.c:20:15: note: expanded from macro
'false' []
// CHECK: +-Range: {{.*[/\\]}}serialized-diags.c:20:15
{{.*[/\\]}}serialized-diags.c:20:16
-// CHECK: +-{{.*[/\\]}}serialized-diags.c:19:1: note: 'taz' declared here []
+// CHECK: +-{{.*[/\\]}}serialized-diags.c:19:6: note: 'taz' declared here []
// CHECK: {{.*[/\\]}}serialized-diags.h:5:7: warning: incompatible integer to
pointer conversion initializing 'char *' with an expression of type 'int'
[-Wint-conversion]
// CHECK: Range: {{.*[/\\]}}serialized-diags.h:5:16
{{.*[/\\]}}serialized-diags.h:5:17
// CHECK: +-{{.*[/\\]}}serialized-diags.c:26:10: note: in file included from
{{.*[/\\]}}serialized-diags.c:26: []
diff --git a/clang/test/Sema/exprs.c b/clang/test/Sema/exprs.c
index 760c45e02f3..4e144041aca 100644
--- a/clang/test/Sema/exprs.c
+++ b/clang/test/Sema/exprs.c
@@ -163,12 +163,15 @@ void test17(int x) {
x = sizeof(x/0); // no warning.
}
-// PR6501 & PR11857
+// PR6501, PR11857, and PR23564
void test18_a(int a); // expected-note 2 {{'test18_a' declared here}}
void test18_b(int); // expected-note {{'test18_b' declared here}}
void test18_c(int a, int b); // expected-note 2 {{'test18_c' declared here}}
void test18_d(int a, ...); // expected-note {{'test18_d' declared here}}
void test18_e(int a, int b, ...); // expected-note {{'test18_e' declared here}}
+#define MY_EXPORT __attribute__((visibility("default")))
+MY_EXPORT void // (no "declared here" notes on this line, no "expanded from
MY_EXPORT" notes either)
+test18_f(int a, int b); // expected-note 2 {{'test18_f' declared here}}
void test18(int b) {
test18_a(b, b); // expected-error {{too many arguments to function call,
expected single argument 'a', have 2}}
test18_a(); // expected-error {{too few arguments to function call, single
argument 'a' was not specified}}
@@ -177,6 +180,8 @@ void test18(int b) {
test18_c(b, b, b); // expected-error {{too many arguments to function call,
expected 2, have 3}}
test18_d(); // expected-error {{too few arguments to function call, at least
argument 'a' must be specified}}
test18_e(); // expected-error {{too few arguments to function call, expected
at least 2, have 0}}
+ test18_f(b); // expected-error {{too few arguments to function call,
expected 2, have 1}}
+ test18_f(b, b, b); // expected-error {{too many arguments to function call,
expected 2, have 3}}
}
typedef int __attribute__((address_space(256))) int_AS256;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
