diff mbox

[C++] PR 66644

Message ID 572389F4.6000103@oracle.com
State New
Headers show

Commit Message

Paolo Carlini April 29, 2016, 4:21 p.m. UTC
Hi,

On 29/04/2016 15:58, Jason Merrill wrote:
> On 04/28/2016 08:18 PM, Paolo Carlini wrote:
>>     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);
>
> The logic here seems convoluted.  I guess we don't need to handle 
> anonymous structs and unions differently here because we'll call 
> check_field_decls for the anonymous union itself, and complain then?  
> In that case, instead of passing down any_default_members at all, can 
> we just pass it up and complain in check_field_decls?
Indeed. Code reworked along these lines becomes even comprehensible ;) 
Something as simple as the below passes testing....

Thanks,
Paolo.

////////////////////////////

Comments

Jason Merrill April 29, 2016, 6:32 p.m. UTC | #1
OK, thanks.

Jason
diff mbox

Patch

Index: cp/class.c
===================================================================
--- cp/class.c	(revision 235643)
+++ 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 *);
 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,14 @@  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)
+		  int* no_const_asn_ref)
 {
   tree type = strip_array_types (TREE_TYPE (field));
+  bool any_default_members = 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 +3558,12 @@  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 |= check_field_decl (fields, t,
+						   cant_have_const_ctor,
+						   no_const_asn_ref);
     }
   /* Check members with class type for constructors, destructors,
      etc.  */
@@ -3620,13 +3620,11 @@  check_field_decl (tree field,
   check_abi_tags (t, field);
 
   if (DECL_INITIAL (field) != NULL_TREE)
-    {
-      /* `build_class_init_list' does not recognize
-	 non-FIELD_DECLs.  */
-      if (TREE_CODE (t) == UNION_TYPE && *any_default_members != 0)
-	error ("multiple fields in union %qT initialized", t);
-      *any_default_members = 1;
-    }
+    /* `build_class_init_list' does not recognize
+       non-FIELD_DECLs.  */
+    any_default_members = true;
+
+  return any_default_members;
 }
 
 /* Check the data members (both static and non-static), class-scoped
@@ -3662,7 +3660,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 +3670,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)
     {
@@ -3867,11 +3865,16 @@  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);
+      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))
+	{
+	  if (any_default_members
+	      && TREE_CODE (t) == UNION_TYPE)
+	    error ("multiple fields in union %qT initialized", t);
+	  any_default_members = true;
+	}
 
       /* 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];
+  };
+};