[C++ Patch] PR 58102 aka DR 1405

[C++ Patch] PR 58102 aka DR 1405

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

Hi,

I think that in order to implement the resolution we simply have to 
remove the check. Tested x86_64-linux.

Thanks,
Paolo.

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

[-- Attachment #2: CL_58102 --]
[-- Type: text/plain, Size: 351 bytes --]

/cp
2014-09-01  Paolo Carlini  <paolo.carlini@oracle.com>

	DR 1405
	PR c++/58102
	* semantics.c (cxx_eval_outermost_constant_expr): Do not check
	for mutable sub-objects.

/testsuite
2014-09-01  Paolo Carlini  <paolo.carlini@oracle.com>

	DR 1405
	PR c++/58102
	* g++.dg/cpp0x/constexpr-mutable2.C: New.
	* g++.dg/cpp0x/constexpr-mutable1.C: Adjust.

[-- Attachment #3: patch_58102 --]
[-- Type: text/plain, Size: 1839 bytes --]

Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 214779)
+++ cp/semantics.c	(working copy)
@@ -9858,18 +9858,6 @@ cxx_eval_outermost_constant_expr (tree t, bool all
 
   verify_constant (r, allow_non_constant, &non_constant_p, &overflow_p);
 
-  if (TREE_CODE (t) != CONSTRUCTOR
-      && cp_has_mutable_p (TREE_TYPE (t)))
-    {
-      /* We allow a mutable type if the original expression was a
-	 CONSTRUCTOR so that we can do aggregate initialization of
-	 constexpr variables.  */
-      if (!allow_non_constant)
-	error ("%qT cannot be the type of a complete constant expression "
-	       "because it has mutable sub-objects", TREE_TYPE (t));
-      non_constant_p = true;
-    }
-
   /* Technically we should check this for all subexpressions, but that
      runs into problems with our internal representation of pointer
      subtraction and the 5.19 rules are still in flux.  */
Index: testsuite/g++.dg/cpp0x/constexpr-mutable1.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-mutable1.C	(revision 214785)
+++ testsuite/g++.dg/cpp0x/constexpr-mutable1.C	(working copy)
@@ -7,6 +7,6 @@ struct A
 };
 
 constexpr A a = { 0, 1 };
-constexpr A b = a;		// { dg-error "mutable" }
+constexpr A b = a;
 constexpr int i = a.i;
 constexpr int j = a.j;		// { dg-error "mutable" }
Index: testsuite/g++.dg/cpp0x/constexpr-mutable2.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-mutable2.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/constexpr-mutable2.C	(working copy)
@@ -0,0 +1,10 @@
+// DR 1405, PR c++/58102
+// { dg-do compile { target c++11 } }
+
+struct S {
+  mutable int n;
+  constexpr S() : n() {}
+};
+
+constexpr S s1 {};
+constexpr S s2 = {};

Re: [C++ Patch] PR 58102 aka DR 1405

[Jason Merrill]

On 09/01/2014 09:47 AM, Paolo Carlini wrote:
> -constexpr A b = a;		// { dg-error "mutable" }
> +constexpr A b = a;

This is wrong; we still need to get an error here.

Jason

Re: [C++ Patch] PR 58102 aka DR 1405

[Paolo Carlini]

Hi,

On 09/02/2014 04:11 PM, Jason Merrill wrote:
> On 09/01/2014 09:47 AM, Paolo Carlini wrote:
>> -constexpr A b = a;        // { dg-error "mutable" }
>> +constexpr A b = a;
>
> This is wrong; we still need to get an error here.
Hum, interesting. Neither current EDG nor current clang error out there. 
Let's see if I can tease the case out...

Thanks,
Paolo.

Re: [C++ Patch] PR 58102 aka DR 1405

[Jason Merrill]

On 09/02/2014 10:17 AM, Paolo Carlini wrote:
> Let's see if I can tease the case out...

I think you need to leave that hunk alone, and instead fix the new 
testcase by treating = {} more like {}, just as we already don't require 
a copy constructor call for copy-list-initialization.

Jason

Re: [C++ Patch] PR 58102 aka DR 1405

[Paolo Carlini]

Hi,

On 09/02/2014 04:28 PM, Jason Merrill wrote:
> On 09/02/2014 10:17 AM, Paolo Carlini wrote:
>> Let's see if I can tease the case out...
>
> I think you need to leave that hunk alone, and instead fix the new 
> testcase by treating = {} more like {}, just as we already don't 
> require a copy constructor call for copy-list-initialization.
I see. Thanks a lot for the tip!

Paolo.

Re: [C++ Patch] PR 58102 aka DR 1405

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

Hi again,

On 09/02/2014 04:28 PM, Jason Merrill wrote:
> On 09/02/2014 10:17 AM, Paolo Carlini wrote:
>> Let's see if I can tease the case out...
>
> I think you need to leave that hunk alone, and instead fix the new 
> testcase by treating = {} more like {}, just as we already don't 
> require a copy constructor call for copy-list-initialization.
By the way, now I really understand the DR (the wording in the 
resolution clarifies what we are *already* doing correctly!).

Anyway, what about the below? Certainly works for the tests which we 
have got.

Thanks,
Paolo.

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


[-- Attachment #2: patch_58102_2 --]
[-- Type: text/plain, Size: 1369 bytes --]

Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 214808)
+++ cp/semantics.c	(working copy)
@@ -9859,11 +9859,14 @@ cxx_eval_outermost_constant_expr (tree t, bool all
   verify_constant (r, allow_non_constant, &non_constant_p, &overflow_p);
 
   if (TREE_CODE (t) != CONSTRUCTOR
+      && (TREE_CODE (t) != TARGET_EXPR
+	  || TREE_CODE (TARGET_EXPR_INITIAL (t)) != AGGR_INIT_EXPR)
       && cp_has_mutable_p (TREE_TYPE (t)))
     {
       /* We allow a mutable type if the original expression was a
 	 CONSTRUCTOR so that we can do aggregate initialization of
-	 constexpr variables.  */
+	 constexpr variables.  Likewise for TARGET_EXPRs with an
+	 AGGR_INIT_EXPR as TARGET_EXPR_INITIAL (c++/58102).  */
       if (!allow_non_constant)
 	error ("%qT cannot be the type of a complete constant expression "
 	       "because it has mutable sub-objects", TREE_TYPE (t));
Index: testsuite/g++.dg/cpp0x/constexpr-mutable2.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-mutable2.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/constexpr-mutable2.C	(working copy)
@@ -0,0 +1,10 @@
+// DR 1405, PR c++/58102
+// { dg-do compile { target c++11 } }
+
+struct S {
+  mutable int n;
+  constexpr S() : n() {}
+};
+
+constexpr S s1 {};
+constexpr S s2 = {};

Re: [C++ Patch] PR 58102 aka DR 1405

[Jason Merrill]

On 09/02/2014 11:07 AM, Paolo Carlini wrote:
> Anyway, what about the below? Certainly works for the tests which we
> have got.

Hmm.  This is definitely an improvement, as it allows a subset of

a non-volatile glvalue of literal type that refers to a non-volatile 
object whose lifetime began within the evalution of e

But it doesn't cover all of that, and in any case we shouldn't need to 
explicitly handle that just for types with mutable subobjects.

I think perhaps it would be better to remove that hunk as in your 
initial patch and replace it with a check in constant_value_1 and an 
explanation in non_const_var_error.

Jason

Re: [C++ Patch] PR 58102 aka DR 1405

[Paolo Carlini]

Hi,

On 09/02/2014 05:45 PM, Jason Merrill wrote:
> On 09/02/2014 11:07 AM, Paolo Carlini wrote:
>> Anyway, what about the below? Certainly works for the tests which we
>> have got.
>
> Hmm.  This is definitely an improvement, as it allows a subset of
>
> a non-volatile glvalue of literal type that refers to a non-volatile 
> object whose lifetime began within the evalution of e
>
> But it doesn't cover all of that, and in any case we shouldn't need to 
> explicitly handle that just for types with mutable subobjects.
>
> I think perhaps it would be better to remove that hunk as in your 
> initial patch and replace it with a check in constant_value_1 and an 
> explanation in non_const_var_error.
In practice, I'm encountering a rather serious problem with moving away 
the check, I'm looking more into it, but maybe I can already explain it 
to you...

The issue, AFAICS, boils down to the difference itself between 
cxx_eval_outermost_constant_expr and cxx_eval_constant_expression: 
changing constant_value_1 means that in principle all the calls of the 
latter (for VAR_DECLs) are impacted. Thus, for example, for the call at 
the beginning of cxx_eval_component_reference:

struct A
{
   int i;
   mutable int j;
};

constexpr A a = { 0, 1 };
constexpr int i = a.i;

how do we avoid emitting a wrong error for the a of a.i?

Paolo.

Re: [C++ Patch] PR 58102 aka DR 1405

[Jason Merrill]

On 09/03/2014 06:53 AM, Paolo Carlini wrote:
> The issue, AFAICS, boils down to the difference itself between
> cxx_eval_outermost_constant_expr and cxx_eval_constant_expression:
> changing constant_value_1 means that in principle all the calls of the
> latter (for VAR_DECLs) are impacted.

Oh, right.

> Thus, for example, for the call at
> the beginning of cxx_eval_component_reference:
>
> struct A
> {
>    int i;
>    mutable int j;
> };
>
> constexpr A a = { 0, 1 };
> constexpr int i = a.i;
>
> how do we avoid emitting a wrong error for the a of a.i?

Perhaps when we get the value of a we replace the mutable initializer 
with a magic "mutable" value and then make sure what we return from 
_outermost_ doesn't contain it?

Jason