Re: C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

Re: C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

[Jason Merrill]

No, that's still the case, just getting confused by the need to also support constant initialization of non-literal types.

Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:

On Sun, Jun 26, 2011 at 9:00 AM, Jason Merrill <jason@redhat.com> wrote:
> The constant expression evaluation code was happily replacing an expression
> involving a temporary with its constant value even if the temporary had a
> destructor that needed to be run.

I thought we wanted literal types to have trivial destructor; did that change?

-- Gaby

Re: C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

[Gabriel Dos Reis]

On Sun, Jun 26, 2011 at 12:09 PM, Jason Merrill <jason@redhat.com> wrote:
> No, that's still the case, just getting confused by the need to also support constant initialization of non-literal types.

Ah, thanks!

-- Gaby

Re: C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

[Jason Merrill]

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

But yes, I think checking for literal type is a better idea.

Tested x86_64-pc-linux-gnu, applying to trunk.


[-- Attachment #2: 49528-2.patch --]
[-- Type: text/x-patch, Size: 1808 bytes --]

commit 81e093c1d6f2657c8c3c2abc2af559111ca82da4
Author: Jason Merrill <jason@redhat.com>
Date:   Sun Jun 26 15:23:00 2011 -0400

    	PR c++/49528
    	* semantics.c (potential_constant_expression_1): Check
    	for non-literality rather than cleanup.
    	(cxx_eval_constant_expression): Likewise.

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 5404c9f..d1af0c6 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7020,11 +7020,10 @@ cxx_eval_constant_expression (const constexpr_call *call, tree t,
       break;
 
     case TARGET_EXPR:
-      /* A cleanup isn't constant.  */
-      if (TARGET_EXPR_CLEANUP (t))
+      if (!literal_type_p (TREE_TYPE (t)))
 	{
 	  if (!allow_non_constant)
-	    error ("temporary of type %qT needing destruction in a "
+	    error ("temporary of non-literal type %qT in a "
 		   "constant expression", TREE_TYPE (t));
 	  *non_constant_p = true;
 	  break;
@@ -7851,11 +7850,10 @@ potential_constant_expression_1 (tree t, bool want_rval, tsubst_flags_t flags)
 					      want_rval, flags);
 
     case TARGET_EXPR:
-      /* A cleanup isn't constant.  */
-      if (TARGET_EXPR_CLEANUP (t))
+      if (!literal_type_p (TREE_TYPE (t)))
 	{
 	  if (flags & tf_error)
-	    error ("temporary of type %qT needing destruction in a "
+	    error ("temporary of non-literal type %qT in a "
 		   "constant expression", TREE_TYPE (t));
 	  return false;
 	}
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-cleanup.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-cleanup.C
index b3fb9a8..de17f3d 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-cleanup.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-cleanup.C
@@ -6,4 +6,4 @@ struct A
   ~A();
 };
 
-constexpr int i = A().i;	// { dg-error "destruction" }
+constexpr int i = A().i;	// { dg-error "non-literal" }

Re: C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

[Jakub Jelinek]

On Sun, Jun 26, 2011 at 10:13:23AM -0400, Jason Merrill wrote:
> On 06/26/2011 10:00 AM, Jason Merrill wrote:
> >Tested x86_64-pc-linux-gnu, applying to trunk. Jakub, should this go
> >into 4.6.1 or .2?
> 
> I think it's fine to wait for .2, since a class with a non-trivial
> destructor is likely to also have a non-trivial, non-constexpr
> constructor, which would prevent A() from having a constant value.

Yeah, I'd prefer to defer it for 4.6.2 and be able to roll the release tomorrow.

	Jakub

Re: C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

[Gabriel Dos Reis]

On Sun, Jun 26, 2011 at 9:00 AM, Jason Merrill <jason@redhat.com> wrote:
> The constant expression evaluation code was happily replacing an expression
> involving a temporary with its constant value even if the temporary had a
> destructor that needed to be run.

I thought we wanted literal types to have trivial destructor; did that change?

-- Gaby

Re: C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

[Jason Merrill]

On 06/26/2011 10:00 AM, Jason Merrill wrote:
> Tested x86_64-pc-linux-gnu, applying to trunk. Jakub, should this go
> into 4.6.1 or .2?

I think it's fine to wait for .2, since a class with a non-trivial 
destructor is likely to also have a non-trivial, non-constexpr 
constructor, which would prevent A() from having a constant value.

Jason

C++ PATCH for c++/49528 (omitting destructor for temporary with constant value)

[Jason Merrill]

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

The constant expression evaluation code was happily replacing an 
expression involving a temporary with its constant value even if the 
temporary had a destructor that needed to be run.

After fixing that, I needed to adjust expand_default_init to avoid 
adding a TARGET_EXPR when we aren't really creating a temporary.

Tested x86_64-pc-linux-gnu, applying to trunk.  Jakub, should this go 
into 4.6.1 or .2?

[-- Attachment #2: 49528.patch --]
[-- Type: text/x-patch, Size: 2834 bytes --]

commit e42fdb0afe8827e3320239e252d804e2e3bbd63d
Author: Jason Merrill <jason@redhat.com>
Date:   Sat Jun 25 20:52:43 2011 -0400

    	PR c++/49528
    	* semantics.c (potential_constant_expression_1): A TARGET_EXPR
    	with a cleanup isn't constant.
    	(cxx_eval_constant_expression): Likewise.
    	* init.c (expand_default_init): Use maybe_constant_init.

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 3c347a4..3ceed90 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1514,7 +1514,7 @@ expand_default_init (tree binfo, tree true_exp, tree exp, tree init, int flags,
       tree fn = get_callee_fndecl (rval);
       if (fn && DECL_DECLARED_CONSTEXPR_P (fn))
 	{
-	  tree e = maybe_constant_value (rval);
+	  tree e = maybe_constant_init (rval);
 	  if (TREE_CONSTANT (e))
 	    rval = build2 (INIT_EXPR, type, exp, e);
 	}
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index f4aa350..5404c9f 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7020,6 +7020,16 @@ cxx_eval_constant_expression (const constexpr_call *call, tree t,
       break;
 
     case TARGET_EXPR:
+      /* A cleanup isn't constant.  */
+      if (TARGET_EXPR_CLEANUP (t))
+	{
+	  if (!allow_non_constant)
+	    error ("temporary of type %qT needing destruction in a "
+		   "constant expression", TREE_TYPE (t));
+	  *non_constant_p = true;
+	  break;
+	}
+      /* else fall through.  */
     case INIT_EXPR:
       /* Pass false for 'addr' because these codes indicate
 	 initialization of a temporary.  */
@@ -7840,8 +7850,16 @@ potential_constant_expression_1 (tree t, bool want_rval, tsubst_flags_t flags)
       return potential_constant_expression_1 (TREE_OPERAND (t, 1),
 					      want_rval, flags);
 
-    case INIT_EXPR:
     case TARGET_EXPR:
+      /* A cleanup isn't constant.  */
+      if (TARGET_EXPR_CLEANUP (t))
+	{
+	  if (flags & tf_error)
+	    error ("temporary of type %qT needing destruction in a "
+		   "constant expression", TREE_TYPE (t));
+	  return false;
+	}
+    case INIT_EXPR:
       return potential_constant_expression_1 (TREE_OPERAND (t, 1),
 					      rval, flags);
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-cleanup.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-cleanup.C
new file mode 100644
index 0000000..b3fb9a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-cleanup.C
@@ -0,0 +1,9 @@
+// { dg-options -std=c++0x }
+
+struct A
+{
+  int i;
+  ~A();
+};
+
+constexpr int i = A().i;	// { dg-error "destruction" }
diff --git a/gcc/testsuite/g++.dg/init/ref19.C b/gcc/testsuite/g++.dg/init/ref19.C
new file mode 100644
index 0000000..ed78c93
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/ref19.C
@@ -0,0 +1,17 @@
+// PR c++/49528
+// { dg-do run }
+
+int d;
+
+struct A
+{
+  int i;
+  ~A() { ++d; };
+};
+
+int main()
+{
+  const int &r = A().i;
+  if (d != 1)
+    return 1;
+}