simple fix for a few warning (0).

Manuel López-Ibáñez lopezibanez@gmail.com
Mon Jan 29 19:53:00 GMT 2007


:ADDPATCH C/C++:

Following suggestions from Paolo and Gabriel, I have updated the patch
by moving the warnings to its own function in c-common.c.

Bootstrapped and regression tested with --enable-languages=all on
i686-pc-linux-gnu.

Is this OK?


2007-01-29  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  * c-decl.c (pop_scope): Replace warnings with call to warn_for_unused_label.
  * c-common.h (warn_for_unused_label): Declare.
  * c-common.c (warn_for_unused_label): Define.

cp/
2007-01-29  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  * decl.c (pop_label): Replace warning with call to warn_for_unused_label.



On 28 Jan 2007 09:29:39 -0600, Gabriel Dos Reis <gdr@cs.tamu.edu> wrote:
> "Manuel López-Ibáñez" <lopezibanez@gmail.com> writes:
>
> | On 28/01/07, Paolo Bonzini <paolo.bonzini@lu.unisi.ch> wrote:
> | >
> | > >         else if (!TREE_USED (p) && warn_unused_label)
> | > >           {
> | > >             if (DECL_INITIAL (p))
> | > > -             warning (0, "label %q+D defined but not used", p);
> | > > +             warning (OPT_Wunused_label, "label %q+D defined but not used", p);
> | > >             else
> | > > -             warning (0, "label %q+D declared but not defined", p);
> | > > +             warning (OPT_Wunused_label, "label %q+D declared but not defined", p);
> | >
> | > You should also remove the "&& warn_unused_label" above.
> | >
> |
> | I think that is a bad practice in general. The advantages of keeping
> | the warn_* around are 1) it shows that any operation in the block is
> | done only for emitting the warning, not for anything essential and 2)
> | if -Wunused-label is disabled, it saves us from calling warning() at
> | all, which is certainly more expensive than checking
> | warn_unused_label.
>
> We have already discussed this in the pass.  We guard the call
> to warning() functions with warn_xxx only if the warning is
> going to require an expensive computation, otherwise it should
> be removed.  That leads to simpler to read and comprehend codes
> as opposed to clutr of nested if.  Please do consider the suggestion.
>
> My personal preference is to move such things into seperate small
> functions:
>
>    void
>    warn_for_unused_label (tree t)
>    {
>      if (!TREE_USE (p) && warn_unused_label)
>         ....
>    }
>
> | You should take into account that the goal of using warning (OPT_W*, )
> | instead of warning (0,) is to enable printing the option that
> | generated the warning and to be able to reclassificate the warning as
> | something else (warning, error, disable), through, for example, a
> | #pragma directive, even if warn_unused_label is true.
>
> You can assume that Paolo knows that; he has been around long enough.
>
> -- Gaby
>
-------------- next part --------------
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 121027)
+++ gcc/cp/decl.c	(working copy)
@@ -365,8 +365,8 @@ pop_label (tree label, tree old_value)
 	  /* Avoid crashing later.  */
 	  define_label (location, DECL_NAME (label));
 	}
-      else if (!TREE_USED (label))
-	warning (OPT_Wunused_label, "label %q+D defined but not used", label);
+      else 
+	warn_for_unused_label (label);
     }
 
   SET_IDENTIFIER_LABEL_VALUE (DECL_NAME (label), old_value);
Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 121027)
+++ gcc/c-decl.c	(working copy)
@@ -761,13 +761,9 @@ pop_scope (void)
 	      error ("label %q+D used but not defined", p);
 	      DECL_INITIAL (p) = error_mark_node;
 	    }
-	  else if (!TREE_USED (p) && warn_unused_label)
-	    {
-	      if (DECL_INITIAL (p))
-		warning (0, "label %q+D defined but not used", p);
-	      else
-		warning (0, "label %q+D declared but not defined", p);
-	    }
+	  else 
+	    warn_for_unused_label (p);
+
 	  /* Labels go in BLOCK_VARS.  */
 	  TREE_CHAIN (p) = BLOCK_VARS (block);
 	  BLOCK_VARS (block) = p;
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 121027)
+++ gcc/c-common.c	(working copy)
@@ -6758,5 +6758,16 @@ warn_about_parentheses (enum tree_code c
 	     "have their mathematical meaning");
 }
 
+void
+warn_for_unused_label (tree label)
+{
+  if (!TREE_USED (label))
+    {
+      if (DECL_INITIAL (label))
+	warning (OPT_Wunused_label, "label %q+D defined but not used", label);
+      else
+	warning (OPT_Wunused_label, "label %q+D declared but not defined", label);
+    }
+}
 
 #include "gt-c-common.h"
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 121027)
+++ gcc/c-common.h	(working copy)
@@ -866,6 +866,7 @@ extern tree builtin_type_for_size (int, 
 extern void warn_array_subscript_with_type_char (tree);
 extern void warn_about_parentheses (enum tree_code, enum tree_code,
 				    enum tree_code);
+extern void warn_for_unused_label (tree label);
 
 
 /* In c-gimplify.c  */


More information about the Gcc-patches mailing list