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

2021-02-03 Thread Tom Greenslade (thomgree) via Gcc-patches
Update of https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562259.html

build_aggr_conv did not correctly handle string literal member initializers.
Extended can_convert_array to handle this case. For the additional check of
compatibility of character types, factored out code from digest_init_r into a 
new function.

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.
* typeck2.c (character_array_from_string_literal): New function.
(digest_init_r): call character_array_from_string_literal
* cp-tree.h: (character_array_from_string_literal): Declare.
* g++.dg/cpp1y/nsdmi-aggr12.C: New test.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 87a7af12796..b917c67204f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -895,28 +895,40 @@ 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)
+{ 
+  return character_array_from_string_literal(atype, from);
+}
+
+  /* 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
@@ -973,8 +985,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,
@@ -1021,9 +1032,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/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f31319904eb..8dfc581 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7946,6 +7946,7 @@ extern tree split_nonconstant_init(tree, 
tree);
 extern bool check_narrowing(tree, tree, tsubst_flags_t,
 bool = false);
 extern bool ordinary_char_type_p   (tree);
+extern bool character_array_from_string_literal (tree, tree);
 extern tree digest_init(tree, tree, 
tsubst_flags_t);
 extern tree digest_init_flags  (tree, tree, int, 
tsubst_flags_t);
 extern tree digest_nsdmi_init  (tree, tree, tsubst_flags_t);
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 9ba2897390a..8fbabeb46d9 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1003,6 +1003,28 @@ ordinary_char_type_p (tree type)
  || type == unsigned_char_type_node);
 }
 
+/* Checks if char

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

2021-01-28 Thread Tom Greenslade (thomgree) via Gcc-patches
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 
‘’ to ‘A’", but works fine on Clang.
Is this a known bug or a new one?

-Original Message-
From: Jason Merrill  
Sent: 22 January 2021 16:30
To: Tom Greenslade (thomgree) ; 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
> - && 

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

2021-01-21 Thread Tom Greenslade (thomgree) via Gcc-patches
Ping for this patch:
https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562259.html

-Original Message-
From: Thomas Greenslade (thomgree) 
Sent: 17 December 2020 22:12
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] 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.

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 000..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();
+}