Re: PR tree-opt/33616: ICE on callee-copied constructor arguments

[Richard Sandiford <rsandifo at nildram dot co dot uk>]

Eric Botcazou <ebotcazou@libertysurf.fr> writes:
>> We try to take the address of the original CONSTRUCTOR (rather than
>> taking the address of a temporary as in the caller-copied case) and
>> expand_expr_addr_expr_1 assumes that all CONSTRUCTORs are constant:
>>
>>   /* If we are taking the address of a constant and are at the top level,
>>      we have to use output_constant_def since we can't call force_const_mem
>>      at top level.  */
>>   /* ??? This should be considered a front-end bug.  We should not be
>>      generating ADDR_EXPR of something that isn't an LVALUE.  The only
>>      exception here is STRING_CST.  */
>>   if (TREE_CODE (exp) == CONSTRUCTOR
>>
>>       || CONSTANT_CLASS_P (exp))
>>
>>     return XEXP (expand_expr_constant (exp, 0, modifier), 0);
>>
>> However, the comment doesn't really seem to fit this case:
>> the CONSTRUCTOR _is_ an lvalue.
>
> No, a CONSTRUCTOR is not an lvalue, it cannot be the LHS of an assignment.

Sorry, my mistake.  I'd assumed that because C's COMPOUND_LITERAL_EXPRs
are lvalues -- as in:

    struct s { int i, j; };
    void foo (void) { (struct s) { 1, 2 } = (struct s) { 3, 4 }; }

-- that CONSTRUCTORs would be too.

> And I think that you should remove the ??? comment
>
>   /* ??? This should be considered a front-end bug.  We should not be
>      generating ADDR_EXPR of something that isn't an LVALUE.  The only
>      exception here is STRING_CST.  */
>
> now that the blatant non-LVALUE case has been eliminated.

Well, the comment is still above a check for CONSTANT_CLASS_P:

  /* If we are taking the address of a constant and are at the top level,
     we have to use output_constant_def since we can't call force_const_mem
     at top level.  */
  /* ??? This should be considered a front-end bug.  We should not be
     generating ADDR_EXPR of something that isn't an LVALUE.  The only
     exception here is STRING_CST.  */
  if (CONSTANT_CLASS_P (exp))
    return XEXP (expand_expr_constant (exp, 0, modifier), 0);

and if I were reading that without the ???, I'd be wondering why we're
taking tha address of things like INTEGER_CSTs.  I think it's useful
to have a comment that says we aren't really doing so out of choice.

Richard