[C++ Patch, V2] PR 82593 ("Internal compiler error: in process_init_constructor_array, at cp/typeck2.c:1294")

[C++ Patch, V2] PR 82593 ("Internal compiler error: in process_init_constructor_array, at cp/typeck2.c:1294")

[Paolo Carlini]

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

Hi again,

I managed to spend much more time on the issue and I'm starting a new 
thread with a mature - IMHO - proposal: the big thing is the use of the 
existing check_array_designated_initializer in 
process_init_constructor_array,  which calls maybe_constant_value, as we 
want, and covers all the ill-formed cases which I can imagine. I'm also 
tweaking a bit the parser to check the return value of 
require_potential_rvalue_constant_expression in order to avoid redundant 
diagnostic in some cases. Also, a couple more testcases beyond the bug 
report.

Tested x86_64-linux.

Thanks, Paolo.

////////////////////////


[-- Attachment #2: CL_82593_2 --]
[-- Type: text/plain, Size: 556 bytes --]

/cp
2017-11-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/82593
	* decl.c (check_array_designated_initializer): Not static.
	* cp-tree.h (check_array_designated_initializer): Declare.
	* typeck2.c (process_init_constructor_array): Call the latter.
	* parser.c (cp_parser_initializer_list): Check the return value
	of require_potential_rvalue_constant_expression.

/testsuite
2017-11-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/82593
	* g++.dg/cpp0x/desig2.C: New.
	* g++.dg/cpp0x/desig3.C: Likewise.
	* g++.dg/cpp0x/desig4.C: Likewise.

[-- Attachment #3: patch_82593_2 --]
[-- Type: text/plain, Size: 4069 bytes --]

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 254858)
+++ cp/cp-tree.h	(working copy)
@@ -6190,6 +6190,8 @@ extern bool require_deduced_type		(tree, tsubst_fl
 
 extern tree finish_case_label			(location_t, tree, tree);
 extern tree cxx_maybe_build_cleanup		(tree, tsubst_flags_t);
+extern bool check_array_designated_initializer  (constructor_elt *,
+						 unsigned HOST_WIDE_INT);
 
 /* in decl2.c */
 extern void record_mangling			(tree, bool);
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 254858)
+++ cp/decl.c	(working copy)
@@ -5245,7 +5245,7 @@ grok_reference_init (tree decl, tree type, tree in
    initializer.  If it does, an error is issued.  Returns true if CE
    is valid, i.e., does not have a designated initializer.  */
 
-static bool
+bool
 check_array_designated_initializer (constructor_elt *ce,
 				    unsigned HOST_WIDE_INT index)
 {
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 254858)
+++ cp/parser.c	(working copy)
@@ -22193,8 +22193,10 @@ cp_parser_initializer_list (cp_parser* parser, boo
 
 	  if (!cp_parser_parse_definitely (parser))
 	    designator = NULL_TREE;
-	  else if (non_const)
-	    require_potential_rvalue_constant_expression (designator);
+	  else if (non_const
+		   && (!require_potential_rvalue_constant_expression
+		       (designator)))
+	    designator = NULL_TREE; 
 	}
       else
 	designator = NULL_TREE;
Index: cp/typeck2.c
===================================================================
--- cp/typeck2.c	(revision 254858)
+++ cp/typeck2.c	(working copy)
@@ -1289,17 +1289,8 @@ process_init_constructor_array (tree type, tree in
 
   FOR_EACH_VEC_SAFE_ELT (v, i, ce)
     {
-      if (ce->index)
-	{
-	  gcc_assert (TREE_CODE (ce->index) == INTEGER_CST);
-	  if (compare_tree_int (ce->index, i) != 0)
-	    {
-	      ce->value = error_mark_node;
-	      sorry ("non-trivial designated initializers not supported");
-	    }
-	}
-      else
-	ce->index = size_int (i);
+      ce->index = (check_array_designated_initializer (ce, i)
+		   ? size_int (i) : error_mark_node);
       gcc_assert (ce->value);
       ce->value = massage_init_elt (TREE_TYPE (type), ce->value, complain);
 
Index: testsuite/g++.dg/cpp0x/desig2.C
===================================================================
--- testsuite/g++.dg/cpp0x/desig2.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/desig2.C	(working copy)
@@ -0,0 +1,23 @@
+// PR c++/82593
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+enum {
+ INDEX1 = 0,
+ INDEX2
+};
+
+class SomeClass {
+public:
+ SomeClass();
+private:
+ struct { int field; } member[2];
+};
+
+SomeClass::SomeClass()
+ : member({
+   [INDEX1] = { .field = 0 },
+   [INDEX2] = { .field = 1 }
+ })
+{
+}
Index: testsuite/g++.dg/cpp0x/desig3.C
===================================================================
--- testsuite/g++.dg/cpp0x/desig3.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/desig3.C	(working copy)
@@ -0,0 +1,21 @@
+// PR c++/82593
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+const int INDEX1 = 0;
+const int INDEX2 = 1;
+
+class SomeClass {
+public:
+ SomeClass();
+private:
+ struct { int field; } member[2];
+};
+
+SomeClass::SomeClass()
+ : member({
+   [INDEX1] = { .field = 0 },
+   [INDEX2] = { .field = 1 }
+ })
+{
+}
Index: testsuite/g++.dg/cpp0x/desig4.C
===================================================================
--- testsuite/g++.dg/cpp0x/desig4.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/desig4.C	(working copy)
@@ -0,0 +1,21 @@
+// PR c++/82593
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+int INDEX1 = 0;
+int INDEX2 = 1;
+
+class SomeClass {
+public:
+ SomeClass();
+private:
+ struct { int field; } member[2];
+};
+
+SomeClass::SomeClass()
+ : member({
+   [INDEX1] = { .field = 0 },  // { dg-error "constant expression" }
+   [INDEX2] = { .field = 1 }   // { dg-error "constant expression" }
+ })
+{
+}

[C++ PING] Re: [C++ Patch, V2] PR 82593 ("Internal compiler error: in process_init_constructor_array, at cp/typeck2.c:1294")

[Paolo Carlini]

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

Hi,

On 17/11/2017 15:09, Paolo Carlini wrote:
> Hi again,
>
> I managed to spend much more time on the issue and I'm starting a new 
> thread with a mature - IMHO - proposal: the big thing is the use of 
> the existing check_array_designated_initializer in 
> process_init_constructor_array,  which calls maybe_constant_value, as 
> we want, and covers all the ill-formed cases which I can imagine. I'm 
> also tweaking a bit the parser to check the return value of 
> require_potential_rvalue_constant_expression in order to avoid 
> redundant diagnostic in some cases. Also, a couple more testcases 
> beyond the bug report.
I'm gently pinging this. I rebased it vs a very minor conflict due to 
Jakub's implementation of P0329R4. While I was at it, I'm also proposing 
a small tweak vs the previous version in the way 
check_array_designated_initializer is used: only if ce->index is 
non-null, more consistently with the current code. Or see the original post:

     https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01481.html

Thanks!
Paolo.

///////////////////////////

[-- Attachment #2: patch_82593_3 --]
[-- Type: text/plain, Size: 4154 bytes --]

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 255161)
+++ cp/cp-tree.h	(working copy)
@@ -6190,6 +6190,8 @@ extern bool require_deduced_type		(tree, tsubst_fl
 
 extern tree finish_case_label			(location_t, tree, tree);
 extern tree cxx_maybe_build_cleanup		(tree, tsubst_flags_t);
+extern bool check_array_designated_initializer  (constructor_elt *,
+						 unsigned HOST_WIDE_INT);
 
 /* in decl2.c */
 extern void record_mangling			(tree, bool);
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 255161)
+++ cp/decl.c	(working copy)
@@ -5249,7 +5249,7 @@ grok_reference_init (tree decl, tree type, tree in
    initializer.  If it does, an error is issued.  Returns true if CE
    is valid, i.e., does not have a designated initializer.  */
 
-static bool
+bool
 check_array_designated_initializer (constructor_elt *ce,
 				    unsigned HOST_WIDE_INT index)
 {
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 255161)
+++ cp/parser.c	(working copy)
@@ -22218,8 +22218,10 @@ cp_parser_initializer_list (cp_parser* parser, boo
 
 	  if (!cp_parser_parse_definitely (parser))
 	    designator = NULL_TREE;
-	  else if (non_const)
-	    require_potential_rvalue_constant_expression (designator);
+	  else if (non_const
+		   && (!require_potential_rvalue_constant_expression
+		       (designator)))
+	    designator = NULL_TREE;
 	  if (designator)
 	    /* Warn the user that they are using an extension.  */
 	    pedwarn (loc, OPT_Wpedantic,
Index: cp/typeck2.c
===================================================================
--- cp/typeck2.c	(revision 255161)
+++ cp/typeck2.c	(working copy)
@@ -1289,17 +1289,10 @@ process_init_constructor_array (tree type, tree in
 
   FOR_EACH_VEC_SAFE_ELT (v, i, ce)
     {
-      if (ce->index)
-	{
-	  gcc_assert (TREE_CODE (ce->index) == INTEGER_CST);
-	  if (compare_tree_int (ce->index, i) != 0)
-	    {
-	      ce->value = error_mark_node;
-	      sorry ("non-trivial designated initializers not supported");
-	    }
-	}
-      else
+      if (!ce->index)
 	ce->index = size_int (i);
+      else if (!check_array_designated_initializer (ce, i))
+	ce->index = error_mark_node;
       gcc_assert (ce->value);
       ce->value = massage_init_elt (TREE_TYPE (type), ce->value, complain);
 
Index: testsuite/g++.dg/cpp0x/desig2.C
===================================================================
--- testsuite/g++.dg/cpp0x/desig2.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/desig2.C	(working copy)
@@ -0,0 +1,23 @@
+// PR c++/82593
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+enum {
+ INDEX1 = 0,
+ INDEX2
+};
+
+class SomeClass {
+public:
+ SomeClass();
+private:
+ struct { int field; } member[2];
+};
+
+SomeClass::SomeClass()
+ : member({
+   [INDEX1] = { .field = 0 },
+   [INDEX2] = { .field = 1 }
+ })
+{
+}
Index: testsuite/g++.dg/cpp0x/desig3.C
===================================================================
--- testsuite/g++.dg/cpp0x/desig3.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/desig3.C	(working copy)
@@ -0,0 +1,21 @@
+// PR c++/82593
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+const int INDEX1 = 0;
+const int INDEX2 = 1;
+
+class SomeClass {
+public:
+ SomeClass();
+private:
+ struct { int field; } member[2];
+};
+
+SomeClass::SomeClass()
+ : member({
+   [INDEX1] = { .field = 0 },
+   [INDEX2] = { .field = 1 }
+ })
+{
+}
Index: testsuite/g++.dg/cpp0x/desig4.C
===================================================================
--- testsuite/g++.dg/cpp0x/desig4.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/desig4.C	(working copy)
@@ -0,0 +1,21 @@
+// PR c++/82593
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+int INDEX1 = 0;
+int INDEX2 = 1;
+
+class SomeClass {
+public:
+ SomeClass();
+private:
+ struct { int field; } member[2];
+};
+
+SomeClass::SomeClass()
+ : member({
+   [INDEX1] = { .field = 0 },  // { dg-error "constant expression" }
+   [INDEX2] = { .field = 1 }   // { dg-error "constant expression" }
+ })
+{
+}

Re: [C++ PING] Re: [C++ Patch, V2] PR 82593 ("Internal compiler error: in process_init_constructor_array, at cp/typeck2.c:1294")

[Jason Merrill]

On 11/30/2017 01:19 PM, Paolo Carlini wrote:
> On 17/11/2017 15:09, Paolo Carlini wrote:
>> Hi again,
>>
>> I managed to spend much more time on the issue and I'm starting a new 
>> thread with a mature - IMHO - proposal: the big thing is the use of 
>> the existing check_array_designated_initializer in 
>> process_init_constructor_array,  which calls maybe_constant_value, as 
>> we want, and covers all the ill-formed cases which I can imagine. I'm 
>> also tweaking a bit the parser to check the return value of 
>> require_potential_rvalue_constant_expression in order to avoid 
>> redundant diagnostic in some cases. Also, a couple more testcases 
>> beyond the bug report.
> I'm gently pinging this. I rebased it vs a very minor conflict due to 
> Jakub's implementation of P0329R4. While I was at it, I'm also proposing 
> a small tweak vs the previous version in the way 
> check_array_designated_initializer is used: only if ce->index is 
> non-null, more consistently with the current code.

OK.

Jason