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

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

[Kaveh R. Ghazi]

 > 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.  Here are some examples of
annoying or impossible to fix problems:

* 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.

* 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.

* On non-POSIX/non-USG "BSD" systems, e.g. sunos4, we don't include
string.h in tsystem.h.  This causes numerous implicit decl warnings
for the string functions.  We can't provide the backup decls because
they might conflict with platforms that do supply them and we can't
use autoconf in tsystem.h to confitionally provide them.

* On K&R systems, many system header prototypes (especially those
returning int which exist implicitly) aren't available unless one
defines __USE_FIXED_PROTOTYPES__.  We might want to define that during
bootstrap, but I don't know if it is safe for all platforms.
Otherwise it would have been done already.


My point is that -Werror requires *every* platform to have zero
warnings.  I think we could get *some* platforms down to zero, but its
not possible to get *all* of them IMHO.

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

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

[Andreas Jaeger]

>>>>> Kaveh R Ghazi writes:

 > My point is that -Werror requires *every* platform to have zero
 > warnings.  I think we could get *some* platforms down to zero, but its
 > not possible to get *all* of them IMHO.
I agree.

For reference, here're the current numbers on i686-linux with glibc
2.2 (after applying a patch of mine that I'll send separatly):

Counting all warnings,
there are 254 warnings in stage3 of this bootstrap.

Number of warnings per file:
     56	gcc/combine.c
     42	gcc/expr.c
     22	gcc/config/i386/i386.c
     20	gcc/reg-stack.c
     12	gcc/loop.c
     12	gcc/cp/pt.c
     10	gcc/cse.c
     10	gcc/cp/error.c
      6	gcc/reload1.c
      6	gcc/gcc.c
      4	hashtab.c
      4	gcc/reload.c
      4	gcc/regmove.c
      4	gcc/real.c
      4	gcc/libgcc2.c
      4	gcc/function.c
      4	gcc/fold-const.c
      4	gcc/cp/call.c
      4	gcc/alias.c
      2	gcc/sched-deps.c
      2	gcc/objc/lang-specs.h
      2	gcc/final.c
      2	gcc/f/lang-specs.h
      2	gcc/expmed.c
      2	gcc/dwarfout.c
      2	gcc/dwarf2out.c
      2	gcc/cp/lang-specs.h
      2	gcc/cp/init.c
      2	gcc/cp/errfn.c
      2	gcc/c-lex.c

Number of warning types:
    156	comparison between signed and unsigned
     20	`???' might be used uninitialized in this function
     14	unused variable `???'
     14	signed and unsigned type in conditional expression
     12	string length `???' is greater than the minimum length `???' ISO C89 is required to support
     10	initialization from incompatible pointer type
      8	function declaration isn't a prototype
      6	`???' defined but not used
      2	unused parameter `???'
      2	pointer targets in passing arg ??? of `???' differ in signedness
      2	passing arg ??? of `???' discards qualifiers from pointer target type
      2	no previous prototype for `???'
      2	integer constant is unsigned in ISO C, signed with -traditional
      2	function-like macro `strcmp' must be used with arguments in traditional C
      2	decimal constant is so large that it is unsigned

The libgcc2.c warnigns might be fixed now with Richard Kenner's patch
and I'm looking into i386.c now.

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

[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>

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

[Neil Booth]

Geoff Keating wrote:-

> Or perhaps the macro expander should track which macros are defined in
> system headers and not produce warnings for them?

I'm not a fan of this option at all.  The only warnings we can avoid
this way are CPP warnings about the macro definition, which one
would hope was valid anyway being in a system header.

When it comes to usage, it gets really hairy because the invocation of
the macro could have come from other macros or macro arguments,
recursively who is to know whether those tokens come from user code or
system headers?  And to the front ends, tokens all look the same
regardless of whence they came.

Zack showed Ulrich how to fix glibc to avoid the warning, and was
rudely brushed off.

Neil.

"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