RFC: should we use -Werror? (& sample patch to do it)

RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

I got warning levels down to around 30 on solaris2.7 and 20 on irix6.
So I thought it might be worth a try to get -Werror installed in gcc.

There are some problems.  We don't want to -Werror stage1 when using
e.g. 2.95 to bootstrap because older GCCs gives many false positives
that have since been corrected.  Using 2.95 on stage1 yields over 350
warnings on the same solaris2.7 that gets just 30 in all of stage3
(and stage3 builds all languges, not just C!)

Thus IMHO, we should only enable -Werror in stage2 or later.  However
this means if you don't do a full bootstrap (e.g. cross-compiles or
building for embedded targets) then -Werror isn't in effect.  This
also means the automatic tester to powerpc won't detect regressions.

That's a big lose since in effect warning regressions become bootstrap
failures for most people.  So I'm open to suggestion on that front.

Anyway, another problem is we have to get to zero warnings before
using -Werror.  I propose doing it one module at a time.  The patch
below is a candidate which does this for rtl.o to start with.  If that
lasts for a week without major pandemonium, we can do more files.

I also added a mechanism to disable -Werror on a per-triplet basis or
per autoconf test basis.  E.g. if the HOST_PTR_PRINTF autoconf test
fails, you are guaranteed to get some format specifier warnings.  So I
automatically disabled -Werror in that case.  Also if a platform is
hopelessly broken, you can disable -Werror in config.gcc for that
triplet.

This patch is meant to solicit comments on both the approach and
whether -Werror is a good idea in the first place.  Its been
bootstrapped on solaris2.7 and I ensured that -Werror was only passed
to rtl.o in stage2 and later.

		--Kaveh

2001-09-04  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* Makefile.in (WERROR_FLAG, WERROR2_FLAG): New variables.
	(rtl.o): Use WERROR_FLAG.
	(STAGE2_FLAGS_TO_PASS): Pass WERROR_FLAG.

	* configure.in (gcc_AC_FUNC_PRINTF_PTR): If test fails, set
	NO_WERROR=1.
	(werror_flag): Set appropriately depending on NO_WERROR.
	
diff -rup orig/egcs-CVS20010904/gcc/Makefile.in egcs-CVS20010904/gcc/Makefile.in
--- orig/egcs-CVS20010904/gcc/Makefile.in	Fri Aug 31 15:25:25 2001
+++ egcs-CVS20010904/gcc/Makefile.in	Tue Sep  4 14:58:10 2001
@@ -89,6 +89,13 @@ BOOT_CFLAGS = -g -O2
 LOOSE_WARN = -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes
 STRICT_WARN = -Wtraditional @strict1_warn@
 STRICT2_WARN = -Wtraditional -pedantic -Wno-long-long
+# WERROR_FLAG and WERROR2_FLAG are used for setting -Werror in stage2
+# and later passes.  We only ever set -Werror in stage>=2 so that
+# we've guaranteed that the latest gcc is being used for compilation.
+# This ensures that the latest set of warnings is used and also that
+# fixes for false positives are in place.
+WERROR_FLAG = 
+WERROR2_FLAG = @werror_flag@
 
 # This is how we control whether or not the additional warnings are applied.
 .-warn = $(STRICT_WARN)
@@ -1365,7 +1372,7 @@ rtl-error.o: rtl-error.c system.h $(RTL_
    input.h toplev.h intl.h diagnostic.h
 
 rtl.o : rtl.c $(GCONFIG_H) $(SYSTEM_H) $(RTL_H) real.h $(GGC_H) errors.h
-	$(CC) -c $(ALL_CFLAGS) -DGENERATOR_FILE $(ALL_CPPFLAGS) $(INCLUDES) $< $(OUTPUT_OPTION)
+	$(CC) -c $(ALL_CFLAGS) $(WERROR_FLAG) -DGENERATOR_FILE $(ALL_CPPFLAGS) $(INCLUDES) $< $(OUTPUT_OPTION)
 
 print-rtl.o : print-rtl.c $(GCONFIG_H) $(SYSTEM_H) $(RTL_H) hard-reg-set.h \
     $(BASIC_BLOCK_H)
@@ -2938,6 +2945,7 @@ STAGE2_FLAGS_TO_PASS = \
 	CFLAGS="$(BOOT_CFLAGS)" \
 	LDFLAGS="$(BOOT_LDFLAGS)" \
 	WARN_CFLAGS="\$$(GCC_WARN_CFLAGS)" \
+	WERROR_FLAG="$(WERROR2_FLAG)" \
 	STRICT_WARN="$(STRICT2_WARN)" \
 	libdir=$(libdir) \
 	LANGUAGES="$(LANGUAGES)" \
diff -rup orig/egcs-CVS20010904/gcc/configure.in egcs-CVS20010904/gcc/configure.in
--- orig/egcs-CVS20010904/gcc/configure.in	Wed Aug 22 16:30:27 2001
+++ egcs-CVS20010904/gcc/configure.in	Tue Sep  4 15:24:34 2001
@@ -605,6 +605,9 @@ AC_SUBST(TARGET_GETGROUPS_T)
 gcc_AC_FUNC_VFPRINTF_DOPRNT
 gcc_AC_FUNC_STRSTR
 gcc_AC_FUNC_PRINTF_PTR
+if test $gcc_cv_func_printf_ptr = no ; then
+  NO_WERROR=1
+fi
 
 case "${host}" in
 *-*-uwin*)
@@ -2032,6 +2035,18 @@ AC_SUBST(slibdir)
 # Nothing to do for FLOAT_H, float_format already handled.
 objdir=`pwd`
 AC_SUBST(objdir)
+
+# This allows either specific platforms (via config.gcc) or specific
+# autoconf tests to override whether warnings become errors.
+AC_MSG_CHECKING(whether to enable -Werror in stage 2)
+if test x$NO_WERROR = x ; then
+  AC_MSG_RESULT(yes)
+  werror_flag=-Werror
+else
+  AC_MSG_RESULT(no)
+  werror_flag=
+fi
+AC_SUBST(werror_flag)
 
 # Process the language and host/target makefile fragments.
 ${CONFIG_SHELL-/bin/sh} $srcdir/configure.frag $srcdir "$subdirs" "$dep_host_xmake_file" "$dep_tmake_file"

Re: RFC: should we use -Werror? (& sample patch to do it)

[Neil Booth]

Kaveh R. Ghazi wrote:-

> I also added a mechanism to disable -Werror on a per-triplet basis or
> per autoconf test basis.  E.g. if the HOST_PTR_PRINTF autoconf test
> fails, you are guaranteed to get some format specifier warnings.  So I
> automatically disabled -Werror in that case.  Also if a platform is
> hopelessly broken, you can disable -Werror in config.gcc for that
> triplet.

I would imagine that it would be better to do it the other way; I think
it is really only realistic to enable it on a per-triplet basis and
default to off.

Neil.

Re: RFC: should we use -Werror? (& sample patch to do it)

[Richard Henderson]

On Tue, Sep 04, 2001 at 11:32:50PM -0400, Kaveh R. Ghazi wrote:
> That's a big lose since in effect warning regressions become bootstrap
> failures for most people.  So I'm open to suggestion on that front.

Autoconf stage1 on installed gcc version being "new enough"?


r~

Re: RFC: should we use -Werror? (& sample patch to do it)

[Geoff Keating]

"Kaveh R. Ghazi" <ghazi@caip.rutgers.edu> writes:

> I got warning levels down to around 30 on solaris2.7 and 20 on irix6.
> So I thought it might be worth a try to get -Werror installed in gcc.
> 
> There are some problems.  We don't want to -Werror stage1 when using
> e.g. 2.95 to bootstrap because older GCCs gives many false positives
> that have since been corrected.  Using 2.95 on stage1 yields over 350
> warnings on the same solaris2.7 that gets just 30 in all of stage3
> (and stage3 builds all languges, not just C!)
> 
> Thus IMHO, we should only enable -Werror in stage2 or later.  However
> this means if you don't do a full bootstrap (e.g. cross-compiles or
> building for embedded targets) then -Werror isn't in effect.  This
> also means the automatic tester to powerpc won't detect regressions.

That's OK.  The automatic tester also does an x86-linux bootstrap.

-- 
- Geoffrey Keating <geoffk@geoffk.org>

Re: RFC: should we use -Werror? (& sample patch to do it)

[Zack Weinberg]

On Tue, Sep 04, 2001 at 11:32:50PM -0400, Kaveh R. Ghazi wrote:
> I got warning levels down to around 30 on solaris2.7 and 20 on irix6.
> So I thought it might be worth a try to get -Werror installed in gcc.
...
> Anyway, another problem is we have to get to zero warnings before
> using -Werror.  I propose doing it one module at a time.  The patch
> below is a candidate which does this for rtl.o to start with.  If that
> lasts for a week without major pandemonium, we can do more files.

I think this is backward.  First we should get to zero warnings
throughout the compiler, _then_ we talk about -Werror.  If only
because if we do it your way, we'll be adding explicit compile rules
to module after module.  I don't think there's enough marginal benefit
from -Werror applied to a few modules, to eat that kind of complexity
burden in the Makefile which is already too complicated.

Instead, we should be talking seriously about the remaining N
warnings, and what we're going to do about them.  I saw a couple of
patches go by for the 20-odd signed/unsigned comparison warnings in
combine.c, which is the majority of the remaining warnings; they
haven't been reviewed.

Next biggest offender is

       7 string length `???' is greater than the length `???' ISO C89
         compilers are required to support

These are more or less intractable: they come from big hairy spec
strings, e.g.

    "%{E|M|MM:cpp0 -lang-c++ %{!no-gcc:-D__GNUG__=%v1}\
       %{!Wno-deprecated:-D__DEPRECATED}\
       %{!fno-exceptions:-D__EXCEPTIONS}\
       -D__GXX_ABI_VERSION=100\
       %{ansi:-D__STRICT_ANSI__ -trigraphs -$} %(cpp_options)}\
     %{!E:%{!M:%{!MM:\
       %{save-temps:cpp0 -lang-c++ \
                    %{!no-gcc:-D__GNUG__=%v1}\
                    %{!Wno-deprecated:-D__DEPRECATED}\
                    %{!fno-exceptions:-D__EXCEPTIONS}\
                    -D__GXX_ABI_VERSION=100\
                    %{ansi:-D__STRICT_ANSI__ -trigraphs -$}\
                    %(cpp_options) %b.ii \n}\
      cc1plus %{save-temps:-fpreprocessed %b.ii}\
              %{!save-temps:%(cpp_options)\
                            %{!no-gcc:-D__GNUG__=%v1} \
                            %{!Wno-deprecated:-D__DEPRECATED}\
                            %{!fno-exceptions:-D__EXCEPTIONS}\
                            -D__GXX_ABI_VERSION=100\
                            %{ansi:-D__STRICT_ANSI__}}\
       %{ansi:-trigraphs -$}\
       %(cc1_options) %2 %{+e1*}\
       %{!fsyntax-only:%(invoke_as)}}}}",

Anyone have a good suggestion for what to do about that, I would
*love* to hear it.  (Short of Neil's driver rewrite, which isn't
happening fast enough to help.)

zw

Re: RFC: should we use -Werror? (& sample patch to do it)

[Neil Booth]

Zack Weinberg wrote:-

> Anyone have a good suggestion for what to do about that, I would
> *love* to hear it.  (Short of Neil's driver rewrite, which isn't
> happening fast enough to help.)

It's not happening at all until there is more consensus that it's what
we want to do.  I was a bit disappointed in the feedback to be honest.

Neil.

Driver rewrite... (was Re: RFC: should we use -Werror?)

[Zack Weinberg]

On Wed, Sep 05, 2001 at 06:40:18PM +0100, Neil Booth wrote:
> Zack Weinberg wrote:-
> 
> > Anyone have a good suggestion for what to do about that, I would
> > *love* to hear it.  (Short of Neil's driver rewrite, which isn't
> > happening fast enough to help.)
> 
> It's not happening at all until there is more consensus that it's what
> we want to do.  I was a bit disappointed in the feedback to be honest.

I think you're being too negative about the feedback.  Yes, there was
a lot of static thrown out about losing the ability to tweak specs at
install time.  I think that if you show up with a compelling proof-of
-concept implementation that does 80% of what people want, faster and
neater, they'll be willing to work out better ways of doing the other
20%.

Personally I had assumed you had gone off to write some code and would
come back when you had something ready to be tested.

zw

Re: Driver rewrite... (was Re: RFC: should we use -Werror?)

[Neil Booth]

Zack Weinberg wrote:-

> I think you're being too negative about the feedback.

Perhaps...

> Yes, there was a lot of static thrown out about losing the ability
> to tweak specs at install time.  I think that if you show up with a
> compelling proof-of -concept implementation that does 80% of what
> people want, faster and neater, they'll be willing to work out
> better ways of doing the other 20%.

It would be a lot of work to get it to that stage.  For example, to
get something suitable for a commit would mean touching every port,
both for coding up routines to handle port-specific flags and to
remove vairous stuff like all the macro gunk that currently exists.
There's a lot of it, and it's repetetive, dull and can't be automated.

So I didn't want to waste time on it if objections about the runtime
configurability stuff and other nits were sticking, which they seemed
to be as the thread died out.  If other interesting mini projects stop
grabbing my attention, I might pick it up again.

Neil.

Re: Driver rewrite... (was Re: RFC: should we use -Werror?)

[Zack Weinberg]

On Wed, Sep 05, 2001 at 07:12:08PM +0100, Neil Booth wrote:
> Zack Weinberg wrote:-
> 
> > I think that if you show up with a
> > compelling proof-of -concept implementation that does 80% of what
> > people want, faster and neater, they'll be willing to work out
> > better ways of doing the other 20%.
> 
> It would be a lot of work to get it to that stage.  For example, to
> get something suitable for a commit would mean touching every port,
> both for coding up routines to handle port-specific flags and to
> remove vairous stuff like all the macro gunk that currently exists.
> There's a lot of it, and it's repetetive, dull and can't be automated.

Um, what was wrong with my suggestion that you put the new driver in a
separate file and add Makefile+config.gcc bits so that each port can
choose which one it wants?  Then ports can be converted incrementally.
You have to touch all the front ends, but there are only five of them
in the tree.  (Which reminds me, what happened to Ada?  It was going
to be put into the tree in July, but then it wasn't.)

As for the runtime configurability, I very much think we were all
waving hands frantically at each other because your proposal was vague
enough that it was hard to tell what it could do.  Come back with
concrete code and at least we can argue about something solid...

zw

Re: RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

 > From: Richard Henderson <rth@redhat.com>
 > 
 > On Tue, Sep 04, 2001 at 11:32:50PM -0400, Kaveh R. Ghazi wrote:
 > > That's a big lose since in effect warning regressions become bootstrap
 > > failures for most people.  So I'm open to suggestion on that front.
 > 
 > Autoconf stage1 on installed gcc version being "new enough"?
 > r~

Well, "new enough" in this case means "3.1 20010825 (experimental)"
when I put in this patch:

 > 2001-08-24  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
 >  
 >         * fold-const.c (tree_expr_nonnegative_p): Handle *_DIV_EXPR,
 >         *_MOD_EXPR, SAVE_EXPR and NON_LVALUE_EXPR.

That patch got rid of some false positives.

I guess eventually 3.1 will be released and checking for it in stage1
would hit sometimes. :-)

--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions

Re: RFC: should we use -Werror? (& sample patch to do it)

[Richard Henderson]

On Wed, Sep 05, 2001 at 01:39:02PM -0400, Kaveh R. Ghazi wrote:
> I guess eventually 3.1 will be released and checking for it in stage1
> would hit sometimes. :-)

Well, that's not what I had in mind.  For instance, I regularly install
last night's build in a local directory here.  That version of the compiler
is first on my path, so I'm regularly building stage1 with something very
recent.

I suspect other developers do something similar.


r~

Re: RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

 > From: Neil Booth <neil@daikokuya.demon.co.uk>
 > 
 > Kaveh R. Ghazi wrote:-
 > 
 > > I also added a mechanism to disable -Werror on a per-triplet basis or
 > > per autoconf test basis.  E.g. if the HOST_PTR_PRINTF autoconf test
 > > fails, you are guaranteed to get some format specifier warnings.  So I
 > > automatically disabled -Werror in that case.  Also if a platform is
 > > hopelessly broken, you can disable -Werror in config.gcc for that
 > > triplet.
 > 
 > I would imagine that it would be better to do it the other way; I think
 > it is really only realistic to enable it on a per-triplet basis and
 > default to off.
 > Neil.

Oh no, I disagree.  IMHO yours is a formula for never getting it done.
I don't have access to all zillion platforms gcc supports.  This needs
to be a team effort if its to be successful, and making it the default
ensures everyone participates. :-)

--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions

Re: RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

 > From: Zack Weinberg <zack@codesourcery.com>
 > 
 > On Tue, Sep 04, 2001 at 11:32:50PM -0400, Kaveh R. Ghazi wrote:
 > > I got warning levels down to around 30 on solaris2.7 and 20 on irix6.
 > > So I thought it might be worth a try to get -Werror installed in gcc.
 > ...
 > > Anyway, another problem is we have to get to zero warnings before
 > > using -Werror.  I propose doing it one module at a time.  The patch
 > > below is a candidate which does this for rtl.o to start with.  If that
 > > lasts for a week without major pandemonium, we can do more files.
 > 
 > I think this is backward.  First we should get to zero warnings
 > throughout the compiler, _then_ we talk about -Werror.  If only
 > because if we do it your way, we'll be adding explicit compile rules
 > to module after module.  I don't think there's enough marginal benefit
 > from -Werror applied to a few modules, to eat that kind of complexity
 > burden in the Makefile which is already too complicated.

The problem with getting to zero is each platform is likely to have
its own special warnings stemming either from the config directory
files or from platform specific wierdness.  We simply cannot get to
zero on all platforms on all modules unless everyone makes a concerted
effort and no regressions occur.  I might very well get solaris2 down
to zero but I bet other platforms would not be zero unless someone
volunteers to look at those.  I find that highly unlikely, we've
hovered at 100-200 warnings for a year or so.  Without -Werror to
force people, regressions creep in at several per week sometimes.

I thought starting with one file would ease people in slowly.  Once we
got the whole compiler I would take out the individual rules and put
it in the .c.o rule or into GCC_WARN_CFLAGS or something.  So whatever
extra complexity added would be temporary.



 > Instead, we should be talking seriously about the remaining N
 > warnings, and what we're going to do about them.  I saw a couple of
 > patches go by for the 20-odd signed/unsigned comparison warnings in
 > combine.c, which is the majority of the remaining warnings; they
 > haven't been reviewed.
 > 
 > Next biggest offender is
 > 
 >        7 string length `???' is greater than the length `???' ISO C89
 >          compilers are required to support
 > 
 > These are more or less intractable: they come from big hairy spec
 > strings, e.g.

IIRC, these string warnings are from -pedantic, which isn't affected
by -Werror.  To make those hard errors, I think you have to use
-pedantic-errors.  (Again, IIRC.)

--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions

Re: RFC: should we use -Werror? (& sample patch to do it)

[Zack Weinberg]

On Wed, Sep 05, 2001 at 01:59:43PM -0400, Kaveh R. Ghazi wrote:
>  > > Anyway, another problem is we have to get to zero warnings before
>  > > using -Werror.  I propose doing it one module at a time.  The patch
>  > > below is a candidate which does this for rtl.o to start with.  If that
>  > > lasts for a week without major pandemonium, we can do more files.
>  > 
>  > I think this is backward.  First we should get to zero warnings
>  > throughout the compiler, _then_ we talk about -Werror.  If only
>  > because if we do it your way, we'll be adding explicit compile rules
>  > to module after module.  I don't think there's enough marginal benefit
>  > from -Werror applied to a few modules, to eat that kind of complexity
>  > burden in the Makefile which is already too complicated.
> 
> The problem with getting to zero is each platform is likely to have
> its own special warnings stemming either from the config directory
> files or from platform specific wierdness. 

Yes.  When I said zero, I meant zero machine-independent warnings.  We
aren't even there yet, and I think it's simply because the remaining
MI warnings are such a pain to deal with - combine.c comparing mode
bitsizes to rtx integer slots, e.g.

Or there's the cp/error.c mess, which the only sane thing to do with
it is finish rewriting it in terms of diagnostic.c, and *that*'s a
lack-of-time issue; it's been on my list for about a year now.

> I thought starting with one file would ease people in slowly.  Once we
> got the whole compiler I would take out the individual rules and put
> it in the .c.o rule or into GCC_WARN_CFLAGS or something.  So whatever
> extra complexity added would be temporary.

See, that's nice in theory, but I bet you "temporary" is measured in
years.

(There are cleaner ways to do the same thing, e.g. look at the hack
used to avoid -pedantic for the non-C front ends.)

>  > Next biggest offender is
>  > 
>  >        7 string length `???' is greater than the length `???' ISO C89
>  >          compilers are required to support
>  > 
>  > These are more or less intractable: they come from big hairy spec
>  > strings, e.g.
> 
> IIRC, these string warnings are from -pedantic, which isn't affected
> by -Werror.  To make those hard errors, I think you have to use
> -pedantic-errors.  (Again, IIRC.)

Not true.  -Werror turns _all_ warnings into errors.

zw

Re: RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

 > From: Zack Weinberg <zack@codesourcery.com>
 > 
 > >  > Next biggest offender is
 > >  > 
 > >  >        7 string length `???' is greater than the length `???' ISO C89
 > >  >          compilers are required to support
 > >  > 
 > >  > These are more or less intractable: they come from big hairy spec
 > >  > strings, e.g.
 > > 
 > > IIRC, these string warnings are from -pedantic, which isn't affected
 > > by -Werror.  To make those hard errors, I think you have to use
 > > -pedantic-errors.  (Again, IIRC.)
 > 
 > Not true.  -Werror turns _all_ warnings into errors.
 > zw

Hmm, I tested it in 3.1 and 2.95.3 and you're right.  Well, that
certainly puts a monkey wrench into my plan.

Since these are intractable as you say, what if we applied the
__extension__ keyword to these strings?  Since they are from -pedantic
that would eliminate the warning, and thus the hard error when using
-Werror, right?

Separately, perhaps we want something like __extension__ applied to
regular warnings too.  E.g. how about a __nowarn__ keyword that
temporarily does "inhibit_warnings = 1" for the statement in question?

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions

Re: RFC: should we use -Werror? (& sample patch to do it)

[Zack Weinberg]

On Wed, Sep 05, 2001 at 03:09:04PM -0400, Kaveh R. Ghazi wrote:
>  > 
>  > Not true.  -Werror turns _all_ warnings into errors.
>  > zw
> 
> Hmm, I tested it in 3.1 and 2.95.3 and you're right.  Well, that
> certainly puts a monkey wrench into my plan.
> 
> Since these are intractable as you say, what if we applied the
> __extension__ keyword to these strings?  Since they are from -pedantic
> that would eliminate the warning, and thus the hard error when using
> -Werror, right?

No, the error issues from the tokenizer, which doesn't know anything
about __extension__, nor should it (it would have to understand the
expression grammar then).

These are not warnings we want to ignore, either.  Those strings
really are too long to be safe.

Please, can we stop trying to paper over the problems and _fix_ them?

zw

Re: RFC: should we use -Werror? (& sample patch to do it)

[Marc Espie]

In article < 20010905123316.C386@codesourcery.com > you write:
>On Wed, Sep 05, 2001 at 03:09:04PM -0400, Kaveh R. Ghazi wrote:
>>  > 
>>  > Not true.  -Werror turns _all_ warnings into errors.
>>  > zw

>> Hmm, I tested it in 3.1 and 2.95.3 and you're right.  Well, that
>> certainly puts a monkey wrench into my plan.

>> Since these are intractable as you say, what if we applied the
>> __extension__ keyword to these strings?  Since they are from -pedantic
>> that would eliminate the warning, and thus the hard error when using
>> -Werror, right?

>No, the error issues from the tokenizer, which doesn't know anything
>about __extension__, nor should it (it would have to understand the
>expression grammar then).

The technical issue is irrelevant.

It shouldn't be hard to change the tokenizer. Okay, it doesn't have
to understand about extension, but it can insert tokens in the chain
if the string is too long, or if it has any further properties, so
that later stages, that know more about the whole thing semantics,
could decide to process it as an error.
e.g., you could rewrite "string..." as "string..." __attribute((_toolong__))
and have the further stages decide what they want to do with toolong or
something. Then they would see it together with a possible __extension__.

(Works that way thanks to ansi string concatenation)

Of course, your other points are valid.


But please, you already tried to enforce your views once by saying it
was impossible to do such and such a thing technically.

Which is clearly not true.

Re: RFC: should we use -Werror? (& sample patch to do it)

[Fergus Henderson]

On 05-Sep-2001, Kaveh R. Ghazi <ghazi@caip.rutgers.edu> wrote:
> Separately, perhaps we want something like __extension__ applied to
> regular warnings too.  E.g. how about a __nowarn__ keyword that
> temporarily does "inhibit_warnings = 1" for the statement in question?

Yes, please.  I've been wanting that for years.

The counter-argument is that inhibiting all warnings for a whole statement
or a whole expression is dangerous, because it might inhibit warnings that
indicate real problems.  However, what I end up doing to work-around the
absence of __nowarn__ is inhibiting certain categories of warnings for my
whole application, simply because one header file contains an occurrence
of code which triggers such a warning and for which the warning can't
easily be supressed.

-- 
Fergus Henderson <fjh@cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: < http://www.cs.mu.oz.au/~fjh >  |     -- the last words of T. S. Garp.

Re: RFC: should we use -Werror? (& sample patch to do it)

[Joseph S. Myers]

On Thu, 27 Sep 2001, Fergus Henderson wrote:

> Yes, please.  I've been wanting that for years.
> 
> The counter-argument is that inhibiting all warnings for a whole statement
> or a whole expression is dangerous, because it might inhibit warnings that
> indicate real problems.  However, what I end up doing to work-around the
> absence of __nowarn__ is inhibiting certain categories of warnings for my
> whole application, simply because one header file contains an occurrence
> of code which triggers such a warning and for which the warning can't
> easily be supressed.

We should also look at the previous discussion of designs for fine-grained
warning control (e.g. see http://gcc.gnu.org/ml/gcc/2000-06/msg00639.html
- linked to from projects/beginner.html but currently missing from the web
server as one of the 4492 files needed from the old disk listed in
http://gcc.gnu.org/ml/gcc/2001-08/msg01443.html ).

-- 
Joseph S. Myers
jsm28@cam.ac.uk

Re: RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

 > From: Zack Weinberg <zack@codesourcery.com>
 > 
 > On Wed, Sep 05, 2001 at 03:09:04PM -0400, Kaveh R. Ghazi wrote:
 > >  > 
 > >  > Not true.  -Werror turns _all_ warnings into errors.
 > >  > zw
 > > 
 > > Hmm, I tested it in 3.1 and 2.95.3 and you're right.  Well, that
 > > certainly puts a monkey wrench into my plan.
 > > 
 > > Since these are intractable as you say, what if we applied the
 > > __extension__ keyword to these strings?  Since they are from -pedantic
 > > that would eliminate the warning, and thus the hard error when using
 > > -Werror, right?
 > 
 > No, the error issues from the tokenizer, which doesn't know anything
 > about __extension__, nor should it (it would have to understand the
 > expression grammar then).

Er, I just tried it and it works...


 > These are not warnings we want to ignore, either.  Those strings
 > really are too long to be safe.
 > Please, can we stop trying to paper over the problems and _fix_ them?
 > zw

Well that's a separate question.  I don't know of any way short of
rewriting the whole specs stuff that Neil was suggesting.  I'm not
generally in favor of papering over things, but at the same time I
don't think -Werror should necessarily bottleneck on a specs
implementation rewrite if a simple workaround is available and the
long term solution is on someone else's todo list.

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions

Re: RFC: should we use -Werror? (& sample patch to do it)

[Neil Booth]

Kaveh R. Ghazi wrote:-

>  > No, the error issues from the tokenizer, which doesn't know anything
>  > about __extension__, nor should it (it would have to understand the
>  > expression grammar then).
> 
> Er, I just tried it and it works...

Well, only sort of.  You miss strings that appear only to the
preprocessor, like those in #defines that don't get used, #include
etc.

String handling will be moving closer to cpplib and out of the parser
sometime soon, so we really don't want it to rely on something in the
front ends.

Neil.

Re: RFC: should we use -Werror? (& sample patch to do it)

[Zack Weinberg]

On Wed, Sep 05, 2001 at 03:51:59PM -0400, Kaveh R. Ghazi wrote:
>  > These are not warnings we want to ignore, either.  Those strings
>  > really are too long to be safe.
>  > Please, can we stop trying to paper over the problems and _fix_ them?
> 
> Well that's a separate question.  I don't know of any way short of
> rewriting the whole specs stuff that Neil was suggesting.  I'm not
> generally in favor of papering over things, but at the same time I
> don't think -Werror should necessarily bottleneck on a specs
> implementation rewrite if a simple workaround is available and the
> long term solution is on someone else's todo list.

You and I seem to be coming at this from different angles.  I am not
yet convinced that -Werror is actually a good idea, even if we *did*
have zero warnings across all targets, which we don't.  I would like
to discuss how we can get to zero warnings, instead of how we should
enforce its staying that way once we do get there.

The reason I'm not convinced that -Werror would be a good idea even
after we get to zero warnings, is that the existing set of warnings
have been around for a very long time because they're both harmless
and intractable.  (Harmless, in the sense that I am fairly sure the
code is in all cases correct, just pulling dubious tricks.)  I feel
it's likely that, even if we do solve all of them, we will get more
code in the same category - causes harmless, intractable warnings -
and then there will be much finger-pointing and raging and gnashing
of teeth, should we enforce zero warnings with -Werror.

I'd like to make a comparison to the argument we had over whether
patches that exposed bugs elsewhere, should be reverted.  All the
warnings in combine.c happen because someone decided mode_bitsize
should be unsigned.  They were probably right.  With -Werror, the
patch that made it unsigned would have broken bootstrap, and then
we would have had an endless unproductive flamewar about it.

zw

Re: RFC: should we use -Werror? (& sample patch to do it)

[lucier]

Zack Weinberg wrote:

> The reason I'm not convinced that -Werror would be a good idea even
> after we get to zero warnings, is that the existing set of warnings
> have been around for a very long time because they're both harmless
> and intractable.  (Harmless, in the sense that I am fairly sure the
> code is in all cases correct, just pulling dubious tricks.)

I don't think this is true, see, e.g., today's warning on sparc-sun-solaris28:

../../gcc/cp/class.c:6770: warning: int format, different type arg (arg 3)

These types of problems (e.g., printing a HOST_WIDE_INT as an int) are added
all the time, quite often in debug code, and I don't think the code is correct.

Brad Lucier

Re: RFC: should we use -Werror? (& sample patch to do it)

[Dan Nicolaescu]

What about changing -Werror to be a little bit more fine grained: so
it would be possible to specify exactly  which warnings would be
errors. 

Something like -Werror=LIST_OF_WARNINGS_THAT_SHOULD_GENERATE_ERRORS
or -WWARNING_NAME=error

Then we could turn on -Werror for all the un-controversial warnings,
that don't generate false positives, like -Wformat,
-Wstrict-prototypes, etc. 

(currently we have fine grained control for warnings, but not for 
-Werror). 

Just my 2 cents

        --dan

Re: RFC: should we use -Werror? (& sample patch to do it)

[Loïc Joly]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

Dan Nicolaescu wrote:
[...] 
> (currently we have fine grained control for warnings, but not for
> -Werror).
> 
Well, the control is not that fine grained, even for warnings, since is
is not possible to disable a warning locally, but only for a full
translation unit.


-- 
Loïc

Re: RFC: should we use -Werror? (& sample patch to do it)

[dewar]

<<Well, the control is not that fine grained, even for warnings, since is
is not possible to disable a warning locally, but only for a full
translation unit.
>>

In GNAT, we have a pragma Warnings (On|Off) which can be applied locally
and is very useful for eliminating "OK" warnings (in GNAT we require
compilation with -gnatwe which is the equivalent of -Werror.

Re: RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

 > From: Zack Weinberg <zack@codesourcery.com>
 > 
 > > I thought starting with one file would ease people in slowly.  Once we
 > > got the whole compiler I would take out the individual rules and put
 > > it in the .c.o rule or into GCC_WARN_CFLAGS or something.  So whatever
 > > extra complexity added would be temporary.
 > 
 > (There are cleaner ways to do the same thing, e.g. look at the hack
 > used to avoid -pedantic for the non-C front ends.)
 > zw

I suppose you're referring to the following from Makefile.in?

 > # This is how we control whether or not the additional warnings are applied.
 > .-warn = $(STRICT_WARN)
 > GCC_WARN_CFLAGS = $(LOOSE_WARN) $($(@D)-warn)

I can't figure this out, can you please explain it?  How does it work?

		Thanks,
		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions

Re: RFC: should we use -Werror? (& sample patch to do it)

[Zack Weinberg]

On Fri, Sep 07, 2001 at 09:39:38PM -0400, Kaveh R. Ghazi wrote:
>  > From: Zack Weinberg <zack@codesourcery.com>
>  > 
>  > (There are cleaner ways to do the same thing, e.g. look at the hack
>  > used to avoid -pedantic for the non-C front ends.)
>  > zw
> 
> I suppose you're referring to the following from Makefile.in?

Yes.

>  > .-warn = $(STRICT_WARN)
>  > GCC_WARN_CFLAGS = $(LOOSE_WARN) $($(@D)-warn)
> 
> I can't figure this out, can you please explain it?  How does it work?

$(@D) expands to the directory containing the current target
_as it is referred to in the Makefile_.  If the current target is
"toplev.o", $(@D) is ".".  If the current target is "java/parse.o",
$(@D) is "java".

When you write a variable reference inside a variable reference, (all
modern versions of) Make expand the inner reference first and mash the
result into the variable name to be looked up by the outer reference.
Therefore, $($(@D)-warn) is equivalent to $(.-warn) for toplev.o,
$(java-warn) for java/parse.o, etc.  We set .-warn to $(STRICT_WARN)
and java-warn etc. to empty, we get $(STRICT_WARN) added to
GCC_WARN_CFLAGS for the top level only.[1]

So, how'd you use this for -Werror?  Like this:

GCC_WARN_CFLAGS = $(LOOSE_WARN) $($(@D)-warn) $($@-warn)

# These files (and only these) are to have no-warnings enforced.
toplev.o-warn = -Werror
c-common.o-warn = -Werror
cp/parse.o-warn = -Werror
# ...

Each Makefile.in and/or Make-lang.in would have the appropriate list
for the files it controls.

I suppose this could also be used to do

combine.o-warn = -Wno-sign-compare

but that would be cheating.

zw

[1] Make has practically no restrictions on what characters can appear
in a variable name.  I think it's just = : $ ( ).  You can even have
interior white space.

Re: RFC: should we use -Werror? (& sample patch to do it)

[Kaveh R. Ghazi]

 > From: Zack Weinberg <zack@codesourcery.com>
 > 
 > On Wed, Sep 05, 2001 at 03:51:59PM -0400, Kaveh R. Ghazi wrote:
 > >  > These are not warnings we want to ignore, either.  Those strings
 > >  > really are too long to be safe.
 > >  > Please, can we stop trying to paper over the problems and _fix_ them?
 > > 
 > > Well that's a separate question.  I don't know of any way short of
 > > rewriting the whole specs stuff that Neil was suggesting.  I'm not
 > > generally in favor of papering over things, but at the same time I
 > > don't think -Werror should necessarily bottleneck on a specs
 > > implementation rewrite if a simple workaround is available and the
 > > long term solution is on someone else's todo list.
 > 
 > You and I seem to be coming at this from different angles.  I am not
 > yet convinced that -Werror is actually a good idea, even if we *did*
 > have zero warnings across all targets, which we don't.  I would like
 > to discuss how we can get to zero warnings, instead of how we should
 > enforce its staying that way once we do get there.

Dicussing whether -Werror is a good idea is part of why I posted an
RFC.  I'm not wedded to -Werror, but I would like some way of at least
not getting more warning regressions every week.  I previously
suggested incorporating the output from contrib/warn_summary into the
regression tester.  But not having seen that happen (and not having
the time myself) I thought we could take another approach.


 > The reason I'm not convinced that -Werror would be a good idea even
 > after we get to zero warnings, is that the existing set of warnings
 > have been around for a very long time because they're both harmless
 > and intractable.  (Harmless, in the sense that I am fairly sure the
 > code is in all cases correct, just pulling dubious tricks.)  I feel
 > it's likely that, even if we do solve all of them, we will get more
 > code in the same category - causes harmless, intractable warnings -
 > and then there will be much finger-pointing and raging and gnashing
 > of teeth, should we enforce zero warnings with -Werror.
 > 
 > I'd like to make a comparison to the argument we had over whether
 > patches that exposed bugs elsewhere, should be reverted.  All the
 > warnings in combine.c happen because someone decided mode_bitsize
 > should be unsigned.  They were probably right.  With -Werror, the
 > patch that made it unsigned would have broken bootstrap, and then
 > we would have had an endless unproductive flamewar about it.
 > zw

No with -Werror, the patch wouldn't have been accepted in the first
place because the submitter couldn't have said they were able to
bootstrap with it.  This may have encouraged them to fix the new
warnings too along with their patch submission.  Think of it like
ENABLE_CHECKING failures.  Previously we would have allowed in a patch
which quietly introduced bugs.  Now with ENABLE_CHECKING on by
default, you can't bootstrap your patch if it doesn't pass these
internal consistency checks.  Using -Werror would do an analogous
thing, encourage everyone to not introduce new warnings.

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions