Re: [PATCH] c/55976 -Werror=return-type should error on returning a value from a void function

[dave dot pagan at oracle dot com]

On 04/04/2018 10:58 AM, Martin Sebor wrote:
> On 04/04/2018 11:15 AM, Jakub Jelinek wrote:
> > On Tue, Apr 03, 2018 at 01:36:13PM -0600, Martin Sebor wrote:
> > > On 04/03/2018 10:26 AM, dave.pagan@oracle.com wrote:
> > > > This patch fixes handlng of -Werror=return-type. Currently, even with
> > > > the flag specified, return type errors remain warnings.
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55976
> > > >
> > > > Function c_finish_return (), when calling pedwarn, should specifiy
> > > > OPT_Wreturn_type as the diagnostic index if warn_return_type is set so
> > > > that the given diagnostic is generated as an error, not a warning.
> > > >
> > > > Patch was successfully bootstrapped and tested on x86_64-linux.
> > >
> > > I would expect the option to control the warning consistently so
> > > that when the test case is compiled with just -Wno-return-type
> > > (and no other options) the warning is not issued.  But that's not
> > > what happens because pedwarn() is called with a zero argument as
> > > the option.
> >
> > I think we need to make sure we error out even with -Wno-return-type
> > when -pedantic-errors.
>
> That would seem surprising to me.  Is there an existing
> precedent for this in GCC? (Any other warnings or options
> that are treated this way?)
>
> It would also diverge from Clang, which would be particularly
> unhelpful to users of both compilers.  I would suggest to
> follow what Clang does in terms of controlling the warning
> (though not necessarily in its severity).  It's consistent
> and intuitive.  (Clang has -Werror=return-type by default;
> that may be overly strict.)

I think these are both good points. While I tend to lean toward 
consistency (both within GCC and with clang), if this sort of problem is 
potentially worse in GCC 8 (as Jakub suggests) then perhaps it's worth 
thinking about how to help prevent it. If we do choose to go this 
direction with -pedantic-errors, and there isn't a precedent for it, 
then the documentation would require an update to reflect the new behavior.

Also, thoughts on this question from my last email?

> Since this patch does fix the original problem, what do you recommend? 
> Scrap this patch? Or let it proceed and submit a new bug noting the 
> (existing) incorrect behavior of -Wno-return-type? 

We could add the discussion in this email to any new bug we create for 
-Wno-return-type.

--Dave
> Martin
>
> > Especially when issues this pedwarn warns about are
> > very hard to debug show stoppers for anybody calling such functions 
> > in GCC 8
> > (because we turn such spots into __builtin_unreachable () and thus 
> > randomly
> > can execute completely unrelated code).  So, I think consistency 
> > isn't that
> > important here.
> >
> >     Jakub