GNOME.org
 

Re: -Werror considered harmful

On Tue, Feb 26, 2013 at 10:20 PM, Maciej Piechotka
<uzytkownik2 gmail com> wrote:

Wouldn't it be better to selectively enable the -Wall -Werror rather
then selectively disabled it? People compiling the library potentially
care less about warning/errors then developers and know less about build
system (was it --disable-fatal-warnings? --disable-Werror? etc.)

Enabling warnings is as hard as:

CFLAGS="-Wall -Werror" ./configure .... && make

while disabling is more problematic - -w is a workaround but from what I
heard it doesn't play nice with autotools and it doesn't help if -Wall
-Werror (or equivalent) are appended rather then prepended to CFLAGS.

Additional problem is that heuristic of compilers are getting better
each version and there is sometimes no clean way of silencing warning -
unused result warning for example work up to certain version by
assignment to variable - but then compiler change and if the variable
itself was not used gcc also complain.

I would be for not including -Werror inside configure.ac at all and
trust people with commit rights to enable it (either by explicit option
or by CFLAGS variable). Possibly building on buildbot with
--enable-fatal-warnings if the flag is present would be a good thing
(but not CFLAGS="-Wall -Werror as valac produces valid code with
warnings).


Agree, I think it's probably something like an explicit --enable-fatal-warnings
that we want, rather than having it default and requiring others to
--disable-fatal-warnings.

It's really not in my interest to have dependency build breaks because
I dont use
the same compiler as the maintainer of libfoo, however the maintainer of libfoo
might want to explicitly enable them.

However I'm aware that there is not really consensus on this, and I'm happy that
at least people do have the option to --disable-fatal-warnings if they
get trapped
by them.

Cheers,
     -Tristan



Best regards

On Tue, 2013-02-26 at 18:07 +0900, Tristan Van Berkom wrote:

Hi Behdad.

I'll be the first to agree that -Werror is evil stuff, we even fell
into this catch 22 only
around a year ago where one group thought it was a good idea to infect
our builds
with -Werror, while another group thought it was a good idea to add
deprecation warnings,
... I'm not even sure we're out of the woods from that tangled mess yet.
(and I'll side with the latter, deprecation warnings are a good thing,
warnings are a
good thing in general).

That being said, does glib not use the gnome-common macros,
which include the --disable-fatal-warnings configure option ?

I've found this configure option to be helpful, not sure if glib still
needs it to be added...

Cheers,
         -Tristan

On Tue, Feb 26, 2013 at 10:39 AM, Behdad Esfahbod <behdad behdad org> wrote:

[I think I'm not on d-d-l, CC is appreciated.]

<rant>

Hi,

I'm sending this email instead of filing a bug because I think I need to say
this as widely as I can: modules adding -Werror (or -Werror=... variants
thereof) are harmful

Background:

A few years ago Javier opened this bug against gnome-common:

  https://bugzilla.gnome.org/show_bug.cgi?id=608953

which references my blogpost:

  http://mces.blogspot.ca/2008/12/year-end-cleaning-ie-on-warning-options.html

The bug title is "Add some more compiler warning options".  Something that I
thinks is good hygiene.  However, in the course of that happening, things like
these ended up going in:

    dnl These compiler flags typically indicate very broken or suspicious
    dnl code.  Some of them such as implicit-function-declaration are
    dnl just not default because gcc compiles a lot of legacy code.
    dnl We choose to make this set into explicit errors.
    base_error_flags=" \
        -Werror=missing-prototypes \
        -Werror=implicit-function-declaration \
        -Werror=pointer-arith \
        -Werror=init-self \
        -Werror=format-security \
        -Werror=format=2 \
        -Werror=missing-include-dirs \
    "

which then made it into glib as part of bug 687385.

Now, can you spot the problem?  Hint: the comment above says "*typically*
indicate very broken".  I translate: "typically this is fine, in other
situations we are breaking people's build, but that's fine".

Indeed, the email that went out with the change [1] acknowledged the problem:

"This gets to the next problem, which is that -Wall includes
-Wmaybe-uninitialized and other heuristics like -Wstrict-aliasing.  Then
combine that with the fact that some people have got it in their head
that "-Wall -Werror" is the Right Thing, what actually ends up happening
is your code only compiles on a *particular version* of gcc.  That just
doesn't work in a distributed project like GNOME, where various bits get
reused by different projects and products on different schedules etc."

but goes on to conclude:

"So I think what Dan has is more the Right Thing - make the compiler
warnings that you should *never* hit into explicit errors."

Interesting.  So some omniscient hacker decided that compilers compiling GNOME
shall never ever see code that generates a missing-prototypes warning, err,
error.  Not in the cryptic system headers of whatever weird cross compiler I
may be using.  I want your crystal ball seized!

Here I am, trying to cross-compile glib using mingw32, so I can cross-compile
pango, so I can fix a pangowin32 bug, and was stopped getting this:

behdad:libcharset 2$ make V=1
/bin/sh ../../libtool  --tag=CC   --mode=compile i586-mingw32msvc-gcc
-DHAVE_CONFIG_H -I. -I../../../glib/libcharset -I../..
-DLIBDIR=\"/home/behdad/.local/i586-mingw32msvc/lib\" -I../..
-I/home/behdad/.local/i586-mingw32msvc/include   -g -O2 -mms-bitfields  -Wall
-Wstrict-prototypes -Werror=declaration-after-statement
-Werror=missing-prototypes -Werror=implicit-function-declaration
-Werror=pointer-arith -Werror=init-self -Werror=format=2
-Werror=missing-include-dirs -MT localcharset.lo -MD -MP -MF
.deps/localcharset.Tpo -c -o localcharset.lo
../../../glib/libcharset/localcharset.c
libtool: compile:  i586-mingw32msvc-gcc -DHAVE_CONFIG_H -I.
-I../../../glib/libcharset -I../..
-DLIBDIR=\"/home/behdad/.local/i586-mingw32msvc/lib\" -I../..
-I/home/behdad/.local/i586-mingw32msvc/include -g -O2 -mms-bitfields -Wall
-Wstrict-prototypes -Werror=declaration-after-statement
-Werror=missing-prototypes -Werror=implicit-function-declaration
-Werror=pointer-arith -Werror=init-self -Werror=format=2
-Werror=missing-include-dirs -MT localcharset.lo -MD -MP -MF
.deps/localcharset.Tpo -c ../../../glib/libcharset/localcharset.c
-DDLL_EXPORT -DPIC -o .libs/localcharset.o
In file included from ../../../glib/libcharset/localcharset.c:28:
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/stdio.h:372:
error: no previous prototype for 'getc'
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/stdio.h:379:
error: no previous prototype for 'putc'
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/stdio.h:386:
error: no previous prototype for 'getchar'
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/stdio.h:393:
error: no previous prototype for 'putchar'
In file included from ../../../glib/libcharset/localcharset.c:28:
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/stdio.h:535:
error: no previous prototype for 'fopen64'
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/stdio.h:547:
error: no previous prototype for 'ftello64'
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/stdio.h:606:
error: no previous prototype for 'vsnwprintf'


Please, please, please, really, stop acting as if you know my build
environment...  If I want errors, I'll enable them myself.

</rant>

[1] https://mail.gnome.org/archives/desktop-devel-list/2012-July/msg00100.html

--
behdad
http://behdad.org/
_______________________________________________
desktop-devel-list mailing list
desktop-devel-list gnome org
https://mail.gnome.org/mailman/listinfo/desktop-devel-list

_______________________________________________
desktop-devel-list mailing list
desktop-devel-list gnome org
https://mail.gnome.org/mailman/listinfo/desktop-devel-list
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]