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

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Jun 20, 2022 at 04:03:50PM -0400, Jason Merrill wrote:
> > +      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);
> > +	  ctors.safe_push (valp);
> > +	  vec_safe_push (indexes, index);
> > +	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
> > +	  gcc_checking_assert (refs->is_empty ());
> > +	  type = reftype;
> > +	  break;
> > +	}
...
> > @@ -5949,6 +5986,28 @@ cxx_eval_store_expression (const constex
> >         valp = ctx->global->values.get (object);
> >         for (unsigned i = 0; i < vec_safe_length (indexes); i++)
> >   	{
> > +	  ctors[i] = valp;
> > +	  if (TREE_CODE (indexes[i]) == REALPART_EXPR
> > +	      || TREE_CODE (indexes[i]) == IMAGPART_EXPR)
> > +	    {
> > +	      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);
> > +	      valp = &TREE_OPERAND (*valp,
> > +				    TREE_CODE (indexes[i]) == IMAGPART_EXPR);
> > +	      break;
> > +	    }
> 
> Hmm, why do we need to handle complex in the !preeval case?  I'd think we
> want to preevaluate all complex values or components thereof.

Because the late evaluation of the initializer could have touched
the destination, so we need to reevaluate it.
Same reason why we call get_or_insert_ctor_field again in the second
loop as we call it in the first loop.
If it would help, I could move that repeated part into:
tree
canonicalize_complex_to_complex_expr (tree t)
{
  if (TREE_CODE (t) == COMPLEX_CST)
   t = build2 (COMPLEX_EXPR, TREE_TYPE (t),
	       TREE_REALPART (t), TREE_IMAGPART (t));
  else if (TREE_CODE (t) == CONSTRUCTOR
	   && CONSTRUCTOR_NELTS (t) == 0
	   && CONSTRUCTOR_NO_CLEARING (t))
    {
      tree r = build_constructor (TREE_TYPE (TREE_TYPE (t)), NULL);
      CONSTRUCTOR_NO_CLEARING (r) = 1;
      t = build2 (COMPLEX_EXPR, TREE_TYPE (t), r, r);
    }
  return t;
}
and use that to shorten the code.

> 
> >   	  constructor_elt *cep
> >   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
> >   	  valp = &cep->value;
> > @@ -6012,17 +6071,41 @@ cxx_eval_store_expression (const constex
> >     bool c = TREE_CONSTANT (init);
> >     bool s = TREE_SIDE_EFFECTS (init);
> >     if (!c || s || activated_union_member_p)
> > -    for (tree elt : *ctors)
> > +    for (tree *elt : ctors)
> >         {
> > +	if (TREE_CODE (*elt) != CONSTRUCTOR)
> > +	  continue;
> >   	if (!c)
> > -	  TREE_CONSTANT (elt) = false;
> > +	  TREE_CONSTANT (*elt) = false;
> >   	if (s)
> > -	  TREE_SIDE_EFFECTS (elt) = true;
> > +	  TREE_SIDE_EFFECTS (*elt) = true;
> >   	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
> >   	   this union.  */
> > -	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
> > -	  CONSTRUCTOR_NO_CLEARING (elt) = false;
> > +	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
> > +	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
> >         }
> > +  if (!indexes->is_empty ())
> > +    {
> > +      tree last = indexes->last ();
> > +      if (TREE_CODE (last) == REALPART_EXPR
> > +	  || TREE_CODE (last) == IMAGPART_EXPR)
> > +	{
> > +	  tree *cexpr = ctors.last ();
> > +	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
> > +				    TREE_OPERAND (*cexpr, 0),
> > +				    TREE_OPERAND (*cexpr, 1)))
> > +	    *cexpr = c;
> > +	  else
> > +	    {
> > +	      TREE_CONSTANT (*cexpr)
> > +		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
> > +		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
> > +	      TREE_SIDE_EFFECTS (*cexpr)
> > +		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
> > +		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));
> 
> This seems like it needs to come before the ctors loop, so that these flags
> can be propagated out to enclosing constructors.

I could indeed move this in between
  bool c = TREE_CONSTANT (init);
  bool s = TREE_SIDE_EFFECTS (init);
and
  if (!c || s || activated_union_member_p)
and update c and s from *cexpr flags.

	Jakub