This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

c: Improve -Wshadow warnings


This patch + testcase improves -Wshadow to report the location of the
shadowed variable, like I did to C++ a month ago.  In fact, it shares
some of the code with C++.  In implementing this, I noticed that
-Wshadow duplicates warnings about parameters, and fixed that in the
process.  Am I the only one that thinks the way we handle function
parameters is a little bizarre?

Since the C front end contains an abundance of overly-long functions,
I took the opportunity to break some logic out of pushdecl.  The
makefiles do not need updating.

I'm happy to commit the C++ part as obvious, since it just moves a
function to c-common.c.

Bootstrapped x86 Linux without regressions.  OK for the C parts?

Neil.

	* c-common.c (shadow_warning): New function, moved from cp/decl.c.
	* c-common.h (shadow_warning): New.
	* c-decl.c: Include c-common.h.
	(warn_if_shadowing): New, broken out of pushdecl.
	(pushdecl): Use warn_if_shadowing.
	(store_parm_decls): Prevent duplicate -Wshadow warnings.
	* cp/decl.c: Include c-common.h.
	(shadow_warning): Move to c-common.c.

	* gcc.dg/Wshadow-1.c: New test.

============================================================
Index: gcc/c-common.c
--- gcc/c-common.c	2001/12/04 22:55:36	1.278
+++ gcc/c-common.c	2001/12/05 20:35:26
@@ -4017,3 +4017,17 @@ c_common_insert_default_attributes (decl
 #undef DEF_ATTR_TREE_LIST
 #undef DEF_FN_ATTR
 }
+
+/* Output a -Wshadow warning MSGID about NAME, an IDENTIFIER_NODE, and
+   additionally give the location of the previous declaration DECL.  */
+void
+shadow_warning (msgid, name, decl)
+     const char *msgid;
+     tree name, decl;
+{
+  warning ("declaration of `%s' shadows %s", IDENTIFIER_POINTER (name), msgid);
+  warning_with_file_and_line (DECL_SOURCE_FILE (decl),
+			      DECL_SOURCE_LINE (decl),
+			      "shadowed declaration is here");
+}
+
============================================================
Index: gcc/c-common.h
--- gcc/c-common.h	2001/12/04 22:55:36	1.103
+++ gcc/c-common.h	2001/12/05 20:35:28
@@ -330,6 +330,8 @@ extern tree walk_stmt_tree			PARAMS ((tr
 extern void prep_stmt                           PARAMS ((tree));
 extern void expand_stmt                         PARAMS ((tree));
 extern void mark_stmt_tree                      PARAMS ((void *));
+extern void shadow_warning			PARAMS ((const char *,
+							 tree, tree));
 
 /* Extra information associated with a DECL.  Other C dialects extend
    this structure in various ways.  The C front-end only uses this
============================================================
Index: gcc/c-decl.c
--- gcc/c-decl.c	2001/12/05 14:09:45	1.279
+++ gcc/c-decl.c	2001/12/05 20:35:46
@@ -45,6 +45,7 @@ Software Foundation, 59 Temple Place - S
 #include "target.h"
 #include "debug.h"
 #include "timevar.h"
+#include "c-common.h"
 
 /* In grokdeclarator, distinguish syntactic contexts of declarators.  */
 enum decl_context
@@ -280,6 +281,7 @@ static tree grokparms			PARAMS ((tree, i
 static void layout_array_type		PARAMS ((tree));
 static tree c_make_fname_decl           PARAMS ((tree, int));
 static void c_expand_body               PARAMS ((tree, int, int));
+static void warn_if_shadowing		PARAMS ((tree, tree));
 
 /* C-specific option variables.  */
 
@@ -2051,6 +2053,66 @@ duplicate_decls (newdecl, olddecl, diffe
   return 1;
 }
 
+/* Check whether decl-node X shadows an existing declaration.
+   OLDLOCAL is the old IDENTIFIER_LOCAL_VALUE of the DECL_NAME of X,
+   which might be a NULL_TREE.  */
+static void
+warn_if_shadowing (x, oldlocal)
+     tree x, oldlocal;
+{
+  tree name;
+
+  if (DECL_EXTERNAL (x))
+    return;
+
+  name = DECL_NAME (x);
+
+  /* Warn if shadowing an argument at the top level of the body.  */
+  if (oldlocal != 0
+      /* This warning doesn't apply to the parms of a nested fcn.  */
+      && ! current_binding_level->parm_flag
+      /* Check that this is one level down from the parms.  */
+      && current_binding_level->level_chain->parm_flag
+      /* Check that the decl being shadowed
+	 comes from the parm level, one level up.  */
+      && chain_member (oldlocal, current_binding_level->level_chain->names))
+    {
+      if (TREE_CODE (oldlocal) == PARM_DECL)
+	pedwarn ("declaration of `%s' shadows a parameter",
+		 IDENTIFIER_POINTER (name));
+      else
+	pedwarn ("declaration of `%s' shadows a symbol from the parameter list",
+		 IDENTIFIER_POINTER (name));
+    }
+  /* Maybe warn if shadowing something else.  */
+  else if (warn_shadow
+	   /* No shadow warnings for internally generated vars.  */
+	   && DECL_SOURCE_LINE (x) != 0
+	   /* No shadow warnings for vars made for inlining.  */
+	   && ! DECL_FROM_INLINE (x))
+    {
+      if (TREE_CODE (x) == PARM_DECL
+	  && current_binding_level->level_chain->parm_flag)
+	/* Don't warn about the parm names in function declarator
+	   within a function declarator.
+	   It would be nice to avoid warning in any function
+	   declarator in a declaration, as opposed to a definition,
+	   but there is no way to tell it's not a definition.  */
+	;
+      else if (oldlocal)
+	{
+	  if (TREE_CODE (oldlocal) == PARM_DECL)
+	    shadow_warning ("a parameter", name, oldlocal);
+	  else
+	    shadow_warning ("a previous local", name, oldlocal);
+	}
+      else if (IDENTIFIER_GLOBAL_VALUE (name) != 0
+	       && IDENTIFIER_GLOBAL_VALUE (name) != error_mark_node)
+	shadow_warning ("a global declaration", name,
+			IDENTIFIER_GLOBAL_VALUE (name));
+    }
+}
+
 /* Record a decl-node X as belonging to the current lexical scope.
    Check for errors (such as an incompatible declaration for the same
    name already seen in the same scope).
@@ -2434,50 +2496,8 @@ pushdecl (x)
 	      if (IDENTIFIER_LIMBO_VALUE (name) == 0)
 		IDENTIFIER_LIMBO_VALUE (name) = x;
 	    }
-
-	  /* Warn if shadowing an argument at the top level of the body.  */
-	  if (oldlocal != 0 && !DECL_EXTERNAL (x)
-	      /* This warning doesn't apply to the parms of a nested fcn.  */
-	      && ! current_binding_level->parm_flag
-	      /* Check that this is one level down from the parms.  */
-	      && current_binding_level->level_chain->parm_flag
-	      /* Check that the decl being shadowed
-		 comes from the parm level, one level up.  */
-	      && chain_member (oldlocal, current_binding_level->level_chain->names))
-	    {
-	      if (TREE_CODE (oldlocal) == PARM_DECL)
-		pedwarn ("declaration of `%s' shadows a parameter",
-			 IDENTIFIER_POINTER (name));
-	      else
-		pedwarn ("declaration of `%s' shadows a symbol from the parameter list",
-			 IDENTIFIER_POINTER (name));
-	    }
 
-	  /* Maybe warn if shadowing something else.  */
-	  else if (warn_shadow && !DECL_EXTERNAL (x)
-		   /* No shadow warnings for internally generated vars.  */
-		   && DECL_SOURCE_LINE (x) != 0
-		   /* No shadow warnings for vars made for inlining.  */
-		   && ! DECL_FROM_INLINE (x))
-	    {
-	      const char *id = IDENTIFIER_POINTER (name);
-
-	      if (TREE_CODE (x) == PARM_DECL
-		  && current_binding_level->level_chain->parm_flag)
-		/* Don't warn about the parm names in function declarator
-		   within a function declarator.
-		   It would be nice to avoid warning in any function
-		   declarator in a declaration, as opposed to a definition,
-		   but there is no way to tell it's not a definition.  */
-		;
-	      else if (oldlocal != 0 && TREE_CODE (oldlocal) == PARM_DECL)
-		warning ("declaration of `%s' shadows a parameter", id);
-	      else if (oldlocal != 0)
-		warning ("declaration of `%s' shadows previous local", id);
-	      else if (IDENTIFIER_GLOBAL_VALUE (name) != 0
-		       && IDENTIFIER_GLOBAL_VALUE (name) != error_mark_node)
-		warning ("declaration of `%s' shadows global declaration", id);
-	    }
+	  warn_if_shadowing (x, oldlocal);
 
 	  /* If storing a local value, there may already be one (inherited).
 	     If so, record it for restoration when this binding level ends.  */
@@ -6358,12 +6378,17 @@ store_parm_decls ()
      then CONST_DECLs for foo and bar are put here.  */
   tree nonparms = 0;
 
+  /* The function containing FNDECL, if any.  */
+  tree context = decl_function_context (fndecl);
+
   /* Nonzero if this definition is written with a prototype.  */
   int prototype = 0;
 
-  /* The function containing FNDECL, if any.  */
-  tree context = decl_function_context (fndecl);
+  int saved_warn_shadow = warn_shadow;
 
+  /* Don't re-emit shadow warnings.  */
+  warn_shadow = 0;
+
   if (specparms != 0 && TREE_CODE (specparms) != TREE_LIST)
     {
       /* This case is when the function was defined with an ANSI prototype.
@@ -6762,6 +6787,8 @@ store_parm_decls ()
      not safe to try to expand expressions involving them.  */
   immediate_size_expand = 0;
   cfun->x_dont_save_pending_sizes_p = 1;
+
+  warn_shadow = saved_warn_shadow;
 }
 
 /* Finish up a function declaration and compile that function
============================================================
Index: gcc/cp/decl.c
--- gcc/cp/decl.c	2001/12/03 23:15:09	1.832
+++ gcc/cp/decl.c	2001/12/05 20:36:19
@@ -45,6 +45,7 @@ Boston, MA 02111-1307, USA.  */
 #include "ggc.h"
 #include "tm_p.h"
 #include "target.h"
+#include "c-common.h"
 
 extern const struct attribute_spec *lang_attribute_table;
 
@@ -147,7 +148,6 @@ static tree push_cp_library_fn PARAMS ((
 static tree build_cp_library_fn PARAMS ((tree, enum tree_code, tree));
 static void store_parm_decls PARAMS ((tree));
 static int cp_missing_noreturn_ok_p PARAMS ((tree));
-static void shadow_warning PARAMS ((const char *, tree, tree));
 
 #if defined (DEBUG_CP_BINDING_LEVELS)
 static void indent PARAMS ((void));
@@ -3788,20 +3788,6 @@ duplicate_decls (newdecl, olddecl)
 
   return 1;
 }
-
-/* Output a -Wshadow warning MSGID, if non-NULL, and give the location
-   of the previous declaration.  */
-static void
-shadow_warning (msgid, name, decl)
-     const char *msgid;
-     tree name, decl;
-{
-  warning ("declaration of `%s' shadows %s", IDENTIFIER_POINTER (name), msgid);
-  warning_with_file_and_line (DECL_SOURCE_FILE (decl),
-			      DECL_SOURCE_LINE (decl),
-			      "shadowed declaration is here");
-}
-
 
 /* Record a decl-node X as belonging to the current lexical scope.
    Check for errors (such as an incompatible declaration for the same
============================================================
Index: gcc/testsuite/gcc.dg/Wshadow-1.c
--- /dev/null	Tue May  5 13:32:27 1998
+++ gcc/testsuite/gcc.dg/Wshadow-1.c	Wed Dec  5 12:36:19 2001
@@ -0,0 +1,32 @@
+/* Copyright (C) 2001 Free Software Foundation, Inc.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wshadow -pedantic-errors" } */
+
+/* Source: Neil Booth, 5 Dec 2001.  */
+
+int decl1;			/* { dg-warning "shadowed declaration" } */
+void foo (double decl1)		/* { dg-warning "shadows a global decl" } */
+{				
+}
+
+void foo1 (int d)
+{
+  double d;	 /* { dg-bogus "warning" "warning in place of error" } */
+  /* { dg-error "shadows a parameter" "" { target *-*-* } 15 } */
+}
+
+void foo2 (int d)		/* { dg-warning "shadowed declaration" } */
+{
+  {
+    double d;			/* { dg-warning "shadows a parameter" } */
+  }
+}
+
+void foo3 ()
+{
+  int local;			/* { dg-warning "shadowed declaration" } */
+  {
+    int local;			/* { dg-warning "shadows a previous local" } */
+  }
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]