Re: Warning policy?

Re: Warning policy?

[Kaveh R. Ghazi]

 > From: Jeffrey A Law <law@hurl.cygnus.com>
 > 
 >   In message < 199901020028.TAA04541@caip.rutgers.edu >you write:
 >   > Anyway, I went ahead and implemented it.  How's this?
 >   > 
 >   > 	Okay to install?
 >   > 
 >   > 		--Kaveh
 >   > 
 >   > 
 >   > Fri Jan  1 19:11:54 1999  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
 >   > 
 >   > 	* c-common.c (decl_attributes): Allow applying attribute `unused'
 >   > 	on a LABEL_DECL.
 >   > 
 >   > 	* c-parse.in (label): Parse attributes after a label, and call
 >   >  	`decl_attributes' to handle them.
 >   > 
 >   > 	* gansidecl.h (ATTRIBUTE_UNUSED_LABEL): Define.
 >   > 
 >   > 	* genrecog.c (OUTPUT_LABEL, write_tree_1, write_tree): When
 >   > 	generating labels, mark them with ATTRIBUTE_UNUSED_LABEL.
 >   > 
 >   > 	* invoke.texi: Note that labels can be marked `unused'.
 > 
 > Seems fine to me.


	Thanks, installed.


 > I'll assume that the change to the parser did
 > not introduce any new conflicts, right?



	Right, no new conflicts were introduced.

However the objc-parse.y file complains about 67 conflicts but expects
66.  This happens _before_ my patch is installed.  The same number occur
after my patch is installed. 

I wasn't sure if the 67th conflict should be ignored or not, so I
didn't change the expected number.

The c-parse.y file expects the correct number of conflicts and the
amount also did not change as a result of my patch. 



 > Don't forget to rebuild c-parse.y and objc/objc-parse.y when you
 > check in this patch.
 > Thanks!
 > jeff

	Yup, done.

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.

Re: Warning policy?

[Jeffrey A Law]

  In message < 199901041550.KAA21756@caip.rutgers.edu >you write:
  > 	Right, no new conflicts were introduced.
  > 
  > However the objc-parse.y file complains about 67 conflicts but expects
  > 66.  This happens _before_ my patch is installed.  The same number occur
  > after my patch is installed. 
  > 
  > I wasn't sure if the 67th conflict should be ignored or not, so I
  > didn't change the expected number.
  > 
  > The c-parse.y file expects the correct number of conflicts and the
  > amount also did not change as a result of my patch. 
Wonderful :( 

Mark Mitchell brought up an issue that we need to resolve.

Unconditionally adding the unused attribute to labels loses for older versions
of gcc.

I think a test based on the version of gcc should be sufficient to fix the
problem.  Can you take care of that?

Thanks,
jeff

Re: Warning policy?

[Richard Henderson]

On Tue, Jan 05, 1999 at 01:43:45AM -0700, Jeffrey A Law wrote:
> I think a test based on the version of gcc should be sufficient to fix the
> problem.  Can you take care of that?

Not without bumping the minor version of egcs.  Otherwise
anything post 1.1 but older than a few days will still fail.


r~

Re: Warning policy?

[Jeffrey A Law]

  In message < 19990105064145.A28833@dot.cygnus.com >you write:
  > On Tue, Jan 05, 1999 at 01:43:45AM -0700, Jeffrey A Law wrote:
  > > I think a test based on the version of gcc should be sufficient to fix th
  > e
  > > problem.  Can you take care of that?
  > 
  > Not without bumping the minor version of egcs.  Otherwise
  > anything post 1.1 but older than a few days will still fail.
The test should be > egcs-2.92.33

We can either bump the version # at the same time or wait for the automatic
bump for the snapshot.

jeff

Re: Warning policy?

[Kaveh R. Ghazi]

 > From: Joe Buck <jbuck@Synopsys.COM>
 > 
 > >  > Andreas Schwab writes: 
 > >  > > Well, the brute force method would be to just emit a `goto L' before
 > >  > > each label L.
 > >  > 
 > >  > That would only create more warnings, as some of these gotos would be
 > >  > unreachable code, which produces a warning.
 > > 
 > > 	Good point, but I'm not seeing this in actual practice from gcc. 
 > > I wrote a small patch to attempt what Andreas suggested and gcc does not
 > > complain about unreachable code.  Various stage1-cc's might complain,
 > > e.g. SunOS4 cc does in fact, but we could arrange to only output the
 > > goto when __GNUC__ is defined. 
 > 
 > Ah, but if gcc's ability to detect dead code improves, the warnings come
 > back.

	My impression was that gcc's response to dead code was to
eliminate it, not warn about it.  At least that's what the comments/docs
suggest...



 > Now, if there were an attribute that can be attached to a label that says
 > "this label may be unused, if so, don't warn about it" that would be
 > the simplest way to fix things.

	
	I'd call it a better solution, not a simpler one. :-)

Anyway, I went ahead and implemented it.  How's this?

	Okay to install?

		--Kaveh


Fri Jan  1 19:11:54 1999  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* c-common.c (decl_attributes): Allow applying attribute `unused'
	on a LABEL_DECL.

	* c-parse.in (label): Parse attributes after a label, and call
 	`decl_attributes' to handle them.

	* gansidecl.h (ATTRIBUTE_UNUSED_LABEL): Define.

	* genrecog.c (OUTPUT_LABEL, write_tree_1, write_tree): When
	generating labels, mark them with ATTRIBUTE_UNUSED_LABEL.

	* invoke.texi: Note that labels can be marked `unused'.


diff -rup orig/egcs-CVS19981231/gcc/c-common.c egcs-CVS19981231/gcc/c-common.c
--- orig/egcs-CVS19981231/gcc/c-common.c	Thu Dec 31 12:05:12 1998
+++ egcs-CVS19981231/gcc/c-common.c	Thu Dec 31 23:05:54 1998
@@ -521,7 +521,8 @@ decl_attributes (node, attributes, prefi
 	    TREE_USED (type) = 1;
 	  else if (TREE_CODE (decl) == PARM_DECL
 		   || TREE_CODE (decl) == VAR_DECL
-		   || TREE_CODE (decl) == FUNCTION_DECL)
+		   || TREE_CODE (decl) == FUNCTION_DECL
+		   || TREE_CODE (decl) == LABEL_DECL)
 	    TREE_USED (decl) = 1;
 	  else
 	    warning ("`%s' attribute ignored", IDENTIFIER_POINTER (name));
diff -rup orig/egcs-CVS19981231/gcc/c-parse.in egcs-CVS19981231/gcc/c-parse.in
--- orig/egcs-CVS19981231/gcc/c-parse.in	Thu Dec 31 12:05:15 1998
+++ egcs-CVS19981231/gcc/c-parse.in	Thu Dec 31 23:05:54 1998
@@ -2214,12 +2214,15 @@ label:	  CASE expr_no_commas ':'
 		      error_with_decl (duplicate, "this is the first default label");
 		    }
 		  position_after_white_space (); }
-	| identifier ':'
+	| identifier ':' maybe_attribute
 		{ tree label = define_label (input_filename, lineno, $1);
 		  stmt_count++;
 		  emit_nop ();
 		  if (label)
-		    expand_label (label);
+		    {
+		      expand_label (label);
+		      decl_attributes (label, $3, NULL_TREE);
+		    }
 		  position_after_white_space (); }
 	;
 
diff -rup orig/egcs-CVS19981231/gcc/gansidecl.h egcs-CVS19981231/gcc/gansidecl.h
--- orig/egcs-CVS19981231/gcc/gansidecl.h	Thu Dec 31 12:05:11 1998
+++ egcs-CVS19981231/gcc/gansidecl.h	Thu Dec 31 23:05:55 1998
@@ -38,6 +38,14 @@ Boston, MA 02111-1307, USA.  */
 # define __attribute__(x)
 #endif
 
+#ifndef ATTRIBUTE_UNUSED_LABEL
+# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 92)
+#  define ATTRIBUTE_UNUSED_LABEL
+# else
+#  define ATTRIBUTE_UNUSED_LABEL ATTRIBUTE_UNUSED
+# endif /* GNUC < 2.92 */
+#endif /* ATTRIBUTE_UNUSED_LABEL */
+
 #ifndef ATTRIBUTE_UNUSED
 #define ATTRIBUTE_UNUSED __attribute__ ((__unused__))
 #endif /* ATTRIBUTE_UNUSED */
diff -rup orig/egcs-CVS19981231/gcc/genrecog.c egcs-CVS19981231/gcc/genrecog.c
--- orig/egcs-CVS19981231/gcc/genrecog.c	Thu Dec 31 12:06:07 1998
+++ egcs-CVS19981231/gcc/genrecog.c	Thu Dec 31 23:05:55 1998
@@ -51,6 +51,9 @@ Boston, MA 02111-1307, USA.  */
 #include "rtl.h"
 #include "obstack.h"
 
+#define OUTPUT_LABEL(INDENT_STRING, LABEL_NUMBER) \
+  printf("%sL%d: ATTRIBUTE_UNUSED_LABEL\n", (INDENT_STRING), (LABEL_NUMBER))
+
 static struct obstack obstack;
 struct obstack *rtl_obstack = &obstack;
 
@@ -1103,7 +1106,7 @@ write_tree_1 (tree, prevpos, afterward, 
   printf ("\n");
   if (tree && tree->subroutine_number == 0)
     {
-      printf ("  L%d:\n", tree->number);
+      OUTPUT_LABEL ("  ", tree->number);
       tree->label_needed = 0;
     }
 
@@ -1239,7 +1242,7 @@ write_tree_1 (tree, prevpos, afterward, 
 
       if (p->label_needed && (p->retest_mode || p->retest_code))
 	{
-	  printf ("%sL%d:\n", indents[indent - 2], p->number);
+	  OUTPUT_LABEL (indents[indent - 2], p->number);
 	  p->label_needed = 0;
 	}
 
@@ -1330,7 +1333,7 @@ write_tree_1 (tree, prevpos, afterward, 
       /* Now that most mode and code tests have been done, we can write out
 	 a label for an inner node, if we haven't already.  */
       if (p->label_needed)
-	printf ("%sL%d:\n", indents[indent - 2], p->number);
+	OUTPUT_LABEL (indents[indent - 2], p->number);
 
       inner_indent = indent;
 
@@ -1563,7 +1566,7 @@ write_tree (tree, prevpos, afterward, in
 
   if (! initial && tree->subroutine_number > 0)
     {
-      printf (" L%d:\n", tree->number);
+      OUTPUT_LABEL (" ", tree->number);
 
       if (afterward)
 	{
diff -rup orig/egcs-CVS19981231/gcc/invoke.texi egcs-CVS19981231/gcc/invoke.texi
--- orig/egcs-CVS19981231/gcc/invoke.texi	Thu Dec 31 12:06:17 1998
+++ egcs-CVS19981231/gcc/invoke.texi	Thu Dec 31 23:05:55 1998
@@ -1340,7 +1340,7 @@ In order to get a warning about an unuse
 specify both @samp{-W} and @samp{-Wunused}.
 
 To suppress this warning for an expression, simply cast it to void.  For
-unused variables and parameters, use the @samp{unused} attribute
+unused variables, parameters and labels, use the @samp{unused} attribute
 (@pxref{Variable Attributes}).
 
 @item -Wuninitialized

Re: Warning policy?

[Jeffrey A Law]

  In message < 199901020028.TAA04541@caip.rutgers.edu >you write:
  > Anyway, I went ahead and implemented it.  How's this?
  > 
  > 	Okay to install?
  > 
  > 		--Kaveh
  > 
  > 
  > Fri Jan  1 19:11:54 1999  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
  > 
  > 	* c-common.c (decl_attributes): Allow applying attribute `unused'
  > 	on a LABEL_DECL.
  > 
  > 	* c-parse.in (label): Parse attributes after a label, and call
  >  	`decl_attributes' to handle them.
  > 
  > 	* gansidecl.h (ATTRIBUTE_UNUSED_LABEL): Define.
  > 
  > 	* genrecog.c (OUTPUT_LABEL, write_tree_1, write_tree): When
  > 	generating labels, mark them with ATTRIBUTE_UNUSED_LABEL.
  > 
  > 	* invoke.texi: Note that labels can be marked `unused'.
Seems fine to me.  I'll assume that the change to the parser did not introduce
any new conflicts, right?

Don't forget to rebuild c-parse.y and objc/objc-parse.y when you check in this
patch.

Thanks!

jeff

Re: Warning policy?

[David Edelsohn]

>>>>> "Kaveh R Ghazi" writes:

Kaveh> Fri Jan  1 19:11:54 1999  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

Kaveh> * c-common.c (decl_attributes): Allow applying attribute `unused'
Kaveh> on a LABEL_DECL.
Kaveh> * c-parse.in (label): Parse attributes after a label, and call
Kaveh> `decl_attributes' to handle them.
Kaveh> * gansidecl.h (ATTRIBUTE_UNUSED_LABEL): Define.
Kaveh> * genrecog.c (OUTPUT_LABEL, write_tree_1, write_tree): When
Kaveh> generating labels, mark them with ATTRIBUTE_UNUSED_LABEL.
Kaveh> * invoke.texi: Note that labels can be marked `unused'.

	I, personally, am much happier marking labels as possibly unused
than creating artificial uses.  I think this or something like it is a
much more correct solution.  Removing warnings is suppose to IMPROVE the
quality of the code.

David

Re: Warning policy?

[Kamil Iskra]

On Fri, 1 Jan 1999, Kaveh R. Ghazi wrote:

> 	My impression was that gcc's response to dead code was to
> eliminate it, not warn about it.  At least that's what the comments/docs
> suggest...

As far as I remember, this comment comes from gcc2 era.

IMHO, the EGCS project should reconsider this issue. In my experience,
unreachable code is always a result of an oversight on my side.
Therefore, I would much appreciate if GCC warned when if finds dead code.
I find it hard to imagine a human written code that intentionally
contained dead code, unless the code was in a state of flux.

Of course, it's a different story when it comes to machine-generated code
and I agree that GCC should not warn about dead code by default. Still,
I can see nothing wrong in providing this functionality under -Wdead-code.

/ Kamil Iskra    AmigaOS  Linux/i386  Linux/m68k               \
| GeekGadgets m68k-amigaos GCC maintainer                      |
| iskra@student.uci.agh.edu.pl  kiskra@ernie.icslab.agh.edu.pl |
\ kamil@dwd.interkom.pl   http://student.uci.agh.edu.pl/~iskra /

Re: Warning policy?

[Joe Buck]

> I find it hard to imagine a human written code that intentionally
> contained dead code, unless the code was in a state of flux.

#define DEBUG_MSG(text) if (DEBUG) fputs(stderr, text)

Now, if DEBUG is set to 0, the fputs is dead code -- and this is
intentional.

Re: Warning policy?

[Jeffrey A Law]

  In message < 199901022031.MAA02818@atrus.synopsys.com >you write:
  > 
  > > I find it hard to imagine a human written code that intentionally
  > > contained dead code, unless the code was in a state of flux.
  > 
  > #define DEBUG_MSG(text) if (DEBUG) fputs(stderr, text)
  > 
  > Now, if DEBUG is set to 0, the fputs is dead code -- and this is
  > intentional.
That's unreachable code, not dead code.

unreachable code is code that can never be executed regardless of the inputs
to the program.

dead code is code that may execute, but the side effects of executing the code
are never needed/used.

a = 1;
a = b + c;

The statement "a = 1" is dead code since the value computed is never used.
(assuming of course that "a" is not volatile :-)

jeff

Re: Warning policy?

[Chip Salzenberg]

According to Kamil Iskra:
> In my experience, unreachable code is always a result of an
> oversight on my side.

But sometimes it's a result of avoiding #ifdefs for conditional
execution, and that's not a bad thing:

   static const int debug = 0;
   /* ... */
   if (debug) { fprintf(error, "stuff\n"); }

-- 
Chip Salzenberg      - a.k.a. -      <chip@perlsupport.com>
      "When do you work?"   "Whenever I'm not busy."

Re: Warning policy?

[Jeffrey A Law]

  In message < Pine.LNX.3.96.990102150511.2118A-100000@jinks.home >you write:
  > On Fri, 1 Jan 1999, Kaveh R. Ghazi wrote:
  > 
  > > 	My impression was that gcc's response to dead code was to
  > > eliminate it, not warn about it.  At least that's what the comments/docs
  > > suggest...
  > 
  > As far as I remember, this comment comes from gcc2 era.
  > 
  > IMHO, the EGCS project should reconsider this issue. In my experience,
  > unreachable code is always a result of an oversight on my side.
  > Therefore, I would much appreciate if GCC warned when if finds dead code.
  > I find it hard to imagine a human written code that intentionally
  > contained dead code, unless the code was in a state of flux.
Be very careful to distinguish between dead code and unreachable code.

dead code is very common at the source level and is also often created by the
compiler during optimization.  Warning for dead code would not be particularly
useful.

unreachable code is rarer and we may be able to issue a useful warning for it.
It's also more likely to point out bugs in the source than warning about
dead code.

jeff

Re: Warning policy?

[Kaveh R. Ghazi]

 > From: Jeffrey A Law <law@hurl.cygnus.com>
 > 
 >   > Another option would be to simply bump the check to "GCC < 2.93" in
 >   > gansidecl.h (with or without an actual egcs minor version number bump.)
 >  
 > That would be fine with me too.  We could bump the version # to 2.93 if
 > we want.
 > jeff

	How about this?  Okay to install?


Thu Jan  7 09:57:18 1999  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* gansidecl.h: Allow attribute unused on labels only when we are
 	version 2.93 or higher.  Not all versions of 2.92 have this feature.

	* version.c: Bump minor number to 93.


diff -rup orig/egcs-CVS19990105/gcc/gansidecl.h egcs-CVS19990105/gcc/gansidecl.h
--- orig/egcs-CVS19990105/gcc/gansidecl.h	Tue Jan  5 22:27:32 1999
+++ egcs-CVS19990105/gcc/gansidecl.h	Thu Jan  7 09:53:41 1999
@@ -39,7 +39,7 @@ Boston, MA 02111-1307, USA.  */
 #endif
 
 #ifndef ATTRIBUTE_UNUSED_LABEL
-# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 92)
+# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 93)
 #  define ATTRIBUTE_UNUSED_LABEL
 # else
 #  define ATTRIBUTE_UNUSED_LABEL ATTRIBUTE_UNUSED
diff -rup orig/egcs-CVS19990105/gcc/version.c egcs-CVS19990105/gcc/version.c
--- orig/egcs-CVS19990105/gcc/version.c	Tue Jan  5 22:28:56 1999
+++ egcs-CVS19990105/gcc/version.c	Thu Jan  7 09:51:19 1999
@@ -1 +1 @@
-char *version_string = "egcs-2.92.34 19990105 (gcc2 ss-980609 experimental)";
+char *version_string = "egcs-2.93.00 19990105 (gcc2 ss-980609 experimental)";

Re: Warning policy?

[Jeffrey A Law]

  In message < 199901071537.KAA07669@caip.rutgers.edu >you write:
  > Thu Jan  7 09:57:18 1999  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
  > 
  > 	* gansidecl.h: Allow attribute unused on labels only when we are
  >  	version 2.93 or higher.  Not all versions of 2.92 have this feature.
  > 
  > 	* version.c: Bump minor number to 93.
Fine.  Thanks for taking care of this.

jeff

Re: Warning policy?

[Kaveh R. Ghazi]

 > From: Richard Henderson <rth@cygnus.com>
 > 
 > On Tue, Jan 05, 1999 at 01:43:45AM -0700, Jeffrey A Law wrote:
 > > I think a test based on the version of gcc should be sufficient to fix the
 > > problem.  Can you take care of that?
 > 
 > Not without bumping the minor version of egcs.  Otherwise
 > anything post 1.1 but older than a few days will still fail.
 > r~

	Urg.  Yes, if your stage1 compiler is a snapshot between 2.92.1
-> 2.92.33, it'll fail.  (Is this a common practice?  I would have
thought not, because if you had a problem, you wouldn't know whether the
stage1 compiler was at fault or the current snapshot was.  Obviously too
much ass-u-ME-ing :-) on my part though.)


What I need to do is test "GCC < 2.92.34".  Is there a way to check the
egcs micro version number?

	If not, I can write an autoconf test to set __GNUC_MICRO__ if
"GCC == 2.92".

Another option would be to simply bump the check to "GCC < 2.93" in
gansidecl.h (with or without an actual egcs minor version number bump.)

Yet another option would be to somehow enable to attribute when
stage>=2, or put another way, disable the attribute when stage==1.
But I'm not sure of the easiest way to do that off the top of my head. 

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.

Re: Warning policy?

[Jeffrey A Law]

  In message < 199901051514.KAA12156@caip.rutgers.edu >you write:
  > 	If not, I can write an autoconf test to set __GNUC_MICRO__ if
  > "GCC == 2.92".
I'd do a feature test instead of a version # test for autoconf stuff.

ie, compile a trivial program that puts an unused attribute on a label
then try to compile it.  If it fails, then it's not supported.

This means that folks that build with "cc" as their stage1 compiler or
older versions of egcs would still get the warnings, but I can live with
that.


  > Another option would be to simply bump the check to "GCC < 2.93" in
  > gansidecl.h (with or without an actual egcs minor version number bump.)
That would be fine with me too.  We could bump the version # to 2.93 if
we want.

jeff

Re: Warning policy?

[Jeffrey A Law]

  In message < 199901051514.KAA12156@caip.rutgers.edu >you write:
  > 	Urg.  Yes, if your stage1 compiler is a snapshot between 2.92.1
  > -> 2.92.33, it'll fail.  (Is this a common practice?  I would have
  > thought not, because if you had a problem, you wouldn't know whether the
  > stage1 compiler was at fault or the current snapshot was.  Obviously too
  > much ass-u-ME-ing :-) on my part though.)
  > 
  > 
  > What I need to do is test "GCC < 2.92.34".  Is there a way to check the
  > egcs micro version number?
Nuts.  I don't think we have the micro version #.

I'm starting to think an autoconf test is a better solution...

jeff

Re: Warning policy?

[Kaveh R. Ghazi]

 > From: Joe Buck <jbuck@Synopsys.COM>
 > 
 >  "Kaveh R. Ghazi" <ghazi@caip.rutgers.edu> writes:
 > > |> 	The "label `???' defined but not used" warnings mostly appear
 > > |> in insn-recog.c.  In this case genrecog.c needs to be fixed.  It may be
 > > |> non-trivial to deduce which labels are unnecessary, I haven't looked. 
 > > |> Another approach might be to get the compiler to allow attribute
 > > |> __unused__ on a label and just mark all of them unused in the generated
 > > |> insn-recog.c file.
 > 
 > Andreas Schwab writes: 
 > > Well, the brute force method would be to just emit a `goto L' before
 > > each label L.
 > 
 > That would only create more warnings, as some of these gotos would be
 > unreachable code, which produces a warning.


	Good point, but I'm not seeing this in actual practice from gcc. 
I wrote a small patch to attempt what Andreas suggested and gcc does not
complain about unreachable code.  Various stage1-cc's might complain,
e.g. SunOS4 cc does in fact, but we could arrange to only output the
goto when __GNUC__ is defined. 




 > The purpose in removing the warnings is to improve the code quality.
 > A -Wall-clean compile is nice, but if all the warnings are gone except
 > for warnings about unused labels in a generated file that would be enough,
 > in my opinion, for Kaveh to declare victory in his war on the warnings.
 > The idea is that we can say there should be no warnings except certain
 > specific warnings in specific files (like this one).



	I agree in principle, but ... consider the following patch.
It eliminates the 60-80 `unused label' warnings without adding any.
IMHO its relatively clean and painless, and it addresses the concern
you raised above.

		--Kaveh


--- egcs-CVS19981230/gcc/genrecog.c~	Wed Dec 30 09:58:54 1998
+++ egcs-CVS19981230/gcc/genrecog.c	Wed Dec 30 14:28:38 1998
@@ -51,6 +51,17 @@ Boston, MA 02111-1307, USA.  */
 #include "rtl.h"
 #include "obstack.h"
 
+/* We output a `goto' right before the actual label to ensure we don't
+   get `unused label' warnings.  Only do it if we are gcc since other
+   compilers might warn about `unreachable code'. */
+#ifdef __GNUC__
+#define OUTPUT_LABEL(INDENT_STRING, LABEL_NUMBER) \
+  printf("%sgoto L%d; L%d:\n", (INDENT_STRING), (LABEL_NUMBER), (LABEL_NUMBER))
+#else
+#define OUTPUT_LABEL(INDENT_STRING, LABEL_NUMBER) \
+  printf("%sL%d:\n", (INDENT_STRING), (LABEL_NUMBER))
+#endif /* __GNUC__ */
+
 static struct obstack obstack;
 struct obstack *rtl_obstack = &obstack;
 
@@ -1103,7 +1114,7 @@ write_tree_1 (tree, prevpos, afterward, 
   printf ("\n");
   if (tree && tree->subroutine_number == 0)
     {
-      printf ("  L%d:\n", tree->number);
+      OUTPUT_LABEL ("  ", tree->number);
       tree->label_needed = 0;
     }
 
@@ -1239,7 +1250,7 @@ write_tree_1 (tree, prevpos, afterward, 
 
       if (p->label_needed && (p->retest_mode || p->retest_code))
 	{
-	  printf ("%sL%d:\n", indents[indent - 2], p->number);
+	  OUTPUT_LABEL (indents[indent - 2], p->number);
 	  p->label_needed = 0;
 	}
 
@@ -1330,7 +1341,7 @@ write_tree_1 (tree, prevpos, afterward, 
       /* Now that most mode and code tests have been done, we can write out
 	 a label for an inner node, if we haven't already.  */
       if (p->label_needed)
-	printf ("%sL%d:\n", indents[indent - 2], p->number);
+	OUTPUT_LABEL (indents[indent - 2], p->number);
 
       inner_indent = indent;
 
@@ -1563,7 +1574,7 @@ write_tree (tree, prevpos, afterward, in
 
   if (! initial && tree->subroutine_number > 0)
     {
-      printf (" L%d:\n", tree->number);
+      OUTPUT_LABEL (" ", tree->number);
 
       if (afterward)
 	{

--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.

Re: Warning policy?

[Joe Buck]

>  > Andreas Schwab writes: 
>  > > Well, the brute force method would be to just emit a `goto L' before
>  > > each label L.
>  > 
>  > That would only create more warnings, as some of these gotos would be
>  > unreachable code, which produces a warning.
> 
> 
> 	Good point, but I'm not seeing this in actual practice from gcc. 
> I wrote a small patch to attempt what Andreas suggested and gcc does not
> complain about unreachable code.  Various stage1-cc's might complain,
> e.g. SunOS4 cc does in fact, but we could arrange to only output the
> goto when __GNUC__ is defined. 

Ah, but if gcc's ability to detect dead code improves, the warnings come
back.

Now, if there were an attribute that can be attached to a label that says
"this label may be unused, if so, don't warn about it" that would be
the simplest way to fix things.

Re: Warning policy?

[Kaveh R. Ghazi]

 > From: Andreas Schwab <schwab@issan.informatik.uni-dortmund.de>
 > 
 > "Kaveh R. Ghazi" <ghazi@caip.rutgers.edu> writes:
 > 
 > |> 	The "label `???' defined but not used" warnings mostly appear in
 > |> insn-recog.c.  In this case genrecog.c needs to be fixed.  It may be
 > |> non-trivial to deduce which labels are unnecessary, I haven't looked. 
 > |> Another approach might be to get the compiler to allow attribute
 > |> __unused__ on a label and just mark all of them unused in the generated
 > |> insn-recog.c file.
 > 
 > Well, the brute force method would be to just emit a `goto L' before
 > each label L.
 > -- 
 > Andreas Schwab                                      "And now for something


	Sounds like a good idea.

The comments in jump.c seem to indicate that the jump pass will optimize
away these gotos.  I.e. it says it "deletes jumps that jump to the
following insn" which I believe this case fits.  So we don't pay
a penalty for this hack.

	I'll submit a patch after the freeze is lifted.

		Thanks,
		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.

Re: Warning policy?

[Kaveh R. Ghazi]

 > From: Gerald Pfeifer <pfeifer@dbai.tuwien.ac.at>
 >  
 > On Sun, 20 Dec 1998, Chip Salzenberg wrote:
 > > I'm bootstrapping EGCS under Linux, and the EGCS sources are
 > > generating a lot of warnings because the configuration default is to
 > > use "gcc -Wall".
 >  
 > Indeed this is the default only for the CVS and snapshot versions, not
 > for releases, as not to confuse ``users''.
 >  
 > > Are the warnings fair game for patches, or is it a configuration error
 > > to use "gcc -Wall" when compiling EGCS?
 >  
 > It is intended to find (and fix) problems. I believe that Kaveh Ghazi
 > <ghazi@caip.rutgers.edu> has been continually working to reduce the 
 > number of warnings, so you might want to coordinate with him.
 >  
 > Gerald

	
	Yup, I'm working on it.  If you are interested, try using
the egcs/contrib/warn_summary script to categorize the warnings.
It has various flags to help you do this.

	The current breakdown looks like this.  (Your mileage may vary
depending on platform and BOOT_CFLAGS.)

Directory	Warnings
---------	--------
gcc topdir	362
Chill		12
C++		7
F77		2
Java		315

	It should be possible to get the java directory warnings down to
the level of the other language subdirs.  The folks doing development
have actually fixed many of these themselves over time.  I'm working on
zapping the last few hundred real soon now. 

	Those remaining in the gcc top level directory are either annoying
to fix, are caused by bugs in the compiler or are warnings about *possible*
problems which gcc can't say for sure.  Here is the approximate breakdown
of the warnings from the top level gcc directory:

Number of warning types:
 121 `???' might be used uninitialized in this function
  81 label `???' defined but not used
  64 missing initializer for `???'
  46 unused parameter `???'
  20 comparison between signed and unsigned
  14 unused variable `???'
  10 variable `???' might be clobbered by `longjmp' or `vfork'
   5 argument `???' might be clobbered by `longjmp' or `vfork'
   1 `???' defined but not used


	Most of the "uninitialized" and "might be clobbered" warnings
are either detection bugs or in the "gcc can't tell for sure" category
so it warns anyway to be safe (but annoying.) I think Jeff said the
buggy ones are due to problems in gcse.c meddling with the flow.  (?)
In the mean time, we are avoiding giving default initializations so that
if someone ever tries to fix this they have something to test against. 
I don't think its being worked on though.  There are some of these
that gcc is documented as being unable to detect properly which are
okay to zap by initialization.  But it requires meticulous code reading
to differentiate these.  Not fun. :-)

	The "label `???' defined but not used" warnings mostly appear in
insn-recog.c.  In this case genrecog.c needs to be fixed.  It may be
non-trivial to deduce which labels are unnecessary, I haven't looked. 
Another approach might be to get the compiler to allow attribute
__unused__ on a label and just mark all of them unused in the generated
insn-recog.c file.

	The "missing initializer" warnings come from toplev.c and are
due to the config/ dir files not containing description strings in the
TARGET_SWITCHES and/or TARGET_OPTIONS macros.  This one is easy to fix
but may take a while and has to be done individually for every target.

	I think the rest are ordinary and just need someone to apply a
fix.  Contributions on any of these (preferably the hard ones :-) )
are welcome.

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.

Re: Warning policy?

[Andreas Schwab]

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

|> 	The "label `???' defined but not used" warnings mostly appear in
|> insn-recog.c.  In this case genrecog.c needs to be fixed.  It may be
|> non-trivial to deduce which labels are unnecessary, I haven't looked. 
|> Another approach might be to get the compiler to allow attribute
|> __unused__ on a label and just mark all of them unused in the generated
|> insn-recog.c file.

Well, the brute force method would be to just emit a `goto L' before
each label L.

-- 
Andreas Schwab                                      "And now for something
schwab@issan.cs.uni-dortmund.de                      completely different"
schwab@gnu.org

Re: Warning policy?

[Joe Buck]

 "Kaveh R. Ghazi" <ghazi@caip.rutgers.edu> writes:
> |> 	The "label `???' defined but not used" warnings mostly appear in
> |> insn-recog.c.  In this case genrecog.c needs to be fixed.  It may be
> |> non-trivial to deduce which labels are unnecessary, I haven't looked. 
> |> Another approach might be to get the compiler to allow attribute
> |> __unused__ on a label and just mark all of them unused in the generated
> |> insn-recog.c file.

Andreas Schwab writes: 
> Well, the brute force method would be to just emit a `goto L' before
> each label L.

That would only create more warnings, as some of these gotos would be
unreachable code, which produces a warning.

However, if the gotos were only introduced in cases where the code
for the previous node falls through, it would probably be fine.
If there's a simple way to do that, then it might not be bad.
However:

The purpose in removing the warnings is to improve the code quality.
A -Wall-clean compile is nice, but if all the warnings are gone except
for warnings about unused labels in a generated file that would be enough,
in my opinion, for Kaveh to declare victory in his war on the warnings.
The idea is that we can say there should be no warnings except certain
specific warnings in specific files (like this one).

Warning policy?

[Chip Salzenberg]

I'm bootstrapping EGCS under Linux, and the EGCS sources are
generating a lot of warnings because the configuration default is to
use "gcc -Wall".  Are the warnings fair game for patches, or is it
a configuration error to use "gcc -Wall" when compiling EGCS?
-- 
Chip Salzenberg      - a.k.a. -      <chip@perlsupport.com>
      "When do you work?"   "Whenever I'm not busy."

Re: Warning policy?

[Gerald Pfeifer]

On Sun, 20 Dec 1998, Chip Salzenberg wrote:
> I'm bootstrapping EGCS under Linux, and the EGCS sources are
> generating a lot of warnings because the configuration default is to
> use "gcc -Wall".

Indeed this is the default only for the CVS and snapshot versions, not
for releases, as not to confuse ``users''.

> Are the warnings fair game for patches, or is it a configuration error
> to use "gcc -Wall" when compiling EGCS?

It is intended to find (and fix) problems. I believe that Kaveh Ghazi
<ghazi@caip.rutgers.edu> has been continually working to reduce the 
number of warnings, so you might want to coordinate with him.

Gerald
-- 
Gerald Pfeifer (Jerry)      Vienna University of Technology
pfeifer@dbai.tuwien.ac.at   http://www.dbai.tuwien.ac.at/~pfeifer/