Re: [PATCH] middle-end/23155 ICE with union cast and array subscriptting

[Andrew Pinski <pinskia at physics dot uc dot edu>]

> 
> Andrew Pinski <pinskia@physics.uc.edu> writes:
> 
> > ChangeLog:
> > 	* gimplifier.c (gimplify_expr): Create a temporary for lvalue
> > 	CONSTRUCTOR.
> > 
> > testsuite/ChangeLog:
> > 
> > 	* gcc.dg/union-cast-1.c: New test.
> > 	* gcc.dg/union-cast-2.c: New test.
> > 	* gcc.dg/union-cast-3.c: New test.
> 
> This is OK, except for this comment:
> 
> +	  /* A GNU extension that allows a cast to an union and C99 allows
> +	     non lvalue arrays to be subscriptted which cause this issue.
> +	     This has undefined behavior only on
> +	     execution, so create a temporary if an lvalue is
> +	     required.  */
> 
> How about something along the lines of:
> 
> 	  /* C99 code may assign to an array in a constructed
> 	     structure or union, and this has undefined behavior only
> 	     on execution, so create a temporary if an lvalue is
> 	     required.  */
> 
> The intention is to be parallel to the existing comments for the
> similar cases.
> 
> In particular, I don't think this has anything to do with unions.  I
> can recreate the ICE with this C++ test case (the test case works in C
> because constructed structures are handled differently by the C
> frontend).
> 
> struct s { int a[1]; };
> 
> void
> foo5 (void)
> {
>   ((struct s) { { 0 } }).a[0] = 1;
> }
> 
> Please do confirm that your patch fixes this C++ test case as well.

It does but really it is more of an accident that it fixes it as the C++
front-end design mistake in how they handle the C99 anonymous struct
variables  (There are a couple of bugs about this).  In C99, this
construct is a true lvalue while the C++ makes it not fully an lvalue.

Does this clear up why this more related to the GNU extension for union
casts rather than more generic issue?

Or should I just change the comment and apply the C++ testcase also?

Thanks,
Andrew Pinski