This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] accept flexible arrays in struct in unions (c++/71912 - [6/7 regression])
It could be raised with WG14 for clarification or even proposed
as a change but given existing practice I don't think it would
be likely to gain committee support, or worth trying. It seems
simpler and, IMO, more useful for portability to do what others
do, at least in the potentially problematic cases like the one
in c++/71912 where the members overlap.
I definitely agree that following the lead of the C front end is the
right thing to do here. But the C front end doesn't complain about
the definition of union U, only about the use of union U in struct S.
So I still think the first problem mentioned in your comment isn't
actually a problem.
Hmm. I see what you mean. That the C wording makes union U below
valid
union U {
struct X { int i, a[]; } x;
struct Y { long i, a[]; } y;
};
but its use in struct S below invalid:
struct S { union U u; };
C does seem to say that though I confess it doesn't make sense
to me. Let me see what it would take to match C here.
ANON_AGGR_TYPE_P should never be true for an unnamed class that is not
an anonymous aggregate (union or struct). The glitch you mentioned
before was that it is sometimes false for an anonymous struct, which
happens when if we check ANON_AGGR_TYPE_P before we see the ; and so
we don't yet know that it's an anonymous struct, we only know it's
unnamed.
When we're looking at it in the context of the enclosing class,
ANON_AGGR_TYPE_P should always be correct. If it isn't, let's fix
that bug.
ANON_AGGR_TYPE_P is correct in the context of the enclosing class,
but it isn't "correct" when the struct is being finished, before
the enclosing class. That's why check_flexarrays skips unnamed
structs, but it doesn't mean diagnose_flexarrays couldn't use it.
I hadn't realized that.
It seems to me that to do that I would need to somehow follow the
path from an array's DECL_CONTEXT to the DECL_CONTEXT of each of
the "members" (anonymous structs, not just their types) it's
recursively defined in. But given the "glitch", how would I tell
whether the struct is anonymous from just its type? I can't think
of a way to do that.
You should be able to rely on ANON_AGGR_TYPE_P and use TYPE_CONTEXT.
I have not removed anonctx because of the problem mentioned above.
If you're convinced it can be done please show me how.
Does the above help?
Yes. I actually had it mostly working before (the anon_context
function I had pasted into one my earlier replies). After restoring
the function and correcting a silly bug in it, it works for simple
cases of structs and their members, and with some additional
conditioning it also works for base and derived classes. Attached
is the delta.
The change let me replace an if statement and the ANONCTX data
member with a new function plus a conditional. I'll let you be
the judge but to me it doesn't seem like an improvement.
Hmm, doesn't that mean that
typedef struct { int a[]; } B;
is never handled? Perhaps you want to do this check from
cp_parser_simple_declaration, when we know whether or not there's a
declarator?
Yes, it does mean that and it's one of the outstanding bugs that
still need fixing (in 7.0). Same way top-level arrays aren't
handled.
Martin
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 05d97a7..6232070 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -6721,10 +6721,6 @@ struct flexmems_t
};
*/
tree after[2];
-
- /* 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;
};
/* Find either the first flexible array member or the first zero-length
@@ -6826,14 +6822,9 @@ find_flexarrays (tree t, flexmems_t *fmem, bool base_p,
find_flexarrays (eltype, fmem, false, pun);
if (fmem->array != array)
- {
- /* If the member struct contains the first flexible array
- member, store the enclosing struct if it is anonymous. */
- if (anon_p)
- fmem->anonctx = t;
- continue;
- }
- else if (first && !array && !anon_p)
+ continue;
+
+ if (first && !array && !anon_p)
{
/* Restore the FIRST member reset above if no flexible
array member has been found in this member's struct. */
@@ -6904,6 +6895,22 @@ find_flexarrays (tree t, flexmems_t *fmem, bool base_p,
}
}
+/* Return the type in which the member T is effectively declared in.
+ This is either the member's enclosing struct for ordinary members,
+ or for anonymous structs and unions, the first non-anonymou struct
+ or union they are declared in. */
+
+static tree
+anon_context (tree t)
+{
+ if (TREE_CODE (t) == FIELD_DECL)
+ return anon_context (DECL_CONTEXT (t));
+
+ bool anon_p = ANON_AGGR_TYPE_P (t);
+
+ return anon_p ? anon_context (TYPE_CONTEXT (t)) : t;
+}
+
/* Issue diagnostics for invalid flexible array members or zero-length
arrays that are not the last elements of the containing class or its
base classes or that are its sole members. */
@@ -6913,7 +6920,7 @@ diagnose_flexarrays (tree t, const flexmems_t *fmem)
{
/* Members of anonymous structs and unions are considered to be members
of the containing struct or union. */
- if (!fmem->array)
+ if (!fmem->array || (fmem->first && !fmem->after[0] && !fmem->after[1]))
return;
/* Issue errors first, and when no errors are found, then warnings
@@ -6923,7 +6930,10 @@ diagnose_flexarrays (tree t, const flexmems_t *fmem)
/* 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 = fmem->anonctx ? fmem->anonctx : t;
+ tree fmemctx = anon_context (fmem->array);
+ bool anon_p = fmemctx != DECL_CONTEXT (fmem->array);
+ if (!anon_p)
+ fmemctx = t;
const char *msg = 0;
@@ -6931,7 +6941,7 @@ diagnose_flexarrays (tree t, const flexmems_t *fmem)
{
if (fmem->after[in_union])
msg = (in_union
- ? (fmem->anonctx
+ ? (anon_p
? G_("zero-size array member %qD belonging to %q#T")
: G_("zero-size array member %qD"))
: G_("zero-size array member %qD not at end of %q#T"));
@@ -6970,7 +6980,7 @@ diagnose_flexarrays (tree t, const flexmems_t *fmem)
{
if (fmem->after[in_union])
msg = (in_union
- ? (fmem->anonctx
+ ? (anon_p
? G_("flexible array member %qD belonging to %q#T")
: G_("flexible array member %qD"))
: G_("flexible array member %qD not at end of %q#T"));