c++: fix string literal member initializer bug [PR90926]

build_aggr_conv did not correctly handle string literal member initializers.
Extended can_convert_array to handle this case. The additional checks of
compatibility of character types, and whether string literal will fit, would be
quite complicated, so are deferred until the actual conversion takes place.

Testcase added for this.

Bootstrapped/regtested on x86_64-pc-linux-gnu.

gcc/cp/ChangeLog:

        PR c++/90926
        * call.c (can_convert_array): Extend to handle all valid aggregate
        initializers of an array; including by string literals, not just by
        brace-init-list.
        (build_aggr_conv): Call can_convert_array more often, not just in
        brace-init-list case.
        * g++.dg/cpp1y/nsdmi-aggr12.C: New test.

On 12/17/20 5:12 PM, Thomas Greenslade (thomgree) via Gcc-patches wrote:
> build_aggr_conv did not correctly handle string literal member initializers.
> Extended can_convert_array to handle this case. The additional checks of
> compatibility of character types, and whether string literal will fit, would be
> quite complicated, so are deferred until the actual conversion takes place.

It seems that we need to check the type, though not the length.

[over.ics.list]: "Otherwise, if the parameter type is a character array 
125 and the initializer list has a single element that is an 
appropriately-typed string-literal (9.4.2), the implicit conversion 
sequence is the identity conversion."

So this should be unambiguous:

struct A
{
   char str[10];
};

struct B
{
   char16_t str[10];
};

void f(A);
void f(B);

int
main ()
{
   f({"foo"});  // calls A overload 

   f({u"foo"}); // calls B overload 

}

You could factor the type matching code out of digest_init_r and use it 
here.

> Testcase added for this.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu.
> 
> gcc/cp/ChangeLog:
> 
>          PR c++/90926
>          * call.c (can_convert_array): Extend to handle all valid aggregate
>          initializers of an array; including by string literals, not just by
>          brace-init-list.
>          (build_aggr_conv): Call can_convert_array more often, not just in
>          brace-init-list case.
>          * g++.dg/cpp1y/nsdmi-aggr12.C: New test.
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index c2d62e582bf..e4ba31f3f2b 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -887,28 +887,41 @@ strip_standard_conversion (conversion *conv)
>     return conv;
>   }
> 
> -/* Subroutine of build_aggr_conv: check whether CTOR, a braced-init-list,
> -   is a valid aggregate initializer for array type ATYPE.  */
> +/* Subroutine of build_aggr_conv: check whether FROM is a valid aggregate
> +   initializer for array type ATYPE.  */
> 
>   static bool
> -can_convert_array (tree atype, tree ctor, int flags, tsubst_flags_t complain)
> +can_convert_array (tree atype, tree from, int flags, tsubst_flags_t complain)
>   {
> -  unsigned i;
>     tree elttype = TREE_TYPE (atype);
> -  for (i = 0; i < CONSTRUCTOR_NELTS (ctor); ++i)
> +  unsigned i;
> +
> +  if (TREE_CODE (from) == CONSTRUCTOR)
>       {
> -      tree val = CONSTRUCTOR_ELT (ctor, i)->value;
> -      bool ok;
> -      if (TREE_CODE (elttype) == ARRAY_TYPE
> -         && TREE_CODE (val) == CONSTRUCTOR)
> -       ok = can_convert_array (elttype, val, flags, complain);
> -      else
> -       ok = can_convert_arg (elttype, TREE_TYPE (val), val, flags,
> -                             complain);
> -      if (!ok)
> -       return false;
> +      for (i = 0; i < CONSTRUCTOR_NELTS (from); ++i)
> +       {
> +         tree val = CONSTRUCTOR_ELT (from, i)->value;
> +         bool ok;
> +         if (TREE_CODE (elttype) == ARRAY_TYPE)
> +           ok = can_convert_array (elttype, val, flags, complain);
> +         else
> +           ok = can_convert_arg (elttype, TREE_TYPE (val), val, flags,
> +                                 complain);
> +         if (!ok)
> +           return false;
> +       }
> +      return true;
>       }
> -  return true;
> +
> +  if (   char_type_p (TYPE_MAIN_VARIANT (elttype))
> +      && TREE_CODE (tree_strip_any_location_wrapper (from)) == STRING_CST)
> +    /* Defer the other necessary checks (compatibility of character types and
> +       whether string literal will fit) until the conversion actually takes
> +       place.  */
> +    return true;
> +
> +  /* No other valid way to aggregate initialize an array.  */
> +  return false;
>   }
> 
>   /* Helper for build_aggr_conv.  Return true if FIELD is in PSET, or if
> @@ -965,8 +978,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>                tree ftype = TREE_TYPE (idx);
>                bool ok;
> 
> -             if (TREE_CODE (ftype) == ARRAY_TYPE
> -                 && TREE_CODE (val) == CONSTRUCTOR)
> +             if (TREE_CODE (ftype) == ARRAY_TYPE)
>                  ok = can_convert_array (ftype, val, flags, complain);
>                else
>                  ok = can_convert_arg (ftype, TREE_TYPE (val), val, flags,
> @@ -1013,9 +1025,8 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>            val = empty_ctor;
>          }
> 
> -      if (TREE_CODE (ftype) == ARRAY_TYPE
> -         && TREE_CODE (val) == CONSTRUCTOR)
> -       ok = can_convert_array (ftype, val, flags, complain);
> +      if (TREE_CODE (ftype) == ARRAY_TYPE)
> +       ok = can_convert_array (ftype, val, flags, complain);
>         else
>          ok = can_convert_arg (ftype, TREE_TYPE (val), val, flags,
>                                complain);
> diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C
> new file mode 100644
> index 00000000000..ce8c95e8aca
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C
> @@ -0,0 +1,21 @@
> +// PR c++/90926
> +// { dg-do run { target c++14 } }
> +
> +struct A
> +{
> +  char str[4] = "foo";
> +  char str_array[2][4] = {"bar", "baz"};
> +};
> +
> +int
> +main ()
> +{
> +  A a;
> +  a.str[0] = 'g';
> +  a.str_array[0][0] = 'g';
> +  a = {};
> +  if (__builtin_strcmp (a.str, "foo") != 0)
> +    __builtin_abort();
> +  if (__builtin_strcmp (a.str_array[0], "bar") != 0)
> +    __builtin_abort();
> +}
>

While trying to fix the suggested overload resolution issue I run into another bug:

struct A
{
  char str[4];
};

void f(A) {};

int main ()
{
  f({"foo"});
}

Does not compile on GCC: "error: could not convert ‘{"foo"}’ from ‘<brace-enclosed initializer list>’ to ‘A’", but works fine on Clang.
Is this a known bug or a new one?

-----Original Message-----
From: Jason Merrill <jason@redhat.com> 
Sent: 22 January 2021 16:30
To: Tom Greenslade (thomgree) <thomgree@cisco.com>; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: fix string literal member initializer bug [PR90926]

On 12/17/20 5:12 PM, Thomas Greenslade (thomgree) via Gcc-patches wrote:
> build_aggr_conv did not correctly handle string literal member initializers.
> Extended can_convert_array to handle this case. The additional checks 
> of compatibility of character types, and whether string literal will 
> fit, would be quite complicated, so are deferred until the actual conversion takes place.

It seems that we need to check the type, though not the length.

[over.ics.list]: "Otherwise, if the parameter type is a character array
125 and the initializer list has a single element that is an appropriately-typed string-literal (9.4.2), the implicit conversion sequence is the identity conversion."

So this should be unambiguous:

struct A
{
   char str[10];
};

struct B
{
   char16_t str[10];
};

void f(A);
void f(B);

int
main ()
{
   f({"foo"});  // calls A overload 

   f({u"foo"}); // calls B overload 

}

You could factor the type matching code out of digest_init_r and use it here.

> Testcase added for this.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu.
> 
> gcc/cp/ChangeLog:
> 
>          PR c++/90926
>          * call.c (can_convert_array): Extend to handle all valid aggregate
>          initializers of an array; including by string literals, not just by
>          brace-init-list.
>          (build_aggr_conv): Call can_convert_array more often, not just in
>          brace-init-list case.
>          * g++.dg/cpp1y/nsdmi-aggr12.C: New test.
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 
> c2d62e582bf..e4ba31f3f2b 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -887,28 +887,41 @@ strip_standard_conversion (conversion *conv)
>     return conv;
>   }
> 
> -/* Subroutine of build_aggr_conv: check whether CTOR, a braced-init-list,
> -   is a valid aggregate initializer for array type ATYPE.  */
> +/* Subroutine of build_aggr_conv: check whether FROM is a valid aggregate
> +   initializer for array type ATYPE.  */
> 
>   static bool
> -can_convert_array (tree atype, tree ctor, int flags, tsubst_flags_t 
> complain)
> +can_convert_array (tree atype, tree from, int flags, tsubst_flags_t 
> +complain)
>   {
> -  unsigned i;
>     tree elttype = TREE_TYPE (atype);
> -  for (i = 0; i < CONSTRUCTOR_NELTS (ctor); ++i)
> +  unsigned i;
> +
> +  if (TREE_CODE (from) == CONSTRUCTOR)
>       {
> -      tree val = CONSTRUCTOR_ELT (ctor, i)->value;
> -      bool ok;
> -      if (TREE_CODE (elttype) == ARRAY_TYPE
> -         && TREE_CODE (val) == CONSTRUCTOR)
> -       ok = can_convert_array (elttype, val, flags, complain);
> -      else
> -       ok = can_convert_arg (elttype, TREE_TYPE (val), val, flags,
> -                             complain);
> -      if (!ok)
> -       return false;
> +      for (i = 0; i < CONSTRUCTOR_NELTS (from); ++i)
> +       {
> +         tree val = CONSTRUCTOR_ELT (from, i)->value;
> +         bool ok;
> +         if (TREE_CODE (elttype) == ARRAY_TYPE)
> +           ok = can_convert_array (elttype, val, flags, complain);
> +         else
> +           ok = can_convert_arg (elttype, TREE_TYPE (val), val, flags,
> +                                 complain);
> +         if (!ok)
> +           return false;
> +       }
> +      return true;
>       }
> -  return true;
> +
> +  if (   char_type_p (TYPE_MAIN_VARIANT (elttype))
> +      && TREE_CODE (tree_strip_any_location_wrapper (from)) == STRING_CST)
> +    /* Defer the other necessary checks (compatibility of character types and
> +       whether string literal will fit) until the conversion actually takes
> +       place.  */
> +    return true;
> +
> +  /* No other valid way to aggregate initialize an array.  */  return 
> + false;
>   }
> 
>   /* Helper for build_aggr_conv.  Return true if FIELD is in PSET, or 
> if @@ -965,8 +978,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>                tree ftype = TREE_TYPE (idx);
>                bool ok;
> 
> -             if (TREE_CODE (ftype) == ARRAY_TYPE
> -                 && TREE_CODE (val) == CONSTRUCTOR)
> +             if (TREE_CODE (ftype) == ARRAY_TYPE)
>                  ok = can_convert_array (ftype, val, flags, complain);
>                else
>                  ok = can_convert_arg (ftype, TREE_TYPE (val), val, 
> flags, @@ -1013,9 +1025,8 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>            val = empty_ctor;
>          }
> 
> -      if (TREE_CODE (ftype) == ARRAY_TYPE
> -         && TREE_CODE (val) == CONSTRUCTOR)
> -       ok = can_convert_array (ftype, val, flags, complain);
> +      if (TREE_CODE (ftype) == ARRAY_TYPE)
> +       ok = can_convert_array (ftype, val, flags, complain);
>         else
>          ok = can_convert_arg (ftype, TREE_TYPE (val), val, flags,
>                                complain); diff --git 
> a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C 
> b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C
> new file mode 100644
> index 00000000000..ce8c95e8aca
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C
> @@ -0,0 +1,21 @@
> +// PR c++/90926
> +// { dg-do run { target c++14 } }
> +
> +struct A
> +{
> +  char str[4] = "foo";
> +  char str_array[2][4] = {"bar", "baz"}; };
> +
> +int
> +main ()
> +{
> +  A a;
> +  a.str[0] = 'g';
> +  a.str_array[0][0] = 'g';
> +  a = {};
> +  if (__builtin_strcmp (a.str, "foo") != 0)
> +    __builtin_abort();
> +  if (__builtin_strcmp (a.str_array[0], "bar") != 0)
> +    __builtin_abort();
> +}
>

On 1/28/21 9:54 AM, Tom Greenslade (thomgree) wrote:
> While trying to fix the suggested overload resolution issue I run into another bug:
> 
> struct A
> {
>    char str[4];
> };
> 
> void f(A) {};
> 
> int main ()
> {
>    f({"foo"});
> }
> 
> Does not compile on GCC: "error: could not convert ‘{"foo"}’ from ‘<brace-enclosed initializer list>’ to ‘A’", but works fine on Clang.
> Is this a known bug or a new one?

That looks like the same bug you're fixing with this patch.

> -----Original Message-----
> From: Jason Merrill <jason@redhat.com>
> Sent: 22 January 2021 16:30
> To: Tom Greenslade (thomgree) <thomgree@cisco.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] c++: fix string literal member initializer bug [PR90926]
> 
> On 12/17/20 5:12 PM, Thomas Greenslade (thomgree) via Gcc-patches wrote:
>> build_aggr_conv did not correctly handle string literal member initializers.
>> Extended can_convert_array to handle this case. The additional checks
>> of compatibility of character types, and whether string literal will
>> fit, would be quite complicated, so are deferred until the actual conversion takes place.
> 
> It seems that we need to check the type, though not the length.
> 
> [over.ics.list]: "Otherwise, if the parameter type is a character array
> 125 and the initializer list has a single element that is an appropriately-typed string-literal (9.4.2), the implicit conversion sequence is the identity conversion."
> 
> So this should be unambiguous:
> 
> struct A
> {
>     char str[10];
> };
> 
> struct B
> {
>     char16_t str[10];
> };
> 
> void f(A);
> void f(B);
> 
> int
> main ()
> {
>     f({"foo"});  // calls A overload
> 
>     f({u"foo"}); // calls B overload
> 
> }
> 
> You could factor the type matching code out of digest_init_r and use it here.
> 
>> Testcase added for this.
>>
>> Bootstrapped/regtested on x86_64-pc-linux-gnu.
>>
>> gcc/cp/ChangeLog:
>>
>>           PR c++/90926
>>           * call.c (can_convert_array): Extend to handle all valid aggregate
>>           initializers of an array; including by string literals, not just by
>>           brace-init-list.
>>           (build_aggr_conv): Call can_convert_array more often, not just in
>>           brace-init-list case.
>>           * g++.dg/cpp1y/nsdmi-aggr12.C: New test.
>>
>> diff --git a/gcc/cp/call.c b/gcc/cp/call.c index
>> c2d62e582bf..e4ba31f3f2b 100644
>> --- a/gcc/cp/call.c
>> +++ b/gcc/cp/call.c
>> @@ -887,28 +887,41 @@ strip_standard_conversion (conversion *conv)
>>      return conv;
>>    }
>>
>> -/* Subroutine of build_aggr_conv: check whether CTOR, a braced-init-list,
>> -   is a valid aggregate initializer for array type ATYPE.  */
>> +/* Subroutine of build_aggr_conv: check whether FROM is a valid aggregate
>> +   initializer for array type ATYPE.  */
>>
>>    static bool
>> -can_convert_array (tree atype, tree ctor, int flags, tsubst_flags_t
>> complain)
>> +can_convert_array (tree atype, tree from, int flags, tsubst_flags_t
>> +complain)
>>    {
>> -  unsigned i;
>>      tree elttype = TREE_TYPE (atype);
>> -  for (i = 0; i < CONSTRUCTOR_NELTS (ctor); ++i)
>> +  unsigned i;
>> +
>> +  if (TREE_CODE (from) == CONSTRUCTOR)
>>        {
>> -      tree val = CONSTRUCTOR_ELT (ctor, i)->value;
>> -      bool ok;
>> -      if (TREE_CODE (elttype) == ARRAY_TYPE
>> -         && TREE_CODE (val) == CONSTRUCTOR)
>> -       ok = can_convert_array (elttype, val, flags, complain);
>> -      else
>> -       ok = can_convert_arg (elttype, TREE_TYPE (val), val, flags,
>> -                             complain);
>> -      if (!ok)
>> -       return false;
>> +      for (i = 0; i < CONSTRUCTOR_NELTS (from); ++i)
>> +       {
>> +         tree val = CONSTRUCTOR_ELT (from, i)->value;
>> +         bool ok;
>> +         if (TREE_CODE (elttype) == ARRAY_TYPE)
>> +           ok = can_convert_array (elttype, val, flags, complain);
>> +         else
>> +           ok = can_convert_arg (elttype, TREE_TYPE (val), val, flags,
>> +                                 complain);
>> +         if (!ok)
>> +           return false;
>> +       }
>> +      return true;
>>        }
>> -  return true;
>> +
>> +  if (   char_type_p (TYPE_MAIN_VARIANT (elttype))
>> +      && TREE_CODE (tree_strip_any_location_wrapper (from)) == STRING_CST)
>> +    /* Defer the other necessary checks (compatibility of character types and
>> +       whether string literal will fit) until the conversion actually takes
>> +       place.  */
>> +    return true;
>> +
>> +  /* No other valid way to aggregate initialize an array.  */  return
>> + false;
>>    }
>>
>>    /* Helper for build_aggr_conv.  Return true if FIELD is in PSET, or
>> if @@ -965,8 +978,7 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>>                 tree ftype = TREE_TYPE (idx);
>>                 bool ok;
>>
>> -             if (TREE_CODE (ftype) == ARRAY_TYPE
>> -                 && TREE_CODE (val) == CONSTRUCTOR)
>> +             if (TREE_CODE (ftype) == ARRAY_TYPE)
>>                   ok = can_convert_array (ftype, val, flags, complain);
>>                 else
>>                   ok = can_convert_arg (ftype, TREE_TYPE (val), val,
>> flags, @@ -1013,9 +1025,8 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
>>             val = empty_ctor;
>>           }
>>
>> -      if (TREE_CODE (ftype) == ARRAY_TYPE
>> -         && TREE_CODE (val) == CONSTRUCTOR)
>> -       ok = can_convert_array (ftype, val, flags, complain);
>> +      if (TREE_CODE (ftype) == ARRAY_TYPE)
>> +       ok = can_convert_array (ftype, val, flags, complain);
>>          else
>>           ok = can_convert_arg (ftype, TREE_TYPE (val), val, flags,
>>                                 complain); diff --git
>> a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C
>> b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C
>> new file mode 100644
>> index 00000000000..ce8c95e8aca
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr12.C
>> @@ -0,0 +1,21 @@
>> +// PR c++/90926
>> +// { dg-do run { target c++14 } }
>> +
>> +struct A
>> +{
>> +  char str[4] = "foo";
>> +  char str_array[2][4] = {"bar", "baz"}; };
>> +
>> +int
>> +main ()
>> +{
>> +  A a;
>> +  a.str[0] = 'g';
>> +  a.str_array[0][0] = 'g';
>> +  a = {};
>> +  if (__builtin_strcmp (a.str, "foo") != 0)
>> +    __builtin_abort();
>> +  if (__builtin_strcmp (a.str_array[0], "bar") != 0)
>> +    __builtin_abort();
>> +}
>>
>