Re: [RFA] Turn on -Werror by default

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Sun, 15 Jan 2006 13:20:55 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Sun, Jan 15, 2006 at 06:55:28PM +0100, Mark Kettenis wrote:
> > From time to time I play with GCC 4, and try to fix a few problems,
> > but I get the feeling I'm the only one.
> 
> You may be the only one who prioritizes fixing these warnings, which I
> still maintain are mostly useless.  I think your sample is seriously
> biased.  A few points:
>   - You're the only active GDB developer I can think of offhand that
>     considers the warnings a serious problem.  I do consider them a
>     problem - one which should be fixed, before we make policy
>     statements about them.

Sorry, but you misunderstood me here.  I do not consider these *new*
GCC 4 warnings a problem.  What I do consider to be a problem is that
it makes people ignore other, useful warnings.

>   - You're primarily focused on the BSDs, and the BSD system compilers
>     can currently build GDB without warning.

Indeed.

>   - Debian/unstable and Fedora Core, both popular development
>     platforms, can not.

It truly amazes me that they are popular develepment platforms for the
average application developer (as opposed to Debian and Fedora Core
developers).  That shows that these people don't understand what
backwards compatibility means, and fully explains why so many
applications don't even compile on my Linux desktop system at work.

>   - Many people doing development on GDB HEAD are likely to have GCC
>     HEAD lying around, which also can't build GDB with -Werror.

True.  And in fact it is important for us to make sure GDB can be used
to debug code compiled with at least the latest official release of
GCC.  Tracking GCC HEAD, and using it to compile GDB, may very well be
the only way to achieve that goal.

> This patch is a policy statement that the GDB developers agree with you
> on the importance of warning-free code, which will inconvenience you
> not at all, and me a great deal.  Can you see where I'm coming from?

Yes.  But warning-free code used to be the policy back in the GCC 3
days.  I'm not aware that we dropped that policy; MAINTAINERS still
lists -Werror in the "Target ISA" section.

> I maintain that the correct way to turn on -Werror is to first fix the
> warnings.  As the developer who thinks -Werror is an important step
> forward, the burden is on you to make GDB warning-free on a reasonable
> set of platforms - I think we both agree on that already.  I'm
> disputing your reasonable set of platforms, however.

Since we also seem to agree that the "warn for pointer argument
passing or assignment with different signedness" warning from GCC 4 is
mostly pedantry, how about the attached patch, which adds
-Wno-pointer-sign" to the mix.

I can add that bit first, and then when we've fixed the few remaining
warnings, we can enable -Werror.

Mark


Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.25
diff -u -p -r1.25 configure.ac
--- configure.ac	17 Dec 2005 22:33:59 -0000	1.25
+++ configure.ac	15 Jan 2006 20:00:55 -0000
@@ -1109,6 +1109,24 @@ AC_ARG_WITH(sysroot,
 AC_SUBST(TARGET_SYSTEM_ROOT)
 AC_SUBST(TARGET_SYSTEM_ROOT_DEFINE)
 
+AC_ARG_ENABLE(werror,
+  [  --enable-werror    treat compile warnings as errors],
+  [case "${enableval}" in
+     yes | y) ERROR_ON_WARNING="yes" ;;
+     no | n)  ERROR_ON_WARNING="no" ;;
+     *) AC_MSG_ERROR(bad value ${enableval} for --enable-werror) ;;
+   esac])
+
+# Enable -Werror by default when using gcc
+if test "${GCC}" = yes -a -z "${ERROR_ON_WARNING}" ; then
+    ERROR_ON_WARNING=yes
+fi
+
+WERROR_CFLAGS=""
+if test "${ERROR_ON_WARNING}" = yes ; then
+    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
@@ -1116,6 +1134,9 @@ AC_SUBST(TARGET_SYSTEM_ROOT_DEFINE)
 build_warnings="-Wimplicit -Wreturn-type -Wcomment -Wtrigraphs \
 -Wformat -Wparentheses -Wpointer-arith -Wformat-nonliteral \
 -Wunused-label -Wunused-function"
+# NOTE: GCC 4 emits some warnings that are not very useful.  We need
+# to clean up our source tree, but until we do disabled them.
+build_warnings="${build_warnings} -Wno-pointer-sign"
 
 # GCC supports -Wuninitialized only with -O or -On, n != 0.
 if test x${CFLAGS+set} = xset; then
@@ -1164,7 +1185,6 @@ if test x"$silent" != x"yes" && test x"$
   echo "Setting GDB specific compiler warning flags = $build_warnings" 6>&1
 fi])dnl
 WARN_CFLAGS=""
-WERROR_CFLAGS=""
 if test "x${build_warnings}" != x -a "x$GCC" = xyes
 then
     AC_MSG_CHECKING(compiler warning flags)