On 07/23/2016 01:18 PM, Martin Sebor wrote:
+ /* A pair of the first non-static non-empty data members following
+ either the flexible array member, if found, or the zero-length
+ array member otherwise. AFTER[1] refers to the first such data
+ member of a union that the struct containing the flexible array
+ member or zero-length array is a member, or NULL when no such
+ union exists. AFTER[0] refers to the first such data member
+ that is not a member of such a union. */
This is pretty hard to follow, could you add an example? Why do we want
to track these separately rather than look at DECL_CONTEXT at diagnostic
time?
+ /* The type in which an anonymous struct or union containing ARRAY
+ is defined or null if no such anonymous struct or union exists. */
+ tree anonctx;
It seems clearer to find this at diagnostic time by following TYPE_CONTEXT.
/* Find the next non-static data member if it exists. */
for (next = fld;
(next = DECL_CHAIN (next))
&& TREE_CODE (next) != FIELD_DECL; );
I think next_initializable_field would be useful here.
if (TREE_CODE (fld) != TYPE_DECL
&& RECORD_OR_UNION_TYPE_P (fldtype)
- && TYPE_ANONYMOUS_P (fldtype))
+ && VAR_DECL != TREE_CODE (fld)
+ && (FIELD_DECL != TREE_CODE (fld) || !DECL_FIELD_IS_BASE (fld)))
Please put the constant on the RHS of comparisons.
It seems that you're only interested in FIELD_DECL here, so maybe move
up the
/* Skip anything that's not a (non-static) data member. */
if (TREE_CODE (fld) != FIELD_DECL)
continue;
and remove the checks for TYPE_DECL and VAR_DECL? For that matter, you
could also move up the DECL_ARTIFICIAL check so you don't need to check
DECL_FIELD_IS_BASE.
+ /* Is the member an anonymous struct or union? */
+ bool anon_p = (!TYPE_ANONYMOUS_P (fldtype)
+ || !DECL_NAME (fld)
+ || anon_aggrname_p (DECL_NAME (fld)));
This looks like anon_p will be true for any non-static data member of
non-anonymous type? Maybe you want ANON_AGGR_TYPE_P?
+ /* If this member isn't anonymous and a prior non-flexible array
+ member has been seen in one of the enclosing structs, clear
+ the FIRST member since it doesn't contribute to the flexible
+ array struct's members. */
+ if (first && !array && !anon_p)
+ fmem->first = NULL_TREE;
Why is this needed? For a non-anonymous member, presumably we'll give
an error when analyzing the type of the member on its own, so we
shouldn't need to complain again here.
/* Replace the zero-length array if it's been stored and
reset the after pointer. */
if (TYPE_DOMAIN (TREE_TYPE (fmem->array)))
{
fmem->after[bool (pun)] = NULL_TREE;
fmem->array = fld;
}
So we silently allow a zero-length array followed by a flexible array? Why?
+ msg = G_("zero-size array member %qD in an otherwise empty %q#T");
+
+ if (msg && pedwarn (DECL_SOURCE_LOCATION (fmem->array),
+ OPT_Wpedantic, msg, fmem->array, fmemctx))
+ {
+ inform (location_of (t), "in the definition of %q#T", fmemctx);
+
+ /* Prevent the same flexible array member from being diagnosed
+ more than once if it happens to be nested in more than one
+ union and overlap with another member. This avoids multiple
+ warnings for perverse cases like the the following where
+ U::U1::X::a1 would otherwise be diagnosed first followed by
+ S::U1::X::a1:
+ struct S {
+ union U {
+ union U1 { struct X { int n1, a1[]; } x1; } u1;
+ union U2 { struct X { int n2, a2[]; } x1; } u2;
+ } u;
+ } s;
+ */
+ TREE_NO_WARNING (fmem->array) = 1;
+ }
There doesn't seem to be anything that checks TREE_NO_WARNING for the
zero-size array case.
+ /* Avoid issuing further duiagnostics after the error above. */
Typo.
if (fmem == &flexmems
&& !TYPE_ANONYMOUS_P (t) && !anon_aggrname_p (TYPE_IDENTIFIER (t)))
I think you want ANON_AGGR_TYPE_P here, too.
I also wonder about integrating this with layout_class_type, since that
is also looking at layout issues.
Jason
