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

[Jason Merrill via Gcc-patches]

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 <ja...@redhat.com>
Sent: 22 January 2021 16:30
To: Tom Greenslade (thomgree) <thomg...@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();
+}