c++: Improve C++'s -Wshadow; fix PRs 16 and 713

Neil Booth neil@daikokuya.demon.co.uk
Tue Nov 6 21:34:00 GMT 2001


Neil Booth wrote:-

> -Wshadow warns about shadowing class names and global function names,
> (the above two PRs, respectively) contrary to its documentation.

[...]

The patch I posted regressed with g++-mike/for3.C, which I didn't
originally notice.  A small tweak fixes this.

OK for the final patch below? (all my recent gcc-testresults posts
include this patch).

Neil.

	* cp/decl.c (shadow_warning): New function.
	(pushdecl): Improve -Wshadow warnings.  Don't give both a warning
	and an error when a block scope decl shadows a parameter.

	* g++.dg/warn/Wshadow-1.C: New tests.
	* g++.old-deja/g++.mike/for3.C: Update.

Index: cp/decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.822
diff -u -p -r1.822 decl.c
--- decl.c	2001/10/23 09:14:11	1.822
+++ decl.c	2001/11/07 05:32:03
@@ -147,6 +147,7 @@ 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,6 +3789,20 @@ 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
    name already seen in the same scope).
@@ -4173,47 +4188,53 @@ pushdecl (x)
 	  if (oldlocal != NULL_TREE && !DECL_EXTERNAL (x)
 	      /* Inline decls shadow nothing.  */
 	      && !DECL_FROM_INLINE (x)
-	      && TREE_CODE (oldlocal) == PARM_DECL
-	      /* Don't complain if it's from an enclosing function.  */
-	      && DECL_CONTEXT (oldlocal) == current_function_decl
-	      && TREE_CODE (x) != PARM_DECL)
+	      && TREE_CODE (oldlocal) == PARM_DECL)
 	    {
-	      /* Go to where the parms should be and see if we
-		 find them there.  */
-	      struct binding_level *b = current_binding_level->level_chain;
-
-	      if (cleanup_label)
-		b = b->level_chain;
-
-	      /* ARM $8.3 */
-	      if (b->parm_flag == 1)
-		cp_error ("declaration of `%#D' shadows a parameter", name);
+	      bool err = false;
+
+	      /* Don't complain if it's from an enclosing function.  */
+	      if (DECL_CONTEXT (oldlocal) == current_function_decl
+		  && TREE_CODE (x) != PARM_DECL)
+		{
+		  /* Go to where the parms should be and see if we find
+		     them there.  */
+		  struct binding_level *b = current_binding_level->level_chain;
+
+		  if (cleanup_label)
+		    b = b->level_chain;
+
+		  /* ARM $8.3 */
+		  if (b->parm_flag == 1)
+		    {
+		      cp_error ("declaration of `%#D' shadows a parameter",
+				name);
+		      err = true;
+		    }
+		}
+
+	      if (warn_shadow && !err)
+		shadow_warning ("a parameter", name, oldlocal);
 	    }
 
 	  /* Maybe warn if shadowing something else.  */
-	  if (warn_shadow && !DECL_EXTERNAL (x)
-	      /* Inline decls shadow nothing.  */
-	      && !DECL_FROM_INLINE (x)
+	  else if (warn_shadow && !DECL_EXTERNAL (x)
 	      /* No shadow warnings for internally generated vars.  */
 	      && ! DECL_ARTIFICIAL (x)
 	      /* No shadow warnings for vars made for inlining.  */
 	      && ! DECL_FROM_INLINE (x))
 	    {
-	      if (oldlocal != NULL_TREE && TREE_CODE (oldlocal) == PARM_DECL)
-		warning ("declaration of `%s' shadows a parameter",
-			IDENTIFIER_POINTER (name));
-	      else if (IDENTIFIER_CLASS_VALUE (name) != NULL_TREE
+	      if (IDENTIFIER_CLASS_VALUE (name) != NULL_TREE
 		       && current_class_ptr
 		       && !TREE_STATIC (name))
-		warning ("declaration of `%s' shadows a member of `this'",
-			IDENTIFIER_POINTER (name));
-	      else if (oldlocal != NULL_TREE)
-		warning ("declaration of `%s' shadows previous local",
-			IDENTIFIER_POINTER (name));
-	      else if (oldglobal != NULL_TREE)
+		cp_warning ("declaration of `%s' shadows a member of `this'",
+			    IDENTIFIER_POINTER (name));
+	      else if (oldlocal != NULL_TREE
+		       && TREE_CODE (oldlocal) == VAR_DECL)
+		shadow_warning ("a previous local", name, oldlocal);
+	      else if (oldglobal != NULL_TREE
+		       && TREE_CODE (oldglobal) == VAR_DECL)
 		/* XXX shadow warnings in outer-more namespaces */
-		warning ("declaration of `%s' shadows global declaration",
-			IDENTIFIER_POINTER (name));
+		shadow_warning ("a global declaration", name, oldglobal);
 	    }
 	}
 
Index: testsuite/g++.dg/warn/Wshadow-1.C
===================================================================
RCS file: Wshadow-1.C
diff -N Wshadow-1.C
--- /dev/null	Tue May  5 13:32:27 1998
+++ Wshadow-1.C	Tue Nov  6 21:32:03 2001
@@ -0,0 +1,42 @@
+/* Copyright (C) 2001 Free Software Foundation, Inc.  */
+
+/* { dg-do compile } */
+/* { dg-options -Wshadow } */
+
+/* Source: Neil Booth, 3 Nov 2001, and PR 16, 713.  -Wshadow was
+   giving a bunch of warnings we didn't want, and wasn't giving the
+   location of the shadowed variable.  */
+
+struct status			// { dg-bogus "shadowed declaration" }
+{
+  int member;
+  void foo2 ();
+
+  inline static int foo3 (int member) // { dg-bogus "shadows" }
+  {
+    return member;
+  }
+};
+
+int decl1;			// { dg-warning "shadowed declaration" }
+int decl2;			// { dg-warning "shadowed declaration" }
+void foo (struct status &status,// { dg-bogus "shadows a global decl" }
+	  double decl1)
+{				// { dg-warning "shadows a global decl" }
+}
+
+void foo1 (int d)
+{
+  double d;			// { dg-error "shadows a parameter" }
+}
+
+// { dg-error "In member function" "ignored" { target *-*-* } 0 }
+void status::foo2 ()
+{
+  int member;			// { dg-warning "shadows a member" }
+  int decl2;			// { dg-warning "shadows a global decl" }
+  int local;			// { dg-warning "shadowed declaration" }
+  {
+    int local;			// { dg-warning "shadows a previous local" }
+  }
+}
Index: testsuite/g++.old-deja/g++.mike/for3.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.mike/for3.C,v
retrieving revision 1.1
diff -u -p -r1.1 for3.C
--- for3.C	2000/01/27 23:19:47	1.1
+++ for3.C	2001/11/07 05:32:03
@@ -1,9 +1,9 @@
 // Special g++ Options: -Wshadow
 
 int
-main(int i) {
-  for(int i=1; i < 3; i++);	// WARNING - shadows parm
-  for(int i=1; i < 3; i++);	// WARNING - shadows parm
+main(int i) {			// WARNING - shadowed decl
+  for(int i=1; i < 3; i++);	// WARNING - declaration of
+  for(int i=1; i < 3; i++);	// WARNING - declaration of
   for(int j=1; j < 3; j++);
   for(int j=1; j < 3; j++);
 }



More information about the Gcc-patches mailing list