Re: RFC: Use -Wall -Wextra

Daniel Jacobowitz
2006-12-29T21:02:16Z

On Fri, Dec 29, 2006 at 10:44:25PM +0200, Eli Zaretskii wrote:
> > I definitely don't want to back away from -Werror: I think it's done
> > us a lot of good.
>
> I'm not against -Werror, I just don't trust GCC to not flag some
> innocent code as questionable under -Wall -Wextra. These flags are
> for those who intentionally want a noisy compiler; adding -Werror to
> them is asking for too much trouble.

How about -Wall without -Wextra then? This is the compromise adopted
by binutils and GCC.

> > I enabled -Wextra mostly because I wanted -Walways-true
>
> IMO -Walways-true is one of the greatest nuisances with the latest GCC
> versions. Some code, especially sophisticated macros that need to
> work portably with different data sizes, simply cannot be made to work
> around this warning, especially if you want to handle 32-bit and
> 64-bit machines. There already are such problems in GNU Make and in
> Emacs, and developers are unwilling to fix them because it's too much
> trouble, if at all possible.

I think neither of us is actually talking about -Walways-true. It
doesn't seem to control what I thought it did. Anyway, I was looking
for the "comparison of unsigned >= 0 is always true" warning
specifically. The only thing controlling that is -Wextra. I suspect
that's the one you are concerned about, too.

I'd like to enable that warning but I won't cry if we agree not to. It
did find one bug in GDB already, and would have found another if it had
been enabled (the ia64-tdep bug I fixed yesterday). But I can build
with it locally on systems where I know it isn't a big problem.

> Those issued by -Walways-true and the ones that wine about mismatch
> between pointers to signed and unsigned char are the two that come to
> mind.

I actually think the latter is useful, but not useful enough to
continue fixing up GDB for it - my patch left that one disabled,
deliberately. Let's not go back there again, it was a real nuisance.

> > > I'd advise against -Wunused: the problems it finds are harmless,
> > > whereas fixing them is not trivial at all, and quite ugly in some
> > > cases.
> >
> > I'm afraid I don't understand this either. The problems -Wunused finds
> > are usually very easy to fix.
>
> You mean with "i = i;"?

No, either by deleting the unused local variable if it's truly unused,
or by adding ATTRIBUTE_UNUSED if it is conditionally unused (or
avoiding conditional compilation).

> gcc -c -g -O2 -I. -I. -I./config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../readline/.. -I../bfd -I./../bfd -I./../include -DMI_OUT=1 -DTUI=1 -Wall -Wextra -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused-parameter -Wno-unused -Wno-sign-compare -Wno-switch -Wno-missing-field-initializers -Werror infrun.c
> cc1: warnings being treated as errors
> infrun.c: In function 'handle_inferior_event':
> infrun.c:1458: warning: statement with no effect
> make[2]: *** [infrun.o] Error 1

Thanks. That comes from the default definition of a macro which no
longer has any non-default definitions; we may as well garbage collect
it. I don't know why I didn't get the warning; I can provoke it for
a small testcase.

I'll remove the macro, since that's an unrelated cleanup.