From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15744 invoked by alias); 29 Dec 2006 21:02:16 -0000 Received: (qmail 15734 invoked by uid 22791); 29 Dec 2006 21:02:15 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Fri, 29 Dec 2006 21:02:08 +0000 Received: from drow by nevyn.them.org with local (Exim 4.63) (envelope-from ) id 1H0Ore-0006EA-3w; Fri, 29 Dec 2006 16:02:06 -0500 Date: Fri, 29 Dec 2006 21:02:00 -0000 From: Daniel Jacobowitz To: Eli Zaretskii Cc: gdb-patches@sourceware.org Subject: Re: RFC: Use -Wall -Wextra Message-ID: <20061229210206.GA23681@nevyn.them.org> Mail-Followup-To: Eli Zaretskii , gdb-patches@sourceware.org References: <20061228195533.GA18492@nevyn.them.org> <20061229135814.GA9741@nevyn.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.13 (2006-08-11) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-12/txt/msg00351.txt.bz2 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. -- Daniel Jacobowitz CodeSourcery