[C++] Fix decomp handling of unnamed bitfields (PR c++/84031)

Hi!

Unnamed bitfields are not data members, so we should ignore them when
counting the members or picking up members to initialize from, and
also should ignore them in find_decomp_class_base, they can appear
in various bases etc. and still there could be just one base containing
direct non-static data members.

The patch also fixes an issue I ran into on the testcase, when recursing
from type in which we've found any direct non-static data members, we have
ret set to that type and if the recursion doesn't find anything nor an
error, it returns that ret rather than NULL.  So, if t == ret, it just
means it didn't find anything preventing return of ret to the caller.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-01-25  Jakub Jelinek  <jakub@redhat.com>

	PR c++/84031
	* decl.c (find_decomp_class_base): Ignore unnamed bitfields.  Ignore
	recursive calls that return ret.
	(cp_finish_decomp): Ignore unnamed bitfields.

	* g++.dg/cpp1z/decomp36.C: New test.


	Jakub

OK.

On Thu, Jan 25, 2018 at 10:26 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Unnamed bitfields are not data members, so we should ignore them when
> counting the members or picking up members to initialize from, and
> also should ignore them in find_decomp_class_base, they can appear
> in various bases etc. and still there could be just one base containing
> direct non-static data members.
>
> The patch also fixes an issue I ran into on the testcase, when recursing
> from type in which we've found any direct non-static data members, we have
> ret set to that type and if the recursion doesn't find anything nor an
> error, it returns that ret rather than NULL.  So, if t == ret, it just
> means it didn't find anything preventing return of ret to the caller.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-01-25  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/84031
>         * decl.c (find_decomp_class_base): Ignore unnamed bitfields.  Ignore
>         recursive calls that return ret.
>         (cp_finish_decomp): Ignore unnamed bitfields.
>
>         * g++.dg/cpp1z/decomp36.C: New test.
>
> --- gcc/cp/decl.c.jj    2018-01-24 17:18:42.376392255 +0100
> +++ gcc/cp/decl.c       2018-01-25 10:22:51.267122576 +0100
> @@ -7206,7 +7206,9 @@ find_decomp_class_base (location_t loc,
>  {
>    bool member_seen = false;
>    for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> -    if (TREE_CODE (field) != FIELD_DECL || DECL_ARTIFICIAL (field))
> +    if (TREE_CODE (field) != FIELD_DECL
> +       || DECL_ARTIFICIAL (field)
> +       || (DECL_C_BIT_FIELD (field) && !DECL_NAME (field)))
>        continue;
>      else if (ret)
>        return type;
> @@ -7245,7 +7247,7 @@ find_decomp_class_base (location_t loc,
>        tree t = find_decomp_class_base (loc, TREE_TYPE (base_binfo), ret);
>        if (t == error_mark_node)
>         return error_mark_node;
> -      if (t != NULL_TREE)
> +      if (t != NULL_TREE && t != ret)
>         {
>           if (ret == type)
>             {
> @@ -7256,9 +7258,6 @@ find_decomp_class_base (location_t loc,
>             }
>           else if (orig_ret != NULL_TREE)
>             return t;
> -         else if (ret == t)
> -           /* OK, found the same base along another path.  We'll complain
> -              in convert_to_base if it's ambiguous.  */;
>           else if (ret != NULL_TREE)
>             {
>               error_at (loc, "cannot decompose class type %qT: its base "
> @@ -7645,7 +7644,9 @@ cp_finish_decomp (tree decl, tree first,
>           goto error_out;
>         }
>        for (tree field = TYPE_FIELDS (btype); field; field = TREE_CHAIN (field))
> -       if (TREE_CODE (field) != FIELD_DECL || DECL_ARTIFICIAL (field))
> +       if (TREE_CODE (field) != FIELD_DECL
> +           || DECL_ARTIFICIAL (field)
> +           || (DECL_C_BIT_FIELD (field) && !DECL_NAME (field)))
>           continue;
>         else
>           eltscnt++;
> @@ -7660,7 +7661,9 @@ cp_finish_decomp (tree decl, tree first,
>         }
>        unsigned int i = 0;
>        for (tree field = TYPE_FIELDS (btype); field; field = TREE_CHAIN (field))
> -       if (TREE_CODE (field) != FIELD_DECL || DECL_ARTIFICIAL (field))
> +       if (TREE_CODE (field) != FIELD_DECL
> +           || DECL_ARTIFICIAL (field)
> +           || (DECL_C_BIT_FIELD (field) && !DECL_NAME (field)))
>           continue;
>         else
>           {
> --- gcc/testsuite/g++.dg/cpp1z/decomp36.C.jj    2018-01-25 10:25:23.900154509 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp36.C       2018-01-25 10:25:15.996152860 +0100
> @@ -0,0 +1,19 @@
> +// PR c++/84031
> +// { dg-do compile { target c++11 } }
> +// { dg-options "" }
> +
> +struct A { unsigned char : 1, a1 : 1, a2 : 2, : 1, a3 : 3; };
> +struct B { unsigned char : 1, : 7; };
> +struct C : B { constexpr C () : c1 (1), c2 (2), c3 (3) {} unsigned char : 1, c1 : 1, c2 : 2, : 1, c3 : 3; };
> +struct D : C { constexpr D () {} unsigned char : 1, : 7; };
> +
> +int
> +main ()
> +{
> +  static constexpr A a { 1, 2, 3 };
> +  const auto &[a1, a2, a3] = a;                // { dg-warning "only available with" "" { target c++14_down } }
> +  static_assert (a1 == 1 && a2 == 2 && a3 == 3, "");
> +  static constexpr D d;
> +  const auto &[d1, d2, d3] = d;                // { dg-warning "only available with" "" { target c++14_down } }
> +  static_assert (d1 == 1 && d2 == 2 && d3 == 3, "");
> +}
>
>         Jakub

Hi all,

On 25/01/2018 16:26, Jakub Jelinek wrote:
> +	|| (DECL_C_BIT_FIELD (field) && !DECL_NAME (field)))
By the way, I see we are accumulating uses of DECL_C_BIT_FIELD && 
!DECL_NAME, shall we add a new macro? For Stage 1? In case, how shall we 
name it?

Paolo.

On Thu, Jan 25, 2018 at 11:17 AM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> Hi all,
>
> On 25/01/2018 16:26, Jakub Jelinek wrote:
>>
>> +       || (DECL_C_BIT_FIELD (field) && !DECL_NAME (field)))
>
> By the way, I see we are accumulating uses of DECL_C_BIT_FIELD &&
> !DECL_NAME, shall we add a new macro? For Stage 1? In case, how shall we
> name it?

Sure.  DECL_UNNAMED_BIT_FIELD seems like the obvious choice.

Jason

Hi,

On 25/01/2018 17:25, Jason Merrill wrote:
> On Thu, Jan 25, 2018 at 11:17 AM, Paolo Carlini
> <paolo.carlini@oracle.com> wrote:
>> Hi all,
>>
>> On 25/01/2018 16:26, Jakub Jelinek wrote:
>>> +       || (DECL_C_BIT_FIELD (field) && !DECL_NAME (field)))
>> By the way, I see we are accumulating uses of DECL_C_BIT_FIELD &&
>> !DECL_NAME, shall we add a new macro? For Stage 1? In case, how shall we
>> name it?
> Sure.  DECL_UNNAMED_BIT_FIELD seems like the obvious choice.
Great. I'll take care of that. We got quite a few instances, 6 in the C 
front-end too. In the C++ front-end a couple checks were actually 
exploiting the fact that DECL_NAME can also be tested on 
non-FIELD_DECLs. The below is what  I have so far and I'm testing (+ the 
new instances corresponding to Jakub's fix).

Paolo.

//////////////////////
Index: c/c-typeck.c
===================================================================
--- c/c-typeck.c	(revision 257058)
+++ c/c-typeck.c	(working copy)
@@ -7955,8 +7955,7 @@ really_start_incremental_init (tree type)
       constructor_fields = TYPE_FIELDS (constructor_type);
       /* Skip any nameless bit fields at the beginning.  */
       while (constructor_fields != NULL_TREE
-	     && DECL_C_BIT_FIELD (constructor_fields)
-	     && DECL_NAME (constructor_fields) == NULL_TREE)
+	     && DECL_UNNAMED_BIT_FIELD (constructor_fields))
 	constructor_fields = DECL_CHAIN (constructor_fields);
 
       constructor_unfilled_fields = constructor_fields;
@@ -8161,8 +8160,7 @@ push_init_level (location_t loc, int implicit,
       constructor_fields = TYPE_FIELDS (constructor_type);
       /* Skip any nameless bit fields at the beginning.  */
       while (constructor_fields != NULL_TREE
-	     && DECL_C_BIT_FIELD (constructor_fields)
-	     && DECL_NAME (constructor_fields) == NULL_TREE)
+	     && DECL_UNNAMED_BIT_FIELD (constructor_fields))
 	constructor_fields = DECL_CHAIN (constructor_fields);
 
       constructor_unfilled_fields = constructor_fields;
@@ -8930,8 +8928,7 @@ set_nonincremental_init (struct obstack * braced_i
       constructor_unfilled_fields = TYPE_FIELDS (constructor_type);
       /* Skip any nameless bit fields at the beginning.  */
       while (constructor_unfilled_fields != NULL_TREE
-	     && DECL_C_BIT_FIELD (constructor_unfilled_fields)
-	     && DECL_NAME (constructor_unfilled_fields) == NULL_TREE)
+	     && DECL_UNNAMED_BIT_FIELD (constructor_unfilled_fields))
 	constructor_unfilled_fields = TREE_CHAIN (constructor_unfilled_fields);
 
     }
@@ -9300,8 +9297,7 @@ output_init_element (location_t loc, tree value, t
 
       /* Skip any nameless bit fields.  */
       while (constructor_unfilled_fields != NULL_TREE
-	     && DECL_C_BIT_FIELD (constructor_unfilled_fields)
-	     && DECL_NAME (constructor_unfilled_fields) == NULL_TREE)
+	     && DECL_UNNAMED_BIT_FIELD (constructor_unfilled_fields))
 	constructor_unfilled_fields =
 	  DECL_CHAIN (constructor_unfilled_fields);
     }
@@ -9665,8 +9661,8 @@ process_init_element (location_t loc, struct c_exp
 		  constructor_unfilled_fields = DECL_CHAIN (constructor_fields);
 		  /* Skip any nameless bit fields.  */
 		  while (constructor_unfilled_fields != 0
-			 && DECL_C_BIT_FIELD (constructor_unfilled_fields)
-			 && DECL_NAME (constructor_unfilled_fields) == 0)
+			 && (DECL_UNNAMED_BIT_FIELD
+			     (constructor_unfilled_fields)))
 		    constructor_unfilled_fields =
 		      DECL_CHAIN (constructor_unfilled_fields);
 		}
@@ -9675,8 +9671,7 @@ process_init_element (location_t loc, struct c_exp
 	  constructor_fields = DECL_CHAIN (constructor_fields);
 	  /* Skip any nameless bit fields at the beginning.  */
 	  while (constructor_fields != NULL_TREE
-		 && DECL_C_BIT_FIELD (constructor_fields)
-		 && DECL_NAME (constructor_fields) == NULL_TREE)
+		 && DECL_UNNAMED_BIT_FIELD (constructor_fields))
 	    constructor_fields = DECL_CHAIN (constructor_fields);
 	}
       else if (TREE_CODE (constructor_type) == UNION_TYPE)
Index: c-family/c-common.h
===================================================================
--- c-family/c-common.h	(revision 257058)
+++ c-family/c-common.h	(working copy)
@@ -939,6 +939,10 @@ extern void c_parse_final_cleanups (void);
 #define CLEAR_DECL_C_BIT_FIELD(NODE) \
   (DECL_LANG_FLAG_4 (FIELD_DECL_CHECK (NODE)) = 0)
 
+/* True if the decl was an unnamed bitfield.  */
+#define DECL_UNNAMED_BIT_FIELD(NODE) \
+  (DECL_C_BIT_FIELD (NODE) && !DECL_NAME (NODE))
+
 extern tree do_case (location_t, tree, tree);
 extern tree build_stmt (location_t, enum tree_code, ...);
 extern tree build_real_imag_expr (location_t, enum tree_code, tree);
Index: cp/class.c
===================================================================
--- cp/class.c	(revision 257058)
+++ cp/class.c	(working copy)
@@ -8202,7 +8202,7 @@ is_really_empty_class (tree type)
 	if (TREE_CODE (field) == FIELD_DECL
 	    && !DECL_ARTIFICIAL (field)
 	    /* An unnamed bit-field is not a data member.  */
-	    && (DECL_NAME (field) || !DECL_C_BIT_FIELD (field))
+	    && !DECL_UNNAMED_BIT_FIELD (field)
 	    && !is_really_empty_class (TREE_TYPE (field)))
 	  return false;
       return true;
Index: cp/constexpr.c
===================================================================
--- cp/constexpr.c	(revision 257058)
+++ cp/constexpr.c	(working copy)
@@ -783,7 +783,7 @@ cx_check_missing_mem_inits (tree ctype, tree body,
 	  tree ftype;
 	  if (TREE_CODE (field) != FIELD_DECL)
 	    continue;
-	  if (DECL_C_BIT_FIELD (field) && !DECL_NAME (field))
+	  if (DECL_UNNAMED_BIT_FIELD (field))
 	    continue;
 	  if (DECL_ARTIFICIAL (field))
 	    continue;
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 257058)
+++ cp/decl.c	(working copy)
@@ -5634,7 +5634,7 @@ next_initializable_field (tree field)
 {
   while (field
 	 && (TREE_CODE (field) != FIELD_DECL
-	     || (DECL_C_BIT_FIELD (field) && !DECL_NAME (field))
+	     || DECL_UNNAMED_BIT_FIELD (field)
 	     || (DECL_ARTIFICIAL (field)
 		 && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
     field = DECL_CHAIN (field);
Index: cp/typeck2.c
===================================================================
--- cp/typeck2.c	(revision 257058)
+++ cp/typeck2.c	(working copy)
@@ -1395,14 +1395,14 @@ process_init_constructor_record (tree type, tree i
       tree next;
       tree type;
 
-      if (!DECL_NAME (field) && DECL_C_BIT_FIELD (field))
-	continue;
-
       if (TREE_CODE (field) != FIELD_DECL
 	  || (DECL_ARTIFICIAL (field)
 	      && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field))))
 	continue;
 
+      if (DECL_UNNAMED_BIT_FIELD (field))
+	continue;
+
       /* If this is a bitfield, first convert to the declared type.  */
       type = TREE_TYPE (field);
       if (DECL_BIT_FIELD_TYPE (field))
@@ -1547,8 +1547,6 @@ process_init_constructor_record (tree type, tree i
 	      for (field = TYPE_FIELDS (type);
 		   field; field = DECL_CHAIN (field))
 		{
-		  if (!DECL_NAME (field) && DECL_C_BIT_FIELD (field))
-		    continue;
 		  if (TREE_CODE (field) != FIELD_DECL
 		      || (DECL_ARTIFICIAL (field)
 			  && !(cxx_dialect >= cxx17
@@ -1555,6 +1553,9 @@ process_init_constructor_record (tree type, tree i
 			       && DECL_FIELD_IS_BASE (field))))
 		    continue;
 
+		  if (DECL_UNNAMED_BIT_FIELD (field))
+		    continue;
+
 		  if (ce->index == field || ce->index == DECL_NAME (field))
 		    break;
 		  if (ANON_AGGR_TYPE_P (TREE_TYPE (field)))