Patchwork C++ PATCH for c++/48089 (ICE with invalid constexpr ctor)

login
register
mail settings
Submitter Jason Merrill
Date March 17, 2011, 2:32 a.m.
Message ID <4D8172C2.70403@redhat.com>
Download mbox | patch
Permalink /patch/87331/
State New
Headers show

Comments

Jason Merrill - March 17, 2011, 2:32 a.m.
In a normal constexpr function, we treat *this as a potential constant 
expression.  But in a constexpr constructor it isn't, since we're still 
in the process of initializing the object.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 0f23f8a6fca94b1a00a51ea34643268e9ff840e9
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Mar 16 20:55:52 2011 -0400

    	PR c++/48089
    	* semantics.c (potential_constant_expression_1): Don't allow *this
    	in a constructor.
    	(register_constexpr_fundef): Use potential_rvalue_constant_expression.
Gabriel Dos Reis - March 17, 2011, 2:43 a.m.
On Wed, Mar 16, 2011 at 9:32 PM, Jason Merrill <jason@redhat.com> wrote:
> In a normal constexpr function, we treat *this as a potential constant
> expression.  But in a constexpr constructor it isn't, since we're still in
> the process of initializing the object.

Hi Jason,
I believe the patch is too strong, or am I reading it wrong?

The real problem is that the data member of the object is not initialized.
Fully constructed subobjects in constexpr constructor are still
be potential constant expressions.
For example, we should not reject

      struct S {
           int i;
           int j;
           constexpr S(int a) : i(a), j(i*i) { }
      };

-- Gaby

> +
> +struct s {
> +    constexpr s() : v(v) { }   // { dg-error "object being constructed" }
> +    char v;
> +};
> +
> +s bang;
>
>
Jason Merrill - March 17, 2011, 3:02 a.m.
On 03/16/2011 10:43 PM, Gabriel Dos Reis wrote:
> The real problem is that the data member of the object is not initialized.
> Fully constructed subobjects in constexpr constructor are still
> be potential constant expressions.
> For example, we should not reject
>
>        struct S {
>             int i;
>             int j;
>             constexpr S(int a) : i(a), j(i*i) { }
>        };

I thought about that, but it seems to me that the implementation 
complexity for supporting that usage would be much higher than it is 
worth.  Nowhere else in constant expression evaluation do we have to 
deal with assigning a value at one point and then getting it back later.

Jason
Gabriel Dos Reis - March 17, 2011, 3:44 a.m.
On Wed, Mar 16, 2011 at 10:02 PM, Jason Merrill <jason@redhat.com> wrote:
> On 03/16/2011 10:43 PM, Gabriel Dos Reis wrote:
>>
>> The real problem is that the data member of the object is not initialized.
>> Fully constructed subobjects in constexpr constructor are still
>> be potential constant expressions.
>> For example, we should not reject
>>
>>       struct S {
>>            int i;
>>            int j;
>>            constexpr S(int a) : i(a), j(i*i) { }
>>       };
>
> I thought about that, but it seems to me that the implementation complexity
> for supporting that usage would be much higher than it is worth.  Nowhere
> else in constant expression evaluation do we have to deal with assigning a
> value at one point and then getting it back later.

A base class subobject of a literal class has similar property.

I am not sure we need more infrastructure or more complexity
in the implementation.  The (C++98) language already requires
us to initialize subobjects in their order of declaration.  That is what
we do here.  All we need is to check that a member, whose value
is being used in the initialization of another member
(which might be itself like in this PR) is initialized. When we look up
`this' in the evaluation environment, we can tell whether a particular
slot has been initialized.

-- Gaby
Jason Merrill - March 17, 2011, 3:41 p.m.
On 03/16/2011 11:44 PM, Gabriel Dos Reis wrote:
> I am not sure we need more infrastructure or more complexity
> in the implementation.  The (C++98) language already requires
> us to initialize subobjects in their order of declaration.  That is what
> we do here.  All we need is to check that a member, whose value
> is being used in the initialization of another member
> (which might be itself like in this PR) is initialized. When we look up
> `this' in the evaluation environment, we can tell whether a particular
> slot has been initialized.

Right, my point is that initialization and then lookup of that stored 
value is something that does not otherwise exist in constant expressions.

Jason
Gabriel Dos Reis - March 17, 2011, 5:48 p.m.
On Thu, Mar 17, 2011 at 10:41 AM, Jason Merrill <jason@redhat.com> wrote:
> On 03/16/2011 11:44 PM, Gabriel Dos Reis wrote:
>>
>> I am not sure we need more infrastructure or more complexity
>> in the implementation.  The (C++98) language already requires
>> us to initialize subobjects in their order of declaration.  That is what
>> we do here.  All we need is to check that a member, whose value
>> is being used in the initialization of another member
>> (which might be itself like in this PR) is initialized. When we look up
>> `this' in the evaluation environment, we can tell whether a particular
>> slot has been initialized.
>
> Right, my point is that initialization and then lookup of that stored value
> is something that does not otherwise exist in constant expressions.

In C++93, no.  But, we changed that with constexpr :-)

The same capability is offered for any constexpr structures
or arrays -- either at global scopes or local.   For GNU extensions,
the same issue arise for designated initializers in array initializations.

-- Gaby

Patch

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index a0c5ae3..7519d26 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -5674,11 +5674,11 @@  register_constexpr_fundef (tree fun, tree body)
       body = unshare_expr (TREE_OPERAND (body, 0));
     }
 
-  if (!potential_constant_expression (body))
+  if (!potential_rvalue_constant_expression (body))
     {
       DECL_DECLARED_CONSTEXPR_P (fun) = false;
       if (!DECL_TEMPLATE_INSTANTIATION (fun))
-	require_potential_constant_expression (body);
+	require_potential_rvalue_constant_expression (body);
       return NULL;
     }
   fundef->body = body;
@@ -7496,7 +7496,16 @@  potential_constant_expression_1 (tree t, bool want_rval, tsubst_flags_t flags)
         tree x = TREE_OPERAND (t, 0);
         STRIP_NOPS (x);
         if (is_this_parameter (x))
-	  return true;
+	  {
+	    if (DECL_CONSTRUCTOR_P (DECL_CONTEXT (x)) && want_rval)
+	      {
+		if (flags & tf_error)
+		  error ("the value of the object being constructed is "
+			 "not a constant expression");
+		return false;
+	      }
+	    return true;
+	  }
 	return potential_constant_expression_1 (x, rval, flags);
       }
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-48089.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-48089.C
new file mode 100644
index 0000000..88ef3d6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-48089.C
@@ -0,0 +1,9 @@ 
+// PR c++/48089
+// { dg-options -std=c++0x }
+
+struct s {
+    constexpr s() : v(v) { }	// { dg-error "object being constructed" }
+    char v;
+};
+
+s bang;