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

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Jun 10, 2022 at 01:27:28PM -0400, Jason Merrill wrote:
> > --- 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?

No, even when handling the pushed *PART_EXPR, it will set
valp = &TREE_OPERAND (*valp, index != integer_zero_node);
So, valp will be either &TREE_OPERAND (*complex_expr, 0)
or &TREE_OPERAND (*complex_expr, 1).
As *valp = init; is what is usually then stored and we want to store there
the scalar.

> > @@ -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?

Because for most references, the code will do:
      vec_safe_push (ctors, *valp);
      vec_safe_push (indexes, index);
I chose not to do this for *PART_EXPR, because the COMPLEX_EXPR isn't a
CONSTRUCTOR and code later on e.g. walks all the ctors and accesses
CONSTRUCTOR_NO_CLEARING on them etc.  As the *PART_EXPR is asserted to
be outermost only, complex_expr is a variant of that ctors push and
complex_part of the indexes.
The reason for the above if is just in case the evaluation of the rhs
of the store would store to the complex and could e.g. make it a COMPLEX_CST
again.

> > +	}
> >       }
> >     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.

See above.
The COMPLEX_EXPR needs special handling (conversion into COMPLEX_CST if it
is constant) anyway.

	Jakub