On 04/04/2018 05:50 PM, dave.pa...@oracle.com wrote:
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.pa...@oracle.com wrote:
This patch fixes handlng of -Werror=return-type. Currently, even with
the flag specified, return type errors remain warnings.


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.

I actually don't think -Wpedantic is the appropriate option
to control the warning in the case Jakub is concerned about
(it may be appropriate for warning about returning a value
from a void function because that's a constraint violation).
-Wpedantic is meant for diagnostics that are required by
the C/C++ standards but that GCC may otherwise silently
accept as extensions.  Defining a non-void function that
doesn't return a value is valid in C is not incorrect and
does not require a diagnostic.  (Using the return value
is undefined, but when it isn't used there is no problem.)

(This is different in recent versions of C++ where a return
statement with no operand in a non-void function requires
a diagnostic, so the C++ handling may need to be different.)

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

I think for C, handling it under the same bug and in the same
patch would be best.  The C++ bits could come later if needed.




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
can execute completely unrelated code).  So, I think consistency
isn't that
important here.


Reply via email to