[PATCH] Fix PR c++/29727: ICE on invalid initializer for template member

[PATCH] Fix PR c++/29727: ICE on invalid initializer for template member

[Simon Martin]

[-- Attachment #1: Type: text/plain, Size: 912 bytes --]

Hi all

The following invalid snippet triggers an ICE since 3.3

=== cut here ===
template<int> struct A
{
     static int a[1];
};

template<int N> int A<N>::a[1] = { X: 0 };

void foo()
{
     A<0>::a;
}
=== cut here ===

The problem is that 'check_array_designated_initializer', when emitting
an error in the presence of a named initializer, asserts the given name
"has" TREE_IDENTIFIER (since the fix for PR29728), which is not the case
here since it's error_mark_node; hence the ICE. The attached patch fixes
this by letting error_mark_node go through.

I've successfully regtested this on x86_64-apple-darwin-9. Is it
OK for 4.3 and for the mainline?

Thanks in advance,
Simon

:ADDPATCH c++:

PS: In the PR, it's stated that 4.1.2 does not have this problem. It's
due to the fact that the fix for PR29728 that introduced the assert, was
not the same in 4.1 as in the other branches (if vs. gcc_assert).



[-- Attachment #2: CL_29727 --]
[-- Type: text/plain, Size: 148 bytes --]

2008-10-05  Simon Martin  <simartin@users.sourceforge.net>

	PR c++/29727
	* decl.c (check_array_designated_initializer): Handle error_mark_node.



[-- Attachment #3: pr29727.patch --]
[-- Type: text/plain, Size: 598 bytes --]

Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 140866)
+++ gcc/cp/decl.c	(working copy)
@@ -4391,7 +4391,8 @@ check_array_designated_initializer (cons
     {
       /* The parser only allows identifiers as designated
 	 initializers.  */
-      gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
+      gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE
+		  || (ce->index == error_mark_node));
       error ("name %qD used in a GNU-style designated "
 	     "initializer for an array", ce->index);
       return false;



[-- Attachment #4: CL_29727_testsuite --]
[-- Type: text/plain, Size: 111 bytes --]

2008-10-05  Simon Martin  <simartin@users.sourceforge.net>

	PR c++/29727
	* g++.dg/init/error2.C: New test.



[-- Attachment #5: error2.C --]
[-- Type: text/plain, Size: 264 bytes --]

/* PR c++/29727 */
/* { dg-do "compile" } */

template<int> struct A
{
  static int a[1];
};
template<int N> int A<N>::a[1] = { X:0 }; /* { dg-error "does not allow designated|was not declared|designated initializer for an array" } */

void foo()
{
  A<0>::a;
}

Re: [PATCH] Fix PR c++/29727: ICE on invalid initializer for template member

[Paolo Bonzini]

> -      gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
> +      gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE
> +		  || (ce->index == error_mark_node));
>        error ("name %qD used in a GNU-style designated "
>  	     "initializer for an array", ce->index);

Hi Simon, this should be

  if (ce->index != error_mark_node)
    {
      gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
      error ("...");
    }
  return false;

otherwise you get a bogus error when ce->index is passed as a %qD.

Thanks!

Paolo

Re: [PATCH] Fix PR c++/29727: ICE on invalid initializer for template member

[Simon Martin]

Hello Paolo,

> Hi Simon, this should be
> 
>   if (ce->index != error_mark_node)
>     {
>       gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
>       error ("...");
>     }
>   return false;
> 
> otherwise you get a bogus error when ce->index is passed as a %qD.
Well, the %qD gives "<declaration error>", which seemed OK to me. With 
what you suggest, we'll not give any error about the fact that a named 
initializer is not valid here.

So if the "<declaration error>" really is bad, I could submit another 
patch giving the current error message when we have an IDENTIFIER_NODE, 
and another one ("ce independent") when we have an error_mark_node.

Best regards,
Simon

Re: [PATCH] Fix PR c++/29727: ICE on invalid initializer for template member

[Paolo Bonzini]

> Well, the %qD gives "<declaration error>", which seemed OK to me. With
> what you suggest, we'll not give any error about the fact that a named
> initializer is not valid here.

It's your call, it depends on how the errors before this one look like,
and whether this one would be automatically fixed after correcting the
errors before.

> So if the "<declaration error>" really is bad, I could submit another
> patch giving the current error message when we have an IDENTIFIER_NODE,
> and another one ("ce independent") when we have an error_mark_node.

Yes, that's a possibility.

Paolo

Re: [PATCH] Fix PR c++/29727: ICE on invalid initializer for template member

[Simon Martin]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

Hello,

Paolo Bonzini wrote:
> 
>> Well, the %qD gives "<declaration error>", which seemed OK to me. With
>> what you suggest, we'll not give any error about the fact that a named
>> initializer is not valid here.
> 
> It's your call, it depends on how the errors before this one look like,
> and whether this one would be automatically fixed after correcting the
> errors before.
I don't have a pre-3.3 compiler around, so I cannot tell for sure, but
looking at the code, I think that we would not report the error. I think
it's not perfect because even if the user fixes the first error (X being
unknown; could be a typo for a member if we were working on an array of
structs instead of an array of ints), the code will still be erroneous.

>> So if the "<declaration error>" really is bad, I could submit another
>> patch giving the current error message when we have an IDENTIFIER_NODE,
>> and another one ("ce independent") when we have an error_mark_node.
> 
> Yes, that's a possibility.
The attached patch does this; the test case and Changelogs are unchanged.

I've successfully tested it on x86_64-apple-darwin-9. Is it better?
OK for 4.3 and for the mainline?

Thanks,
Simon





[-- Attachment #2: pr29727_2.patch --]
[-- Type: text/plain, Size: 813 bytes --]

Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 140923)
+++ gcc/cp/decl.c	(working copy)
@@ -4391,9 +4391,15 @@ check_array_designated_initializer (cons
     {
       /* The parser only allows identifiers as designated
 	 initializers.  */
-      gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
-      error ("name %qD used in a GNU-style designated "
-	     "initializer for an array", ce->index);
+      if (ce->index == error_mark_node)
+	error ("name used in a GNU-style designated "
+	       "initializer for an array");
+      else
+	{
+	  gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
+	  error ("name %qD used in a GNU-style designated "
+		 "initializer for an array", ce->index);
+	}
       return false;
     }