Hi Jason,
On 28/04/2016 23:45, Jason Merrill wrote:
I would expect this to cause a false negative on a union of two
anonymous structs, both of which have initialized members.
I think better would be to have a local any_default_members rather
than passing the same pointer through all levels.
Also, you can look at 'type' rather than DECL_CONTEXT (field).
.
... and thanks a lot for your very helpful reply. Now I realize that
something was wrong in the general logic here, not a tiny detail in a
conditional. Thus the below tries to closely follow your advice: the
idea is that any_default_members as required by check_field_decls should
still compute to the same value but the logic to emit the "multiple
fields in union initialized" diagnostic in check_field_decl is
different: it relies on the incoming any_default_members, not on what is
computed and returned at that level. Thus we can reject, eg, your case
of two anonymous struct. Also, for test5, which we already correctly
rejected, we do not emit an additional redundant error, and that's more
evidence that something bigger was wrong in check_field_decl. Anyway,
the below passes testing. How does it look?
Thanks,
Paolo.
/////////////////////
Index: cp/class.c
===================================================================
--- cp/class.c (revision 235615)
+++ cp/class.c (working copy)
@@ -139,7 +139,7 @@ static int count_fields (tree);
static int add_fields_to_record_type (tree, struct sorted_fields_type*, int);
static void insert_into_classtype_sorted_fields (tree, tree, int);
static bool check_bitfield_decl (tree);
-static void check_field_decl (tree, tree, int *, int *, int *);
+static bool check_field_decl (tree, tree, int *, int *, bool);
static void check_field_decls (tree, tree *, int *, int *);
static tree *build_base_field (record_layout_info, tree, splay_tree, tree *);
static void build_base_fields (record_layout_info, splay_tree, tree *);
@@ -3541,14 +3541,15 @@ check_bitfield_decl (tree field)
enclosing type T. Issue any appropriate messages and set appropriate
flags. */
-static void
+static bool
check_field_decl (tree field,
tree t,
int* cant_have_const_ctor,
int* no_const_asn_ref,
- int* any_default_members)
+ bool any_default_members)
{
tree type = strip_array_types (TREE_TYPE (field));
+ bool any_default_members_field = false;
/* In C++98 an anonymous union cannot contain any fields which would change
the settings of CANT_HAVE_CONST_CTOR and friends. */
@@ -3558,12 +3559,13 @@ check_field_decl (tree field,
structs. So, we recurse through their fields here. */
else if (ANON_AGGR_TYPE_P (type))
{
- tree fields;
-
- for (fields = TYPE_FIELDS (type); fields; fields = DECL_CHAIN (fields))
+ for (tree fields = TYPE_FIELDS (type); fields;
+ fields = DECL_CHAIN (fields))
if (TREE_CODE (fields) == FIELD_DECL && !DECL_C_BIT_FIELD (field))
- check_field_decl (fields, t, cant_have_const_ctor,
- no_const_asn_ref, any_default_members);
+ any_default_members_field |= check_field_decl (fields, t,
+ cant_have_const_ctor,
+ no_const_asn_ref,
+ any_default_members);
}
/* Check members with class type for constructors, destructors,
etc. */
@@ -3623,10 +3625,12 @@ check_field_decl (tree field,
{
/* `build_class_init_list' does not recognize
non-FIELD_DECLs. */
- if (TREE_CODE (t) == UNION_TYPE && *any_default_members != 0)
+ if (TREE_CODE (t) == UNION_TYPE && any_default_members)
error ("multiple fields in union %qT initialized", t);
- *any_default_members = 1;
+ any_default_members_field = true;
}
+
+ return any_default_members_field;
}
/* Check the data members (both static and non-static), class-scoped
@@ -3662,7 +3666,7 @@ check_field_decls (tree t, tree *access_decls,
tree *field;
tree *next;
bool has_pointers;
- int any_default_members;
+ bool any_default_members;
int cant_pack = 0;
int field_access = -1;
@@ -3672,7 +3676,7 @@ check_field_decls (tree t, tree *access_decls,
has_pointers = false;
/* Assume none of the members of this class have default
initializations. */
- any_default_members = 0;
+ any_default_members = false;
for (field = &TYPE_FIELDS (t); *field; field = next)
{
@@ -3868,10 +3872,10 @@ check_field_decls (tree t, tree *access_decls,
/* We set DECL_C_BIT_FIELD in grokbitfield.
If the type and width are valid, we'll also set DECL_BIT_FIELD. */
if (! DECL_C_BIT_FIELD (x) || ! check_bitfield_decl (x))
- check_field_decl (x, t,
- cant_have_const_ctor_p,
- no_const_asn_ref_p,
- &any_default_members);
+ any_default_members |= check_field_decl (x, t,
+ cant_have_const_ctor_p,
+ no_const_asn_ref_p,
+ any_default_members);
/* Now that we've removed bit-field widths from DECL_INITIAL,
anything left in DECL_INITIAL is an NSDMI that makes the class
Index: testsuite/g++.dg/cpp0x/nsdmi-anon-struct1.C
===================================================================
--- testsuite/g++.dg/cpp0x/nsdmi-anon-struct1.C (revision 0)
+++ testsuite/g++.dg/cpp0x/nsdmi-anon-struct1.C (working copy)
@@ -0,0 +1,48 @@
+// PR c++/66644
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wno-pedantic" }
+
+struct test1
+{
+ union
+ {
+ struct { char a=0, b=0; };
+ char buffer[16];
+ };
+};
+
+struct test2
+{
+ union
+ {
+ struct { char a=0, b; };
+ char buffer[16];
+ };
+};
+
+struct test3
+{
+ union
+ {
+ struct { char a, b; } test2{0,0};
+ char buffer[16];
+ };
+};
+
+struct test4
+{
+ union
+ { // { dg-error "multiple fields" }
+ struct { char a=0, b=0; };
+ struct { char c=0, d; };
+ };
+};
+
+struct test5
+{
+ union
+ {
+ union { char a=0, b=0; }; // { dg-error "multiple fields" }
+ char buffer[16];
+ };
+};
