Re: [PATCH v3] c++: Fix P0960 in member init list and array [PR92812]

[Marek Polacek <polacek@redhat.com>]

On Wed, Sep 02, 2020 at 05:06:45PM -0400, Jason Merrill wrote:
> On 9/2/20 4:37 PM, Marek Polacek wrote:
> > I've added do_aggregate_paren_init to factor some common code.  It's not
> > perfect because the enclosing conditions couldn't really be factored out,
> 
> This condition:
> 
> > +	       && (list_length (init) > 1
> > +		   /* A single-element list: handle non-standard extensions
> > +		      like compound literals.  This also prevents triggering
> > +		      aggregate ()-initialization in compiler-generated code
> > +		      for =default.  */
> > +		   || (list_length (init) == 1
> > +		       && !same_type_ignoring_top_level_qualifiers_p
> > +			   (type, TREE_TYPE (TREE_VALUE (init))))))
> 
> seems like it could move into do_aggregate_paren_init as well, even if it's
> redundant with code in the caller in some cases; we never want to add { } to
> a single parenthesized expression of the same type.

True, done in this patch.

> And don't we need to
> check for the same-type situation for the array case in check_initializer?

Yea, I think so.  It can be reached using either

  char c[4]("foo");
 
which is already handled by the string literal case, or with an array prvalue:

  using T = int[2];
  T t(T{1, 1});

> Incidentally, for checking whether a list is length 1 or more, probably
> slightly more efficient to look at TREE_CHAIN directly like
> do_aggregate_paren_init does in this patch, rather than use list_length.

Done.

How does this look?  Testing in progress, but dg.exp and old-deja.exp
is clean.

Ok if testing passes?

-- >8 --
This patch nails down the remaining P0960 case in PR92812:

  struct A {
    int ar[2];
    A(): ar(1, 2) {} // doesn't work without this patch
  };

Note that when the target object is not of array type, this already
works:

  struct S { int x, y; };
  struct A {
    S s;
    A(): s(1, 2) { } // OK in C++20
  };

because build_new_method_call_1 takes care of the P0960 magic.

It proved to be quite hairy.  When the ()-list has more than one
element, we can always create a CONSTRUCTOR, because the code was
previously invalid.  But when the ()-list has just one element, it
gets all kinds of difficult.  As usual, we have to handle a("foo")
so as not to wrap the STRING_CST in a CONSTRUCTOR.  Always turning
x(e) into x{e} would run into trouble as in c++/93790.  Another
issue was what to do about x({e}): previously, this would trigger
"list-initializer for non-class type must not be parenthesized".
I figured I'd make this work in C++20, so that given

  struct S { int x, y; };

you can do

   S a[2];
   [...]
   A(): a({1, 2}) // initialize a[0] with {1, 2} and a[1] with {}

It also turned out that, as an extension, we support compound literals:

  F (): m((S[1]) { 1, 2 })

so this has to keep working as before.  Moreover, make sure not to trigger
in compiler-generated code, like =default, where array assignment is allowed.

I've factored out a function that turns a TREE_LIST into a CONSTRUCTOR
to simplify handling of P0960.

paren-init35.C also tests this with vector types.

gcc/cp/ChangeLog:

	PR c++/92812
	* cp-tree.h (do_aggregate_paren_init): Declare.
	* decl.c (do_aggregate_paren_init): New.
	(grok_reference_init): Use it.
	(check_initializer): Likewise.
	* init.c (perform_member_init): Handle initializing an array from
	a ()-list.  Use do_aggregate_paren_init.

gcc/testsuite/ChangeLog:

	PR c++/92812
	* g++.dg/cpp0x/constexpr-array23.C: Adjust dg-error.
	* g++.dg/cpp0x/initlist69.C: Likewise.
	* g++.dg/diagnostic/mem-init1.C: Likewise.
	* g++.dg/init/array28.C: Likewise.
	* g++.dg/cpp2a/paren-init33.C: New test.
	* g++.dg/cpp2a/paren-init34.C: New test.
	* g++.dg/cpp2a/paren-init35.C: New test.
	* g++.old-deja/g++.brendan/crash60.C: Adjust dg-error.
	* g++.old-deja/g++.law/init10.C: Likewise.
	* g++.old-deja/g++.other/array3.C: Likewise.
---
 gcc/cp/cp-tree.h                              |   1 +
 gcc/cp/decl.c                                 |  62 +++++----
 gcc/cp/init.c                                 |  26 +++-
 .../g++.dg/cpp0x/constexpr-array23.C          |   6 +-
 gcc/testsuite/g++.dg/cpp0x/initlist69.C       |   4 +-
 gcc/testsuite/g++.dg/cpp2a/paren-init33.C     | 128 ++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/paren-init34.C     |  25 ++++
 gcc/testsuite/g++.dg/cpp2a/paren-init35.C     |  21 +++
 gcc/testsuite/g++.dg/diagnostic/mem-init1.C   |   4 +-
 gcc/testsuite/g++.dg/init/array28.C           |   2 +-
 .../g++.old-deja/g++.brendan/crash60.C        |   2 +-
 gcc/testsuite/g++.old-deja/g++.law/init10.C   |   2 +-
 gcc/testsuite/g++.old-deja/g++.other/array3.C |   3 +-
 13 files changed, 239 insertions(+), 47 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init33.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init34.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init35.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 2b7c9b9913a..708de83eb46 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6582,6 +6582,7 @@ extern bool check_array_designated_initializer  (constructor_elt *,
 extern bool check_for_uninitialized_const_var   (tree, bool, tsubst_flags_t);
 extern tree build_explicit_specifier		(tree, tsubst_flags_t);
 extern void do_push_parm_decls			(tree, tree, tree *);
+extern tree do_aggregate_paren_init		(tree, tree);
 
 /* in decl2.c */
 extern void record_mangling			(tree, bool);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 4c84f2d0a9b..31d68745844 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5557,6 +5557,37 @@ start_decl_1 (tree decl, bool initialized)
   maybe_push_cleanup_level (type);
 }
 
+/* Given a parenthesized list of values INIT, create a CONSTRUCTOR to handle
+   C++20 P0960.  TYPE is the type of the object we're initializing.  */
+
+tree
+do_aggregate_paren_init (tree init, tree type)
+{
+  tree val = TREE_VALUE (init);
+
+  if (TREE_CHAIN (init) == NULL_TREE)
+    {
+      /* If the list has a single element and it's a string literal,
+	 then it's the initializer for the array as a whole.  */
+      if (TREE_CODE (type) == ARRAY_TYPE
+	  && char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type)))
+	  && TREE_CODE (tree_strip_any_location_wrapper (val))
+	     == STRING_CST)
+	return val;
+      /* Handle non-standard extensions like compound literals.  This also
+	 prevents triggering aggregate parenthesized-initialization in
+	 compiler-generated code for =default.  */
+      else if (same_type_ignoring_top_level_qualifiers_p (type,
+							  TREE_TYPE (val)))
+	return val;
+    }
+
+  init = build_constructor_from_list (init_list_type_node, init);
+  CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
+  CONSTRUCTOR_IS_PAREN_INIT (init) = true;
+  return init;
+}
+
 /* Handle initialization of references.  DECL, TYPE, and INIT have the
    same meaning as in cp_finish_decl.  *CLEANUP must be NULL on entry,
    but will be set to a new CLEANUP_STMT if a temporary is created
@@ -5604,11 +5635,7 @@ grok_reference_init (tree decl, tree type, tree init, int flags)
 	  /* If the list had more than one element, the code is ill-formed
 	     pre-C++20, so we can build a constructor right away.  */
 	  else
-	    {
-	      init = build_constructor_from_list (init_list_type_node, init);
-	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
-	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
-	    }
+	    init = do_aggregate_paren_init (init, ttype);
 	}
       else
 	init = build_x_compound_expr_from_list (init, ELK_INIT,
@@ -6794,30 +6821,7 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
 	       && TREE_CODE (type) == ARRAY_TYPE
 	       && !DECL_DECOMPOSITION_P (decl)
 	       && (cxx_dialect >= cxx20))
-	{
-	  /* [dcl.init.string] "An array of ordinary character type [...]
-	     can be initialized by an ordinary string literal [...] by an
-	     appropriately-typed string literal enclosed in braces" only
-	     talks about braces, but GCC has always accepted
-
-	       char a[]("foobar");
-
-	     so we continue to do so.  */
-	  tree val = TREE_VALUE (init);
-	  if (TREE_CHAIN (init) == NULL_TREE
-	      && char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type)))
-	      && TREE_CODE (tree_strip_any_location_wrapper (val))
-		 == STRING_CST)
-	    /* If the list has a single element and it's a string literal,
-	       then it's the initializer for the array as a whole.  */
-	    init = val;
-	  else
-	    {
-	      init = build_constructor_from_list (init_list_type_node, init);
-	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
-	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
-	    }
-	}
+	init = do_aggregate_paren_init (init, type);
       else if (TREE_CODE (init) == TREE_LIST
 	       && TREE_TYPE (init) != unknown_type_node
 	       && !MAYBE_CLASS_TYPE_P (type))
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index d4540db3605..3268ae4ad3f 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -809,13 +809,25 @@ perform_member_init (tree member, tree init)
       return;
     }
 
-  if (init && TREE_CODE (init) == TREE_LIST
-      && (DIRECT_LIST_INIT_P (TREE_VALUE (init))
-	  /* FIXME C++20 parenthesized aggregate init (PR 92812).  */
-	  || !(/* cxx_dialect >= cxx20 ? CP_AGGREGATE_TYPE_P (type) */
-	       /* :  */CLASS_TYPE_P (type))))
-    init = build_x_compound_expr_from_list (init, ELK_MEM_INIT,
-					    tf_warning_or_error);
+  if (init && TREE_CODE (init) == TREE_LIST)
+    {
+      /* A(): a{e} */
+      if (DIRECT_LIST_INIT_P (TREE_VALUE (init)))
+	init = build_x_compound_expr_from_list (init, ELK_MEM_INIT,
+						tf_warning_or_error);
+      /* We are trying to initialize an array from a ()-list.  If we
+	 should attempt to do so, conjure up a CONSTRUCTOR.  */
+      else if (TREE_CODE (type) == ARRAY_TYPE
+	       /* P0960 is a C++20 feature.  */
+	       && cxx_dialect >= cxx20)
+	init = do_aggregate_paren_init (init, type);
+      else if (!CLASS_TYPE_P (type))
+	init = build_x_compound_expr_from_list (init, ELK_MEM_INIT,
+						tf_warning_or_error);
+      /* If we're initializing a class from a ()-list, leave the TREE_LIST
+	 alone: we might call an appropriate constructor, or (in C++20)
+	 do aggregate-initialization.  */
+    }
 
   if (init == void_type_node)
     {
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array23.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array23.C
index 1323271a0a5..1829fa7a653 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-array23.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array23.C
@@ -9,9 +9,9 @@ struct A
 };
 
 struct B
-{				// This should really be target { ! c++2a }
-  typedef A W[4];		// { dg-error "paren" "" { target *-*-* } .+1 }
-  constexpr B () : w ({ A::z, A::z, A::z, A::z }) {} // { dg-error "constant" }
+{
+  typedef A W[4];		// { dg-error "paren" "" { target { ! c++20 } } .+1 }
+  constexpr B () : w ({ A::z, A::z, A::z, A::z }) {} // { dg-error "constant|could not convert" }
   W w;
 };
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist69.C b/gcc/testsuite/g++.dg/cpp0x/initlist69.C
index 7995f595a47..893a4e92cd3 100644
--- a/gcc/testsuite/g++.dg/cpp0x/initlist69.C
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist69.C
@@ -6,14 +6,14 @@ struct ca {
   T elem[1];
 
   ca(const T (&s)[1]): elem{{s}} { }	   // { dg-error "invalid" }
-  ca(const T (&s)[1],int): elem({{s}}) { } // { dg-error "paren|invalid" }
+  ca(const T (&s)[1],int): elem({{s}}) { } // { dg-error "paren|invalid|too many" }
   ca(const T (&s)[1],char): elem(s) { }	   // { dg-error "array" }
   ca(const T (&s)[1],double): elem{s} { }  // { dg-error "invalid" }
 
   ca(const T &v): elem{{v}} { }	      // OK
   ca(const T &v,int): elem{{{v}}} { } // { dg-error "braces" }
   ca(const T &v,char): elem{v} { }    // OK
-  ca(const T &v,double): elem({v}) { } // { dg-error "paren" }
+  ca(const T &v,double): elem({v}) { } // { dg-error "paren" "" { target { ! c++20 } } }
 };
 
 int main() {
diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init33.C b/gcc/testsuite/g++.dg/cpp2a/paren-init33.C
new file mode 100644
index 00000000000..43f323e8f14
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/paren-init33.C
@@ -0,0 +1,128 @@
+// PR c++/92812
+// { dg-do run { target c++20 } }
+// { dg-options "-Wall -Wextra" }
+// Initializing arrays in a member init list using ()-init, valid cases.
+
+#define assert(X) do { if (!(X)) __builtin_abort(); } while(0)
+
+struct S { int x, y; };
+struct N { int x, y; N(int, int); };
+
+static S s{10, 11};
+
+struct A {
+  S s1;
+  S s2;
+  S a1[2];
+  S a2[2];
+  S a3[2];
+  S a4[2];
+  S a5[2];
+  S a6[2];
+  A() : s1(1, 2),
+	s2(1), // { dg-warning "missing initializer for member" }
+	a1({1}), // { dg-warning "missing initializer for member" }
+	a2({1, 2}),
+	a3({}, {}),
+	a4(),
+	a5(s),
+	a6(s, s)
+    { }
+};
+
+struct C {
+  int a1[2];
+  int a2[2];
+  int a3[2];
+  int a4[2];
+  int a5[2];
+  C() : a1(1),
+	a2(1, 2),
+	a3({1}),
+	a4({}, {}),
+	a5()
+    { }
+};
+
+struct D {
+  N n;
+  // Not an aggregate, should work pre-C++20 too.
+  D() : n(1, 2) { }
+};
+
+struct E {
+  char a1[4];
+  char a2[4];
+  E() : a1("ab"),
+	a2("abc")
+    { }
+};
+
+// Compound literal.
+struct F {
+  F ();
+  S m[1];
+};
+
+F::F () : m(__extension__(S[1]) { 1, 2 })
+{
+}
+
+struct B { int i; };
+struct Der : B { };
+Der d;
+struct G {
+  B b1[1];
+  B b2[2];
+  G(): b1(d),
+       b2(d, d)
+    { }
+};
+
+// Variation of c++/93790.
+struct Empty { };
+struct Empty_refwrap {
+  Empty& r;
+  Empty_refwrap(Empty &e) : r(e) { }
+  operator Empty&() { return r; }
+};
+
+Empty empty;
+Empty_refwrap empty_refwrap(empty);
+
+struct H {
+  Empty &e;
+  // Turning this into {empty_refwrap} would break things.
+  H() : e(empty_refwrap) { }
+};
+
+int
+main ()
+{
+  A a;
+  assert (a.s1.x == 1 && a.s1.y == 2);
+  assert (a.s2.x == 1 && a.s2.y == 0);
+  assert (a.a1[0].x == 1 && a.a1[0].y == 0
+	  && a.a1[1].x == 0 && a.a1[1].y == 0);
+  assert (a.a2[0].x == 1 && a.a2[0].y == 2
+	  && a.a2[1].x == 0 && a.a2[1].y == 0);
+  assert (a.a3[0].x == 0 && a.a3[0].y == 0
+	  && a.a3[1].x == 0 && a.a3[1].y == 0);
+  assert (a.a4[0].x == 0 && a.a4[0].y == 0
+	  && a.a4[1].x == 0 && a.a4[1].y == 0);
+  assert (a.a5[0].x == 10 && a.a5[0].y == 11
+	  && a.a5[1].x == 0 && a.a5[1].y == 0);
+  assert (a.a6[0].x == 10 && a.a6[0].y == 11
+	  && a.a6[1].x == 10 && a.a6[1].y == 11);
+
+  C c;
+  assert (c.a1[0] == 1 && c.a1[1] == 0);
+  assert (c.a2[0] == 1 && c.a2[1] == 2);
+  assert (c.a3[0] == 1 && c.a3[1] == 0);
+  assert (c.a4[0] == 0 && c.a4[1] == 0);
+  assert (c.a5[0] == 0 && c.a5[1] == 0);
+
+  E e;
+  assert (__builtin_strcmp (e.a1, "ab") == 0
+	  && __builtin_strcmp (e.a2, "abc") == 0);
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init34.C b/gcc/testsuite/g++.dg/cpp2a/paren-init34.C
new file mode 100644
index 00000000000..24942764cb7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/paren-init34.C
@@ -0,0 +1,25 @@
+// PR c++/92812
+// { dg-do compile { target c++20 } }
+// Initializing arrays in a member init list using ()-init, invalid cases.
+
+struct S { int x, y; };
+struct N { int x, y; N(int, int); };
+
+struct A {
+  N a[2];
+  A() : a(1, 2) { } // { dg-error "could not convert" }
+};
+
+struct B {
+  S a[2];
+  B() : a(1) // { dg-error "could not convert" }
+    { }
+};
+
+// Copy-initialization does not consider explicit ctors.
+struct E { explicit E(int); };
+
+struct C {
+  E a[2];
+  C() : a(4, 5) { } // { dg-error "could not convert" }
+};
diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init35.C b/gcc/testsuite/g++.dg/cpp2a/paren-init35.C
new file mode 100644
index 00000000000..4f1892742c4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/paren-init35.C
@@ -0,0 +1,21 @@
+// PR c++/92812
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-Wno-psabi" }
+// Paren-init in a member init list with vector types.
+
+typedef float __m128 __attribute__ ((__vector_size__ (16), __may_alias__));
+
+__m128 m;
+__m128 g(m);
+__m128 ag[](m, m, m);
+__m128 ag2[]({}, {}, {});
+
+struct A {
+  __m128 a1;
+  __m128 a2[2];
+  __m128 a3[2];
+  A() : a1(m),
+	a2(m, m),
+	a3({}, m)
+    { }
+};
diff --git a/gcc/testsuite/g++.dg/diagnostic/mem-init1.C b/gcc/testsuite/g++.dg/diagnostic/mem-init1.C
index b749c72cd8b..8ea5264b0e0 100644
--- a/gcc/testsuite/g++.dg/diagnostic/mem-init1.C
+++ b/gcc/testsuite/g++.dg/diagnostic/mem-init1.C
@@ -4,7 +4,7 @@
 struct A {
     A()
     : a()       // { dg-error "reference type" }
-    , b(1)      // { dg-error "incompatible" }
+    , b(1)      // { dg-error "incompatible" "" { target { ! c++20 } } }
     , c(0)      // { dg-bogus "" }
     {}
 
@@ -17,7 +17,7 @@ template<typename T, typename U>
 struct B {
     B()
     : a()       // { dg-error "reference type" }
-    , b(1)      // { dg-error "incompatible" }
+    , b(1)      // { dg-error "incompatible" "" { target { ! c++20 } } }
     , c(0)      // { dg-bogus "" }
     {}
 
diff --git a/gcc/testsuite/g++.dg/init/array28.C b/gcc/testsuite/g++.dg/init/array28.C
index 9869354279d..a75c36215f9 100644
--- a/gcc/testsuite/g++.dg/init/array28.C
+++ b/gcc/testsuite/g++.dg/init/array28.C
@@ -2,6 +2,6 @@
 
 struct Foo { explicit Foo(int) { } };
 struct Goo {
-  Goo() : x(Foo(4), Foo(5)) { } // { dg-error "" }
+  Goo() : x(Foo(4), Foo(5)) { } // { dg-error "" "" { target { ! c++20 } } }
   Foo x[2];
 };
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/crash60.C b/gcc/testsuite/g++.old-deja/g++.brendan/crash60.C
index a59d72a7bf6..1f5629b5134 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/crash60.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/crash60.C
@@ -9,4 +9,4 @@ public:
 
 // Note that we mistakenly initialize the array data member as if it
 // was scalar
-X::X () : f (0) {}// { dg-error "" } .*
+X::X () : f (0) {}// { dg-error "" "" { target { ! c++20 } } }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/init10.C b/gcc/testsuite/g++.old-deja/g++.law/init10.C
index 90e3e45d682..1a020ec730b 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/init10.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/init10.C
@@ -20,7 +20,7 @@ public:
         b();
 };
 
-b::b() : three(this)  // { dg-error "array" }
+b::b() : three(this)  // { dg-error "array|could not convert" }
 {
 }
 
diff --git a/gcc/testsuite/g++.old-deja/g++.other/array3.C b/gcc/testsuite/g++.old-deja/g++.other/array3.C
index f89090f1747..6eeeb4f3888 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/array3.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/array3.C
@@ -20,6 +20,7 @@ class B
 };
 
 B::B (const A a[])
-  : ary(a)        // { dg-error "array" }
+		  // { dg-error "could not convert|invalid conversion" "" { target { c++20 } } .+1 }
+  : ary(a)        // { dg-error "array" "" { target { ! c++20 } } }
 {
 }

base-commit: f0a3bab43fda3084eaf1bdaac58936757f30ea35
-- 
2.26.2