[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #7 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to David Binderman from comment #6)
> I'd like to vote for -Wduplicated-cond being in either -Wextra or -Wall.
> 
> I only just found it last week (thanks to Weverything discussion)
> and it is proving useful in finding bugs in fedora rawhide.

-Wduplicated-branches in -Wextra too. 

I've switched on both in my local copy of gcc. Useful.

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #8 from Eric Gallager <egallager at gcc dot gnu.org> ---
Any reason not to put -Wnested-externs in -Wall or -Wextra?

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #9 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Eric Gallager from comment #8)
> Any reason not to put -Wnested-externs in -Wall or -Wextra?

It warns for any "extern" declaration in other than file scope.  This is
completely standard C, and useful as well.

It is harmful to warn about idiomatic code, especially if there is no real
alternative.

I don't think anyone will accidentally do this either (you don't type "extern"
if you don't mean it, after all).

Do you have any example where this warning helped you find a bug?

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #10 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #9)
> (In reply to Eric Gallager from comment #8)
> > Any reason not to put -Wnested-externs in -Wall or -Wextra?
> 
> It warns for any "extern" declaration in other than file scope.  This is
> completely standard C, and useful as well.
> 
> It is harmful to warn about idiomatic code, especially if there is no real
> alternative.
> 
> I don't think anyone will accidentally do this either (you don't type
> "extern" if you don't mean it, after all).
> 
> Do you have any example where this warning helped you find a bug?

It may have been idiomatic 30 years ago, but it isn't any longer. And there's
an easy alternative: just move the declaration out of the function and up to
file scope. It makes more sense that way and looks better. Or, better yet,
include the proper header file for the extern declaration. Most instances I've
seen of the warning have been stuff like people putting "extern int errno"
inside of a function instead of realizing that they're supposed to just do
#include <errno.h> instead (possibly because the code is from before the header
was standardized?). In the errno case it's particularly harmful because errno
may be defined as a macro that's incompatible with the declaration being used.

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #11 from David Binderman <dcb314 at hotmail dot com> ---
-Wold-style-definition

KnR style function definitions have been deprecated for about 35 years.

Yes, there is a warning for it in gcc, but that warning isn't in either
-Wall or -Wextra. 

Also, switching on -std=c2x only makes it a warning, not an error. 
That doesn't sound correct to me. AFAIK KnR is removed from c2x.

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #12 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to David Binderman from comment #11)
> -Wold-style-definition
> 
> KnR style function definitions have been deprecated for about 35 years.

+1

> Yes, there is a warning for it in gcc, but that warning isn't in either
> -Wall or -Wextra. 
> 
> Also, switching on -std=c2x only makes it a warning, not an error. 
> That doesn't sound correct to me. AFAIK KnR is removed from c2x.

It can stay there as a GCC extension no problem.  If there is a warning for it
it is not a problem anymore, esp. if that warning is in -Wall or on by default
even.

The only reason to remove support for this is if it actually simplifies the
compiler code itself, which means we have to completely not support it anymore.
So let's first have a default warning, and then see if anything in the wild
still uses old-style functions defs?

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[schwab@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #13 from Andreas Schwab <schwab@linux-m68k.org> ---
Like bash.

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #14 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to David Binderman from comment #11)
> -Wold-style-definition
> 
> KnR style function definitions have been deprecated for about 35 years.
> 
> Yes, there is a warning for it in gcc, but that warning isn't in either
> -Wall or -Wextra. 
> 
> Also, switching on -std=c2x only makes it a warning, not an error. 
> That doesn't sound correct to me. AFAIK KnR is removed from c2x.

Related: Bug 82922 for -Wstrict-prototypes, which warns in some of the same
cases as -Wold-style-definition
(82922 is already in the "Depends On" field)

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #15 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to Segher Boessenkool from comment #12)
> The only reason to remove support for this is if it actually simplifies the
> compiler code itself, which means we have to completely not support it
> anymore.

Standards conformance might well be a more important reason.

> So let's first have a default warning, and then see if anything in the wild
> still uses old-style functions defs?

I just compiled about 250 packages from Fedora Rawhide
with compiler flag -Wold-style-definition.

The average number of occurrences of the warning message per package is current
running around 30.

Admittedly a small sample of the 8,000 or so packages in Fedora Rawhide,
but current estimates are about 240,000 old style function definitions
for that distribution.

It might well be worthwhile putting -Wold-style-definition into -Wextra
for a year or so and see how we get on.

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, what is most important with this are configure scripts, if we start
warning on something still widely used in configure snippets, we'll get
silently different results of configure checks.
For old style definitions, the question is if we want to warn about
void foo () {} style of functions or just those which actually have some
arguments.

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #17 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #16)
> Note, what is most important with this are configure scripts, if we start
> warning on something still widely used in configure snippets, we'll get
> silently different results of configure checks.

A configure check that isn't specifically for some warning) that gives
different
results if some random warning happens, is fundamentally broken already.  I
would
hope existing checks are more robust (but I certainly believe they are not :-(
)

> For old style definitions, the question is if we want to warn about
> void foo () {} style of functions or just those which actually have some
> arguments.

We can have a =2 to warn for everything, and =1 for just the more serious
things?
Easy to switch default for -Wall and -W that way, too.

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

--- Comment #18 from Eric Gallager <egallager at gcc dot gnu.org> ---
-Wmissing-declarations came up as a possibility for this in IRC

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656
Bug 87656 depends on bug 87729, which changed state.

Bug 87729 Summary: Please include -Woverloaded-virtual in -Wall
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87729

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

[Bug c++/87656] Useful flags to enable with -Wall or -Wextra

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tschwinge at gcc dot gnu.org

--- Comment #19 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(In reply to David Binderman from comment #6)
> I'd like to vote for -Wduplicated-cond being in either -Wextra or -Wall.
> 
> [...] it is proving useful in finding bugs [...]

Generally ACK, but note that '-Wduplicated-cond' once has been in '-Wall', but
then again was removed; PR67819,
<https://inbox.sourceware.org/gcc-patches/20151002162737.GD6184@redhat.com>.