diff mbox series

[C++] Fix handling of location wrappers in constexpr evaluation (PR c++/92015)

Message ID 20191017071826.GW2116@tucnak
State New
Headers show
Series [C++] Fix handling of location wrappers in constexpr evaluation (PR c++/92015) | expand

Commit Message

Jakub Jelinek Oct. 17, 2019, 7:18 a.m. UTC
Hi!

As written in the PR, location wrappers are stripped by
cxx_eval_constant_expression as the first thing it does after dealing with
jump_target.  The problem is that when this function is called on a
CONSTRUCTOR, is TREE_CONSTANT and reduced_constant_expression_p (which
allows location wrappers around constants), we don't recurse on the
CONSTRUCTOR elements and so nothing strips them away.  Then when
cxx_eval_component_reference or bit_field_ref we actually can run into those
location wrappers and the callers assume they don't appear anymore.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

I admit I'm not 100% sure about the second case which will call the function
to do the stripping for all the elts until the one found.  The alternative
would be:
       tree bitpos = bit_position (field);
       if (bitpos == start && DECL_SIZE (field) == TREE_OPERAND (t, 1))
-	return value;
+	{
+	  STRIP_ANY_LOCATION_WRAPPER (value);
+	  return value;
+	}
       if (TREE_CODE (TREE_TYPE (field)) == INTEGER_TYPE
-	  && TREE_CODE (value) == INTEGER_CST
 	  && tree_fits_shwi_p (bitpos)
 	  && tree_fits_shwi_p (DECL_SIZE (field)))
 	{
 	  HOST_WIDE_INT bit = tree_to_shwi (bitpos);
 	  HOST_WIDE_INT sz = tree_to_shwi (DECL_SIZE (field));
 	  HOST_WIDE_INT shift;
 	  if (bit >= istart && bit + sz <= istart + isize)
 	    {
+	      STRIP_ANY_LOCATION_WRAPPER (value);
+	      if (TREE_CODE (value) != INTEGER_CST)
+		continue;
but then we'd do those tree_fits_shwi_p/tree_to_shwi.  Guess
a micro-optimization without a clear winner.

2019-10-17  Jakub Jelinek  <jakub@redhat.com>

	PR c++/92015
	* constexpr.c (cxx_eval_component_reference, cxx_eval_bit_field_ref):
	Use STRIP_ANY_LOCATION_WRAPPER on CONSTRUCTOR elts.

	* g++.dg/cpp0x/constexpr-92015.C: New test.


	Jakub

Comments

Jason Merrill Oct. 21, 2019, 6:20 p.m. UTC | #1
On 10/17/19 3:18 AM, Jakub Jelinek wrote:
> Hi!
> 
> As written in the PR, location wrappers are stripped by
> cxx_eval_constant_expression as the first thing it does after dealing with
> jump_target.  The problem is that when this function is called on a
> CONSTRUCTOR, is TREE_CONSTANT and reduced_constant_expression_p (which
> allows location wrappers around constants), we don't recurse on the
> CONSTRUCTOR elements and so nothing strips them away.  Then when
> cxx_eval_component_reference or bit_field_ref we actually can run into those
> location wrappers and the callers assume they don't appear anymore.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

OK.

> I admit I'm not 100% sure about the second case which will call the function
> to do the stripping for all the elts until the one found.  The alternative
> would be:
>         tree bitpos = bit_position (field);
>         if (bitpos == start && DECL_SIZE (field) == TREE_OPERAND (t, 1))
> -	return value;
> +	{
> +	  STRIP_ANY_LOCATION_WRAPPER (value);
> +	  return value;
> +	}
>         if (TREE_CODE (TREE_TYPE (field)) == INTEGER_TYPE
> -	  && TREE_CODE (value) == INTEGER_CST
>   	  && tree_fits_shwi_p (bitpos)
>   	  && tree_fits_shwi_p (DECL_SIZE (field)))
>   	{
>   	  HOST_WIDE_INT bit = tree_to_shwi (bitpos);
>   	  HOST_WIDE_INT sz = tree_to_shwi (DECL_SIZE (field));
>   	  HOST_WIDE_INT shift;
>   	  if (bit >= istart && bit + sz <= istart + isize)
>   	    {
> +	      STRIP_ANY_LOCATION_WRAPPER (value);
> +	      if (TREE_CODE (value) != INTEGER_CST)
> +		continue;
> but then we'd do those tree_fits_shwi_p/tree_to_shwi.  Guess
> a micro-optimization without a clear winner.
> 
> 2019-10-17  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/92015
> 	* constexpr.c (cxx_eval_component_reference, cxx_eval_bit_field_ref):
> 	Use STRIP_ANY_LOCATION_WRAPPER on CONSTRUCTOR elts.
> 
> 	* g++.dg/cpp0x/constexpr-92015.C: New test.
> 
> --- gcc/cp/constexpr.c.jj	2019-10-16 09:30:57.300112739 +0200
> +++ gcc/cp/constexpr.c	2019-10-16 17:06:03.943539476 +0200
> @@ -2887,7 +2887,10 @@ cxx_eval_component_reference (const cons
>   	  : field == part)
>   	{
>   	  if (value)
> -	    return value;
> +	    {
> +	      STRIP_ANY_LOCATION_WRAPPER (value);
> +	      return value;
> +	    }
>   	  else
>   	    /* We're in the middle of initializing it.  */
>   	    break;
> @@ -2977,6 +2980,7 @@ cxx_eval_bit_field_ref (const constexpr_
>     FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (whole), i, field, value)
>       {
>         tree bitpos = bit_position (field);
> +      STRIP_ANY_LOCATION_WRAPPER (value);
>         if (bitpos == start && DECL_SIZE (field) == TREE_OPERAND (t, 1))
>   	return value;
>         if (TREE_CODE (TREE_TYPE (field)) == INTEGER_TYPE
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-92015.C.jj	2019-10-16 17:16:44.204871319 +0200
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-92015.C	2019-10-16 17:14:14.773127884 +0200
> @@ -0,0 +1,7 @@
> +// PR c++/92015
> +// { dg-do compile { target c++11 } }
> +
> +struct S1 { char c[6] {'h', 'e', 'l', 'l', 'o', 0}; };
> +struct S2 { char c[6] = "hello"; };
> +static_assert (S1{}.c[0] == 'h', "");
> +static_assert (S2{}.c[0] == 'h', "");
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/cp/constexpr.c.jj	2019-10-16 09:30:57.300112739 +0200
+++ gcc/cp/constexpr.c	2019-10-16 17:06:03.943539476 +0200
@@ -2887,7 +2887,10 @@  cxx_eval_component_reference (const cons
 	  : field == part)
 	{
 	  if (value)
-	    return value;
+	    {
+	      STRIP_ANY_LOCATION_WRAPPER (value);
+	      return value;
+	    }
 	  else
 	    /* We're in the middle of initializing it.  */
 	    break;
@@ -2977,6 +2980,7 @@  cxx_eval_bit_field_ref (const constexpr_
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (whole), i, field, value)
     {
       tree bitpos = bit_position (field);
+      STRIP_ANY_LOCATION_WRAPPER (value);
       if (bitpos == start && DECL_SIZE (field) == TREE_OPERAND (t, 1))
 	return value;
       if (TREE_CODE (TREE_TYPE (field)) == INTEGER_TYPE
--- gcc/testsuite/g++.dg/cpp0x/constexpr-92015.C.jj	2019-10-16 17:16:44.204871319 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-92015.C	2019-10-16 17:14:14.773127884 +0200
@@ -0,0 +1,7 @@ 
+// PR c++/92015
+// { dg-do compile { target c++11 } }
+
+struct S1 { char c[6] {'h', 'e', 'l', 'l', 'o', 0}; };
+struct S2 { char c[6] = "hello"; };
+static_assert (S1{}.c[0] == 'h', "");
+static_assert (S2{}.c[0] == 'h', "");