Re: [PATCH] c++: Add support for __real__/__imag__ modifications in constant expressions [PR88174]

[Jason Merrill <jason@redhat.com>]

On 6/9/22 04:37, Jakub Jelinek wrote:
> Hi!
> 
> We claim we support P0415R1 (constexpr complex), but e.g.
> #include <complex>
> 
> constexpr bool
> foo ()
> {
>    std::complex<double> a (1.0, 2.0);
>    a += 3.0;
>    a.real (6.0);
>    return a.real () == 6.0 && a.imag () == 2.0;
> }
> 
> static_assert (foo ());
> 
> fails with
> test.C:12:20: error: non-constant condition for static assertion
>     12 | static_assert (foo ());
>        |                ~~~~^~
> test.C:12:20:   in ‘constexpr’ expansion of ‘foo()’
> test.C:8:10:   in ‘constexpr’ expansion of ‘a.std::complex<double>::real(6.0e+0)’
> test.C:12:20: error: modification of ‘__real__ a.std::complex<double>::_M_value’ is not a constant expression
> 
> The problem is we don't handle REALPART_EXPR and IMAGPART_EXPR
> in cxx_eval_store_expression.
> The following patch attempts to support it (with a requirement
> that those are the outermost expressions, ARRAY_REF/COMPONENT_REF
> etc. are just not possible on the result of these, BIT_FIELD_REF
> would be theoretically possible if trying to extract some bits
> from one part of a complex int, but I don't see how it could appear
> in the FE trees.
> 
> For these references, the code handles value being COMPLEX_CST,
> COMPLEX_EXPR or CONSTRUCTOR_NO_CLEARING empty CONSTRUCTOR (what we use
> to represent uninitialized values for C++20 and later) and the
> code starts by rewriting it to COMPLEX_EXPR, so that we can freely
> adjust the individual parts and later on possibly optimize it back
> to COMPLEX_CST if both halves are constant.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-06-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/88174
> 	* constexpr.cc (cxx_eval_store_expression): Handle REALPART_EXPR
> 	and IMAGPART_EXPR.
> 
> 	* g++.dg/cpp1y/constexpr-complex1.C: New test.
> 
> --- gcc/cp/constexpr.cc.jj	2022-06-08 08:21:02.973448193 +0200
> +++ gcc/cp/constexpr.cc	2022-06-08 17:13:04.986040449 +0200
> @@ -5707,6 +5707,20 @@ cxx_eval_store_expression (const constex
>   	  }
>   	  break;
>   
> +	case REALPART_EXPR:
> +	  gcc_assert (probe == target);

Doesn't this assert mean that complex_expr will always be == valp?

> +	  vec_safe_push (refs, integer_zero_node);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
> +	case IMAGPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, integer_one_node);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
>   	default:
>   	  if (evaluated)
>   	    object = probe;
> @@ -5749,6 +5763,8 @@ cxx_eval_store_expression (const constex
>     auto_vec<int> index_pos_hints;
>     bool activated_union_member_p = false;
>     bool empty_base = false;
> +  int complex_part = -1;
> +  tree *complex_expr = NULL;
>     while (!refs->is_empty ())
>       {
>         if (*valp == NULL_TREE)
> @@ -5785,14 +5801,36 @@ cxx_eval_store_expression (const constex
>   	  *valp = ary_ctor;
>   	}
>   
> -      /* If the value of object is already zero-initialized, any new ctors for
> -	 subobjects will also be zero-initialized.  */
> -      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> -
>         enum tree_code code = TREE_CODE (type);
>         tree reftype = refs->pop();
>         tree index = refs->pop();
>   
> +      if (code == COMPLEX_TYPE)
> +	{
> +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> +	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
> +			    TREE_IMAGPART (*valp));
> +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> +		   && CONSTRUCTOR_NELTS (*valp) == 0
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> +	    {
> +	      tree r = build_constructor (reftype, NULL);
> +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> +	      *valp = build2 (COMPLEX_EXPR, type, r, r);
> +	    }
> +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	  complex_expr = valp;
> +	  valp = &TREE_OPERAND (*valp, index != integer_zero_node);
> +	  gcc_checking_assert (refs->is_empty ());
> +	  type = reftype;
> +	  complex_part = index != integer_zero_node;
> +	  break;
> +	}
> +
> +      /* If the value of object is already zero-initialized, any new ctors for
> +	 subobjects will also be zero-initialized.  */
> +      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> +
>         if (code == RECORD_TYPE && is_empty_field (index))
>   	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
>   	   have no data and might have an offset lower than previously declared
> @@ -5946,6 +5984,24 @@ cxx_eval_store_expression (const constex
>   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>   	  valp = &cep->value;
>   	}
> +      if (complex_part != -1)
> +	{
> +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> +	    *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp),
> +			    TREE_REALPART (*valp),
> +			    TREE_IMAGPART (*valp));
> +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> +		   && CONSTRUCTOR_NELTS (*valp) == 0
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> +	    {
> +	      tree r = build_constructor (TREE_TYPE (TREE_TYPE (*valp)), NULL);
> +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> +	      *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp), r, r);
> +	    }
> +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	  complex_expr = valp;
> +	  valp = &TREE_OPERAND (*valp, complex_part);

I don't understand this block; shouldn't valp point to the real or imag 
part of the complex number at this point?  How could complex_part be set 
without us handling the complex case in the loop already?

> +	}
>       }
>   
>     if (*non_constant_p)
> @@ -6016,6 +6072,22 @@ cxx_eval_store_expression (const constex
>   	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
>   	  CONSTRUCTOR_NO_CLEARING (elt) = false;
>         }
> +  if (complex_expr)

I might have added the COMPLEX_EXPR to ctors instead of a separate 
variable, but this is fine too.

> +    {
> +      if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*complex_expr),
> +				TREE_OPERAND (*complex_expr, 0),
> +				TREE_OPERAND (*complex_expr, 1)))
> +	*complex_expr = c;
> +      else
> +	{
> +	  TREE_CONSTANT (*complex_expr)
> +	    = (TREE_CONSTANT (TREE_OPERAND (*complex_expr, 0))
> +	       & TREE_CONSTANT (TREE_OPERAND (*complex_expr, 1)));
> +	  TREE_SIDE_EFFECTS (*complex_expr)
> +	    = (TREE_SIDE_EFFECTS (TREE_OPERAND (*complex_expr, 0))
> +	       | TREE_SIDE_EFFECTS (TREE_OPERAND (*complex_expr, 1)));
> +	}
> +    }
>   
>     if (lval)
>       return target;
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-06-08 17:32:39.190148964 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-06-08 17:29:04.413321741 +0200
> @@ -0,0 +1,24 @@
> +// PR c++/88174
> +// { dg-do compile { target c++14 } }
> +
> +constexpr bool
> +foo (double x, double y, double z, double w)
> +{
> +  __complex__ double a = 0;
> +  __real__ a = x;
> +  __imag__ a = y;
> +#if __cpp_constexpr >= 201907L
> +  __complex__ double b;
> +  __real__ b = z;
> +#else
> +  __complex__ double b = z;
> +#endif
> +  __imag__ b = w;
> +  a += b;
> +  a -= b;
> +  a *= b;
> +  a /= b;
> +  return __real__ a == x && __imag__ a == y;
> +}
> +
> +static_assert (foo (1.0, 2.0, 3.0, 4.0), "");
> 
> 	Jakub
>