Message ID | 20160601115504.725560-2-marbacz@gmail.com |
---|---|
State | New |
Headers | show |
On Wed, Jun 01, 2016 at 01:55:04PM +0200, Marcin Baczyński wrote: > PR c/48116. > > Botstrapped and tested on x86_64-pc-linux-gnu. > > gcc/ChangeLog: > > * c/c-typeck.c (c_finish_return): emit warning about return with a > void expression in a function returning void if warn_return_type. This is a GNU extension, so I fail to see why you should warn. Furthermore, the way you've done it, you would still warn instead of emit error e.g. with -pedantic-errors on it. Also, c/ prefixes don't belong to ChangeLog entries, gcc/c/ChangeLog is -separate, and after : the first letter should be capitalized. > > gcc/testsuite/ChangeLog: > > * gcc.dg/Wreturn-type3.c: new test. > --- > gcc/c/c-typeck.c | 4 ++++ > gcc/testsuite/gcc.dg/Wreturn-type3.c | 6 ++++++ > 2 files changed, 10 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/Wreturn-type3.c > > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c > index 1520c20..8ac6cd4 100644 > --- a/gcc/c/c-typeck.c > +++ b/gcc/c/c-typeck.c > @@ -9700,6 +9700,10 @@ c_finish_return (location_t loc, tree retval, tree origtype) > warned_here = pedwarn > (xloc, 0, > "%<return%> with a value, in function returning void"); > + else if (warn_return_type) > + warned_here = warning_at > + (loc, OPT_Wreturn_type, > + "%<return%> with expression, in function returning void"); > else > warned_here = pedwarn > (xloc, OPT_Wpedantic, "ISO C forbids " > diff --git a/gcc/testsuite/gcc.dg/Wreturn-type3.c b/gcc/testsuite/gcc.dg/Wreturn-type3.c > new file mode 100644 > index 0000000..e83c94b > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/Wreturn-type3.c > @@ -0,0 +1,6 @@ > +/* PR c/48116 */ > +/* { dg-do compile } */ > +/* { dg-options "-Wreturn-type" } */ > + > +void a() {} > +void b() { return a(); } /* { dg-warning "return" "returning void" } */ > -- > 2.8.3 Jakub
On Wed, Jun 01, 2016 at 02:44:22PM +0200, Marcin Baczyński wrote: > On 1 Jun 2016 14:07, "Jakub Jelinek" <jakub@redhat.com> wrote: > > > > On Wed, Jun 01, 2016 at 01:55:04PM +0200, Marcin Baczyński wrote: > > > PR c/48116. > > > > > > Botstrapped and tested on x86_64-pc-linux-gnu. > > > > > > gcc/ChangeLog: > > > > > > * c/c-typeck.c (c_finish_return): emit warning about return with a > > > void expression in a function returning void if warn_return_type. > > > > This is a GNU extension, so I fail to see why you should warn. > > The bug is on the easyhacks list, I thought it would be a good place to > start digging into the GCC code base. First of all, I'm not the C FE maintainer, so these are just IMHO comments, the maintainers might have different opinion. > Do you have any suggestions for better things to start with? Welcome to GCC hacking! I'm must say I don't know in what state the easy hacks list is, but for bugs that have been sitting in bugzilla for many years there often is a reason why they haven't been addressed - it could be just lack of interest from anybody, or that they fell through without anybody noticing, but could be also that they aren't all that easy. Often the easiest to fix bugs could be among the most recently filed PRs. Jakub
On 06/01/2016 10:39 AM, Jakub Jelinek wrote: > On Wed, Jun 01, 2016 at 02:44:22PM +0200, Marcin Baczyński wrote: >> On 1 Jun 2016 14:07, "Jakub Jelinek" <jakub@redhat.com> wrote: >>> >>> On Wed, Jun 01, 2016 at 01:55:04PM +0200, Marcin Baczyński wrote: >>>> PR c/48116. >>>> >>>> Botstrapped and tested on x86_64-pc-linux-gnu. >>>> >>>> gcc/ChangeLog: >>>> >>>> * c/c-typeck.c (c_finish_return): emit warning about return with a >>>> void expression in a function returning void if warn_return_type. >>> >>> This is a GNU extension, so I fail to see why you should warn. >> >> The bug is on the easyhacks list, I thought it would be a good place to >> start digging into the GCC code base. > > First of all, I'm not the C FE maintainer, so these are just IMHO comments, > the maintainers might have different opinion. > >> Do you have any suggestions for better things to start with? Since you've done a lot of work on this bug already I think it would be worthwhile to see it through to completion. Given that, as Jakub mentioned, the absence of the warning in permissive mode (i.e. without -Wpedantic) is a GCC extension to C, it would be helpful to mention it in the manual (doing so would resolve the submitter's complaint that the -Wreturn-type option doesn't work as documented). With the documentation patch accepted, the bug can then be closed as resolved/fixed and taken off the easy hacks list. Martin
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 1520c20..8ac6cd4 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -9700,6 +9700,10 @@ c_finish_return (location_t loc, tree retval, tree origtype) warned_here = pedwarn (xloc, 0, "%<return%> with a value, in function returning void"); + else if (warn_return_type) + warned_here = warning_at + (loc, OPT_Wreturn_type, + "%<return%> with expression, in function returning void"); else warned_here = pedwarn (xloc, OPT_Wpedantic, "ISO C forbids " diff --git a/gcc/testsuite/gcc.dg/Wreturn-type3.c b/gcc/testsuite/gcc.dg/Wreturn-type3.c new file mode 100644 index 0000000..e83c94b --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wreturn-type3.c @@ -0,0 +1,6 @@ +/* PR c/48116 */ +/* { dg-do compile } */ +/* { dg-options "-Wreturn-type" } */ + +void a() {} +void b() { return a(); } /* { dg-warning "return" "returning void" } */