-Wall

-Wall

[Lars Magne Ingebrigtsen]

There's a lot fewer warnings when compiling with -Wall in emacs/src now
than there used to be, but there should be none, I think.

Are there any of them that, like, are there for a reason (i.e., we
disagree with the gcc people that those things should be warned about),
or are they all things that should be fixed?

If it's the latter, I think we should fix those, and have -Wall as a
standard compilation switch.  It's much easier to write cleanly C code
with -Wall than without.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: -Wall

[Paul Eggert]

On 04/04/2016 11:22 AM, Lars Magne Ingebrigtsen wrote:
> There's a lot fewer warnings when compiling with -Wall in emacs/src now
> than there used to be, but there should be none, I think.

I'm getting no warnings when I build.

> Are there any of them that, like, are there for a reason (i.e., we
> disagree with the gcc people that those things should be warned about),
> or are they all things that should be fixed?
>

It's more the other way around: -Wall doesn't generate enough warnings. 
Please try './configure --enable-gcc-warnings'. This uses -Wall plus 
some other warning flags that are useful in practice. I regularly check 
that this builds cleanly for recent GCC on my platform (Fedora 23, GCC 
5.3.1 20151207 (Red Hat 5.3.1-2)), and occasionally for recent Ubuntu, 
both using default configure-time options.

If you'd like to take up the burden of porting --enable-gcc-warnings to 
other platforms feel free, though my suggestion is to not bother with 
older and/or rarely-used platforms as they have too many bugs in this area.

Re: -Wall

[Lars Magne Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 04/04/2016 11:22 AM, Lars Magne Ingebrigtsen wrote:
>> There's a lot fewer warnings when compiling with -Wall in emacs/src now
>> than there used to be, but there should be none, I think.
>
> I'm getting no warnings when I build.

Hm...  I just added -Wall to the options line in the Makefile, and I get
a couple dozen.  But examining most of these more closely, they're the
result of this:

  RGB_PIXEL_COLOR corner_pixels[4], best IF_LINT (= 0);

So I guess the IF_LINT is triggered by...  something?

Anyway, if you're not seeing any warnings with the setup you're using,
shouldn't those options be the default compilation options?

> It's more the other way around: -Wall doesn't generate enough
> warnings. Please try './configure --enable-gcc-warnings'. This uses
> -Wall plus some other warning flags that are useful in practice. I
> regularly check that this builds cleanly for recent GCC on my platform
> (Fedora 23, GCC 5.3.1 20151207 (Red Hat 5.3.1-2)), and occasionally
> for recent Ubuntu, both using default configure-time options.
>
> If you'd like to take up the burden of porting --enable-gcc-warnings
> to other platforms feel free, though my suggestion is to not bother
> with older and/or rarely-used platforms as they have too many bugs in
> this area.

How about making that option the default on GNU/Linux systems?  There
would be less code for you to clean up after I've done C level changes
and forgotten to twiddle the options.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: -Wall

[John Wiegley]

>>>>> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> There's a lot fewer warnings when compiling with -Wall in emacs/src now than
> there used to be, but there should be none, I think.

I'm very much in favor of addressing and eliminating all warnings on as many
platforms as possible.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: -Wall

[Paul Eggert]

On 04/04/2016 11:58 AM, Lars Magne Ingebrigtsen wrote:
>
> Hm...  I just added -Wall to the options line in the Makefile, and I get
> a couple dozen.  But examining most of these more closely, they're the
> result of this:
>
>    RGB_PIXEL_COLOR corner_pixels[4], best IF_LINT (= 0);
>
> So I guess the IF_LINT is triggered by...  something?

Yes, -Dlint or its equivalent, which --enable-gcc-warnings arranges for.

Re: -Wall

[Lars Magne Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Yes, -Dlint or its equivalent, which --enable-gcc-warnings arranges for.

And I see that --enable-gcc-warnings doesn't just enable warnings, but
makes them into errors:

make[1]: Entering directory `/home/larsi/src/emacs/trunk/src'
  GEN      globals.h
  CC       vm-limit.o
vm-limit.c:58:0: error: macro "__MALLOC_HOOK_VOLATILE" is not used [-Werror=unused-macros]
cc1: all warnings being treated as errors
make[1]: *** [vm-limit.o] Error 1
make[1]: Leaving directory `/home/larsi/src/emacs/trunk/src'
make: *** [src] Error 2

(This is on an older Debian machine.)

I still think that warnings should be enabled by default (on Linux), but
they shouldn't be treated as errors.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: -Wall

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

John Wiegley <jwiegley@gmail.com> schrieb am Di., 5. Apr. 2016 um 01:23 Uhr:

> >>>>> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
>
> > There's a lot fewer warnings when compiling with -Wall in emacs/src now
> than
> > there used to be, but there should be none, I think.
>
> I'm very much in favor of addressing and eliminating all warnings on as
> many
> platforms as possible.
>

That'll be a challenge. When I compile with clang -Weverything, I get
~90,000 warnings.

[-- Attachment #2: Type: text/html, Size: 857 bytes --]

Re: -Wall

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Tue, 05 Apr 2016 09:39:02 +0200
> Cc: emacs-devel@gnu.org
> 
> I still think that warnings should be enabled by default (on Linux), but
> they shouldn't be treated as errors.

I disagree.  "gcc -Wall" is at times very noisy and tends to flag
perfectly valid C as suspect.  The noise level sometimes makes it hard
to find real problems drowned in the sea of false alarms.

I usually compile once with -Wall after significant changes, and that
is enough to catch any real issues.

Re: -Wall

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Tue, 05 Apr 2016 13:16:02 +0000
> 
>  I'm very much in favor of addressing and eliminating all warnings on as many
>  platforms as possible.
> 
> That'll be a challenge. When I compile with clang -Weverything, I get ~90,000 warnings. 

I don't even think it's a worthy use of our resources.  C compilers
move towards more and more noisy defaults, so the real challenge is to
come up with a set of switches that produce a high enough
signal-to-noise ratio.  I'm not saying that the current default is
that sweet spot, but -Wall, let alone -Weverything, is certainly not
it.

Re: -Wall

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

It is GNU project policy that we do NOT make our programs
ugly in the name of having zero warnings in with -Wall.
Such a goal is misguided.

-Wall is too fussy.  It warns about constructs that are
entirely valid and reasonable to use.  I designed it to be
the most extreme pedant possible.  We should not let the
existence of that option dictate to us what is good C code.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: -Wall

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

>> I'm very much in favor of addressing and eliminating all warnings on as many
>> platforms as possible.
>> 
>> That'll be a challenge. When I compile with clang -Weverything, I get
>> ~90,000 warnings.

> I don't even think it's a worthy use of our resources. C compilers move
> towards more and more noisy defaults, so the real challenge is to come up
> with a set of switches that produce a high enough signal-to-noise ratio. I'm
> not saying that the current default is that sweet spot, but -Wall, let alone
> -Weverything, is certainly not it.

-Weverything may be a bit too much, but the alternative -- that of ignoring
warnings because there are too many of them -- is also not a good choice. We
should find the sweet spot we want to adhere to, and then make it clean.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: -Wall

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]

On 04/05/2016 12:39 AM, Lars Magne Ingebrigtsen wrote:
> warnings should be enabled by default (on Linux), but
> they shouldn't be treated as errors.

It's good at times to not treat them as errors, so I installed the 
attached patch to add that capability more clearly, with a new 
configure-time option --enable-gcc-warnings=warn-only. The attached 
patch makes this new option the default if you have a Git checkout of 
Emacs and GCC 5.3 or later. This shouldn't affect Eli any time soon, 
since he uses an old compiler.

If this patch causes trouble by generating too many warnings on some 
platforms, we can dial it back by suppressing warnings on those 
platforms. But the latest GNU/Linux platforms running the latest GTK 
should be warnings-free.

As others have noted in this thread, -Wall generates too many warnings. 
The --enable-gcc-warnings option suppresses GCC warnings that are false 
alarms for Emacs. It also enables some warnings that -Wall does not, but 
which work well with Emacs.

[-- Attachment #2: 0001-Enable-GCC-warnings-in-developer-builds.patch --]
[-- Type: application/x-patch, Size: 5591 bytes --]

Re: -Wall

[Lars Magne Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> It's good at times to not treat them as errors, so I installed the
> attached patch to add that capability more clearly, with a new
> configure-time option --enable-gcc-warnings=warn-only. The attached
> patch makes this new option the default if you have a Git checkout of
> Emacs and GCC 5.3 or later.

Great, thanks!

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: -Wall

[Stefan Monnier]

>> warnings should be enabled by default (on Linux), but
>> they shouldn't be treated as errors.

Agreed.

> It's good at times to not treat them as errors, so I installed the attached
> patch to add that capability more clearly, with a new configure-time
> option --enable-gcc-warnings=warn-only. The attached patch makes this new

I don't think the default behavior for --enable-gcc-warnings should be
to treat them as errors.  IOW I think "--enable-gcc-warnings" should
just ask gcc to emit the warnings, and then have
"--enable-gcc-warnings=error" to treat them as errors.

I think it'd be both closer to what most people would want (just
a guess, obviously) and would make more sense from the point of view of
the meaning of the words.

IOW if we want "--enable-foo" to have errors and "--enable-foo=bar" to
just emit the warnings, then "foo" shouldn't be "gcc-warnings".


        Stefan

Re: -Wall

[Paul Eggert]

Stefan Monnier wrote:
> I don't think the default behavior for --enable-gcc-warnings should be
> to treat them as errors.

Yes, we could change it in that way. For what it's worth, in other GNU projects 
--enable-gcc-warnings treats them as errors by default, I expect because Jim 
Meyering felt that people too often ignore warnings that are not errors. (We 
could change those other projects too...)