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

[Jason Merrill <jason@redhat.com>]

On 9/1/20 6:23 PM, Marek Polacek wrote:
> 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.
> 
> paren-init35.C also tests this with vector types.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/92812
> 	* init.c (do_paren_init_for_array_p): New.
> 	(perform_member_init): Use it.  If true, build up a CONSTRUCTOR
> 	from the list of arguments.
> 
> 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/init.c                                 |  64 ++++++++-
>   .../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 +-
>   11 files changed, 243 insertions(+), 18 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/init.c b/gcc/cp/init.c
> index d4540db3605..2edc9651ad6 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -756,6 +756,41 @@ maybe_warn_list_ctor (tree member, tree init)
>   	     "of the underlying array", member, begin);
>   }
>   
> +/* Return true if we should attempt to perform the P0960 magic when
> +   initializing an array TYPE from a parenthesized list of values LIST.  */
> +
> +static bool
> +do_paren_init_for_array_p (tree list, tree type)
> +{
> +  if (cxx_dialect < cxx20)
> +    /* P0960 is a C++20 feature.  */
> +    return false;
> +
> +  const int len = list_length (list);
> +  if (len == 0)
> +    /* Value-initialization.  */
> +    return false;
> +  else if (len > 1)
> +    /* If the list had more than one element, the code is ill-formed
> +       pre-C++20, so we should attempt to ()-init.  */
> +    return true;
> +
> +  /* Lists with one element are trickier.  */
> +  tree elt = TREE_VALUE (list);
> +
> +  /* For a("foo"), don't wrap the STRING_CST in { }.  */
> +  if (char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type)))
> +      && TREE_CODE (tree_strip_any_location_wrapper (elt)) == STRING_CST)
> +    return false;

Hmm, yet another place we need to implement the special treatment of 
strings?  Can't we factor this better?  Could there be a general e.g. 
maybe_aggregate_paren_init function to turn a list into a CONSTRUCTOR 
that's used in various places?

> +  /* Don't trigger in compiler-generated code for = default.  */
> +  if (current_function_decl && DECL_DEFAULTED_FN (current_function_decl))
> +    return false;
> +
> +  /* Handle non-standard extensions like compound literals.  */
> +  return !same_type_ignoring_top_level_qualifiers_p (type, TREE_TYPE (elt));

Isn't the defaulted function case caught by the same-type check?

Jason