diff mbox

[4.6] detect C++ errors to fix 2288 and 18770

Message ID 20110525192206.GA29722@nightcrawler
State New
Headers show

Commit Message

Nathan Froyd May 25, 2011, 7:22 p.m. UTC
On Sun, May 22, 2011 at 03:25:41PM -0700, H.J. Lu wrote:
> FWIW, I tried Janis's patch on 4.6 branch and I got
> 
> /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C: In
> function 'void e1()':^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:29:11:
> error: redeclaration of 'int k'^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:27:12:
> error: 'int k' previously declared here^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C: In
> function 'void e4()':^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:63:11:
> error: redeclaration of 'int i'^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:61:14:
> error: 'int i' previously declared here^M
> 
> FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 14)
> FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 17)
> PASS: g++.dg/parse/pr18770.C prev (test for errors, line 27)
> PASS: g++.dg/parse/pr18770.C redecl (test for errors, line 29)
> FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 37)
> FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 39)
> FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 47)
> FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 53)
> PASS: g++.dg/parse/pr18770.C prev (test for errors, line 61)
> PASS: g++.dg/parse/pr18770.C redecl (test for errors, line 63)
> FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 71)
> FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 73)
> PASS: g++.dg/parse/pr18770.C (test for excess errors)
> 
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:
> In function 'int main()':^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:22:11:
> error: redeclaration of 'int i'^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:20:14:
> error: 'int i' previously declared here^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:27:11:
> error: redeclaration of 'int i'^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:25:14:
> error: 'int i' previously declared here^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:36:16:
> error: types may not be defined in conditions^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:39:3:
> error: 'A' was not declared in this scope^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:39:5:
> error: expected ';' before 'bar'^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:42:12:
> error: types may not be defined in conditions^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:42:40:
> error: 'one' was not declared in this scope^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:51:14:
> warning: declaration of 'int f()' has 'extern' and is initialized
> [enabled by default]^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:51:18:
> error: function 'int f()' is initialized like a variable^M
> /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:55:23:
> error: extended initializer lists only available with -std=c++0x or
> -std=gnu++0x^M
> 
> FAIL: g++.old-deja/g++.jason/cond.C  (test for errors, line 9)
> FAIL: g++.old-deja/g++.jason/cond.C  (test for errors, line 11)
> FAIL: g++.old-deja/g++.jason/cond.C  (test for errors, line 16)
> PASS: g++.old-deja/g++.jason/cond.C  (test for errors, line 20)
> PASS: g++.old-deja/g++.jason/cond.C  (test for errors, line 22)
> PASS: g++.old-deja/g++.jason/cond.C  (test for errors, line 25)
> PASS: g++.old-deja/g++.jason/cond.C  (test for errors, line 27)
> FAIL: g++.old-deja/g++.jason/cond.C  (test for errors, line 30)
> FAIL: g++.old-deja/g++.jason/cond.C  (test for errors, line 33)
> PASS: g++.old-deja/g++.jason/cond.C  (test for errors, line 36)
> PASS: g++.old-deja/g++.jason/cond.C decl (test for errors, line 39)
> PASS: g++.old-deja/g++.jason/cond.C exp (test for errors, line 39)
> PASS: g++.old-deja/g++.jason/cond.C def (test for errors, line 42)
> PASS: g++.old-deja/g++.jason/cond.C expected (test for errors, line 42)
> PASS: g++.old-deja/g++.jason/cond.C extern (test for warnings, line 51)
> 
> The patch no longer catches all problems.

The patch just requires some shuffling of logic to catch issues now;
below is a version that works for me on the trunk.

This new checking does require modifying g++.dg/cpp0x/range-for5.C.  The
new logic of the patch claims that:

  int i;
  for (int i : a)
  {
    int i;
  }

is incorrect (the innermost `i' is an erroneous redeclaration).  If you
apply the expansion of range-based for loops from [stmt.ranged]p1, you'd
get something like:

  for (...; ...; ...)
   {
     int i = ...;
     int i;
   }

which is bad.  I believe [basic.scope.local]p4 says much the same thing.

Tested with g++ testsuite on x86_64-unknown-linux-gnu; tests in progress
for libstdc++.  OK to commit?

-Nathan

gcc/cp/
2011-xx-xx  Janis Johnson  <janisjo@codesourcery.com>
	    Nathan Froyd  <froydnj@codesourcery.com>

	PR c++/2288
	PR c++/18770
	* name-lookup.h (enum scope_kind): Add sk_cond.
	* name-lookup.c (pushdecl_maybe_friend): Get scope of shadowed local.
	Detect and report error for redeclaration from for-init or if
	or switch condition.
	(begin_scope): Handle sk_cond.
	* semantics.c (begin_if_stmt): Use sk_cond.
	(begin switch_stmt): Ditto.

gcc/testsuite/
2011-xx-xx  Janis Johnson  <janisjo@codesourcery.com>
	    Nathan Froyd  <froydnj@codesourcery.com>

	PR c++/2288
	PR c++/18770
	* g++.old-deja/g++.jason/cond.C: Remove xfails.
	* g++.dg/parse/pr18770.C: New test.
	* g++.dg/cpp0x/range-for5.C: Add dg-error marker.
diff mbox

Patch

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index bb6d4b9..723f36f 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -957,8 +957,15 @@  pushdecl_maybe_friend_1 (tree x, bool is_friend)
       else
 	{
 	  /* Here to install a non-global value.  */
-	  tree oldlocal = innermost_non_namespace_value (name);
 	  tree oldglobal = IDENTIFIER_NAMESPACE_VALUE (name);
+	  tree oldlocal = NULL_TREE;
+	  cxx_scope *oldscope = NULL;
+	  cxx_binding *oldbinding = outer_binding (name, NULL, true);
+	  if (oldbinding)
+	    {
+	      oldlocal = oldbinding->value;
+	      oldscope = oldbinding->scope;
+	    }
 
 	  if (need_new_binding)
 	    {
@@ -1087,6 +1094,20 @@  pushdecl_maybe_friend_1 (tree x, bool is_friend)
 		       }
 		   }
 		}
+	      /* Error if redeclaring a local declared in a
+		 for-init-statement or in the condition of an if or
+		 switch statement when the new declaration is in the
+		 outermost block of the controlled statement.
+		 Redeclaring a variable from a for or while condition is
+		 detected elsewhere.  */
+	      else if (TREE_CODE (oldlocal) == VAR_DECL
+		       && oldscope == current_binding_level->level_chain
+		       && (oldscope->kind == sk_cond
+			   || oldscope->kind == sk_for))
+		{
+		  error ("redeclaration of %q#D", x);
+		  error ("%q+#D previously declared here", oldlocal);
+		}
 
 	      if (warn_shadow && !nowarn)
 		{
@@ -1446,6 +1467,7 @@  begin_scope (scope_kind kind, tree entity)
     case sk_try:
     case sk_catch:
     case sk_for:
+    case sk_cond:
     case sk_class:
     case sk_scoped_enum:
     case sk_function_parms:
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 4bf253f..90b56e4 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -109,6 +109,8 @@  typedef enum scope_kind {
   sk_catch,	     /* A catch-block.  */
   sk_for,	     /* The scope of the variable declared in a
 			for-init-statement.  */
+  sk_cond,	     /* The scope of the variable declared in the condition
+			of an if or switch statement.  */
   sk_function_parms, /* The scope containing function parameters.  */
   sk_class,	     /* The scope containing the members of a class.  */
   sk_scoped_enum,    /* The scope containing the enumertors of a C++0x
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 55ad117..7833d76 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -656,7 +656,7 @@  tree
 begin_if_stmt (void)
 {
   tree r, scope;
-  scope = do_pushlevel (sk_block);
+  scope = do_pushlevel (sk_cond);
   r = build_stmt (input_location, IF_STMT, NULL_TREE,
 		  NULL_TREE, NULL_TREE, scope);
   begin_cond (&IF_COND (r));
@@ -1013,7 +1013,7 @@  begin_switch_stmt (void)
 {
   tree r, scope;
 
-  scope = do_pushlevel (sk_block);
+  scope = do_pushlevel (sk_cond);
   r = build_stmt (input_location, SWITCH_STMT, NULL_TREE, NULL_TREE, NULL_TREE, scope);
 
   begin_cond (&SWITCH_STMT_COND (r));
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for5.C b/gcc/testsuite/g++.dg/cpp0x/range-for5.C
index 9c97ad5..2fe4702 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for5.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for5.C
@@ -49,6 +49,6 @@  void test1()
   int i;
   for (int i : a)
   {
-    int i;
+    int i;			// { dg-error "redeclaration" }
   }
 }
diff --git a/gcc/testsuite/g++.dg/parse/pr18770.C b/gcc/testsuite/g++.dg/parse/pr18770.C
new file mode 100644
index 0000000..df57be4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/pr18770.C
@@ -0,0 +1,175 @@ 
+/* { dg-do compile } */
+
+/* The ISO C++ standard says, in Section 3.3.2 sentence 4, that a name
+   declared in the for-init-statement or in the condition of an if, for
+   while, or switch statement can't be redeclared in the outermost block
+   of the controlled statement.  (Note, this is not an error in C.)  */
+
+extern void foo (int);
+extern int j;
+
+void
+e0 (void)
+{
+  for (int i = 0;	// { dg-error "previously declared here" "prev" }
+       i < 10; ++i)
+    {
+      int i = 2;	// { dg-error "redeclaration" "redecl" }
+      foo (i);
+    }
+}
+
+void
+e1 (void)
+{
+  int i;
+  for (i = 0;
+       int k = j; i++)	// { dg-error "previously declared here" "prev" }
+    {
+      int k = 2;	// { dg-error "redeclaration" "redecl" }
+      foo (k);
+    }
+}
+
+void
+e2 (void)
+{
+  if (int i = 1)	// { dg-error "previously declared here" "prev" }
+    {
+      int i = 2;	// { dg-error "redeclaration" "redecl" }
+      foo (i);
+    }
+}
+
+void
+e3 (void)
+{
+  if (int i = 1)	// { dg-error "previously declared here" "prev" }
+    {
+      foo (i);
+    }
+  else
+    {
+      int i = 2;	// { dg-error "redeclaration" "redecl" }
+      foo (i);
+    }
+}
+
+void
+e4 (void)
+{
+  while (int i = 1)	// { dg-error "previously declared here" "prev" }
+    {
+      int i = 2;	// { dg-error "redeclaration" "redecl" }
+      foo (i);
+    }
+}
+
+void
+e5 (void)
+{
+  switch (int i = j)	// { dg-error "previously declared here" "prev" }
+    {
+    int i;		// { dg-error "redeclaration" "redecl" }
+    default:
+      {
+        i = 2;
+        foo (i);
+      }
+    }
+}
+
+void
+f0 (void)
+{
+  for (int i = 0; i < 10; ++i)
+    {
+      foo (i);
+      {
+        int i = 2;	// OK, not outermost block.
+        foo (i);
+      }
+    }
+}
+
+void
+f1 (void)
+{
+  int i;
+  for (i = 0; int k = j; i++)
+    {
+      foo (k);
+      {
+	int k = 2;	// OK, not outermost block.
+	foo (k);
+      }
+    }
+}
+
+void
+f2 (void)
+{
+  if (int i = 1)
+    {
+      foo (i);
+      {
+	int i = 2;	// OK, not outermost block.
+	foo (i);
+      }
+    }
+}
+
+void
+f3 (void)
+{
+  if (int i = 1)
+    {
+      foo (i);
+    }
+  else
+    {
+      foo (i+2);
+      {
+	int i = 2;	// OK, not outermost block.
+	foo (i);
+      }
+    }
+}
+
+void
+f4 (void)
+{
+  while (int i = 1)
+    {
+      foo (i);
+      {
+	int i = 2;	// OK, not outermost block.
+	foo (i);
+      }
+    }
+}
+
+void
+f5 (void)
+{
+  switch (int i = j)
+    {
+    default:
+      {
+        int i = 2;	// OK, not outermost block.
+        foo (i);
+      }
+    }
+}
+
+void
+f6 (void)
+{
+  int i = 1;
+
+  for (int j = 0; j < 10; j++)
+    {
+      int i = 2;	// OK, not variable from for-init.
+      foo (i);
+    }
+}
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/cond.C b/gcc/testsuite/g++.old-deja/g++.jason/cond.C
index d0616e4..a6e5ba0 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/cond.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/cond.C
@@ -6,14 +6,14 @@  int main()
 {
   float i;
   
-  if (int i = 1)		// { dg-error "" "" { xfail *-*-* } } , 
+  if (int i = 1)		// { dg-error "previously" }
     {
-      char i;			// { dg-error "" "" { xfail *-*-* } } , 
+      char i;			// { dg-error "redeclaration" } 
       char j;
     }
   else
     {
-      short i;			// { dg-error "" "" { xfail *-*-* } } , 
+      short i;			// { dg-error "redeclaration" }
       char j;
     }
 
@@ -27,10 +27,10 @@  int main()
       int i;			// { dg-error "redeclaration" }
     }
 
-  switch (int i = 0)		// { dg-error "" "" { xfail *-*-* } } 
+  switch (int i = 0)		// { dg-error "previously" }
     {
     default:
-      int i;			// { dg-error "" "" { xfail *-*-* } } 
+      int i;			// { dg-error "redeclaration" } 
     }
 
   if (struct A { operator int () { return 1; } } *foo = new A) // { dg-error "defined" }