Re: prevent creation of invalid static constructors from gimplifier

[Ian Lance Taylor <iant at google dot com>]

Olivier Hainque <hainque@adacore.com> writes:

> > In any case it suggests a simple fix: change
> > categorize_ctor_elements_1 so that when it sees a constructor for
> > which TREE_CONSTANT is not set, it increments nc_elts.
> 
>  I had thought of something along these lines but this unfortunately
>  doesn't catch all the cases. The basic point is that, as the name
>  conveys, categorize_ctor_elements_1 states facts about the constructor
>  _elements_ and not about the outer constructor itself.

Good point.  That tells me that I suggested checking TREE_CONSTANT in
the wrong place; we should check it at the end of
categorize_ctor_elements_1, not in the CONSTRUCTOR case in that
function.

>  Whether this should be handled by categorize_ctor_elements_1 was not clear
>  to me, since the implementation matches what the name conveys and we seemed
>  to be looking for a more general property. This is what prompted my question
>  at http://gcc.gnu.org/ml/gcc/2006-03/msg00729.html.

In general, you are certainly correct.  However, I note that the value
returned in num_nonconstant_elements by categorize_ctor_elements is
only used in one place: gimplify_init_constructor.  And I note that
what we are really trying to do here is work around a deficiency in
output_constructor.  Short of fixing output_constructor, we are
picking among bad options.

So to me it seems at least plausible to block the call to
output_constructor at the point of least damage, which to me seems
like categorize_ctor_elements_1.  With of course an appropriate
comment and FIXME.

But here is another option, which may be more reasonable.  We add a
new return value to categorize_ctor_elements: whether the value is a
valid constant initializer.  This is not as crazy as it sounds, since
categorize_ctor_elements_1 already defers to
initializer_constant_valid_p for most cases.  The only case which
needs any special handling in categorize_ctor_elements_1 is
CONSTRUCTOR itself, and it would need to be handled much as we have
already discussed.  This does raise the issue that the two different
functions may get out of synch, but as your bug shows we already face
that issue and just didn't know it.

And, of course, if we do add that new return value, we might as well
remove the p_nc_elts return value, since it will no longer be used.

Ian