patch via mailing list: Use getLocation() in too few/many arguments diagnostic
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 18 08:24:02 PST 2020
I've commit on your behalf in
260b91f379c8f86d3d6008648b3f2a945a007888, thank you for the patch!
~Aaron
On Mon, Feb 10, 2020 at 12:42 PM Aaron Ballman <aaron at aaronballman.com> wrote:
>
> On Mon, Feb 10, 2020 at 10:06 AM John Marshall
> <John.W.Marshall at glasgow.ac.uk> wrote:
> >
> > 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.
>
> Thank you for the patch -- I think it is fine to not check the column
> for the cases you've added, so this iteration LGTM. I'm happy to
> commit this on your behalf, but it will have to wait until next week
> because I'm out at wg21 meetings this week. If no one else commits
> this before I get home, I'll handle it then.
>
> ~Aaron
>
> >
> > 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;
> >
More information about the cfe-commits
mailing list