diff mbox

[C++] 79118 bitfields & constexpr

Message ID 1beb4ea3-2741-b293-c845-ced98bd8dd0e@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Jan. 24, 2017, 6:53 p.m. UTC
On 01/24/2017 01:41 PM, Jason Merrill wrote:
> On Tue, Jan 24, 2017 at 1:31 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> On 01/23/2017 04:06 PM, Jason Merrill wrote:
>>
>>>> In this particular case we've also made the base object the containing
>>>> class, not the unnamed struct member.  That means we're looking in the
>>>> wrong
>>>> CONSTRUCTOR and see CONSTRUCTOR_NO_IMPLICIT_ZERO is true.  Whereas for
>>>> the
>>>> subobj's CONSTRUCTOR it is false.
>>>
>>>
>>> Why is it false?
>>
>>
>> I thought it was because we're looking at a different level of ctor,
>> investigation shows there may be a bug there too. because in one place we
>> do:
>>      if (*valp == NULL_TREE)
>>         {
>>           *valp = build_constructor (type, NULL);
>>           CONSTRUCTOR_NO_IMPLICIT_ZERO (*valp) = no_zero_init;
>> and another we do:
>>       if (vec_safe_is_empty (*vec)
>>           || (*vec)->last().index != field)
>>         {
>>           ctor = build_constructor (TREE_TYPE (field), NULL);
>>           CONSTRUCTOR_APPEND_ELT (*vec, field, ctor);
>>
>> However, further looking at this problem, I discovered we're not properly
>> checking the initialization of anonymous members.  Once we do that, we
>> reject the ctor as a constexpr, because it fails to initialize the 'type'
>> member (regardless of bitfieldness).
>>
>> This patch augments cx_check_missing_mem_inits.  I change the first parm to
>> be the CTYPE not the FUN from whence we pull the CTYPE.  That way we don't
>> have to cons up an empty CONSTRUCTOR for the recursive case of discovering
>> no initializer at all for the anon member.
>>
>> With this in place we don't try and evaluate the constexpr in the original
>> testcase.
>>
>> ok?
>
> I'm not seeing the patch.

d'oh!

Comments

Jason Merrill Jan. 24, 2017, 7:20 p.m. UTC | #1
OK.

On Tue, Jan 24, 2017 at 1:53 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 01/24/2017 01:41 PM, Jason Merrill wrote:
>>
>> On Tue, Jan 24, 2017 at 1:31 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>>
>>> On 01/23/2017 04:06 PM, Jason Merrill wrote:
>>>
>>>>> In this particular case we've also made the base object the containing
>>>>> class, not the unnamed struct member.  That means we're looking in the
>>>>> wrong
>>>>> CONSTRUCTOR and see CONSTRUCTOR_NO_IMPLICIT_ZERO is true.  Whereas for
>>>>> the
>>>>> subobj's CONSTRUCTOR it is false.
>>>>
>>>>
>>>>
>>>> Why is it false?
>>>
>>>
>>>
>>> I thought it was because we're looking at a different level of ctor,
>>> investigation shows there may be a bug there too. because in one place we
>>> do:
>>>      if (*valp == NULL_TREE)
>>>         {
>>>           *valp = build_constructor (type, NULL);
>>>           CONSTRUCTOR_NO_IMPLICIT_ZERO (*valp) = no_zero_init;
>>> and another we do:
>>>       if (vec_safe_is_empty (*vec)
>>>           || (*vec)->last().index != field)
>>>         {
>>>           ctor = build_constructor (TREE_TYPE (field), NULL);
>>>           CONSTRUCTOR_APPEND_ELT (*vec, field, ctor);
>>>
>>> However, further looking at this problem, I discovered we're not properly
>>> checking the initialization of anonymous members.  Once we do that, we
>>> reject the ctor as a constexpr, because it fails to initialize the 'type'
>>> member (regardless of bitfieldness).
>>>
>>> This patch augments cx_check_missing_mem_inits.  I change the first parm
>>> to
>>> be the CTYPE not the FUN from whence we pull the CTYPE.  That way we
>>> don't
>>> have to cons up an empty CONSTRUCTOR for the recursive case of
>>> discovering
>>> no initializer at all for the anon member.
>>>
>>> With this in place we don't try and evaluate the constexpr in the
>>> original
>>> testcase.
>>>
>>> ok?
>>
>>
>> I'm not seeing the patch.
>
>
> d'oh!
>
>
>
> --
> Nathan Sidwell
diff mbox

Patch

2017-01-24  Nathan Sidwell  <nathan@acm.org>

	PR c++/79118 - anon-members and constexpr
	* constexpr.c (cx_check_missing_mem_inits): Caller passes type not
	ctor decl.  Recursively check anonymous members.
	(register_constexpr_fundef): Adjust cx_check_missing_mem_inits
	call.
	(explain_invalid_constexpr_fn): Likewise.


	PR c++/79118
	* g++.dg/cpp0x/pr79118.C: New.

Index: cp/constexpr.c
===================================================================
--- cp/constexpr.c	(revision 244874)
+++ cp/constexpr.c	(working copy)
@@ -696,23 +696,21 @@  massage_constexpr_body (tree fun, tree b
   return body;
 }
 
-/* FUN is a constexpr constructor with massaged body BODY.  Return true
-   if some bases/fields are uninitialized, and complain if COMPLAIN.  */
+/* CTYPE is a type constructed from BODY.  Return true if some
+   bases/fields are uninitialized, and complain if COMPLAIN.  */
 
 static bool
-cx_check_missing_mem_inits (tree fun, tree body, bool complain)
+cx_check_missing_mem_inits (tree ctype, tree body, bool complain)
 {
-  bool bad;
-  tree field;
-  unsigned i, nelts;
-  tree ctype;
-
-  if (TREE_CODE (body) != CONSTRUCTOR)
-    return false;
-
-  nelts = CONSTRUCTOR_NELTS (body);
-  ctype = DECL_CONTEXT (fun);
-  field = TYPE_FIELDS (ctype);
+  unsigned nelts = 0;
+  
+  if (body)
+    {
+      if (TREE_CODE (body) != CONSTRUCTOR)
+	return false;
+      nelts = CONSTRUCTOR_NELTS (body);
+    }
+  tree field = TYPE_FIELDS (ctype);
 
   if (TREE_CODE (ctype) == UNION_TYPE)
     {
@@ -726,13 +724,13 @@  cx_check_missing_mem_inits (tree fun, tr
       return false;
     }
 
-  bad = false;
-  for (i = 0; i <= nelts; ++i)
+  /* Iterate over the CONSTRUCTOR, checking any missing fields don't
+     need an explicit initialization.  */
+  bool bad = false;
+  for (unsigned i = 0; i <= nelts; ++i)
     {
-      tree index;
-      if (i == nelts)
-	index = NULL_TREE;
-      else
+      tree index = NULL_TREE;
+      if (i < nelts)
 	{
 	  index = CONSTRUCTOR_ELT (body, i)->index;
 	  /* Skip base and vtable inits.  */
@@ -740,13 +738,25 @@  cx_check_missing_mem_inits (tree fun, tr
 	      || DECL_ARTIFICIAL (index))
 	    continue;
 	}
+
       for (; field != index; field = DECL_CHAIN (field))
 	{
 	  tree ftype;
-	  if (TREE_CODE (field) != FIELD_DECL
-	      || (DECL_C_BIT_FIELD (field) && !DECL_NAME (field))
-	      || DECL_ARTIFICIAL (field))
+	  if (TREE_CODE (field) != FIELD_DECL)
 	    continue;
+	  if (DECL_C_BIT_FIELD (field) && !DECL_NAME (field))
+	    continue;
+	  if (DECL_ARTIFICIAL (field))
+	    continue;
+	  if (ANON_AGGR_TYPE_P (TREE_TYPE (field)))
+	    {
+	      /* Recurse to check the anonummous aggregate member.  */
+	      bad |= cx_check_missing_mem_inits
+		(TREE_TYPE (field), NULL_TREE, complain);
+	      if (bad && !complain)
+		return true;
+	      continue;
+	    }
 	  ftype = strip_array_types (TREE_TYPE (field));
 	  if (type_has_constexpr_default_constructor (ftype))
 	    {
@@ -766,6 +776,15 @@  cx_check_missing_mem_inits (tree fun, tr
 	}
       if (field == NULL_TREE)
 	break;
+
+      if (ANON_AGGR_TYPE_P (TREE_TYPE (index)))
+	{
+	  /* Check the anonymous aggregate initializer is valid.  */
+	  bad |= cx_check_missing_mem_inits
+	    (TREE_TYPE (index), CONSTRUCTOR_ELT (body, i)->value, complain);
+	  if (bad && !complain)
+	    return true;
+	}
       field = DECL_CHAIN (field);
     }
 
@@ -803,7 +822,8 @@  register_constexpr_fundef (tree fun, tre
     }
 
   if (DECL_CONSTRUCTOR_P (fun)
-      && cx_check_missing_mem_inits (fun, massaged, !DECL_GENERATED_P (fun)))
+      && cx_check_missing_mem_inits (DECL_CONTEXT (fun),
+				     massaged, !DECL_GENERATED_P (fun)))
     return NULL;
 
   /* Create the constexpr function table if necessary.  */
@@ -864,7 +884,7 @@  explain_invalid_constexpr_fn (tree fun)
 	  body = massage_constexpr_body (fun, DECL_SAVED_TREE (fun));
 	  require_potential_rvalue_constant_expression (body);
 	  if (DECL_CONSTRUCTOR_P (fun))
-	    cx_check_missing_mem_inits (fun, body, true);
+	    cx_check_missing_mem_inits (DECL_CONTEXT (fun), body, true);
 	}
     }
   input_location = save_loc;
Index: testsuite/g++.dg/cpp0x/pr79118.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr79118.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr79118.C	(working copy)
@@ -0,0 +1,42 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-additional-options { -Wno-pedantic } }
+// PR c++/79118 failure to check initialization of anonymous members.
+
+struct One
+{
+  union
+  {
+    int a;
+    int b;
+  };
+
+  constexpr One () : a(), b() {} // { dg-error "multiple" }
+  constexpr One (int) : a() {}
+  constexpr One (unsigned) : b () {}
+  constexpr One (void *) {} // { dg-error "exactly one" }
+};
+
+One a ();
+One b (0);
+One c (0u);
+One d ((void *)0);
+
+struct Two
+{
+  struct
+  {
+    int a;
+    int b;
+  };
+
+  constexpr Two () : a(), b() {}
+  constexpr Two (int) : a() {} // { dg-error "b' must be initialized" }
+  constexpr Two (unsigned) : b () {} // { dg-error "a' must be initialized" }
+  constexpr Two (void *) {} // { dg-error "a' must be initialized" }
+   // { dg-error "b' must be initialized" "" { target *-*-* } 35 }
+};
+
+Two e ();
+Two f (0);
+Two g (0u);
+Two h ((void *)0);