RFC: Use -Wall -Wextra

[Daniel Jacobowitz <drow at false dot org>]

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  <dan@codesourcery.com>

	* configure.ac (build_warnings): Use -Wall -Wextra.
	* configure: Regenerated.

2006-12-28  Daniel Jacobowitz  <dan@codesourcery.com>

	* 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