public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Call braced_list_to_string after array size is fixed
@ 2018-08-17 12:01 Bernd Edlinger
  2018-08-22 13:18 ` Bernd Edlinger
  0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2018-08-17 12:01 UTC (permalink / raw)
  To: gcc-patches, Jeff Law, Richard Biener, Jason Merrill,
	Joseph Myers, Martin Sebor

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

Hi,

Yes I know Martin will see this as a capital offense, but to my excuse I must say
it only happened in self defense :-)

As my other patch series depends on STRNG_CSTs with certain properties, it got
broken by the way the string constants are created now, due to the 71625 patch.

In order to be able to bootstrap my patch with current trunk, I had to move
the point where the char array constructor is converted to a STRING_CST to the
point directly after the array bounds are fixed, so that it is possible to create
the string constant (with implicit NUL termination) like all other C strings, without
making the array larger.

I added a test case for a crash with an invalid string intializer (in C++).
and removed an xfail in the new test case string2.C.  This way the C++ diagnostics
matches the state before 71625 was applied.

This does not fix the other problems with 71625, but I don't think they are related
to the way the string constants are created, so there should be no conflict.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-bracedstr.diff --]
[-- Type: text/x-patch; name="patch-bracedstr.diff", Size: 11325 bytes --]

c-family:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-common.c (braced_list_to_string): Remove eval parameter.
	Add some more checks.  Always create zero-terminated STRING_CST.
	* c-common.h (braced_list_to_string): Adjust prototype.

c:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-decl.c (finish_decl): Call braced_list_to_string here ...
	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.


cp:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* decl.c (eval_check_narrowing): Remove.
	(check_initializer): Move call to braced_list_to_string from here ...
	* typeck2.c (store_init_value): ... to here.
	(digest_init_r): Remove handing of signed/unsigned char strings.

testsuite:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-c++-common/array-init.c: New test.
	* g++.dg/init/string2.C: Remove xfail.


Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 263554)
+++ gcc/c/c-decl.c	(working copy)
@@ -5011,6 +5011,17 @@ finish_decl (tree decl, location_t init_
       relayout_decl (decl);
     }
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && DECL_INITIAL (decl)
+      && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR)
+    {
+      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (typ1 == char_type_node
+	  || typ1 == signed_char_type_node
+	  || typ1 == unsigned_char_type_node)
+	DECL_INITIAL (decl) = braced_list_to_string (type, DECL_INITIAL (decl));
+    }
+
   if (VAR_P (decl))
     {
       if (init && TREE_CODE (init) == CONSTRUCTOR)
Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 263554)
+++ gcc/c/c-parser.c	(working copy)
@@ -2126,15 +2126,6 @@ c_parser_declaration_or_fndef (c_parser
 	      if (d != error_mark_node)
 		{
 		  maybe_warn_string_init (init_loc, TREE_TYPE (d), init);
-
-		  /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-		  tree valtype = TREE_TYPE (init.value);
-		  if (TREE_CODE (init.value) == CONSTRUCTOR
-		      && TREE_CODE (valtype) == ARRAY_TYPE
-		      && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
-		    if (tree str = braced_list_to_string (valtype, init.value))
-		      init.value = str;
-
 		  finish_decl (d, init_loc, init.value,
 			       init.original_type, asm_name);
 		}
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 263554)
+++ gcc/c-family/c-common.c	(working copy)
@@ -8510,39 +8510,24 @@ maybe_add_include_fixit (rich_location *
 }
 
 /* Attempt to convert a braced array initializer list CTOR for array
-   TYPE into a STRING_CST for convenience and efficiency.  When non-null,
-   use EVAL to attempt to evalue constants (used by C++).  Return
-   the converted string on success or null on failure.  */
+   TYPE into a STRING_CST for convenience and efficiency.  Return
+   the converted string on success or the original ctor on failure.  */
 
 tree
-braced_list_to_string (tree type, tree ctor, tree (*eval)(tree, tree))
+braced_list_to_string (tree type, tree ctor)
 {
   unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
 
-  /* If the array has an explicit bound, use it to constrain the size
-     of the string.  If it doesn't, be sure to create a string that's
-     as long as implied by the index of the last zero specified via
-     a designator, as in:
-       const char a[] = { [7] = 0 };  */
+  /* When this function is called, the array has already an explicit bound,
+     but in the case of VLAs it can be a variable length.
+     See testsuite/g++.dg/ext/vla19.C for an example.
+     Allow this conversion to succeed in this case.  */
   unsigned HOST_WIDE_INT maxelts = HOST_WIDE_INT_M1U;
-  if (tree size = TYPE_SIZE_UNIT (type))
+  if (tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
     {
-      if (tree_fits_uhwi_p (size))
-	{
-	  maxelts = tree_to_uhwi (size);
-	  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
-
-	  /* Avoid converting initializers for zero-length arrays.  */
-	  if (!maxelts)
-	    return NULL_TREE;
-	}
+      maxelts = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+      maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
     }
-  else if (!nelts)
-    /* Avoid handling the undefined/erroneous case of an empty
-       initializer for an arrays with unspecified bound.  */
-    return NULL_TREE;
-
-  tree eltype = TREE_TYPE (type);
 
   auto_vec<char> str;
   str.reserve (nelts + 1);
@@ -8552,19 +8537,21 @@ braced_list_to_string (tree type, tree c
 
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), i, index, value)
     {
-      unsigned HOST_WIDE_INT idx = index ? tree_to_uhwi (index) : i;
+      unsigned HOST_WIDE_INT idx = i;
+      if (index)
+	{
+	  if (!tree_fits_uhwi_p (index))
+	    return ctor;
+	  idx = tree_to_uhwi (index);
+	}
 
       /* auto_vec is limited to UINT_MAX elements.  */
       if (idx > UINT_MAX)
-	return NULL_TREE;
+	return ctor;
 
-      /* Attempt to evaluate constants.  */
-      if (eval)
-	value = eval (eltype, value);
-
-      /* Avoid non-constant initializers.  */
+     /* Avoid non-constant initializers.  */
      if (!tree_fits_shwi_p (value))
-	return NULL_TREE;
+	return ctor;
 
       /* Skip over embedded nuls except the last one (initializer
 	 elements are in ascending order of indices).  */
@@ -8572,11 +8559,14 @@ braced_list_to_string (tree type, tree c
       if (!val && i + 1 < nelts)
 	continue;
 
+      if (idx < str.length())
+	return ctor;
+
       /* Bail if the CTOR has a block of more than 256 embedded nuls
 	 due to implicitly initialized elements.  */
       unsigned nchars = (idx - str.length ()) + 1;
       if (nchars > 256)
-	return NULL_TREE;
+	return ctor;
 
       if (nchars > 1)
 	{
@@ -8585,17 +8575,15 @@ braced_list_to_string (tree type, tree c
 	}
 
       if (idx > maxelts)
-	return NULL_TREE;
+	return ctor;
 
       str.safe_insert (idx, val);
     }
 
-  if (!nelts)
-    /* Append a nul for the empty initializer { }.  */
-    str.safe_push (0);
+  /* Append a nul string termination.  */
+  str.safe_push (0);
 
-  /* Build a STRING_CST with the same type as the array, which
-     may be an array of unknown bound.  */
+  /* Build a STRING_CST with the same type as the array */
   tree res = build_string (str.length (), str.begin ());
   TREE_TYPE (res) = type;
   return res;
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 263554)
+++ gcc/c-family/c-common.h	(working copy)
@@ -1331,7 +1331,7 @@ extern void maybe_add_include_fixit (ric
 extern void maybe_suggest_missing_token_insertion (rich_location *richloc,
 						   enum cpp_ttype token_type,
 						   location_t prev_token_loc);
-extern tree braced_list_to_string (tree, tree, tree (*)(tree, tree) = NULL);
+extern tree braced_list_to_string (tree, tree);
 
 #if CHECKING_P
 namespace selftest {
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 263554)
+++ gcc/cp/decl.c	(working copy)
@@ -6282,30 +6282,6 @@ build_aggr_init_full_exprs (tree decl, t
   return build_aggr_init (decl, init, flags, tf_warning_or_error);
 }
 
-/* Attempt to determine the constant VALUE of integral type and convert
-   it to TYPE, issuing narrowing warnings/errors as necessary.  Return
-   the constant result or null on failure.  Callback for
-   braced_list_to_string.  */
-
-static tree
-eval_check_narrowing (tree type, tree value)
-{
-  if (tree valtype = TREE_TYPE (value))
-    {
-      if (TREE_CODE (valtype) != INTEGER_TYPE)
-	return NULL_TREE;
-    }
-  else
-    return NULL_TREE;
-
-  value = scalar_constant_value (value);
-  if (!value)
-    return NULL_TREE;
-
-  check_narrowing (type, value, tf_warning_or_error);
-  return value;
-}
-
 /* Verify INIT (the initializer for DECL), and record the
    initialization in DECL_INITIAL, if appropriate.  CLEANUP is as for
    grok_reference_init.
@@ -6421,17 +6397,7 @@ check_initializer (tree decl, tree init,
 	    }
 	  else
 	    {
-	      /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-	      tree valtype = TREE_TYPE (decl);
-	      if (TREE_CODE (valtype) == ARRAY_TYPE
-		  && TYPE_STRING_FLAG (TREE_TYPE (valtype))
-		  && BRACE_ENCLOSED_INITIALIZER_P (init))
-		if (tree str = braced_list_to_string (valtype, init,
-						      eval_check_narrowing))
-		  init = str;
-
-	      if (TREE_CODE (init) != STRING_CST)
-		init = reshape_init (type, init, tf_warning_or_error);
+	      init = reshape_init (type, init, tf_warning_or_error);
 	      flags |= LOOKUP_NO_NARROWING;
 	    }
 	}
Index: gcc/cp/typeck2.c
===================================================================
--- gcc/cp/typeck2.c	(revision 263554)
+++ gcc/cp/typeck2.c	(working copy)
@@ -813,6 +813,16 @@ store_init_value (tree decl, tree init, vec<tree,
     /* Digest the specified initializer into an expression.  */
     value = digest_init_flags (type, init, flags, tf_warning_or_error);
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TREE_CODE (value) == CONSTRUCTOR)
+    {
+      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (typ1 == char_type_node
+	 || typ1 == signed_char_type_node
+	 || typ1 == unsigned_char_type_node)
+	value = braced_list_to_string (type, value);
+    }
+
   value = extend_ref_init_temps (decl, value, cleanups);
 
   /* In C++11 constant expression is a semantic, not syntactic, property.
@@ -1056,9 +1066,7 @@ digest_init_r (tree type, tree init, int
 
 	  if (TYPE_PRECISION (typ1) == BITS_PER_UNIT)
 	    {
-	      if (char_type != char_type_node
-		  && char_type != signed_char_type_node
-		  && char_type != unsigned_char_type_node)
+	      if (char_type != char_type_node)
 		{
 		  if (complain & tf_error)
 		    error_at (loc, "char-array initialized from wide string");
Index: gcc/testsuite/c-c++-common/array-init.c
===================================================================
--- gcc/testsuite/c-c++-common/array-init.c	(revision 0)
+++ gcc/testsuite/c-c++-common/array-init.c	(working copy)
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-prune-output "sorry, unimplemented: non-trivial designated initializers not supported" } */
+
+char x[] = { [-1] = 1, 2, 3 }; /* { dg-error "array index in initializer exceeds array bounds" "" { target c } } */
Index: gcc/testsuite/g++.dg/init/string2.C
===================================================================
--- gcc/testsuite/g++.dg/init/string2.C	(revision 263554)
+++ gcc/testsuite/g++.dg/init/string2.C	(working copy)
@@ -54,7 +54,7 @@ template <class T>
 int tmplen ()
 {
   static const T
-    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
+    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
   return __builtin_strlen (a);
 }
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Call braced_list_to_string after array size is fixed
  2018-08-17 12:01 [PATCH] Call braced_list_to_string after array size is fixed Bernd Edlinger
@ 2018-08-22 13:18 ` Bernd Edlinger
  2018-08-24 19:52   ` [PATCHv2] " Bernd Edlinger
  0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2018-08-22 13:18 UTC (permalink / raw)
  To: gcc-patches, Jeff Law, Richard Biener, Jason Merrill,
	Joseph Myers, Martin Sebor

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

Hi,

this is a minor update to the previous version, which does no longer try
to fold the array initializer of VLAs, due to this initializer hitting
an assertion in my varasm patch, I believe it is certainly not worth the
risk to do that optimization with VLAs.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

On 08/17/18 14:01, Bernd Edlinger wrote:
> Hi,
> 
> Yes I know Martin will see this as a capital offense, but to my excuse I must say
> it only happened in self defense :-)
> 
> As my other patch series depends on STRNG_CSTs with certain properties, it got
> broken by the way the string constants are created now, due to the 71625 patch.
> 
> In order to be able to bootstrap my patch with current trunk, I had to move
> the point where the char array constructor is converted to a STRING_CST to the
> point directly after the array bounds are fixed, so that it is possible to create
> the string constant (with implicit NUL termination) like all other C strings, without
> making the array larger.
> 
> I added a test case for a crash with an invalid string intializer (in C++).
> and removed an xfail in the new test case string2.C.  This way the C++ diagnostics
> matches the state before 71625 was applied.
> 
> This does not fix the other problems with 71625, but I don't think they are related
> to the way the string constants are created, so there should be no conflict.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-bracedstr.diff --]
[-- Type: text/x-patch; name="patch-bracedstr.diff", Size: 11169 bytes --]

c-family:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-common.c (braced_list_to_string): Remove eval parameter.
	Add some more checks.  Always create zero-terminated STRING_CST.
	* c-common.h (braced_list_to_string): Adjust prototype.

c:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-decl.c (finish_decl): Call braced_list_to_string here ...
	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.


cp:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* decl.c (eval_check_narrowing): Remove.
	(check_initializer): Move call to braced_list_to_string from here ...
	* typeck2.c (store_init_value): ... to here.
	(digest_init_r): Remove handing of signed/unsigned char strings.

testsuite:
2018-08-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-c++-common/array-init.c: New test.
	* g++.dg/init/string2.C: Remove xfail.


Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 263554)
+++ gcc/c/c-decl.c	(working copy)
@@ -5011,6 +5011,17 @@ finish_decl (tree decl, location_t init_
       relayout_decl (decl);
     }
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && DECL_INITIAL (decl)
+      && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR)
+    {
+      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (typ1 == char_type_node
+	  || typ1 == signed_char_type_node
+	  || typ1 == unsigned_char_type_node)
+	DECL_INITIAL (decl) = braced_list_to_string (type, DECL_INITIAL (decl));
+    }
+
   if (VAR_P (decl))
     {
       if (init && TREE_CODE (init) == CONSTRUCTOR)
Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 263554)
+++ gcc/c/c-parser.c	(working copy)
@@ -2126,15 +2126,6 @@ c_parser_declaration_or_fndef (c_parser
 	      if (d != error_mark_node)
 		{
 		  maybe_warn_string_init (init_loc, TREE_TYPE (d), init);
-
-		  /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-		  tree valtype = TREE_TYPE (init.value);
-		  if (TREE_CODE (init.value) == CONSTRUCTOR
-		      && TREE_CODE (valtype) == ARRAY_TYPE
-		      && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
-		    if (tree str = braced_list_to_string (valtype, init.value))
-		      init.value = str;
-
 		  finish_decl (d, init_loc, init.value,
 			       init.original_type, asm_name);
 		}
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 263554)
+++ gcc/c-family/c-common.c	(working copy)
@@ -8510,39 +8510,24 @@ maybe_add_include_fixit (rich_location *
 }
 
 /* Attempt to convert a braced array initializer list CTOR for array
-   TYPE into a STRING_CST for convenience and efficiency.  When non-null,
-   use EVAL to attempt to evalue constants (used by C++).  Return
-   the converted string on success or null on failure.  */
+   TYPE into a STRING_CST for convenience and efficiency.  Return
+   the converted string on success or the original ctor on failure.  */
 
 tree
-braced_list_to_string (tree type, tree ctor, tree (*eval)(tree, tree))
+braced_list_to_string (tree type, tree ctor)
 {
-  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
+  if (!tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
+    return ctor;
 
   /* If the array has an explicit bound, use it to constrain the size
      of the string.  If it doesn't, be sure to create a string that's
      as long as implied by the index of the last zero specified via
      a designator, as in:
        const char a[] = { [7] = 0 };  */
-  unsigned HOST_WIDE_INT maxelts = HOST_WIDE_INT_M1U;
-  if (tree size = TYPE_SIZE_UNIT (type))
-    {
-      if (tree_fits_uhwi_p (size))
-	{
-	  maxelts = tree_to_uhwi (size);
-	  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
-
-	  /* Avoid converting initializers for zero-length arrays.  */
-	  if (!maxelts)
-	    return NULL_TREE;
-	}
-    }
-  else if (!nelts)
-    /* Avoid handling the undefined/erroneous case of an empty
-       initializer for an arrays with unspecified bound.  */
-    return NULL_TREE;
+  unsigned HOST_WIDE_INT maxelts = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
 
-  tree eltype = TREE_TYPE (type);
+  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
 
   auto_vec<char> str;
   str.reserve (nelts + 1);
@@ -8552,19 +8537,21 @@ braced_list_to_string (tree type, tree c
 
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), i, index, value)
     {
-      unsigned HOST_WIDE_INT idx = index ? tree_to_uhwi (index) : i;
+      unsigned HOST_WIDE_INT idx = i;
+      if (index)
+	{
+	  if (!tree_fits_uhwi_p (index))
+	    return ctor;
+	  idx = tree_to_uhwi (index);
+	}
 
       /* auto_vec is limited to UINT_MAX elements.  */
       if (idx > UINT_MAX)
-	return NULL_TREE;
+	return ctor;
 
-      /* Attempt to evaluate constants.  */
-      if (eval)
-	value = eval (eltype, value);
-
-      /* Avoid non-constant initializers.  */
+     /* Avoid non-constant initializers.  */
      if (!tree_fits_shwi_p (value))
-	return NULL_TREE;
+	return ctor;
 
       /* Skip over embedded nuls except the last one (initializer
 	 elements are in ascending order of indices).  */
@@ -8572,11 +8559,14 @@ braced_list_to_string (tree type, tree c
       if (!val && i + 1 < nelts)
 	continue;
 
+      if (idx < str.length())
+	return ctor;
+
       /* Bail if the CTOR has a block of more than 256 embedded nuls
 	 due to implicitly initialized elements.  */
       unsigned nchars = (idx - str.length ()) + 1;
       if (nchars > 256)
-	return NULL_TREE;
+	return ctor;
 
       if (nchars > 1)
 	{
@@ -8585,17 +8575,15 @@ braced_list_to_string (tree type, tree c
 	}
 
       if (idx > maxelts)
-	return NULL_TREE;
+	return ctor;
 
       str.safe_insert (idx, val);
     }
 
-  if (!nelts)
-    /* Append a nul for the empty initializer { }.  */
-    str.safe_push (0);
+  /* Append a nul string termination.  */
+  str.safe_push (0);
 
-  /* Build a STRING_CST with the same type as the array, which
-     may be an array of unknown bound.  */
+  /* Build a STRING_CST with the same type as the array */
   tree res = build_string (str.length (), str.begin ());
   TREE_TYPE (res) = type;
   return res;
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 263554)
+++ gcc/c-family/c-common.h	(working copy)
@@ -1331,7 +1331,7 @@ extern void maybe_add_include_fixit (ric
 extern void maybe_suggest_missing_token_insertion (rich_location *richloc,
 						   enum cpp_ttype token_type,
 						   location_t prev_token_loc);
-extern tree braced_list_to_string (tree, tree, tree (*)(tree, tree) = NULL);
+extern tree braced_list_to_string (tree, tree);
 
 #if CHECKING_P
 namespace selftest {
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 263554)
+++ gcc/cp/decl.c	(working copy)
@@ -6282,30 +6282,6 @@ build_aggr_init_full_exprs (tree decl, t
   return build_aggr_init (decl, init, flags, tf_warning_or_error);
 }
 
-/* Attempt to determine the constant VALUE of integral type and convert
-   it to TYPE, issuing narrowing warnings/errors as necessary.  Return
-   the constant result or null on failure.  Callback for
-   braced_list_to_string.  */
-
-static tree
-eval_check_narrowing (tree type, tree value)
-{
-  if (tree valtype = TREE_TYPE (value))
-    {
-      if (TREE_CODE (valtype) != INTEGER_TYPE)
-	return NULL_TREE;
-    }
-  else
-    return NULL_TREE;
-
-  value = scalar_constant_value (value);
-  if (!value)
-    return NULL_TREE;
-
-  check_narrowing (type, value, tf_warning_or_error);
-  return value;
-}
-
 /* Verify INIT (the initializer for DECL), and record the
    initialization in DECL_INITIAL, if appropriate.  CLEANUP is as for
    grok_reference_init.
@@ -6421,17 +6397,7 @@ check_initializer (tree decl, tree init,
 	    }
 	  else
 	    {
-	      /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-	      tree valtype = TREE_TYPE (decl);
-	      if (TREE_CODE (valtype) == ARRAY_TYPE
-		  && TYPE_STRING_FLAG (TREE_TYPE (valtype))
-		  && BRACE_ENCLOSED_INITIALIZER_P (init))
-		if (tree str = braced_list_to_string (valtype, init,
-						      eval_check_narrowing))
-		  init = str;
-
-	      if (TREE_CODE (init) != STRING_CST)
-		init = reshape_init (type, init, tf_warning_or_error);
+	      init = reshape_init (type, init, tf_warning_or_error);
 	      flags |= LOOKUP_NO_NARROWING;
 	    }
 	}
Index: gcc/cp/typeck2.c
===================================================================
--- gcc/cp/typeck2.c	(revision 263554)
+++ gcc/cp/typeck2.c	(working copy)
@@ -813,6 +813,16 @@ store_init_value (tree decl, tree init, vec<tree,
     /* Digest the specified initializer into an expression.  */
     value = digest_init_flags (type, init, flags, tf_warning_or_error);
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TREE_CODE (value) == CONSTRUCTOR)
+    {
+      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (typ1 == char_type_node
+	 || typ1 == signed_char_type_node
+	 || typ1 == unsigned_char_type_node)
+	value = braced_list_to_string (type, value);
+    }
+
   value = extend_ref_init_temps (decl, value, cleanups);
 
   /* In C++11 constant expression is a semantic, not syntactic, property.
@@ -1056,9 +1066,7 @@ digest_init_r (tree type, tree init, int
 
 	  if (TYPE_PRECISION (typ1) == BITS_PER_UNIT)
 	    {
-	      if (char_type != char_type_node
-		  && char_type != signed_char_type_node
-		  && char_type != unsigned_char_type_node)
+	      if (char_type != char_type_node)
 		{
 		  if (complain & tf_error)
 		    error_at (loc, "char-array initialized from wide string");
Index: gcc/testsuite/c-c++-common/array-init.c
===================================================================
--- gcc/testsuite/c-c++-common/array-init.c	(revision 0)
+++ gcc/testsuite/c-c++-common/array-init.c	(working copy)
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-prune-output "sorry, unimplemented: non-trivial designated initializers not supported" } */
+
+char x[] = { [-1] = 1, 2, 3 }; /* { dg-error "array index in initializer exceeds array bounds" "" { target c } } */
Index: gcc/testsuite/g++.dg/init/string2.C
===================================================================
--- gcc/testsuite/g++.dg/init/string2.C	(revision 263554)
+++ gcc/testsuite/g++.dg/init/string2.C	(working copy)
@@ -54,7 +54,7 @@ template <class T>
 int tmplen ()
 {
   static const T
-    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
+    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
   return __builtin_strlen (a);
 }
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-22 13:18 ` Bernd Edlinger
@ 2018-08-24 19:52   ` Bernd Edlinger
  2018-08-26  3:34     ` Jeff Law
  2018-08-30  4:34     ` Jason Merrill
  0 siblings, 2 replies; 11+ messages in thread
From: Bernd Edlinger @ 2018-08-24 19:52 UTC (permalink / raw)
  To: gcc-patches, Jeff Law, Richard Biener, Jason Merrill,
	Joseph Myers, Martin Sebor

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Hi,

this updated patch fixes one regression with current trunk due
to a new test case.  Sorry for the confusion.

The change to the previous version is:
1) the check to avoid folding on empty char arrays is restored.
2) A null-termination character is added except when the string is full length.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-bracedstr-v2.diff --]
[-- Type: text/x-patch; name="patch-bracedstr-v2.diff", Size: 11318 bytes --]

c-family:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-common.c (braced_list_to_string): Remove eval parameter.
	Add some more checks.  Always create zero-terminated STRING_CST.
	* c-common.h (braced_list_to_string): Adjust prototype.

c:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-decl.c (finish_decl): Call braced_list_to_string here ...
	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.


cp:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* decl.c (eval_check_narrowing): Remove.
	(check_initializer): Move call to braced_list_to_string from here ...
	* typeck2.c (store_init_value): ... to here.
	(digest_init_r): Remove handing of signed/unsigned char strings.

testsuite:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-c++-common/array-init.c: New test.
	* g++.dg/init/string2.C: Remove xfail.

diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
--- gcc/c/c-decl.c	2018-08-21 08:17:41.000000000 +0200
+++ gcc/c/c-decl.c	2018-08-24 20:03:12.508230259 +0200
@@ -5030,6 +5030,17 @@ finish_decl (tree decl, location_t init_
       relayout_decl (decl);
     }
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && DECL_INITIAL (decl)
+      && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR)
+    {
+      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (typ1 == char_type_node
+	  || typ1 == signed_char_type_node
+	  || typ1 == unsigned_char_type_node)
+	DECL_INITIAL (decl) = braced_list_to_string (type, DECL_INITIAL (decl));
+    }
+
   if (VAR_P (decl))
     {
       if (init && TREE_CODE (init) == CONSTRUCTOR)
diff -Npur gcc/c/c-parser.c gcc/c/c-parser.c
--- gcc/c/c-parser.c	2018-08-21 08:17:41.000000000 +0200
+++ gcc/c/c-parser.c	2018-08-24 20:03:12.511230218 +0200
@@ -2127,15 +2127,6 @@ c_parser_declaration_or_fndef (c_parser
 	      if (d != error_mark_node)
 		{
 		  maybe_warn_string_init (init_loc, TREE_TYPE (d), init);
-
-		  /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-		  tree valtype = TREE_TYPE (init.value);
-		  if (TREE_CODE (init.value) == CONSTRUCTOR
-		      && TREE_CODE (valtype) == ARRAY_TYPE
-		      && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
-		    if (tree str = braced_list_to_string (valtype, init.value))
-		      init.value = str;
-
 		  finish_decl (d, init_loc, init.value,
 			       init.original_type, asm_name);
 		}
diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
--- gcc/c-family/c-common.c	2018-08-17 05:02:11.000000000 +0200
+++ gcc/c-family/c-common.c	2018-08-24 20:03:12.512230205 +0200
@@ -8511,39 +8511,28 @@ maybe_add_include_fixit (rich_location *
 }
 
 /* Attempt to convert a braced array initializer list CTOR for array
-   TYPE into a STRING_CST for convenience and efficiency.  When non-null,
-   use EVAL to attempt to evalue constants (used by C++).  Return
-   the converted string on success or null on failure.  */
+   TYPE into a STRING_CST for convenience and efficiency.  Return
+   the converted string on success or the original ctor on failure.  */
 
 tree
-braced_list_to_string (tree type, tree ctor, tree (*eval)(tree, tree))
+braced_list_to_string (tree type, tree ctor)
 {
-  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
+  if (!tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
+    return ctor;
 
   /* If the array has an explicit bound, use it to constrain the size
      of the string.  If it doesn't, be sure to create a string that's
      as long as implied by the index of the last zero specified via
      a designator, as in:
        const char a[] = { [7] = 0 };  */
-  unsigned HOST_WIDE_INT maxelts = HOST_WIDE_INT_M1U;
-  if (tree size = TYPE_SIZE_UNIT (type))
-    {
-      if (tree_fits_uhwi_p (size))
-	{
-	  maxelts = tree_to_uhwi (size);
-	  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
+  unsigned HOST_WIDE_INT maxelts = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
 
-	  /* Avoid converting initializers for zero-length arrays.  */
-	  if (!maxelts)
-	    return NULL_TREE;
-	}
-    }
-  else if (!nelts)
-    /* Avoid handling the undefined/erroneous case of an empty
-       initializer for an arrays with unspecified bound.  */
-    return NULL_TREE;
+  /* Avoid converting initializers for zero-length arrays.  */
+  if (!maxelts)
+    return ctor;
 
-  tree eltype = TREE_TYPE (type);
+  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
 
   auto_vec<char> str;
   str.reserve (nelts + 1);
@@ -8553,19 +8542,21 @@ braced_list_to_string (tree type, tree c
 
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), i, index, value)
     {
-      unsigned HOST_WIDE_INT idx = index ? tree_to_uhwi (index) : i;
+      unsigned HOST_WIDE_INT idx = i;
+      if (index)
+	{
+	  if (!tree_fits_uhwi_p (index))
+	    return ctor;
+	  idx = tree_to_uhwi (index);
+	}
 
       /* auto_vec is limited to UINT_MAX elements.  */
       if (idx > UINT_MAX)
-	return NULL_TREE;
+	return ctor;
 
-      /* Attempt to evaluate constants.  */
-      if (eval)
-	value = eval (eltype, value);
-
-      /* Avoid non-constant initializers.  */
+     /* Avoid non-constant initializers.  */
      if (!tree_fits_shwi_p (value))
-	return NULL_TREE;
+	return ctor;
 
       /* Skip over embedded nuls except the last one (initializer
 	 elements are in ascending order of indices).  */
@@ -8573,11 +8564,14 @@ braced_list_to_string (tree type, tree c
       if (!val && i + 1 < nelts)
 	continue;
 
+      if (idx < str.length())
+	return ctor;
+
       /* Bail if the CTOR has a block of more than 256 embedded nuls
 	 due to implicitly initialized elements.  */
       unsigned nchars = (idx - str.length ()) + 1;
       if (nchars > 256)
-	return NULL_TREE;
+	return ctor;
 
       if (nchars > 1)
 	{
@@ -8585,18 +8579,17 @@ braced_list_to_string (tree type, tree c
 	  str.quick_grow_cleared (idx);
 	}
 
-      if (idx > maxelts)
-	return NULL_TREE;
+      if (idx >= maxelts)
+	return ctor;
 
       str.safe_insert (idx, val);
     }
 
-  if (!nelts)
-    /* Append a nul for the empty initializer { }.  */
+  /* Append a nul string termination.  */
+  if (str.length () < maxelts)
     str.safe_push (0);
 
-  /* Build a STRING_CST with the same type as the array, which
-     may be an array of unknown bound.  */
+  /* Build a STRING_CST with the same type as the array.  */
   tree res = build_string (str.length (), str.begin ());
   TREE_TYPE (res) = type;
   return res;
diff -Npur gcc/c-family/c-common.h gcc/c-family/c-common.h
--- gcc/c-family/c-common.h	2018-08-17 05:02:11.000000000 +0200
+++ gcc/c-family/c-common.h	2018-08-24 20:03:12.512230205 +0200
@@ -1331,7 +1331,7 @@ extern void maybe_add_include_fixit (ric
 extern void maybe_suggest_missing_token_insertion (rich_location *richloc,
 						   enum cpp_ttype token_type,
 						   location_t prev_token_loc);
-extern tree braced_list_to_string (tree, tree, tree (*)(tree, tree) = NULL);
+extern tree braced_list_to_string (tree, tree);
 
 #if CHECKING_P
 namespace selftest {
diff -Npur gcc/cp/decl.c gcc/cp/decl.c
--- gcc/cp/decl.c	2018-08-22 22:35:38.000000000 +0200
+++ gcc/cp/decl.c	2018-08-24 20:03:12.514230178 +0200
@@ -6299,30 +6299,6 @@ build_aggr_init_full_exprs (tree decl, t
   return build_aggr_init (decl, init, flags, tf_warning_or_error);
 }
 
-/* Attempt to determine the constant VALUE of integral type and convert
-   it to TYPE, issuing narrowing warnings/errors as necessary.  Return
-   the constant result or null on failure.  Callback for
-   braced_list_to_string.  */
-
-static tree
-eval_check_narrowing (tree type, tree value)
-{
-  if (tree valtype = TREE_TYPE (value))
-    {
-      if (TREE_CODE (valtype) != INTEGER_TYPE)
-	return NULL_TREE;
-    }
-  else
-    return NULL_TREE;
-
-  value = scalar_constant_value (value);
-  if (!value)
-    return NULL_TREE;
-
-  check_narrowing (type, value, tf_warning_or_error);
-  return value;
-}
-
 /* Verify INIT (the initializer for DECL), and record the
    initialization in DECL_INITIAL, if appropriate.  CLEANUP is as for
    grok_reference_init.
@@ -6438,17 +6414,7 @@ check_initializer (tree decl, tree init,
 	    }
 	  else
 	    {
-	      /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-	      tree valtype = TREE_TYPE (decl);
-	      if (TREE_CODE (valtype) == ARRAY_TYPE
-		  && TYPE_STRING_FLAG (TREE_TYPE (valtype))
-		  && BRACE_ENCLOSED_INITIALIZER_P (init))
-		if (tree str = braced_list_to_string (valtype, init,
-						      eval_check_narrowing))
-		  init = str;
-
-	      if (TREE_CODE (init) != STRING_CST)
-		init = reshape_init (type, init, tf_warning_or_error);
+	      init = reshape_init (type, init, tf_warning_or_error);
 	      flags |= LOOKUP_NO_NARROWING;
 	    }
 	}
diff -Npur gcc/cp/typeck2.c gcc/cp/typeck2.c
--- gcc/cp/typeck2.c	2018-08-22 22:35:38.000000000 +0200
+++ gcc/cp/typeck2.c	2018-08-24 20:03:12.515230164 +0200
@@ -814,6 +814,16 @@ store_init_value (tree decl, tree init,
     /* Digest the specified initializer into an expression.  */
     value = digest_init_flags (type, init, flags, tf_warning_or_error);
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TREE_CODE (value) == CONSTRUCTOR)
+    {
+      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (typ1 == char_type_node
+	 || typ1 == signed_char_type_node
+	 || typ1 == unsigned_char_type_node)
+	value = braced_list_to_string (type, value);
+    }
+
   value = extend_ref_init_temps (decl, value, cleanups);
 
   /* In C++11 constant expression is a semantic, not syntactic, property.
@@ -1065,9 +1075,7 @@ digest_init_r (tree type, tree init, int
 
 	  if (TYPE_PRECISION (typ1) == BITS_PER_UNIT)
 	    {
-	      if (char_type != char_type_node
-		  && char_type != signed_char_type_node
-		  && char_type != unsigned_char_type_node)
+	      if (char_type != char_type_node)
 		{
 		  if (complain & tf_error)
 		    error_at (loc, "char-array initialized from wide string");
diff -Npur gcc/testsuite/c-c++-common/array-init.c gcc/testsuite/c-c++-common/array-init.c
--- gcc/testsuite/c-c++-common/array-init.c	1970-01-01 01:00:00.000000000 +0100
+++ gcc/testsuite/c-c++-common/array-init.c	2018-08-24 20:03:12.515230164 +0200
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-prune-output "sorry, unimplemented: non-trivial designated initializers not supported" } */
+
+char x[] = { [-1] = 1, 2, 3 }; /* { dg-error "array index in initializer exceeds array bounds" "" { target c } } */
diff -Npur gcc/testsuite/g++.dg/init/string2.C gcc/testsuite/g++.dg/init/string2.C
--- gcc/testsuite/g++.dg/init/string2.C	2018-08-16 20:21:59.000000000 +0200
+++ gcc/testsuite/g++.dg/init/string2.C	2018-08-24 20:03:12.515230164 +0200
@@ -54,7 +54,7 @@ template <class T>
 int tmplen ()
 {
   static const T
-    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
+    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
   return __builtin_strlen (a);
 }
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-24 19:52   ` [PATCHv2] " Bernd Edlinger
@ 2018-08-26  3:34     ` Jeff Law
  2018-08-26  7:46       ` Bernd Edlinger
  2018-08-30  4:34     ` Jason Merrill
  1 sibling, 1 reply; 11+ messages in thread
From: Jeff Law @ 2018-08-26  3:34 UTC (permalink / raw)
  To: Bernd Edlinger, gcc-patches, Richard Biener, Jason Merrill,
	Joseph Myers, Martin Sebor

On 08/24/2018 01:52 PM, Bernd Edlinger wrote:
> Hi,
> 
> this updated patch fixes one regression with current trunk due
> to a new test case.  Sorry for the confusion.
> 
> The change to the previous version is:
> 1) the check to avoid folding on empty char arrays is restored.
> 2) A null-termination character is added except when the string is full length.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> patch-bracedstr-v2.diff
> 
> 
> c-family:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* c-common.c (braced_list_to_string): Remove eval parameter.
> 	Add some more checks.  Always create zero-terminated STRING_CST.
> 	* c-common.h (braced_list_to_string): Adjust prototype.
> 
> c:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* c-decl.c (finish_decl): Call braced_list_to_string here ...
> 	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.
> 
> 
> cp:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* decl.c (eval_check_narrowing): Remove.
> 	(check_initializer): Move call to braced_list_to_string from here ...
> 	* typeck2.c (store_init_value): ... to here.
> 	(digest_init_r): Remove handing of signed/unsigned char strings.
> 
> testsuite:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* c-c++-common/array-init.c: New test.
> 	* g++.dg/init/string2.C: Remove xfail.
My concern here is that you removed code that was explicitly added
during the initial review work of the patch that turned braced
initializers into strings.


> -    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
> +    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
This is not an XFAIL, this is a selector.  Essentially it says that the
diagnostic is appropriate when not in c++98 mode.

You can see that to be the case if you compile the test in c++98 mode
without your change.  It will compile with no errors or warnings.

However, after your change it issues a warning for the narrowing
conversion in c++98 mode, which AFAICT it should not do.

jeff





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-26  3:34     ` Jeff Law
@ 2018-08-26  7:46       ` Bernd Edlinger
  2018-08-31 16:45         ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2018-08-26  7:46 UTC (permalink / raw)
  To: Jeff Law, gcc-patches, Richard Biener, Jason Merrill,
	Joseph Myers, Martin Sebor

On 08/26/18 05:34, Jeff Law wrote:
> On 08/24/2018 01:52 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> this updated patch fixes one regression with current trunk due
>> to a new test case.  Sorry for the confusion.
>>
>> The change to the previous version is:
>> 1) the check to avoid folding on empty char arrays is restored.
>> 2) A null-termination character is added except when the string is full length.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> patch-bracedstr-v2.diff
>>
>>
>> c-family:
>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* c-common.c (braced_list_to_string): Remove eval parameter.
>> 	Add some more checks.  Always create zero-terminated STRING_CST.
>> 	* c-common.h (braced_list_to_string): Adjust prototype.
>>
>> c:
>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* c-decl.c (finish_decl): Call braced_list_to_string here ...
>> 	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.
>>
>>
>> cp:
>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* decl.c (eval_check_narrowing): Remove.
>> 	(check_initializer): Move call to braced_list_to_string from here ...
>> 	* typeck2.c (store_init_value): ... to here.
>> 	(digest_init_r): Remove handing of signed/unsigned char strings.
>>
>> testsuite:
>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* c-c++-common/array-init.c: New test.
>> 	* g++.dg/init/string2.C: Remove xfail.
> My concern here is that you removed code that was explicitly added
> during the initial review work of the patch that turned braced
> initializers into strings.
> 

Yes, you mean BRACE_ENCLOSED_INITIALIZER_P
which is (TREE_CODE (NODE) == CONSTRUCTOR && TREE_TYPE (NODE) == init_list_type_node)

I tried that first but the TREE_TYPE of the CONSTRUCTOR was no longer init_list_type_node
at that point.  I think the middle-end needs the structure type here, and digest_init must
have fixed that.

This did not break in the debugger:
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TREE_CODE (value) == CONSTRUCTOR)
+    {
+      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (typ1 == char_type_node
+	 || typ1 == signed_char_type_node
+	 || typ1 == unsigned_char_type_node)
+       {
+        if (BRACE_ENCLOSED_INITIALIZER_P(value))
+          asm("int3");
+	value = braced_list_to_string (type, value);
+       }
+    }
+

while this
+        if (!BRACE_ENCLOSED_INITIALIZER_P(value))
+          printf("%p - %p\n", TREE_TYPE(value), init_list_type_node);

does this:

$ cat test.cc
char x[] = {1,2,3};
$ ../gcc-build/gcc/xgcc -B ../gcc-build/gcc/ -S test.cc
0x7fdedb821b28 - 0x7fdedb81f738

while the string is folded as expected.

> 
>> -    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
>> +    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
> This is not an XFAIL, this is a selector.  Essentially it says that the
> diagnostic is appropriate when not in c++98 mode.
> 
> You can see that to be the case if you compile the test in c++98 mode
> without your change.  It will compile with no errors or warnings.
> 
> However, after your change it issues a warning for the narrowing
> conversion in c++98 mode, which AFAICT it should not do.
> 

This just restores the state _before_ Martin's braced initializer patch.
So I have the impression that is actually a regression due to the
original braced initializer patch.
It is unfortunate that Martin did not check that.


Bernd.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-24 19:52   ` [PATCHv2] " Bernd Edlinger
  2018-08-26  3:34     ` Jeff Law
@ 2018-08-30  4:34     ` Jason Merrill
  2018-08-30  7:07       ` Bernd Edlinger
  1 sibling, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2018-08-30  4:34 UTC (permalink / raw)
  To: Bernd Edlinger, gcc-patches, Jeff Law, Richard Biener,
	Joseph Myers, Martin Sebor

On 08/24/2018 03:52 PM, Bernd Edlinger wrote:
> this updated patch fixes one regression with current trunk due
> to a new test case.  Sorry for the confusion.
> 
> The change to the previous version is:
> 1) the check to avoid folding on empty char arrays is restored.
> 2) A null-termination character is added except when the string is full length.

> -		      && TYPE_STRING_FLAG (TREE_TYPE (valtype)))

> +      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
> +      if (typ1 == char_type_node
> +	  || typ1 == signed_char_type_node
> +	  || typ1 == unsigned_char_type_node)


Why stop using TYPE_STRING_FLAG?

Jason

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-30  4:34     ` Jason Merrill
@ 2018-08-30  7:07       ` Bernd Edlinger
  2018-08-31  6:47         ` Bernd Edlinger
  0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2018-08-30  7:07 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches, Jeff Law, Richard Biener,
	Joseph Myers, Martin Sebor

On 08/30/18 06:34, Jason Merrill wrote:
> On 08/24/2018 03:52 PM, Bernd Edlinger wrote:
>> this updated patch fixes one regression with current trunk due
>> to a new test case.  Sorry for the confusion.
>>
>> The change to the previous version is:
>> 1) the check to avoid folding on empty char arrays is restored.
>> 2) A null-termination character is added except when the string is full length.
> 
>> -              && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
> 
>> +      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
>> +      if (typ1 == char_type_node
>> +      || typ1 == signed_char_type_node
>> +      || typ1 == unsigned_char_type_node)
> 
> 
> Why stop using TYPE_STRING_FLAG?
> 

No longer sure, I will try it with TYPE_STRING_FLAG.


Bernd.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-30  7:07       ` Bernd Edlinger
@ 2018-08-31  6:47         ` Bernd Edlinger
  2018-09-02 15:19           ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2018-08-31  6:47 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches, Jeff Law, Richard Biener,
	Joseph Myers, Martin Sebor

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On 08/30/18 09:07, Bernd Edlinger wrote:
> On 08/30/18 06:34, Jason Merrill wrote:
>> On 08/24/2018 03:52 PM, Bernd Edlinger wrote:
>>> this updated patch fixes one regression with current trunk due
>>> to a new test case.  Sorry for the confusion.
>>>
>>> The change to the previous version is:
>>> 1) the check to avoid folding on empty char arrays is restored.
>>> 2) A null-termination character is added except when the string is full length.
>>
>>> -              && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
>>
>>> +      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
>>> +      if (typ1 == char_type_node
>>> +      || typ1 == signed_char_type_node
>>> +      || typ1 == unsigned_char_type_node)
>>
>>
>> Why stop using TYPE_STRING_FLAG?
>>
> 
> No longer sure, I will try it with TYPE_STRING_FLAG.
> 

This is an update that uses TYPE_STRING_FLAG.
It does not seem to make any difference, though.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.


Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-bracedstr-v2.diff --]
[-- Type: text/x-patch; name="patch-bracedstr-v2.diff", Size: 11044 bytes --]

c-family:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-common.c (braced_list_to_string): Remove eval parameter.
	Add some more checks.  Always create zero-terminated STRING_CST.
	* c-common.h (braced_list_to_string): Adjust prototype.

c:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-decl.c (finish_decl): Call braced_list_to_string here ...
	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.


cp:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* decl.c (eval_check_narrowing): Remove.
	(check_initializer): Move call to braced_list_to_string from here ...
	* typeck2.c (store_init_value): ... to here.
	(digest_init_r): Remove handing of signed/unsigned char strings.

testsuite:
2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-c++-common/array-init.c: New test.
	* g++.dg/init/string2.C: Remove xfail.

diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
--- gcc/c/c-decl.c	2018-08-21 08:17:41.000000000 +0200
+++ gcc/c/c-decl.c	2018-08-30 12:03:12.508230259 +0200
@@ -5031,6 +5031,12 @@ finish_decl (tree decl, location_t init_
       relayout_decl (decl);
     }
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TYPE_STRING_FLAG (TREE_TYPE (type))
+      && DECL_INITIAL (decl)
+      && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR)
+    DECL_INITIAL (decl) = braced_list_to_string (type, DECL_INITIAL (decl));
+
   if (VAR_P (decl))
     {
       if (init && TREE_CODE (init) == CONSTRUCTOR)
diff -Npur gcc/c/c-parser.c gcc/c/c-parser.c
--- gcc/c/c-parser.c	2018-08-21 08:17:41.000000000 +0200
+++ gcc/c/c-parser.c	2018-08-24 20:03:12.511230218 +0200
@@ -2127,15 +2127,6 @@ c_parser_declaration_or_fndef (c_parser
 	      if (d != error_mark_node)
 		{
 		  maybe_warn_string_init (init_loc, TREE_TYPE (d), init);
-
-		  /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-		  tree valtype = TREE_TYPE (init.value);
-		  if (TREE_CODE (init.value) == CONSTRUCTOR
-		      && TREE_CODE (valtype) == ARRAY_TYPE
-		      && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
-		    if (tree str = braced_list_to_string (valtype, init.value))
-		      init.value = str;
-
 		  finish_decl (d, init_loc, init.value,
 			       init.original_type, asm_name);
 		}
diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
--- gcc/c-family/c-common.c	2018-08-17 05:02:11.000000000 +0200
+++ gcc/c-family/c-common.c	2018-08-24 20:03:12.512230205 +0200
@@ -8511,39 +8511,28 @@ maybe_add_include_fixit (rich_location *
 }
 
 /* Attempt to convert a braced array initializer list CTOR for array
-   TYPE into a STRING_CST for convenience and efficiency.  When non-null,
-   use EVAL to attempt to evalue constants (used by C++).  Return
-   the converted string on success or null on failure.  */
+   TYPE into a STRING_CST for convenience and efficiency.  Return
+   the converted string on success or the original ctor on failure.  */
 
 tree
-braced_list_to_string (tree type, tree ctor, tree (*eval)(tree, tree))
+braced_list_to_string (tree type, tree ctor)
 {
-  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
+  if (!tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
+    return ctor;
 
   /* If the array has an explicit bound, use it to constrain the size
      of the string.  If it doesn't, be sure to create a string that's
      as long as implied by the index of the last zero specified via
      a designator, as in:
        const char a[] = { [7] = 0 };  */
-  unsigned HOST_WIDE_INT maxelts = HOST_WIDE_INT_M1U;
-  if (tree size = TYPE_SIZE_UNIT (type))
-    {
-      if (tree_fits_uhwi_p (size))
-	{
-	  maxelts = tree_to_uhwi (size);
-	  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
+  unsigned HOST_WIDE_INT maxelts = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
 
-	  /* Avoid converting initializers for zero-length arrays.  */
-	  if (!maxelts)
-	    return NULL_TREE;
-	}
-    }
-  else if (!nelts)
-    /* Avoid handling the undefined/erroneous case of an empty
-       initializer for an arrays with unspecified bound.  */
-    return NULL_TREE;
+  /* Avoid converting initializers for zero-length arrays.  */
+  if (!maxelts)
+    return ctor;
 
-  tree eltype = TREE_TYPE (type);
+  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
 
   auto_vec<char> str;
   str.reserve (nelts + 1);
@@ -8553,19 +8542,21 @@ braced_list_to_string (tree type, tree c
 
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), i, index, value)
     {
-      unsigned HOST_WIDE_INT idx = index ? tree_to_uhwi (index) : i;
+      unsigned HOST_WIDE_INT idx = i;
+      if (index)
+	{
+	  if (!tree_fits_uhwi_p (index))
+	    return ctor;
+	  idx = tree_to_uhwi (index);
+	}
 
       /* auto_vec is limited to UINT_MAX elements.  */
       if (idx > UINT_MAX)
-	return NULL_TREE;
+	return ctor;
 
-      /* Attempt to evaluate constants.  */
-      if (eval)
-	value = eval (eltype, value);
-
-      /* Avoid non-constant initializers.  */
+     /* Avoid non-constant initializers.  */
      if (!tree_fits_shwi_p (value))
-	return NULL_TREE;
+	return ctor;
 
       /* Skip over embedded nuls except the last one (initializer
 	 elements are in ascending order of indices).  */
@@ -8573,11 +8564,14 @@ braced_list_to_string (tree type, tree c
       if (!val && i + 1 < nelts)
 	continue;
 
+      if (idx < str.length())
+	return ctor;
+
       /* Bail if the CTOR has a block of more than 256 embedded nuls
 	 due to implicitly initialized elements.  */
       unsigned nchars = (idx - str.length ()) + 1;
       if (nchars > 256)
-	return NULL_TREE;
+	return ctor;
 
       if (nchars > 1)
 	{
@@ -8585,18 +8579,17 @@ braced_list_to_string (tree type, tree c
 	  str.quick_grow_cleared (idx);
 	}
 
-      if (idx > maxelts)
-	return NULL_TREE;
+      if (idx >= maxelts)
+	return ctor;
 
       str.safe_insert (idx, val);
     }
 
-  if (!nelts)
-    /* Append a nul for the empty initializer { }.  */
+  /* Append a nul string termination.  */
+  if (str.length () < maxelts)
     str.safe_push (0);
 
-  /* Build a STRING_CST with the same type as the array, which
-     may be an array of unknown bound.  */
+  /* Build a STRING_CST with the same type as the array.  */
   tree res = build_string (str.length (), str.begin ());
   TREE_TYPE (res) = type;
   return res;
diff -Npur gcc/c-family/c-common.h gcc/c-family/c-common.h
--- gcc/c-family/c-common.h	2018-08-17 05:02:11.000000000 +0200
+++ gcc/c-family/c-common.h	2018-08-24 20:03:12.512230205 +0200
@@ -1331,7 +1331,7 @@ extern void maybe_add_include_fixit (ric
 extern void maybe_suggest_missing_token_insertion (rich_location *richloc,
 						   enum cpp_ttype token_type,
 						   location_t prev_token_loc);
-extern tree braced_list_to_string (tree, tree, tree (*)(tree, tree) = NULL);
+extern tree braced_list_to_string (tree, tree);
 
 #if CHECKING_P
 namespace selftest {
diff -Npur gcc/cp/decl.c gcc/cp/decl.c
--- gcc/cp/decl.c	2018-08-22 22:35:38.000000000 +0200
+++ gcc/cp/decl.c	2018-08-24 20:03:12.514230178 +0200
@@ -6299,30 +6299,6 @@ build_aggr_init_full_exprs (tree decl, t
   return build_aggr_init (decl, init, flags, tf_warning_or_error);
 }
 
-/* Attempt to determine the constant VALUE of integral type and convert
-   it to TYPE, issuing narrowing warnings/errors as necessary.  Return
-   the constant result or null on failure.  Callback for
-   braced_list_to_string.  */
-
-static tree
-eval_check_narrowing (tree type, tree value)
-{
-  if (tree valtype = TREE_TYPE (value))
-    {
-      if (TREE_CODE (valtype) != INTEGER_TYPE)
-	return NULL_TREE;
-    }
-  else
-    return NULL_TREE;
-
-  value = scalar_constant_value (value);
-  if (!value)
-    return NULL_TREE;
-
-  check_narrowing (type, value, tf_warning_or_error);
-  return value;
-}
-
 /* Verify INIT (the initializer for DECL), and record the
    initialization in DECL_INITIAL, if appropriate.  CLEANUP is as for
    grok_reference_init.
@@ -6438,17 +6414,7 @@ check_initializer (tree decl, tree init,
 	    }
 	  else
 	    {
-	      /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-	      tree valtype = TREE_TYPE (decl);
-	      if (TREE_CODE (valtype) == ARRAY_TYPE
-		  && TYPE_STRING_FLAG (TREE_TYPE (valtype))
-		  && BRACE_ENCLOSED_INITIALIZER_P (init))
-		if (tree str = braced_list_to_string (valtype, init,
-						      eval_check_narrowing))
-		  init = str;
-
-	      if (TREE_CODE (init) != STRING_CST)
-		init = reshape_init (type, init, tf_warning_or_error);
+	      init = reshape_init (type, init, tf_warning_or_error);
 	      flags |= LOOKUP_NO_NARROWING;
 	    }
 	}
diff -Npur gcc/cp/typeck2.c gcc/cp/typeck2.c
--- gcc/cp/typeck2.c	2018-08-22 22:35:38.000000000 +0200
+++ gcc/cp/typeck2.c	2018-08-30 12:03:12.515230164 +0200
@@ -807,6 +807,11 @@ store_init_value (tree decl, tree init,
     /* Digest the specified initializer into an expression.  */
     value = digest_init_flags (type, init, flags, tf_warning_or_error);
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TYPE_STRING_FLAG (TREE_TYPE (type))
+      && TREE_CODE (value) == CONSTRUCTOR)
+    value = braced_list_to_string (type, value);
+
   value = extend_ref_init_temps (decl, value, cleanups);
 
   /* In C++11 constant expression is a semantic, not syntactic, property.
@@ -1058,9 +1063,7 @@ digest_init_r (tree type, tree init, int
 
 	  if (TYPE_PRECISION (typ1) == BITS_PER_UNIT)
 	    {
-	      if (char_type != char_type_node
-		  && char_type != signed_char_type_node
-		  && char_type != unsigned_char_type_node)
+	      if (char_type != char_type_node)
 		{
 		  if (complain & tf_error)
 		    error_at (loc, "char-array initialized from wide string");
diff -Npur gcc/testsuite/c-c++-common/array-init.c gcc/testsuite/c-c++-common/array-init.c
--- gcc/testsuite/c-c++-common/array-init.c	1970-01-01 01:00:00.000000000 +0100
+++ gcc/testsuite/c-c++-common/array-init.c	2018-08-24 20:03:12.515230164 +0200
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-prune-output "sorry, unimplemented: non-trivial designated initializers not supported" } */
+
+char x[] = { [-1] = 1, 2, 3 }; /* { dg-error "array index in initializer exceeds array bounds" "" { target c } } */
diff -Npur gcc/testsuite/g++.dg/init/string2.C gcc/testsuite/g++.dg/init/string2.C
--- gcc/testsuite/g++.dg/init/string2.C	2018-08-16 20:21:59.000000000 +0200
+++ gcc/testsuite/g++.dg/init/string2.C	2018-08-24 20:03:12.515230164 +0200
@@ -54,7 +54,7 @@ template <class T>
 int tmplen ()
 {
   static const T
-    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
+    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
   return __builtin_strlen (a);
 }
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-26  7:46       ` Bernd Edlinger
@ 2018-08-31 16:45         ` Jeff Law
  2018-08-31 17:38           ` Bernd Edlinger
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2018-08-31 16:45 UTC (permalink / raw)
  To: Bernd Edlinger, gcc-patches, Richard Biener, Jason Merrill,
	Joseph Myers, Martin Sebor

On 08/26/2018 01:45 AM, Bernd Edlinger wrote:

>>> cp:
>>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>
>>> 	* decl.c (eval_check_narrowing): Remove.
>>> 	(check_initializer): Move call to braced_list_to_string from here ...
>>> 	* typeck2.c (store_init_value): ... to here.
>>> 	(digest_init_r): Remove handing of signed/unsigned char strings.
>>>
>>> testsuite:
>>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>
>>> 	* c-c++-common/array-init.c: New test.
>>> 	* g++.dg/init/string2.C: Remove xfail.
>> My concern here is that you removed code that was explicitly added
>> during the initial review work of the patch that turned braced
>> initializers into strings.
>>
> 
> Yes, you mean BRACE_ENCLOSED_INITIALIZER_P
> which is (TREE_CODE (NODE) == CONSTRUCTOR && TREE_TYPE (NODE) == init_list_type_node)
I was referring to the callback into the eval routine from within
braced_list_to_string which is related to the concern where you dropped
the c++98_only selector.  Sorry I wasn't clear about that.

[ snip ]

> 
>>
>>> -    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
>>> +    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
>> This is not an XFAIL, this is a selector.  Essentially it says that the
>> diagnostic is appropriate when not in c++98 mode.
>>
>> You can see that to be the case if you compile the test in c++98 mode
>> without your change.  It will compile with no errors or warnings.
>>
>> However, after your change it issues a warning for the narrowing
>> conversion in c++98 mode, which AFAICT it should not do.
>>
> 
> This just restores the state _before_ Martin's braced initializer patch.
> So I have the impression that is actually a regression due to the
> original braced initializer patch.
> It is unfortunate that Martin did not check that.
The ChangeLog said it was removing an xfail, so naturally when I saw it
was removing a selector I assumed that the selector was right
(particularly since it made sense given current behavior) and you'd made
a minor goof.

But I can confirm that prior to Martin's changes we did issue a
diagnostic when in C++98 mode:

j.C: In instantiation of ‘int tmplen() [with T = char]’:
j.C:61:27:   required from here
j.C:57:5: warning: overflow in conversion from ‘int’ to ‘char’ changes
value from ‘333’ to ‘'M'’ [-Woverflow]
57 |     a[] = { 1, 2, 333, 0 };         // { dg-warning
"\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
   |     ^

You can see that with the trunk just before Martin's change or with
older compilers (I also tested with 6.4.1 as I had it handy).

So I'll withdraw my objection to this change.  I'll dig into your
updated patch momentarily.

jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-31 16:45         ` Jeff Law
@ 2018-08-31 17:38           ` Bernd Edlinger
  0 siblings, 0 replies; 11+ messages in thread
From: Bernd Edlinger @ 2018-08-31 17:38 UTC (permalink / raw)
  To: Jeff Law, gcc-patches, Richard Biener, Jason Merrill,
	Joseph Myers, Martin Sebor

On 08/31/18 18:45, Jeff Law wrote:
> On 08/26/2018 01:45 AM, Bernd Edlinger wrote:
> 
>>>> cp:
>>>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>>
>>>> 	* decl.c (eval_check_narrowing): Remove.
>>>> 	(check_initializer): Move call to braced_list_to_string from here ...
>>>> 	* typeck2.c (store_init_value): ... to here.
>>>> 	(digest_init_r): Remove handing of signed/unsigned char strings.
>>>>
>>>> testsuite:
>>>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>>
>>>> 	* c-c++-common/array-init.c: New test.
>>>> 	* g++.dg/init/string2.C: Remove xfail.
>>> My concern here is that you removed code that was explicitly added
>>> during the initial review work of the patch that turned braced
>>> initializers into strings.
>>>
>>
>> Yes, you mean BRACE_ENCLOSED_INITIALIZER_P
>> which is (TREE_CODE (NODE) == CONSTRUCTOR && TREE_TYPE (NODE) == init_list_type_node)
> I was referring to the callback into the eval routine from within
> braced_list_to_string which is related to the concern where you dropped
> the c++98_only selector.  Sorry I wasn't clear about that.
> 

Ah, that you mean.

I think it should be fine, since the main purpose of eval_check_narrowing
was to call check_narrowing on the value, which is normally done by reshape_init
but this is by-passed if braced_list_to_string is successful.

It is much smarter to call braced_list_to_string that after digest_init completed.


> 
>>
>>>
>>>> -    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
>>>> +    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
>>> This is not an XFAIL, this is a selector.  Essentially it says that the
>>> diagnostic is appropriate when not in c++98 mode.
>>>
>>> You can see that to be the case if you compile the test in c++98 mode
>>> without your change.  It will compile with no errors or warnings.
>>>
>>> However, after your change it issues a warning for the narrowing
>>> conversion in c++98 mode, which AFAICT it should not do.
>>>
>>
>> This just restores the state _before_ Martin's braced initializer patch.
>> So I have the impression that is actually a regression due to the
>> original braced initializer patch.
>> It is unfortunate that Martin did not check that.
> The ChangeLog said it was removing an xfail, so naturally when I saw it
> was removing a selector I assumed that the selector was right
> (particularly since it made sense given current behavior) and you'd made
> a minor goof.
> 

Yes, indeed, the change log is wrong, it should be

* g++.dg/init/string2.C: Adjust test expectations.


> But I can confirm that prior to Martin's changes we did issue a
> diagnostic when in C++98 mode:
> 
> j.C: In instantiation of ‘int tmplen() [with T = char]’:
> j.C:61:27:   required from here
> j.C:57:5: warning: overflow in conversion from ‘int’ to ‘char’ changes
> value from ‘333’ to ‘'M'’ [-Woverflow]
> 57 |     a[] = { 1, 2, 333, 0 };         // { dg-warning
> "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
>     |     ^
> 
> You can see that with the trunk just before Martin's change or with
> older compilers (I also tested with 6.4.1 as I had it handy).
> 
> So I'll withdraw my objection to this change.  I'll dig into your
> updated patch momentarily.
> 
> jeff
> 


Thanks
Bernd.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2] Call braced_list_to_string after array size is fixed
  2018-08-31  6:47         ` Bernd Edlinger
@ 2018-09-02 15:19           ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2018-09-02 15:19 UTC (permalink / raw)
  To: Bernd Edlinger, Jason Merrill, gcc-patches, Richard Biener,
	Joseph Myers, Martin Sebor

On 08/31/2018 12:47 AM, Bernd Edlinger wrote:
> On 08/30/18 09:07, Bernd Edlinger wrote:
>> On 08/30/18 06:34, Jason Merrill wrote:
>>> On 08/24/2018 03:52 PM, Bernd Edlinger wrote:
>>>> this updated patch fixes one regression with current trunk due
>>>> to a new test case.  Sorry for the confusion.
>>>>
>>>> The change to the previous version is:
>>>> 1) the check to avoid folding on empty char arrays is restored.
>>>> 2) A null-termination character is added except when the string is full length.
>>>> -              && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
>>>> +      tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
>>>> +      if (typ1 == char_type_node
>>>> +      || typ1 == signed_char_type_node
>>>> +      || typ1 == unsigned_char_type_node)
>>>
>>> Why stop using TYPE_STRING_FLAG?
>>>
>> No longer sure, I will try it with TYPE_STRING_FLAG.
>>
> This is an update that uses TYPE_STRING_FLAG.
> It does not seem to make any difference, though.
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> 
> 
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> patch-bracedstr-v2.diff
> 
> 
> c-family:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* c-common.c (braced_list_to_string): Remove eval parameter.
> 	Add some more checks.  Always create zero-terminated STRING_CST.
> 	* c-common.h (braced_list_to_string): Adjust prototype.
> 
> c:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* c-decl.c (finish_decl): Call braced_list_to_string here ...
> 	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.
> 
> 
> cp:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* decl.c (eval_check_narrowing): Remove.
> 	(check_initializer): Move call to braced_list_to_string from here ...
> 	* typeck2.c (store_init_value): ... to here.
> 	(digest_init_r): Remove handing of signed/unsigned char strings.
> 
> testsuite:
> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* c-c++-common/array-init.c: New test.
> 	* g++.dg/init/string2.C: Remove xfail.

Thanks.  Committed.

Jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-09-02 15:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-17 12:01 [PATCH] Call braced_list_to_string after array size is fixed Bernd Edlinger
2018-08-22 13:18 ` Bernd Edlinger
2018-08-24 19:52   ` [PATCHv2] " Bernd Edlinger
2018-08-26  3:34     ` Jeff Law
2018-08-26  7:46       ` Bernd Edlinger
2018-08-31 16:45         ` Jeff Law
2018-08-31 17:38           ` Bernd Edlinger
2018-08-30  4:34     ` Jason Merrill
2018-08-30  7:07       ` Bernd Edlinger
2018-08-31  6:47         ` Bernd Edlinger
2018-09-02 15:19           ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).