On 09/14/2016 01:03 PM, Martin Sebor wrote:
Attached is an updated patch that does away with the "overlapping"
warning and instead issues the same warnings as the C front end
(though with more context).
In struct flexmems_t I've retained the NEXT array even though only
the first element is used to diagnose problems. The second element
is used by the find_flexarrays function to differentiate structs
with flexible array members in unions (which are valid) from those
in structs (which are not).
FWIW, I think this C restriction on structs is unnecessary and will
propose to remove it so that's valid to define a struct that contains
another struct (possibly recursively) with a flexible array member.
I.e., I think this should be made valid in C (and accepted without
the pedantic warning that GCC, and with this patch also G++, issues):
Agreed.
+ /* Is FLD a typedef for an anonymous struct? */
+ bool anon_type_p
+ = (TREE_CODE (fld) == TYPE_DECL
+ && DECL_IMPLICIT_TYPEDEF_P (fld)
+ && anon_aggrname_p (DECL_NAME (fld)));
We talked earlier about handling typedefs in
cp_parser_{simple,single}_declaration, so that we catch
typedef struct { int a[]; } B;
or at least adding a FIXME comment here explaining that looking at
typedefs is a temporary hack.
+ /* Type of the member. */
+ tree fldtype = TREE_CODE (fld) == FIELD_DECL ? TREE_TYPE (fld) : fld;
Why set "fldtype" to be a TYPE_DECL rather than its type?
+ /* Determine the type of the array element or object referenced
+ by the member so that it can be checked for flexible array
+ members if it hasn't been yet. */
+ tree eltype = TREE_CODE (fld) == FIELD_DECL ? fldtype : TREE_TYPE (fld);
Given the above, this seems equivalent to
tree eltype = TREE_TYPE (fld);
+ if (RECORD_OR_UNION_TYPE_P (eltype))
+ {
...
+ if (fmem->array && !fmem->after[bool (pun)])
+ {
+ /* Once the member after the flexible array has been found
+ we're done. */
+ fmem->after[bool (pun)] = fld;
+ break;
+ }
...
if (field_nonempty_p (fld))
{
...
/* Remember the first non-static data member after the flexible
array member, if one has been found, or the zero-length array
if it has been found. */
if (fmem->array && !fmem->after[bool (pun)])
fmem->after[bool (pun)] = fld;
}
Can we do this in one place rather than two?
+ if (eltype == fldtype || TYPE_UNNAMED_P (eltype))
This is excluding arrays, so we don't diagnose, say,
struct A
{
int i;
int ar[];
};
struct B {
A a[2];
};
Should we complain elsewhere about an array of a class with a flexible
array member?
+ /* Does the field represent an anonymous struct? */
+ bool anon_p = !DECL_NAME (fld) && ANON_AGGR_TYPE_P (eltype);
You don't need to check DECL_NAME; ANON_AGGR_TYPE_P by itself indicates
that we're dealing with an anonymous struct/union.
Similarly, PSTR is set to the outermost struct of which T is a member
if one such struct exists, otherwise to NULL. */
...
find_flexarrays (eltype, fmem, false, pun,
!pstr && TREE_CODE (t) == RECORD_TYPE ? fld :
pstr);
...
+diagnose_invalid_flexarray (const flexmems_t *fmem)
+{
+ if (fmem->array && fmem->enclosing
+ && pedwarn (location_of (fmem->enclosing), OPT_Wpedantic,
+ TYPE_DOMAIN (TREE_TYPE (fmem->array))
+ ? G_("invalid use of %q#T with a zero-size array "
+ "in %q#D")
+ : G_("invalid use of %q#T with a flexible array member "
+ "in %q#T"),
+ DECL_CONTEXT (fmem->array),
+ DECL_CONTEXT (fmem->enclosing)))
PSTR is documented to be the outermost struct, but it (and thus
fmem->enclosing) end up being a data member of that outermost struct,
which is why you need to take its DECL_CONTEXT. What's the advantage of
doing that over passing t itself to the recursive call?
+ /* The context of the flexible array member. Either the struct
+ in which it's declared or, for anonymous structs and unions,
+ the struct/union of which the array is effectively a member. */
+ tree fmemctx = anon_context (fmem->array);
+ bool anon_p = fmemctx != DECL_CONTEXT (fmem->array);
+ if (!anon_p)
+ fmemctx = t;
Why do you need to do something different here based on anon_p? I don't
see why that would affect whether we want to look through intermediate
non-anonymous classes.
+ check_flexarrays (basetype, fmem, true);
Please decorate boolean literal arguments like this with the name of the
parameter, e.g. /*base_p*/true.
+ bool maybe_anon_p = anon_aggrname_p (TYPE_IDENTIFIER (t));
Now we can use TYPE_UNNAMED_P.
Jason
