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

[Jack Howarth <howarth at bromo dot med dot uc dot edu>]

On Thu, Mar 18, 2010 at 01:50:54PM -0700, Janis Johnson wrote:
> 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 (this is not an error in C).  This patch
> detects those errors that were not already detected elsewhere, adds
> a new test, and removes xfails from an existing test.
> 
> Tested on powerpc64-linux with bootstrap of all languages but Ada and
> regtest for -m32/-m64.  OK for stage 1 of GCC 4.6?
> 
> 2010-03-18  Janis Johnson  <janis187@us.ibm.com>
> 
> gcc/cp
> 	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/
> 	PR c++/2288
> 	PR c++/18770
> 	* g++.old-deja/g++.jason/cond.C: Remove xfails.
> 	* g++.dg/parse/pr18770.C: New test.
> 
> Index: gcc/cp/name-lookup.h
> ===================================================================
> --- gcc/cp/name-lookup.h	(revision 157490)
> +++ gcc/cp/name-lookup.h	(working copy)
> @@ -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
> Index: gcc/cp/name-lookup.c
> ===================================================================
> --- gcc/cp/name-lookup.c	(revision 157490)
> +++ gcc/cp/name-lookup.c	(working copy)
> @@ -946,8 +946,15 @@ pushdecl_maybe_friend (tree x, bool is_f
>        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)
>  	    {
> @@ -1051,6 +1058,21 @@ pushdecl_maybe_friend (tree x, bool is_f
>  		}
>  	    }
>  
> +	  /* 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 (oldlocal != NULL_TREE
> +		   && 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);
> +	    }
> +
>  	  /* Maybe warn if shadowing something else.  */
>  	  else if (warn_shadow && !DECL_EXTERNAL (x)
>  	      /* No shadow warnings for internally generated vars.  */
> @@ -1383,6 +1405,7 @@ begin_scope (scope_kind kind, tree entit
>      case sk_try:
>      case sk_catch:
>      case sk_for:
> +    case sk_cond:
>      case sk_class:
>      case sk_scoped_enum:
>      case sk_function_parms:
> Index: gcc/cp/semantics.c
> ===================================================================
> --- gcc/cp/semantics.c	(revision 157490)
> +++ gcc/cp/semantics.c	(working copy)
> @@ -644,7 +644,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);
>    TREE_CHAIN (r) = scope;
>    begin_cond (&IF_COND (r));
> @@ -929,7 +929,7 @@ begin_switch_stmt (void)
>  
>    r = build_stmt (input_location, SWITCH_STMT, NULL_TREE, NULL_TREE, NULL_TREE);
>  
> -  scope = do_pushlevel (sk_block);
> +  scope = do_pushlevel (sk_cond);
>    TREE_CHAIN (r) = scope;
>    begin_cond (&SWITCH_STMT_COND (r));
>  
> Index: gcc/testsuite/g++.old-deja/g++.jason/cond.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.jason/cond.C	(revision 157490)
> +++ gcc/testsuite/g++.old-deja/g++.jason/cond.C	(working copy)
> @@ -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" } 
> Index: gcc/testsuite/g++.dg/parse/pr18770.C
> ===================================================================
> --- gcc/testsuite/g++.dg/parse/pr18770.C	(revision 0)
> +++ gcc/testsuite/g++.dg/parse/pr18770.C	(revision 0)
> @@ -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);
> +    }
> +}
> 

  Any chance we can get this committed to gcc trunk while in 4.7 stage 1
(since it was already approved for 4.6 stage 1 and never applied...
http://gcc.gnu.org/ml/gcc-patches/2010-05/msg00264.html). It would be nice
to have in gcc 4.7 so that other open source projects that only build against
g++ would be alerted to fix their sources.
            Jack