From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7065 invoked by alias); 28 Dec 2006 19:55:44 -0000 Received: (qmail 6954 invoked by uid 22791); 28 Dec 2006 19:55:42 -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; Thu, 28 Dec 2006 19:55:35 +0000 Received: from drow by nevyn.them.org with local (Exim 4.63) (envelope-from ) id 1H01Lh-0004qO-3Y for gdb-patches@sourceware.org; Thu, 28 Dec 2006 14:55:33 -0500 Date: Thu, 28 Dec 2006 19:55:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: RFC: Use -Wall -Wextra Message-ID: <20061228195533.GA18492@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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/msg00333.txt.bz2 I'd like to hear opinions on this patch. It changes the default set of GDB build warnings from: build_warnings="-Wimplicit -Wreturn-type -Wcomment -Wtrigraphs \ -Wformat -Wparentheses -Wpointer-arith -Wformat-nonliteral \ -Wunused-label -Wunused-function -Wno-pointer-sign" to: build_warnings="-Wall -Wextra -Wpointer-arith -Wformat-nonliteral \ -Wno-pointer-sign -Wno-unused-parameter \ -Wno-unused -Wno-sign-compare -Wno-switch -Wno-missing-field-initializers" The main reason for avoiding -Wall, as far as I can remember and according to the comments, was to avoid -Wunused-parameter. But it's easy to do that explicitly and hold on to -Wall (which doesn't even enable that warning unless -Wextra is added too, at least nowadays). I went even further, and added -Wextra. My original motivation for this patch was a bug I'll be fixing later today on ia64, which turned out to be a comparison of size_t < 0 - always false. GCC can warn about that, but we don't ask it to. I picked the above set of warnings after some experimentation. GDB doesn't build with them without the patch I'll post after this one. Some of that patch was placating GCC, but more of it was fixing actual bugs that haven't bitten yet, so I think this is a good set! I'd really like to turn on -Wunused too, but it has been off for so long that we have a substantial number of unused local variables - it will take some work to clean up. I'm sure this patch will break some other targets, but we have a while before the next release to clean that up, and it's very easy to do so. I'll volunteer to do everything else that shows up building cross debuggers if we agree on this change. By the way, a nice side benefit is that -Wall enables -Wuninitialized iff optimization is enabled so you'll be able to say CFLAGS="-g -O0" without having to disable -Werror for the "-Wuninitialized requires -O1" warning. -- Daniel Jacobowitz CodeSourcery 2006-12-28 Daniel Jacobowitz * configure.ac (build_warnings): Use -Wall -Wextra. * configure: Regenerated. 2006-12-28 Daniel Jacobowitz * doc/gdbint.texinfo (Compiler Warnings): Update for -Wall -Wextra usage. Index: configure.ac =================================================================== RCS file: /cvs/src/src/gdb/configure.ac,v retrieving revision 1.36 diff -u -p -r1.36 configure.ac --- configure.ac 22 Nov 2006 17:34:15 -0000 1.36 +++ configure.ac 28 Dec 2006 19:39:05 -0000 @@ -1111,32 +1111,17 @@ if test "${ERROR_ON_WARNING}" = yes ; th WERROR_CFLAGS="-Werror" fi -# NOTE: Don't add -Wall or -Wunused, they both include -# -Wunused-parameter which reports bogus warnings. -# NOTE: If you add to this list, remember to update +# We avoid -Wunused-parameter, because GDB has a lot of +# callback interfaces with unused arguments. +# The entries after -Wno-unused-parameter are disabled warnings +# which may or may not be enabled in the future, which can not +# currently be used to build GDB. +# NOTE: If you change this list, remember to update # gdb/doc/gdbint.texinfo. -build_warnings="-Wimplicit -Wreturn-type -Wcomment -Wtrigraphs \ --Wformat -Wparentheses -Wpointer-arith -Wformat-nonliteral \ --Wunused-label -Wunused-function -Wno-pointer-sign" - -# GCC supports -Wuninitialized only with -O or -On, n != 0. -if test x${CFLAGS+set} = xset; then - case "${CFLAGS}" in - *"-O0"* ) ;; - *"-O"* ) - build_warnings="${build_warnings} -Wuninitialized" - ;; - esac -else - build_warnings="${build_warnings} -Wuninitialized" -fi +build_warnings="-Wall -Wextra -Wpointer-arith -Wformat-nonliteral \ +-Wno-pointer-sign -Wno-unused-parameter \ +-Wno-unused -Wno-sign-compare -Wno-switch -Wno-missing-field-initializers" -# Up for debate: -Wswitch -Wcomment -trigraphs -Wtrigraphs -# -Wunused-function -Wunused-variable -Wunused-value -# -Wchar-subscripts -Wtraditional -Wshadow -Wcast-qual -# -Wcast-align -Wwrite-strings -Wconversion -Wstrict-prototypes -# -Wmissing-prototypes -Wmissing-declarations -Wredundant-decls -# -Woverloaded-virtual -Winline -Werror" AC_ARG_ENABLE(build-warnings, [ --enable-build-warnings Enable build-time compiler warnings if gcc is used], [case "${enableval}" in Index: doc/gdbint.texinfo =================================================================== RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v retrieving revision 1.247 diff -u -p -r1.247 gdbint.texinfo --- doc/gdbint.texinfo 10 Nov 2006 19:20:37 -0000 1.247 +++ doc/gdbint.texinfo 28 Dec 2006 19:39:07 -0000 @@ -5471,64 +5471,30 @@ errors.} @subsection Compiler Warnings @cindex compiler warnings -With few exceptions, developers should include the configuration option -@samp{--enable-gdb-build-warnings=,-Werror} when building @value{GDBN}. -The exceptions are listed in the file @file{gdb/MAINTAINERS}. +With few exceptions, developers should avoid the configuration option +@samp{--disable-werror} when building @value{GDBN}. The exceptions +are listed in the file @file{gdb/MAINTAINERS}. The default, when +building with @sc{gcc}, is @samp{--enable-werror}. This option causes @value{GDBN} (when built using GCC) to be compiled with a carefully selected list of compiler warning flags. Any warnings -from those flags being treated as errors. +from those flags are treated as errors. The current list of warning flags includes: @table @samp -@item -Wimplicit -Since @value{GDBN} coding standard requires all functions to be declared -using a prototype, the flag has the side effect of ensuring that -prototyped functions are always visible with out resorting to -@samp{-Wstrict-prototypes}. - -@item -Wreturn-type -Such code often appears to work except on instruction set architectures -that use register windows. +@item -Wall -Wextra +All typical @sc{gcc} warnings. -@item -Wcomment - -@item -Wtrigraphs +@item -Wpointer-arith -@item -Wformat -@itemx -Wformat-nonliteral +@item -Wformat-nonliteral +Non-literal format strings, with a few exceptions, are bugs - they +might contain unintented user-supplied format specifiers. Since @value{GDBN} uses the @code{format printf} attribute on all -@code{printf} like functions these check not just @code{printf} calls +@code{printf} like functions this checks not just @code{printf} calls but also calls to functions such as @code{fprintf_unfiltered}. -@item -Wparentheses -This warning includes uses of the assignment operator within an -@code{if} statement. - -@item -Wpointer-arith - -@item -Wuninitialized - -@item -Wunused-label -This warning has the additional benefit of detecting the absence of the -@code{case} reserved word in a switch statement: -@smallexample -enum @{ FD_SCHEDULED, NOTHING_SCHEDULED @} sched; -@dots{} -switch (sched) - @{ - case FD_SCHEDULED: - @dots{}; - break; - NOTHING_SCHEDULED: - @dots{}; - break; - @} -@end smallexample - -@item -Wunused-function - @item -Wno-pointer-sign In version 4.0, GCC began warning about pointer argument passing or assignment even when the source and destination differed only in @@ -5537,19 +5503,21 @@ carefully between @code{char} and @code{ the @value{GDBN} developers decided correcting these warnings wasn't worth the time it would take. -@end table - -@emph{Pragmatics: Due to the way that @value{GDBN} is implemented most -functions have unused parameters. Consequently the warning -@samp{-Wunused-parameter} is precluded from the list. The macro +@item -Wno-unused-parameter +Due to the way that @value{GDBN} is implemented many functions have +unused parameters. Consequently this warning is avoided. The macro @code{ATTRIBUTE_UNUSED} is not used as it leads to false negatives --- it is not an error to have @code{ATTRIBUTE_UNUSED} on a parameter that -is being used. The options @samp{-Wall} and @samp{-Wunused} are also -precluded because they both include @samp{-Wunused-parameter}.} +is being used. + +@item -Wno-unused +@itemx -Wno-sign-compare +@itemx -Wno-switch +@itemx -Wno-missing-field-initializers +These are warnings which might be useful for @value{GDBN}, but are +currently too noisy to enable with @samp{-Werror}. -@emph{Pragmatics: @value{GDBN} has not simply accepted the warnings -enabled by @samp{-Wall -Werror -W...}. Instead it is selecting warnings -when and where their benefits can be demonstrated.} +@end table @subsection Formatting