[C++ Patch] PR 66644

[Paolo Carlini]

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.

/////////////////////
-------------- next part --------------
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];
+  };
+};