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: dewar@gnat.com
 > 
 > <<* 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.  >>
 > 
 > Yes, it does, you can always get the effect of a warning free cast by
 > casting pointers to two objects. So you can cast a pointer to the
 > pointer to a pointer to int, and then print the contents of this
 > pointer in the proper format.

Ok, using casts at each call site, its possible to fix.  But its still
annoying.  These were just a few examples off the top of my head.  My
point is that many -Wall -pedantic bogosities pop up on oddball
systems.

So I still stand by my assertion that we'd have a heck of a time
getting all warnings ironed out on *all* systems without breaking
bootstrap on some of them if we activate -Werror.

Better to start with a regression checker as a starting point.

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

[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

[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

[Kaveh R. Ghazi]

 > From: Neil Booth <neil@daikokuya.demon.co.uk>
 > 
 > The particular example of the warning that started this thread, about
 > ISO macros and a missing argument, *is* easily fixable if we track
 > where macros were defined (since it refers merely to the form of the
 > invocation of that macro).
 > 
 > > Geoff suggested fixincludes, but it would be a real pain to fix every
 > > macro that could potentially cause problems.  And some cases are a
 > > catch-22, e.g. what about UINT_MAX which contains a U suffix?  Using
 > > UINT_MAX in user code causes -Wtraditional to complain about the U,
 > > but without the U you get "decimal constant is so large it is
 > > unsigned" problems.  (Not that I would recommend getting rid of the U,
 > > just that either way we're hosed.)
 > 
 > I think people using -Wtraditional would have to accept the U suffix
 > warning - it is what they asked for, after all.  That warning is IMO
 > less important than the signedness one.  I don't think current
 > behaviour is particularly broken.

My point was not whether -Wtraditional users should accept U, it was
merely to point out that some warnings have inherent conflicts getting
down to zero.  Remember -Werror *requires* absolutely zero warnings on
every platform its used on to avoid breaking bootstrap.  Just one of
the obstacles are system header macros called in user code.

If we're having this much controversy over just one issue, I don't see
how we can ever turn on -Werror.

So... I reiterate that I favor the regression tester approach. :-)

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

[Kaveh R. Ghazi]

 > From: Russ Allbery <rra at stanford dot edu> 
 > 
 > Another problem that I ran into recently is the SIG_DFL macro in
 > <sys/signal.h> on Solaris causing warnings with -Wstrict-prototypes,
 > which makes it harder to track down missing prototypes in old code
 > that used to be K&R (and similarly prevents -Werror from being used,
 > which is what I'd originally intended on doing).  Fixing things like
 > that would require adding a prototype to a function pointer constant,
 > which doesn't sound like very much fun.

I put code in the trunk in August to avoid prototype warnings for
function pointer casts on integer constants for exactly this problem.

However not all bogus warnings are addressable like this one was.

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

[Zack Weinberg]

[CC list brutally trimmed.]

On Sun, Jan 14, 2001 at 08:40:44AM -0500, Kaveh R. Ghazi wrote:
>  > From: Neil Booth <neil@daikokuya.demon.co.uk>
>  > 
>  > 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.
> 
> I'm not sure its as hairy as you suggest, Zack seemed to think it
> could be done.  See my previous posting on this topic which contains
> references to his opinions from August.
> http://gcc.gnu.org/ml/gcc/2001-01/msg00886.html

I think we can all agree that the ideal situation, from the user's
point of view, would be:

/* stdio.h */
#define printf(format, args...) fprintf(stdout, format, ##args)

/* user */
printf("string")

	-> No warning about C99 vararg semantics.
	   (If the varargs extension is used by a macro defined in a
	   system header, suppress the warning.)

/* limits.h */
#define UINT_MAX 4294967295U

/* user */
  if (expression involving UINT_MAX)

	-> No warning about 'U' not understood by K+R C.
	   (If the numeric token in question originated in a system
	   header, suppress the warning.)

/* string.h */
#define strcmp(a, b) /* monstrous horror */

/* user */
  qsort(array, elts, sizeof (char *), strcmp);

	-> No warning about function macro used in non-function context.
	   (If the function macro in question was defined in a system
	   header, suppress the warning.)

/* syslog.h */
extern void syslog(int, char *, ...)
	__attribute__ ((format (printf, 2, 3)));

/* user */

syslog(LOG_INFO, "i=%s", 34);

	-> Yes, -Wformat warning.
	(-Wformat pays no attention to the location of the prototype
	for the printflike function.)

But I do expect that implementing this would take substantial effort.
We'd need to implement tests for every individual warning to determine
whether we're warning about a construct in the code the user wrote.
That is not the same in general as not warning about constructs
composed of tokens from system headers; nor is it the same as
suppressing all warnings in system headers.  Notice that the
appropriate check is different in each context.

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

To be fair to Ulrich, he explained in private mail that he wants the
error message when printf is used without arguments to be the same
with or without -O.

My personal opinion about glibc's clever macros is that they make the
generated code worse, and they should all go away.  [This one is
perhaps the least problematic.]  As such, I'm not particularly
interested in catering to them in cpp.

However, I think effort put into improving gcc's warning generation is
always effort well spent.  And that includes reducing the number of
false positives.

zw

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

[Neil Booth]

Kaveh R. Ghazi wrote:-

> I'm not sure its as hairy as you suggest, Zack seemed to think it
> could be done.  See my previous posting on this topic which contains
> references to his opinions from August.

You could set in_system_header when using a system macro, but that's a
bit of a sledgehammer approach.

For macros in general, it is not easy to know where they start or
finish e.g. macros arguments can cross the boundary of one macro and
another and / or the source file.  Also, the number of macros (even in
GCC itself) that take arguments which are or contain the name of
another macro to invoke is huge.  Look at some of the hairy testcases
- one from Jamie Lokier makes my mind boggle.

These things really do make the general situation more complex. You
could also be turning off warnings that are useful, e.g. all the
printf format checking warnings would go, since that was defined in a
system header, right?

The particular example of the warning that started this thread, about
ISO macros and a missing argument, *is* easily fixable if we track
where macros were defined (since it refers merely to the form of the
invocation of that macro).

But I remain _very_ sceptical that any of this is a good idea.  Also
GCC has enough overhead as it is - I don't want to add more without
good reason.

> Geoff suggested fixincludes, but it would be a real pain to fix every
> macro that could potentially cause problems.  And some cases are a
> catch-22, e.g. what about UINT_MAX which contains a U suffix?  Using
> UINT_MAX in user code causes -Wtraditional to complain about the U,
> but without the U you get "decimal constant is so large it is
> unsigned" problems.  (Not that I would recommend getting rid of the U,
> just that either way we're hosed.)

I think people using -Wtraditional would have to accept the U suffix
warning - it is what they asked for, after all.  That warning is IMO
less important than the signedness one.  I don't think current
behaviour is particularly broken.

> The only solution to this kind of problem IMHO is to teach gcc to
> ignore warnings from macros defined in system headers.  Zack outlined
> a way to do this given an integrated preprocessor, is it still
> workable given all the changes to the cpp infrastructure since August?

As I said, you could take the approach that once a system header macro
is used, you suppress warnings.  But I think this would end up being
really confusing, and causing a lot of stuff to be suppressed.  The
front ends currently keep track of systemheader-ness themselves, they
would need to be updated to use CPP exclusively for this information
(which by itself is a good idea anyway, IMO).

Neil.

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

[Russ Allbery]

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

> Geoff suggested fixincludes, but it would be a real pain to fix every
> macro that could potentially cause problems.  And some cases are a
> catch-22, e.g. what about UINT_MAX which contains a U suffix?  Using
> UINT_MAX in user code causes -Wtraditional to complain about the U, but
> without the U you get "decimal constant is so large it is unsigned"
> problems.  (Not that I would recommend getting rid of the U, just that
> either way we're hosed.)

Another problem that I ran into recently is the SIG_DFL macro in
<sys/signal.h> on Solaris causing warnings with -Wstrict-prototypes, which
makes it harder to track down missing prototypes in old code that used to
be K&R (and similarly prevents -Werror from being used, which is what I'd
originally intended on doing).  Fixing things like that would require
adding a prototype to a function pointer constant, which doesn't sound
like very much fun.

-- 
Russ Allbery (rra@stanford.edu)             < http://www.eyrie.org/~eagle/ >

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

[Kaveh R. Ghazi]

 > From: Neil Booth <neil@daikokuya.demon.co.uk>
 > 
 > 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.

I'm not sure its as hairy as you suggest, Zack seemed to think it
could be done.  See my previous posting on this topic which contains
references to his opinions from August.
http://gcc.gnu.org/ml/gcc/2001-01/msg00886.html


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

Well that aside, we still have problems with proprietary vendors whose
system header macros produce warnings when utilized in user code.

Geoff suggested fixincludes, but it would be a real pain to fix every
macro that could potentially cause problems.  And some cases are a
catch-22, e.g. what about UINT_MAX which contains a U suffix?  Using
UINT_MAX in user code causes -Wtraditional to complain about the U,
but without the U you get "decimal constant is so large it is
unsigned" problems.  (Not that I would recommend getting rid of the U,
just that either way we're hosed.)

The only solution to this kind of problem IMHO is to teach gcc to
ignore warnings from macros defined in system headers.  Zack outlined
a way to do this given an integrated preprocessor, is it still
workable given all the changes to the cpp infrastructure since August?

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

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

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

[Kaveh R. Ghazi]

 > From: Geoff Keating <geoffk@geoffk.org>
 > 
 > > 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 might be workable.  But IMHO, this list should be the same as the
"primary evaluation platforms", so perhaps you add irix6 and hpux?? to
the list.


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

That's a worthwhile project IMHO, even outside this context.  See:

http://gcc.gnu.org/ml/gcc-patches/2000-08/msg00334.html
http://gcc.gnu.org/ml/gcc-patches/2000-08/msg00432.html

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

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

[dewar]

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

Yes, it does, you can always get the effect of a warning free cast by
casting pointers to two objects. So you can cast a pointer to the
pointer to a pointer to int, and then print the contents of this pointer
in the proper format.

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

[Kaveh R. Ghazi]

 > From: Geoff Keating <geoffk@geoffk.org>
 > 
 > There's a big gotcha here: the regression tester uses gcc 2.95.2 as
 > the host compiler.  Thus, it gets quite a number of spurious
 > warnings due to bugs in 2.95.2.

So we'd have to run a separate build for the warning regression
checker and use a relatively recent CVS gcc to generate the warnings.


 >   It's also not reproducible,
 > because the warnings you get on sparc-solaris are not the same set
 > as the warnings you get on x86-linux or even sparc-linux.  This is
 > likely to be a fatal problem for any such tool.

I think what you mean here is that once the checker finds a
regression, the person who caused it may not be able to reproduce it
on their host platforms.  That's quite possible, but having the
checker is better then not having it.  I think if we run the checker
native on x86-linux-gnu, we start with a common enough platform that
most developers have access to.  If someone working elsewhere
introduces a warning only seen on x86-linux-gnu, they may still be
able to fix it because most warnings are obvious by inspection.  Still
we'll have to work out how to fix it some other way via a volunteer if
the original culprit can't do so.  I don't think this is fatal.


 > IMO, it would be better to switch off -Wall, switch on -Werror, and
 > switch on individual warning flags until something breaks.
 > - Geoffrey Keating <geoffk@geoffk.org>

I'm not sure if you mean to do this for all bootstraps or just the
warning checker host.

If done for everyone, you'd still run into the problems I outlined in
my previous message about unfixable warnings.  If done only for the
checker system, unless you fix every remaining warning you'd have so
few -W* flags turned on that the inactive ones (e.g. -Wsign-compare)
will regress very quickly since no one sees them any more.

IMHO, we could get a regression checker up relatively quickly, but
getting -Werror working is far away.  I'm willing to be proven false
by a working example though. :-)

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

[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

[dewar]

<<If rewriting is 'too big a burden', I suspect that means the warning
is not 'easy to avoid', and so it should not be in -Wall.
>>

But if your criterion is that no use ever anywhere could be in the 
category of not "easy to avoid", that's a rather fierce requirement.

However, I suspect that most of the remaining warnings are indeed
fixable.

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

[Geoff Keating]

> From: dewar@gnat.com
> Cc: aj@suse.de, dewar@gnat.com, dkorn@pixelpower.com, gcc@gcc.gnu.org,
>         jsm28@cam.ac.uk, robertlipe@usa.net
> Date: Fri, 12 Jan 2001 17:34:32 -0500 (EST)
> 
> <<Why do we have unfixable warnings in -Wall?  They're not supposed to
> be there.
> >>
> 
> I am not sure what this means, any warning can be wrong sometimes .. yes
> you can always fix them in the sense of rewriting (e.g. in super standard
> ANSI C), but sometimes that's too big a burden.

The definition of -Wall is:

  This enables all the warnings about constructions that some users
  consider questionable, and that are easy to avoid (or modify to
  prevent the warning), even in conjunction with macros.

If rewriting is 'too big a burden', I suspect that means the warning
is not 'easy to avoid', and so it should not be in -Wall.

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

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

[dewar]

<<Why do we have unfixable warnings in -Wall?  They're not supposed to
be there.
>>

I am not sure what this means, any warning can be wrong sometimes .. yes
you can always fix them in the sense of rewriting (e.g. in super standard
ANSI C), but sometimes that's too big a burden.

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

[Geoff Keating]

> Date: Fri, 12 Jan 2001 11:07:39 -0500 (EST)
> 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.

> Instead, my suggestion would be to run some kind of regression checker
> like what we have for the testsuite.  Have it email whoever introduces
> warnings until they fix it, just like the testsuite nag.  That way we
> at least hold the line on the current warning count, and getting
> closer to zero is attainable.
> 
> I don't have time to make this happen, but I suspect it wouldn't be
> too hard using Geoff's current regression checker hooked up to the
> contrib/warn_summary script in some fashion.  Focus on the "Number of
> warning types:" but ignore line numbers or filename in case moving
> code around changes where the warning comes from.

There's a big gotcha here: the regression tester uses gcc 2.95.2 as
the host compiler.  Thus, it gets quite a number of spurious warnings
due to bugs in 2.95.2.  It's also not reproducible, because the
warnings you get on sparc-solaris are not the same set as the warnings
you get on x86-linux or even sparc-linux.  This is likely to be a
fatal problem for any such tool.

IMO, it would be better to switch off -Wall, switch on -Werror, and
switch on individual warning flags until something breaks.

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

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

[Kaveh R. Ghazi]

 > From: Robert Lipe <robertlipe@usa.net> 
 > 
 > 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

Needless to say, I'm in favor of this.  However just making it a
criteria isn't enough.  Unless some automated enforcement is put in
place, it won't be adhered to IMHO.

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.

Instead, my suggestion would be to run some kind of regression checker
like what we have for the testsuite.  Have it email whoever introduces
warnings until they fix it, just like the testsuite nag.  That way we
at least hold the line on the current warning count, and getting
closer to zero is attainable.

I don't have time to make this happen, but I suspect it wouldn't be
too hard using Geoff's current regression checker hooked up to the
contrib/warn_summary script in some fashion.  Focus on the "Number of
warning types:" but ignore line numbers or filename in case moving
code around changes where the warning comes from.

Also, you'll want to use stage3 warnings and not some prior version of
gcc on stage1 since the most recent snapshot has been tuned to emit
the warnings we care about.

That's my $.02 in case anyone wants to tackle this.

		Good luck,
		--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

[Akim Demaille]

>>>>> "Robert" == Robert Dewar <dewar@gnat.com> writes:

Robert> In GNAT, we have a compiler option to make all warnings fatal,
Robert> and we always use this option for all builds. We have also
Robert> adopted a zero warnings tolerance for all our C code.

In forthcoming Autoconf 2.50 we have introduced support for a new
hopefully standard envvar, WARNINGS.  I plan to propagate its support
in several tools, the ultimate goals being to make it possible to run
say a test suite with an ultra severe environment, something which is
extremely useful in the development of Autoconf for instance.

Bison is next on my list, and it would be great if GCC could support
it too.

Here is the documentation provided with Autoconf:

   `autoconf' accepts the following options:

[--- CUT ---]

`--warnings=CATEGORY'
`-W CATEGORY'
     Report the warnings related to CATEGORY (which can actually be a
     comma separated list).  *Note Reporting Messages::, macro
     `AC_DIAGNOSE', for a comprehensive list of categories.  Special
     values include:

    `all'
          report all the warnings

    `none'
          report none

    `error'
          treats warnings as errors

    `no-CATEGORY'
          disable warnings falling into CATEGORY

     Warnings about `syntax' are enabled by default, and the environment
     variable `WARNINGS', a comma separated list of categories, is
     honored. `autoconf' will actually behave as if you had run

          autoconf --warnings=syntax,$WARNINGS,CATEGORIES

     If you want to disable `autoconf''s defaults and `WARNING' but
     enable the warnings about obsolete constructs, use `-W
     none,obsolete'.

     `autoconf' displays a back trace for errors, but not for warnings;
     if you want them, just pass `-W error'.  For instance on this
     `configure.ac':

          AC_DEFUN([INNER],
          [AC_TRY_RUN([true])])
          
          AC_DEFUN([OUTTER],
          [INNER])
          
          AC_INIT
          OUTTER

     you get:

          /tmp % ace -Wcross
          configure.ac:8: warning: AC_TRY_RUN called without default \
          to allow cross compiling
          /tmp % ace -Wcross,error
          configure.ac:8: error: AC_TRY_RUN called without default \
          to allow cross compiling
          acgeneral.m4:3044: AC_TRY_RUN is expanded from...
          configure.ac:2: INNER is expanded from...
          configure.ac:5: OUTTER is expanded from...
          configure.ac:8: the top level

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

[David Korn]

>Obviously this cannot happen over night, but I think it is a very
>useful goal. There is something about zero warnings that is considerably
>more useful than "few" warnings.

  Seconded.  We adhere to this standard in my daily work, although we are
flexible about it if the circumstances seem to justify.  Hard experience
has shown us that the more warnings fly by during a build, the less likely
anyone is to pay attention and notice new ones that actually mean something
serious.  It's proved to be a source of bugs, so we decided to stop it.

  Of course it may or may not be an attainable goal considering the number
of targets that gcc builds for, but that's a practical rather than
theoretical problem ......

     DaveK
-- 
The Boulder Pledge: "Under no circumstances will I ever purchase anything 
offered to me as the result of an unsolicited email message. Nor will I 
forward chain letters, petitions, mass mailings, or virus warnings to large 
numbers of others. This is my contribution to the survival of the online
community." 


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

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

[dewar]

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

I prefer a zero warnings criterion for the bootstrap, but for that you
definitely need the feature to selectively turn off warnings for a 
particular section of code.

In GNAT, we have a compiler option to make all warnings fatal, and we
always use this option for all builds. We have also adopted a zero
warnings tolerance for all our C code.

Obviously this cannot happen over night, but I think it is a very
useful goal. There is something about zero warnings that is considerably
more useful than "few" warnings.

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

[dewar]

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

Ada does not have this feature, but it allows an implementation to 
introduce new pragmas, and we introduced pragma Warnings (Off | On)
precisely for this purpose, and have found it very useful.

After all almost any warning has the property that it is wrong sometimes,
(otherwise it should be an error).

"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