patch via mailing list: Use getLocation() in too few/many arguments diagnostic

[John Marshall via cfe-commits]

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.reinterpretcast at 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.Marshall at glasgow.ac.uk>
AuthorDate: Mon Jan 20 14:58:14 2020 +0000
Commit:     John Marshall <John.W.Marshall at 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;