"introduce no new bootstrap warning" criteria. was: Loop iv debugging patch

"introduce no new bootstrap warning" criteria. was: Loop iv debugging patch

[Robert Lipe]

About a recent patch, Andreas Jaeger wrote:

> Bootstrapping gcc I get:
> /cvs/gcc/gcc/doloop.c:60: warning: static declaration for `doloop_condition_get' follows non-static

For some compilers, this is a hard error and breaks the build.


I'm not going to fuss about the commit in question.  But in recent
years, a LOT of effort has been spent on getting the warning level in
a full bootstrap down to a manageable number.  (Thanks, Kaveh!)  It's
easy to watch that number decay as code is added back in that isn't held
to the same standards of zero warnings.  Yeah, when major new libraries
come it I can see it taking some time for them to stabilize on all
combinations of builds, but we're consistently seeing defects introduced
into tree that gcc itself would have told us about.

Is it time, in the name of quality/damage control in this project, to
make it an acceptance criteria for any commit that it introduce no new
warnings?

RJL

Re: "introduce no new bootstrap warning" criteria. was: Loop ivdebugging patch

[Joseph S. Myers]

On Fri, 12 Jan 2001, Robert Lipe wrote:

> Is it time, in the name of quality/damage control in this project, to
> make it an acceptance criteria for any commit that it introduce no new
> warnings?

 ... unless the point of the patch is to improve GCC's warnings, and this
causes previously undetected problems in GCC to be diagnosed.

What happened to the idea of a pragma to mark particular warnings as
expected in code - whether globally, scoped with push/pop, or for a
particular line of code only?  (With the preferred design from previous
discussions being, I think, a regular expression describing the expected
English language warning text, with an unresolved issue over whether the
expression should also be matched against the local language text.)  This
way, the spurious warnings that can't readily be fixed (e.g. macro strcmp
used without args on glibc systems - a -Wtraditional warning, but
presumably no system for which it is relevant has the strcmp function as a
macro) can be removed from the build.

See e.g. <URL: http://gcc.gnu.org/ml/gcc/2000-06/msg00639.html >.

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

Re: "introduce no new bootstrap warning" criteria. was: Loop iv debugging patch

[Philipp Thomas]

* Joseph S. Myers (jsm28@cam.ac.uk) [20010112 15:15]:

> What happened to the idea of a pragma to mark particular warnings as
> expected in code - whether globally, scoped with push/pop, or for a
> particular line of code only?

Well, it got discussed and then none had time (or interest?) to actually
implement it. Now that we have _Pragma in, there is nothing really blocking
implementation of '#pragma GCC waring ...', that is besides of volunteers to
actually do it ;-)

> With the preferred design from previous
> discussions being, I think, a regular expression describing the expected
> English language warning text,

Well, my favourite is still much easier to implement. I'd simply use the
name of a warning to specify it. Thus you'd do something like this:

#pragma GCC warning push
#pragma GCC warning unused off
<some code>
#pragma GCC warning pop.

What we might need for this to get really usefull would be to make warnings
more fine grained.

Philipp

-- 
Philipp Thomas <pthomas@suse.de>
Development, SuSE GmbH, Schanzaecker Str. 10, D-90443 Nuremberg, Germany

Penguins shall save the dinosaurs
                          -- Handelsblatt about Linux on S/390

Re: "introduce no new bootstrap warning" criteria. was: Loop iv debugging patch

[Fergus Henderson]

On 15-Jan-2001, Philipp Thomas <pthomas@suse.de> wrote:
> #pragma GCC warning push
> #pragma GCC warning unused off
> <some code>
> #pragma GCC warning pop.

That's very prolix syntax.  It would be quite unfortunate if you have to
write three #pragmas just to disable one warning.

I'd much prefer something more similar to the current `__extension__'.
For example `__nowarn__', used as a prefix on an expression,
declaration, or statement, could disable all warnings for just that
expression/declaration/statement.  Alternatively,
`__disablewarn__(NAME)' could be used in the same contexts to
selectively disable specific warnings.

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

Re: "introduce no new bootstrap warning" criteria. was: Loop iv debugging patch

[Andreas Jaeger]

>>>>> Robert Lipe writes:

 > About a recent patch, Andreas Jaeger wrote:
>> Bootstrapping gcc I get:
>> /cvs/gcc/gcc/doloop.c:60: warning: static declaration for `doloop_condition_get' follows non-static

 > For some compilers, this is a hard error and breaks the build.


 > I'm not going to fuss about the commit in question.  But in recent
 > years, a LOT of effort has been spent on getting the warning level in
 > a full bootstrap down to a manageable number.  (Thanks, Kaveh!)  It's
 > easy to watch that number decay as code is added back in that isn't held
 > to the same standards of zero warnings.  Yeah, when major new libraries
 > come it I can see it taking some time for them to stabilize on all
 > combinations of builds, but we're consistently seeing defects introduced
 > into tree that gcc itself would have told us about.

 > Is it time, in the name of quality/damage control in this project, to
 > make it an acceptance criteria for any commit that it introduce no new
 > warnings?

I don't think we can demand absolutly no new warnings.  But we could
classify the warnings and demand that some warnings (like the one I
reported and fixed) are forbidden but others are ok
(e.g. signed/unsigned compares).

Another problem is also that you might not get any warning on the
platform you're bootstrapping on but would get a warning if you
bootstrap, e.g. on a 64 bit platform instead of a 32 bit platform.

I'm in favor of decreasing the number of warnings,
Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: "introduce no new bootstrap warning" criteria. was: Loop iv debugging patch

[Toon Moene]

Robert Lipe wrote:

> About a recent patch, Andreas Jaeger wrote:

> > Bootstrapping gcc I get:
> > /cvs/gcc/gcc/doloop.c:60: warning: static declaration for `doloop_condition_get' follows non-static

> For some compilers, this is a hard error and breaks the build.

"Zachte heelmeesters maken stinkende wonden".

> I'm not going to fuss about the commit in question.  But in recent
> years, a LOT of effort has been spent on getting the warning level in
> a full bootstrap down to a manageable number.  (Thanks, Kaveh!)

Not only that, but real bugs have been fixed because they showed up once
it became practical to add -Wall to the bootstrap flags.

If people are really interested, I can dig up the first one.

-- 
Toon Moene - mailto:toon@moene.indiv.nluug.nl - phoneto: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
Maintainer, GNU Fortran 77: http://gcc.gnu.org/onlinedocs/g77_news.html
Join GNU Fortran 95: http://g95.sourceforge.net/ (under construction)

Re: "introduce no new bootstrap warning" criteria. was: Loop iv debugging, patch

[Geoff Keating]

> Date: Sat, 13 Jan 2001 07:33:46 -0500 (EST)
> From: "Kaveh R. Ghazi" <ghazi@caip.rutgers.edu>
> Cc: aj@suse.de, dewar@gnat.com, dkorn@pixelpower.com, gcc@gcc.gnu.org,
>         jsm28@cam.ac.uk, robertlipe@usa.net
> 
> 
>  > From: Geoff Keating <geoffk@geoffk.org>
>  > 
>  > > From: "Kaveh R. Ghazi" <ghazi@caip.rutgers.edu>
>  > >
>  > > I looked into what it would take to turn on -Werror and/or
>  > > -pedatic-errors and that doesn't seem possible.  There are too many
>  > > unfixable messages requiring a pragma silencer and also many warnings
>  > > only appear on unusual platforms so we'd have a real hard time getting
>  > > this to work without breaking bootstrap on lots of systems.  Plus any
>  > > time a new warning is added to -Wall, it would break systems until
>  > > completely silenced also.
>  > 
>  > Why do we have unfixable warnings in -Wall?  They're not supposed
>  > to be there.
> 
> Recall that if we activate -Werror, we must have zero warnings on all
> platforms, even old strange broken ones.

Perhaps we could only define -Werror on non-strange-broken platforms?

You know, like Linux, Solaris, Cygwin, AIX.  If you do those, you'll
cover probably 99% of the GCC developers.

> * On solaris, the system header definition of __GTHREAD_ONCE_INIT
> causes missing initializer warnings.  Generically, any warning caused
> by a macro defined in system headers but used in user code is trouble.
> Checking in_system_header doesn't work for these.

Fixincludes!  Fixincludes!  :-)

Actually, this is not unreasonable.  It's just as annoying for the
user to see these messages as it is for GCC.  They probably should be
fixed in fixincludes.

> * On systems where %p isn't supported, the backup method for printing
> pointers using appropriate sized integer specifiers always causes
> -Wformat warnings.  I don't think this one has a solution.

Ugh.  The warnings are actually accurate, so you don't want to just
remove them.  I guess these count as strange broken platforms.

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

Re: "introduce no new bootstrap warning" criteria. was: Loop ivdebugging, patch

[Joseph S. Myers]

On Sun, 14 Jan 2001, Geoff Keating wrote:

> Perhaps we could only define -Werror on non-strange-broken platforms?
>
> You know, like Linux, Solaris, Cygwin, AIX.  If you do those, you'll
> cover probably 99% of the GCC developers.

That will break every time glibc changes in a way that causes warnings.

e.g., the "macro strcmp used without args" ones aren't reasonably fixable;
when glibc 2.2 changed the iconv prototype, that caused warnings; glibc
2.2.1 causes the "ISO C99 requires rest arguments to be used" warnings
every place printf is used with just one argument.

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

Re: "introduce no new bootstrap warning" criteria. was: Loop ivdebugging, patch

[Geoff Keating]

> Date: Sun, 14 Jan 2001 11:30:36 +0000 (GMT)
> From: "Joseph S. Myers" <jsm28@cam.ac.uk>
> cc: <ghazi@caip.rutgers.edu>, <aj@suse.de>, <dewar@gnat.com>,
>         <dkorn@pixelpower.com>, <gcc@gcc.gnu.org>, <robertlipe@usa.net>
> 
> On Sun, 14 Jan 2001, Geoff Keating wrote:
> 
> > Perhaps we could only define -Werror on non-strange-broken platforms?
> >
> > You know, like Linux, Solaris, Cygwin, AIX.  If you do those, you'll
> > cover probably 99% of the GCC developers.
> 
> That will break every time glibc changes in a way that causes warnings.
> 
> e.g., the "macro strcmp used without args" ones aren't reasonably fixable;
> when glibc 2.2 changed the iconv prototype, that caused warnings; glibc
> 2.2.1 causes the "ISO C99 requires rest arguments to be used" warnings
> every place printf is used with just one argument.

... so perhaps we should try to avoid having glibc change like that?
Or perhaps the macro expander should track which macros are defined in
system headers and not produce warnings for them?

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