diff mbox series

[C++] PR 84632 ("[8 Regression] internal compiler error: tree check: expected record_type or union_type or qual_union_type, have array_type in reduced_constant_expression_p...")

Message ID 8a93abd6-69ae-4ff5-5332-eb8791d492f1@oracle.com
State New
Headers show
Series [C++] PR 84632 ("[8 Regression] internal compiler error: tree check: expected record_type or union_type or qual_union_type, have array_type in reduced_constant_expression_p...") | expand

Commit Message

Paolo Carlini March 22, 2018, 8:05 a.m. UTC
Hi,

yesterday I figured out that this issue is in fact orthogonal to 
possible improvements to maybe_deduce_size_from_array_init, indeed it 
happens also when we are not deducing the size. The testcase is very 
similar to that recently submitted for c++/78345, which led Jason to add 
the bit of additional checking around the middle of build_aggr_init. 
Since we did already have a similar check a bit earlier - not requiring 
the computation of from_array - I added one for VAR_DECLs too in the 
same place and tweaked/improved a bit the error message in the process 
(a few greps revealed that we don't seem to have any other generic error 
message using the form "bad ..."), consistently with the diagnostic that 
we already provide for, eg, a simple

     int a[2] = a;

FYI, I must also add that entire testsuite doesn't have a test 
triggering the existing TREE_CODE (init) == TREE_LIST check, and failed 
so far to construct one... (many such issues are normally catched by 
digest_init). Tested x86_64-linux.

Thanks, Paolo.

/////////////////////
/cp
2018-03-22  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/84632
	* init.c (build_aggr_init): Reject VAR_DECLs too as initializer.

/testsuite
2018-03-22  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/84632
	* g++.dg/init/array49.C: New.

Comments

Jason Merrill March 22, 2018, 6:08 p.m. UTC | #1
On Thu, Mar 22, 2018 at 4:05 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> yesterday I figured out that this issue is in fact orthogonal to possible
> improvements to maybe_deduce_size_from_array_init, indeed it happens also
> when we are not deducing the size. The testcase is very similar to that
> recently submitted for c++/78345, which led Jason to add the bit of
> additional checking around the middle of build_aggr_init. Since we did
> already have a similar check a bit earlier - not requiring the computation
> of from_array - I added one for VAR_DECLs too in the same place and
> tweaked/improved a bit the error message in the process (a few greps
> revealed that we don't seem to have any other generic error message using
> the form "bad ..."), consistently with the diagnostic that we already
> provide for, eg, a simple
>
>     int a[2] = a;
>
> FYI, I must also add that entire testsuite doesn't have a test triggering
> the existing TREE_CODE (init) == TREE_LIST check, and failed so far to
> construct one... (many such issues are normally catched by digest_init).
> Tested x86_64-linux.

That whole block is there to support this sort of initialization, as
an ancient extension.  Since it isn't generally allowed, and is
causing trouble, let's remove it and reject anything that isn't an
initializer list.

Jason
Paolo Carlini March 22, 2018, 6:58 p.m. UTC | #2
Hi,

On 22/03/2018 19:08, Jason Merrill wrote:
> On Thu, Mar 22, 2018 at 4:05 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi,
>>
>> yesterday I figured out that this issue is in fact orthogonal to possible
>> improvements to maybe_deduce_size_from_array_init, indeed it happens also
>> when we are not deducing the size. The testcase is very similar to that
>> recently submitted for c++/78345, which led Jason to add the bit of
>> additional checking around the middle of build_aggr_init. Since we did
>> already have a similar check a bit earlier - not requiring the computation
>> of from_array - I added one for VAR_DECLs too in the same place and
>> tweaked/improved a bit the error message in the process (a few greps
>> revealed that we don't seem to have any other generic error message using
>> the form "bad ..."), consistently with the diagnostic that we already
>> provide for, eg, a simple
>>
>>      int a[2] = a;
>>
>> FYI, I must also add that entire testsuite doesn't have a test triggering
>> the existing TREE_CODE (init) == TREE_LIST check, and failed so far to
>> construct one... (many such issues are normally catched by digest_init).
>> Tested x86_64-linux.
> That whole block is there to support this sort of initialization, as
> an ancient extension.  Since it isn't generally allowed, and is
> causing trouble, let's remove it and reject anything that isn't an
> initializer list.
Nice. While I start implementing the above, any hint about thing like 
g++.dg/cpp0x/initlist68.C, which we would reject because involves a 
plain constructor of type ARRAY_TYPE, not a proper 
BRACE_ENCLOSED_INITIALIZER_P? Also, less important I guess, we would 
also reject g++.dg/torture/pr70499.C to which you added -fpermissive in 
the occasion of c++/78345. Or maybe you meant something a bit less 
drastic ;)

Paolo.
Paolo Carlini March 22, 2018, 7:35 p.m. UTC | #3
Hi again,

On 22/03/2018 19:58, Paolo Carlini wrote:
> Nice. While I start implementing the above, any hint about thing like 
> g++.dg/cpp0x/initlist68.C, which we would reject because involves a 
> plain constructor of type ARRAY_TYPE, not a proper 
> BRACE_ENCLOSED_INITIALIZER_P? Also, less important I guess, we would 
> also reject g++.dg/torture/pr70499.C to which you added -fpermissive 
> in the occasion of c++/78345. Or maybe you meant something a bit less 
> drastic ;)
I suspect that allowing for either BRACE_ENCLOSED_INITIALIZER_P or 
CONSTRUCTOR with a check of the type (like you changed build_vec_init at 
the time: 
https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cp/init.c?r1=246244&r2=246243&pathrev=246244 
) would work fine.

Paolo.
Jason Merrill March 22, 2018, 9:19 p.m. UTC | #4
On Thu, Mar 22, 2018 at 3:35 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi again,
>
> On 22/03/2018 19:58, Paolo Carlini wrote:
>
> Nice. While I start implementing the above, any hint about thing like
> g++.dg/cpp0x/initlist68.C, which we would reject because involves a plain
> constructor of type ARRAY_TYPE, not a proper BRACE_ENCLOSED_INITIALIZER_P?
> Also, less important I guess, we would also reject g++.dg/torture/pr70499.C
> to which you added -fpermissive in the occasion of c++/78345. Or maybe you
> meant something a bit less drastic ;)
>
> I suspect that allowing for either BRACE_ENCLOSED_INITIALIZER_P or
> CONSTRUCTOR with a check of the type (like you changed build_vec_init at the
> time:
> https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cp/init.c?r1=246244&r2=246243&pathrev=246244
> ) would work fine.

Yes, that sounds good.  And if pr70499.C fails, let's fix the testcase
to add the missing braces.

Jason
Paolo Carlini March 22, 2018, 9:35 p.m. UTC | #5
Hi,

On 22/03/2018 22:19, Jason Merrill wrote:
> On Thu, Mar 22, 2018 at 3:35 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi again,
>>
>> On 22/03/2018 19:58, Paolo Carlini wrote:
>>
>> Nice. While I start implementing the above, any hint about thing like
>> g++.dg/cpp0x/initlist68.C, which we would reject because involves a plain
>> constructor of type ARRAY_TYPE, not a proper BRACE_ENCLOSED_INITIALIZER_P?
>> Also, less important I guess, we would also reject g++.dg/torture/pr70499.C
>> to which you added -fpermissive in the occasion of c++/78345. Or maybe you
>> meant something a bit less drastic ;)
>>
>> I suspect that allowing for either BRACE_ENCLOSED_INITIALIZER_P or
>> CONSTRUCTOR with a check of the type (like you changed build_vec_init at the
>> time:
>> https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cp/init.c?r1=246244&r2=246243&pathrev=246244
>> ) would work fine.
> Yes, that sounds good.  And if pr70499.C fails, let's fix the testcase
> to add the missing braces.
Ok about pr70499.C.

Unfortunately, however, the above idea still is not enough: for, say, 
lambda-array.C we have to accept as init an INDIRECT_REF with 
ARRAY_TYPE, thus qualified CONSTRUCTOR isn't enough, it really looks 
like we have to accept various (?) TREE_CODEs with ARRAY_TYPE as type, 
not just CONSTRUCTORs, to handle normal well formed code. Thus my best 
try so far would be the below, in testing. What do you think?

Cheers,
Paolo.

/////////////////////
Paolo Carlini March 22, 2018, 9:39 p.m. UTC | #6
... with patch ;)

If you are curious where the heck that INDIRECT_REF is coming from, is 
coming from the gimplifier, cp_gimpliify_expr, via build_vec_init. Grrr.

Paolo.

///////////////////////
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 258758)
+++ cp/init.c	(working copy)
@@ -1688,14 +1688,6 @@ build_aggr_init (tree exp, tree init, int flags, t
 	}
       else
 	{
-	  /* An array may not be initialized use the parenthesized
-	     initialization form -- unless the initializer is "()".  */
-	  if (init && TREE_CODE (init) == TREE_LIST)
-	    {
-	      if (complain & tf_error)
-		error ("bad array initializer");
-	      return error_mark_node;
-	    }
 	  /* Must arrange to initialize each element of EXP
 	     from elements of INIT.  */
 	  if (cv_qualified_p (type))
@@ -1705,14 +1697,15 @@ build_aggr_init (tree exp, tree init, int flags, t
 	  from_array = (itype && same_type_p (TREE_TYPE (init),
 					      TREE_TYPE (exp)));
 
-	  if (init && !from_array
-	      && !BRACE_ENCLOSED_INITIALIZER_P (init))
+	  if (init && ((!from_array
+			&& !BRACE_ENCLOSED_INITIALIZER_P (init))
+		       /* See c++/84632.  */
+		       || VAR_P (init)))
 	    {
 	      if (complain & tf_error)
-		permerror (init_loc, "array must be initialized "
-			   "with a brace-enclosed initializer");
-	      else
-		return error_mark_node;
+		error_at (init_loc, "array must be initialized "
+			  "with a brace-enclosed initializer");
+	      return error_mark_node;
 	    }
 	}
 
Index: testsuite/g++.dg/init/array49.C
===================================================================
--- testsuite/g++.dg/init/array49.C	(nonexistent)
+++ testsuite/g++.dg/init/array49.C	(working copy)
@@ -0,0 +1,6 @@
+// PR c++/84632
+// { dg-additional-options "-w" }
+
+class {
+  &a;  // { dg-error "forbids declaration" }
+} b[2] = b;  // { dg-error "initialized" }
Index: testsuite/g++.dg/torture/pr70499.C
===================================================================
--- testsuite/g++.dg/torture/pr70499.C	(revision 258758)
+++ testsuite/g++.dg/torture/pr70499.C	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-additional-options "-w -fpermissive -Wno-psabi" }
+// { dg-additional-options "-w -Wno-psabi" }
 // { dg-additional-options "-mavx" { target x86_64-*-* i?86-*-* } }
 
 typedef double __m256d __attribute__ ((__vector_size__ (32), __may_alias__));
@@ -30,7 +30,7 @@ struct Foo {
 template<typename Tx>  
 __attribute__((__always_inline__)) inline void inlineFunc(Tx hx[]) {
     Tx x = hx[0], y = hx[1];
-    Tx lam[1] = (x*y);
+    Tx lam[1] = {(x*y)};
 }
 
 void FooBarFunc () {
Jason Merrill March 22, 2018, 10:26 p.m. UTC | #7
On Thu, Mar 22, 2018 at 5:39 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> ... with patch ;)
>
> If you are curious where the heck that INDIRECT_REF is coming from, is
> coming from the gimplifier, cp_gimpliify_expr, via build_vec_init. Grrr.

Hmm, maybe build_vec_init should call itself directly rather than via
build_aggr_init in the case of multidimensional arrays.

Jason
Paolo Carlini March 23, 2018, 10:13 a.m. UTC | #8
Hi,

On 22/03/2018 23:26, Jason Merrill wrote:
> On Thu, Mar 22, 2018 at 5:39 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> ... with patch ;)
>>
>> If you are curious where the heck that INDIRECT_REF is coming from, is
>> coming from the gimplifier, cp_gimpliify_expr, via build_vec_init. Grrr.
> Hmm, maybe build_vec_init should call itself directly rather than via
> build_aggr_init in the case of multidimensional arrays.
Yes, arranging things like that seems doable. However, yesterday, while 
fiddling with the idea and instrumenting the code with some gcc_asserts, 
I noticed that we have yet another tree code to handle, TARGET_EXPR, as 
in lines #41, #47, #56 of ext/complit12.C, and in that case 
build_aggr_init is simply called by check_initializer via 
build_aggr_init_full_exprs, the "normal" path. Well, unless we want to 
adjust/reject complit12.C too, which clang rejects, in fact with errors 
on lines #19 and #29 too. The below passes testing.

Thanks,
Paolo.

/////////////////
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 258758)
+++ cp/init.c	(working copy)
@@ -1688,14 +1688,6 @@ build_aggr_init (tree exp, tree init, int flags, t
 	}
       else
 	{
-	  /* An array may not be initialized use the parenthesized
-	     initialization form -- unless the initializer is "()".  */
-	  if (init && TREE_CODE (init) == TREE_LIST)
-	    {
-	      if (complain & tf_error)
-		error ("bad array initializer");
-	      return error_mark_node;
-	    }
 	  /* Must arrange to initialize each element of EXP
 	     from elements of INIT.  */
 	  if (cv_qualified_p (type))
@@ -1705,14 +1697,16 @@ build_aggr_init (tree exp, tree init, int flags, t
 	  from_array = (itype && same_type_p (TREE_TYPE (init),
 					      TREE_TYPE (exp)));
 
-	  if (init && !from_array
-	      && !BRACE_ENCLOSED_INITIALIZER_P (init))
+	  if (init && !BRACE_ENCLOSED_INITIALIZER_P (init)
+	      && (!from_array
+		  || (TREE_CODE (init) != CONSTRUCTOR
+		      && TREE_CODE (init) != INDIRECT_REF
+		      && TREE_CODE (init) != TARGET_EXPR)))
 	    {
 	      if (complain & tf_error)
-		permerror (init_loc, "array must be initialized "
-			   "with a brace-enclosed initializer");
-	      else
-		return error_mark_node;
+		error_at (init_loc, "array must be initialized "
+			  "with a brace-enclosed initializer");
+	      return error_mark_node;
 	    }
 	}
 
Index: testsuite/g++.dg/init/array49.C
===================================================================
--- testsuite/g++.dg/init/array49.C	(nonexistent)
+++ testsuite/g++.dg/init/array49.C	(working copy)
@@ -0,0 +1,6 @@
+// PR c++/84632
+// { dg-additional-options "-w" }
+
+class {
+  &a;  // { dg-error "forbids declaration" }
+} b[2] = b;  // { dg-error "initialized" }
Index: testsuite/g++.dg/torture/pr70499.C
===================================================================
--- testsuite/g++.dg/torture/pr70499.C	(revision 258758)
+++ testsuite/g++.dg/torture/pr70499.C	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-additional-options "-w -fpermissive -Wno-psabi" }
+// { dg-additional-options "-w -Wno-psabi" }
 // { dg-additional-options "-mavx" { target x86_64-*-* i?86-*-* } }
 
 typedef double __m256d __attribute__ ((__vector_size__ (32), __may_alias__));
@@ -30,7 +30,7 @@ struct Foo {
 template<typename Tx>  
 __attribute__((__always_inline__)) inline void inlineFunc(Tx hx[]) {
     Tx x = hx[0], y = hx[1];
-    Tx lam[1] = (x*y);
+    Tx lam[1] = {(x*y)};
 }
 
 void FooBarFunc () {
Jason Merrill March 23, 2018, 12:39 p.m. UTC | #9
On Fri, Mar 23, 2018 at 6:13 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 22/03/2018 23:26, Jason Merrill wrote:
>>
>> On Thu, Mar 22, 2018 at 5:39 PM, Paolo Carlini <paolo.carlini@oracle.com>
>> wrote:
>>>
>>> ... with patch ;)
>>>
>>> If you are curious where the heck that INDIRECT_REF is coming from, is
>>> coming from the gimplifier, cp_gimpliify_expr, via build_vec_init. Grrr.
>>
>> Hmm, maybe build_vec_init should call itself directly rather than via
>> build_aggr_init in the case of multidimensional arrays.
>
> Yes, arranging things like that seems doable. However, yesterday, while
> fiddling with the idea and instrumenting the code with some gcc_asserts, I
> noticed that we have yet another tree code to handle, TARGET_EXPR, as in
> lines #41, #47, #56 of ext/complit12.C, and in that case build_aggr_init is
> simply called by check_initializer via build_aggr_init_full_exprs, the
> "normal" path. Well, unless we want to adjust/reject complit12.C too, which
> clang rejects, in fact with errors on lines #19 and #29 too. The below
> passes testing.

I think I'd like to allow TARGET_EXPR here, with a comment about
compound literals, but avoid INDIRECT_REF with that build_vec_init
change.

Jason
Paolo Carlini March 26, 2018, 9:19 a.m. UTC | #10
Hi,

On 23/03/2018 13:39, Jason Merrill wrote:
> On Fri, Mar 23, 2018 at 6:13 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> On 22/03/2018 23:26, Jason Merrill wrote:
>>> On Thu, Mar 22, 2018 at 5:39 PM, Paolo Carlini <paolo.carlini@oracle.com>
>>> wrote:
>>>> ... with patch ;)
>>>>
>>>> If you are curious where the heck that INDIRECT_REF is coming from, is
>>>> coming from the gimplifier, cp_gimpliify_expr, via build_vec_init. Grrr.
>>> Hmm, maybe build_vec_init should call itself directly rather than via
>>> build_aggr_init in the case of multidimensional arrays.
>> Yes, arranging things like that seems doable. However, yesterday, while
>> fiddling with the idea and instrumenting the code with some gcc_asserts, I
>> noticed that we have yet another tree code to handle, TARGET_EXPR, as in
>> lines #41, #47, #56 of ext/complit12.C, and in that case build_aggr_init is
>> simply called by check_initializer via build_aggr_init_full_exprs, the
>> "normal" path. Well, unless we want to adjust/reject complit12.C too, which
>> clang rejects, in fact with errors on lines #19 and #29 too. The below
>> passes testing.
> I think I'd like to allow TARGET_EXPR here, with a comment about
> compound literals, but avoid INDIRECT_REF with that build_vec_init
> change.
I see. Having run the full testsuite a number of times with additional 
gcc_asserts, I'm very confident that something as simple as the below is 
fine, at least as far as the testsuite + variants of lambda-array.C is 
concerned. In it I'm also proposing a gcc_assert verifying that the very 
idea of not using any diagnostic conditional makes sense for the 
internally generated INDIRECT_REFs: in the existing build_aggr_init if 
the types were different from_array would be zero and, for INDIRECT_REF 
as init, the condition true.

Thanks, Paolo.

///////////////////////
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 258846)
+++ cp/init.c	(working copy)
@@ -1688,14 +1688,6 @@ build_aggr_init (tree exp, tree init, int flags, t
 	}
       else
 	{
-	  /* An array may not be initialized use the parenthesized
-	     initialization form -- unless the initializer is "()".  */
-	  if (init && TREE_CODE (init) == TREE_LIST)
-	    {
-	      if (complain & tf_error)
-		error ("bad array initializer");
-	      return error_mark_node;
-	    }
 	  /* Must arrange to initialize each element of EXP
 	     from elements of INIT.  */
 	  if (cv_qualified_p (type))
@@ -1705,14 +1697,17 @@ build_aggr_init (tree exp, tree init, int flags, t
 	  from_array = (itype && same_type_p (TREE_TYPE (init),
 					      TREE_TYPE (exp)));
 
-	  if (init && !from_array
-	      && !BRACE_ENCLOSED_INITIALIZER_P (init))
+	  if (init && !BRACE_ENCLOSED_INITIALIZER_P (init)
+	      && (!from_array
+		  || (TREE_CODE (init) != CONSTRUCTOR
+		      /* Can happen, eg, handling the compound-literals
+			 extension (ext/complit12.C).  */
+		      && TREE_CODE (init) != TARGET_EXPR)))
 	    {
 	      if (complain & tf_error)
-		permerror (init_loc, "array must be initialized "
-			   "with a brace-enclosed initializer");
-	      else
-		return error_mark_node;
+		error_at (init_loc, "array must be initialized "
+			  "with a brace-enclosed initializer");
+	      return error_mark_node;
 	    }
 	}
 
@@ -4371,7 +4366,19 @@ build_vec_init (tree base, tree maxindex, tree ini
 	    elt_init = cp_build_modify_expr (input_location, to, NOP_EXPR,
 					     from, complain);
 	  else if (type_build_ctor_call (type))
-	    elt_init = build_aggr_init (to, from, 0, complain);
+	    {
+	      if (TREE_CODE (type) == ARRAY_TYPE
+		  && from && TREE_CODE (from) == INDIRECT_REF)
+		{
+		  gcc_assert (same_type_ignoring_top_level_qualifiers_p
+			      (type, TREE_TYPE (from)));
+		  elt_init = build_vec_init (to, NULL_TREE, from,
+					     /*explicit_value_init_p=*/false,
+					     /*from_array=*/1, complain);
+		}
+	      else
+		elt_init = build_aggr_init (to, from, 0, complain);
+	    }
 	  else if (from)
 	    elt_init = cp_build_modify_expr (input_location, to, NOP_EXPR, from,
 					     complain);
Index: testsuite/g++.dg/init/array49.C
===================================================================
--- testsuite/g++.dg/init/array49.C	(nonexistent)
+++ testsuite/g++.dg/init/array49.C	(working copy)
@@ -0,0 +1,6 @@
+// PR c++/84632
+// { dg-additional-options "-w" }
+
+class {
+  &a;  // { dg-error "forbids declaration" }
+} b[2] = b;  // { dg-error "initialized" }
Index: testsuite/g++.dg/torture/pr70499.C
===================================================================
--- testsuite/g++.dg/torture/pr70499.C	(revision 258846)
+++ testsuite/g++.dg/torture/pr70499.C	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-additional-options "-w -fpermissive -Wno-psabi" }
+// { dg-additional-options "-w -Wno-psabi" }
 // { dg-additional-options "-mavx" { target x86_64-*-* i?86-*-* } }
 
 typedef double __m256d __attribute__ ((__vector_size__ (32), __may_alias__));
@@ -30,7 +30,7 @@ struct Foo {
 template<typename Tx>  
 __attribute__((__always_inline__)) inline void inlineFunc(Tx hx[]) {
     Tx x = hx[0], y = hx[1];
-    Tx lam[1] = (x*y);
+    Tx lam[1] = {(x*y)};
 }
 
 void FooBarFunc () {
Jason Merrill March 26, 2018, 5:12 p.m. UTC | #11
On Mon, Mar 26, 2018 at 5:19 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 23/03/2018 13:39, Jason Merrill wrote:
>> On Fri, Mar 23, 2018 at 6:13 AM, Paolo Carlini <paolo.carlini@oracle.com>
>> wrote:
>>>
>>> On 22/03/2018 23:26, Jason Merrill wrote:
>>>>
>>>> On Thu, Mar 22, 2018 at 5:39 PM, Paolo Carlini
>>>> <paolo.carlini@oracle.com>
>>>> wrote:
>>>>>
>>>>> ... with patch ;)
>>>>>
>>>>> If you are curious where the heck that INDIRECT_REF is coming from, is
>>>>> coming from the gimplifier, cp_gimpliify_expr, via build_vec_init.
>>>>> Grrr.
>>>>
>>>> Hmm, maybe build_vec_init should call itself directly rather than via
>>>> build_aggr_init in the case of multidimensional arrays.
>>>
>>> Yes, arranging things like that seems doable. However, yesterday, while
>>> fiddling with the idea and instrumenting the code with some gcc_asserts,
>>> I
>>> noticed that we have yet another tree code to handle, TARGET_EXPR, as in
>>> lines #41, #47, #56 of ext/complit12.C, and in that case build_aggr_init
>>> is
>>> simply called by check_initializer via build_aggr_init_full_exprs, the
>>> "normal" path. Well, unless we want to adjust/reject complit12.C too,
>>> which
>>> clang rejects, in fact with errors on lines #19 and #29 too. The below
>>> passes testing.
>>
>> I think I'd like to allow TARGET_EXPR here, with a comment about
>> compound literals, but avoid INDIRECT_REF with that build_vec_init
>> change.
>
> I see. Having run the full testsuite a number of times with additional
> gcc_asserts, I'm very confident that something as simple as the below is
> fine, at least as far as the testsuite + variants of lambda-array.C is
> concerned. In it I'm also proposing a gcc_assert verifying that the very
> idea of not using any diagnostic conditional makes sense for the internally
> generated INDIRECT_REFs: in the existing build_aggr_init if the types were
> different from_array would be zero and, for INDIRECT_REF as init, the
> condition true.

Your build_aggr_init change is OK, but I had in mind something more
general in build_vec_init:
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index ff52c42c1ad..d689390d117 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4367,7 +4367,10 @@ build_vec_init (tree base, tree maxindex, tree init,
 	  else
 	    from = NULL_TREE;
 
-	  if (from_array == 2)
+	  if (TREE_CODE (type) == ARRAY_TYPE)
+	    elt_init = build_vec_init (to, NULL_TREE, from, /*val_init*/false,
+				       from_array, complain);
+	  else if (from_array == 2)
 	    elt_init = cp_build_modify_expr (input_location, to, NOP_EXPR,
 					     from, complain);
 	  else if (type_build_ctor_call (type))
Paolo Carlini March 26, 2018, 5:57 p.m. UTC | #12
Hi,

On 26/03/2018 19:12, Jason Merrill wrote:
> Your build_aggr_init change is OK, but I had in mind something more
> general in build_vec_init:
Oh nice. Shall I test it together with my build_aggr_type bits and the 
testcases and commit it if everything is Ok? By the way - FYI - what I 
had tested was used *only* by lambda-array.C and lambda-errloc.C.

Paolo.
Jason Merrill March 26, 2018, 6:25 p.m. UTC | #13
On Mon, Mar 26, 2018 at 1:57 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 26/03/2018 19:12, Jason Merrill wrote:
>>
>> Your build_aggr_init change is OK, but I had in mind something more
>> general in build_vec_init:
>
> Oh nice. Shall I test it together with my build_aggr_type bits and the
> testcases and commit it if everything is Ok?

Please.

Jason
diff mbox series

Patch

Index: cp/init.c
===================================================================
--- cp/init.c	(revision 258745)
+++ cp/init.c	(working copy)
@@ -1688,12 +1688,14 @@  build_aggr_init (tree exp, tree init, int flags, t
 	}
       else
 	{
-	  /* An array may not be initialized use the parenthesized
-	     initialization form -- unless the initializer is "()".  */
-	  if (init && TREE_CODE (init) == TREE_LIST)
+	  /* An array may not be initialized usinge the parenthesized
+	     initialization form -- unless the initializer is "()".  
+	     Also reject VAR_DECLs (c++/84632).  */
+	  if (init && (TREE_CODE (init) == TREE_LIST || VAR_P (init)))
 	    {
 	      if (complain & tf_error)
-		error ("bad array initializer");
+		error_at (init_loc, "array must be initialized "
+			  "with a brace-enclosed initializer");
 	      return error_mark_node;
 	    }
 	  /* Must arrange to initialize each element of EXP
Index: testsuite/g++.dg/init/array49.C
===================================================================
--- testsuite/g++.dg/init/array49.C	(nonexistent)
+++ testsuite/g++.dg/init/array49.C	(working copy)
@@ -0,0 +1,6 @@ 
+// PR c++/84632
+// { dg-additional-options "-w" }
+
+class {
+  &a;  // { dg-error "forbids declaration" }
+} b[2] = b;  // { dg-error "initialized" }