[PATCH 2/3] Avoid creating an initializer for a flexible array member

[Patrick Palka]

On Sun, Jan 3, 2016 at 3:14 PM, Martin Sebor <msebor@gmail.com> wrote:
> On 12/31/2015 08:40 AM, Patrick Palka wrote:
>>
>> If we do create such an initializer, we end up with an error_mark_node
>> during gimplification, because in cp-gimplify.c we pass this
>> VEC_INIT_EXPR of the flexible array member to build_vec_init, for which
>> it spits on an error_mark_node.  This happens in e.g. the test case
>> g++.dg/ext/array1.C.
>>
>> This patch makes it so that we avoid generating an initializer for a
>> flexible array member, thus avoiding to end up with an error_mark_node
>> during gimplification.  The same kind of thing is done ~90 lines down
>> from the code I changed.
>
>
> I don't think this change is correct (or complete).  We could
> decide that the kind of code it's meant to allow should be
> accepted in general but if we did, this change alone would not
> be sufficient.
>
> In the way of background, the existing special treatment for
> flexible array members was added to prevent rejecting cases
> like this one:
>
>   struct A { A (int); };
>   struct B {
>       B ();
>       int n;
>       A a[];
>   };
>
> Since B's ctor above doesn't initialize the flexible array
> member it's well-formed.  Ditto if B's ctor is defined but
> avoids initializing B::a.
>
> The proposed change has the effect of also accepting the
> following modified version of the example above that is
> currently rejected:
>
>   struct A { A (int); };
>   struct B {
>       B (): n (), a () { }
>       int n;
>       A a[];
>   };
>
> In this modified example, B's ctor attempts to default-initialize
> the flexible array member and its elements using their respective
> default ctors even though no such ctor for the latter exists.
>
> Since C++ flexible array members are a GCC extension whose C++
> specific aspects are essentially undocumented it's open to
> discussion whether or not code like this should be accepted.
> Clang rejects it, but one could make the argument that since
> the flexible array member has no elements, default initializing
> it should be allowed even if the same initialization would be
> ill-formed if the array wasn't empty.
>
> If the code should be accepted as written above then it should
> also be accepted if B's ctor were omitted or defaulted. I.e.,
> the following should be treated the same as the above:
>
>   struct A { A (int); };
>   struct B {
>       // ditto with B() = default;
>       int n;
>       A a[];
>   } b;
>
> The patch doesn't handle either of these cases and the code
> is rejected both with and without it.

Good catch.  I had not intended to change the validity of certain uses
of flexible array members with this patch.

So at least for now it seems that we should not avoid building a
VEC_INIT_EXPR of a flexible array member, since calling
build_vec_init_expr is necessary to diagnose some invalid uses of
flexible array members. So, I see three possible solutions to avoid
propagating an error_mark_node to the gimplifier while preserving the
exact behavior of flexible array members:

Continue to unconditionally call_build_vec_init_expr() in
perform_member_init(), and

1. Don't add the resulting VEC_INIT_EXPR to the statement tree if the
array in question has no upper bound (i.e. is a flexible array
member); or
2. Modify build_vec_init() to return an empty statement instead of
returning error_mark_node, if the array to be initialized has no upper
bound (and its initializer is NULL_TREE); or
3. Modify cp_gimplify_expr() to avoid calling build_vec_init() if the
VEC_INIT_EXPR in question is for an array with no upper bound (and its
initializer is NULL_TREE).

Which approach is best? The third one is the most conservative and the
one least likely to cause inadvertent changes in behavior elsewhere, I
think.

(In the end however, this "bug" is mostly harmless, and a patch fixing
it is really only useful in combination with patch #3/3 to avoid a
spurious ICE-on-valid.)