public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Fix up default initialization with consteval default ctor [PR96994]
@ 2020-09-15  7:57 Jakub Jelinek
  2020-09-25 12:49 ` Stephan Bergmann
  2020-09-25 20:30 ` Jason Merrill
  0 siblings, 2 replies; 5+ messages in thread
From: Jakub Jelinek @ 2020-09-15  7:57 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

The following testcase is miscompiled (in particular the a and i
initialization).  The problem is that build_special_member_call due to
the immediate constructors (but not evaluated in constant expression mode)
doesn't create a CALL_EXPR, but returns a TARGET_EXPR with CONSTRUCTOR
as the initializer for it, and then expand_default_init just emits
the returned statement, but this one doesn't have any side-effects and does
nothing.  There is an if to handle constexpr ctors which emits an INIT_EXPR
but constexpr ctors still show up as CALL_EXPR and need to be manually
evaluated to constant expressions (if possible).

The following patch fixes that, though I'm not sure about several things.
One is that the earlier if also has expr == true_exp && in the condition,
not sure if we want it in this case or not.
Another is that for delegating constructors, we emit two separate calls
and build_if_in_charge them together.  Not sure if consteval could come into
play in that case.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2020-09-15  Jakub Jelinek  <jakub@redhat.com>

	PR c++/96994
	* init.c (expand_default_init): If rval is TARGET_EXPR with
	TREE_CONSTANT TARGET_EXPR_INITIAL, emit INIT_EXPR.

	* g++.dg/cpp2a/consteval18.C: New test.

--- gcc/cp/init.c.jj	2020-09-10 11:24:05.019805303 +0200
+++ gcc/cp/init.c	2020-09-14 15:06:59.467341241 +0200
@@ -1999,6 +1999,9 @@ expand_default_init (tree binfo, tree tr
 	    rval = build2 (INIT_EXPR, type, exp, e);
 	}
     }
+  else if (TREE_CODE (rval) == TARGET_EXPR
+	   && TREE_CONSTANT (TARGET_EXPR_INITIAL (rval)))
+    rval = build2 (INIT_EXPR, type, exp, rval);
 
   /* FIXME put back convert_to_void?  */
   if (TREE_SIDE_EFFECTS (rval))
--- gcc/testsuite/g++.dg/cpp2a/consteval18.C.jj	2020-09-14 15:12:50.036282784 +0200
+++ gcc/testsuite/g++.dg/cpp2a/consteval18.C	2020-09-14 15:12:42.834386644 +0200
@@ -0,0 +1,26 @@
+// PR c++/96994
+// { dg-do run { target c++20 } }
+
+struct A { consteval A () { i = 1; } consteval A (int x) : i (x) {} int i = 0; };
+struct B { constexpr B () { i = 1; } constexpr B (int x) : i (x) {} int i = 0; };
+A const a;
+constexpr A b;
+B const c;
+A const constinit d;
+A const e = 2;
+constexpr A f = 3;
+B const g = 4;
+A const constinit h = 5;
+A i;
+B j;
+A k = 6;
+B l = 7;
+static_assert (b.i == 1 && f.i == 3);
+
+int
+main()
+{
+  if (a.i != 1 || c.i != 1 || d.i != 1 || e.i != 2 || g.i != 4 || h.i != 5
+      || i.i != 1 || j.i != 1 || k.i != 6 || l.i != 7)
+    __builtin_abort ();
+}

	Jakub


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] c++: Fix up default initialization with consteval default ctor [PR96994]
  2020-09-15  7:57 [PATCH] c++: Fix up default initialization with consteval default ctor [PR96994] Jakub Jelinek
@ 2020-09-25 12:49 ` Stephan Bergmann
  2020-09-25 20:30 ` Jason Merrill
  1 sibling, 0 replies; 5+ messages in thread
From: Stephan Bergmann @ 2020-09-25 12:49 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches

On 15/09/2020 09:57, Jakub Jelinek via Gcc-patches wrote:
> The following testcase is miscompiled (in particular the a and i
> initialization).  The problem is that build_special_member_call due to
> the immediate constructors (but not evaluated in constant expression mode)
> doesn't create a CALL_EXPR, but returns a TARGET_EXPR with CONSTRUCTOR
> as the initializer for it, and then expand_default_init just emits
> the returned statement, but this one doesn't have any side-effects and does
> nothing.  There is an if to handle constexpr ctors which emits an INIT_EXPR
> but constexpr ctors still show up as CALL_EXPR and need to be manually
> evaluated to constant expressions (if possible).
> 
> The following patch fixes that, though I'm not sure about several things.
> One is that the earlier if also has expr == true_exp && in the condition,
> not sure if we want it in this case or not.
> Another is that for delegating constructors, we emit two separate calls
> and build_if_in_charge them together.  Not sure if consteval could come into
> play in that case.

(Just reporting that with this patch applied, my build of LibreOffice 
using consteval, cf. 
<https://git.libreoffice.org/core/+/4b9e440c51be3e40326bc90c33ae69885bfb51e4%5E!/> 
"Turn OStringLiteral into a consteval'ed, static-refcound rtl_String", 
works fine.)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] c++: Fix up default initialization with consteval default ctor [PR96994]
  2020-09-15  7:57 [PATCH] c++: Fix up default initialization with consteval default ctor [PR96994] Jakub Jelinek
  2020-09-25 12:49 ` Stephan Bergmann
@ 2020-09-25 20:30 ` Jason Merrill
  2020-09-30  7:57   ` [PATCH v2] " Jakub Jelinek
  1 sibling, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2020-09-25 20:30 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 9/15/20 3:57 AM, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase is miscompiled (in particular the a and i
> initialization).  The problem is that build_special_member_call due to
> the immediate constructors (but not evaluated in constant expression mode)
> doesn't create a CALL_EXPR, but returns a TARGET_EXPR with CONSTRUCTOR
> as the initializer for it,

That seems like the bug; at the end of build_over_call, after you

>        call = cxx_constant_value (call, obj_arg);

You need to build an INIT_EXPR if obj_arg isn't a dummy.

Jason


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] c++: Fix up default initialization with consteval default ctor [PR96994]
  2020-09-25 20:30 ` Jason Merrill
@ 2020-09-30  7:57   ` Jakub Jelinek
  2020-09-30 20:18     ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2020-09-30  7:57 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Fri, Sep 25, 2020 at 04:30:26PM -0400, Jason Merrill via Gcc-patches wrote:
> On 9/15/20 3:57 AM, Jakub Jelinek wrote:
> > The following testcase is miscompiled (in particular the a and i
> > initialization).  The problem is that build_special_member_call due to
> > the immediate constructors (but not evaluated in constant expression mode)
> > doesn't create a CALL_EXPR, but returns a TARGET_EXPR with CONSTRUCTOR
> > as the initializer for it,
> 
> That seems like the bug; at the end of build_over_call, after you
> 
> >        call = cxx_constant_value (call, obj_arg);
> 
> You need to build an INIT_EXPR if obj_arg isn't a dummy.

That works.  obj_arg is NULL if it is a dummy from the earlier code.

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

2020-09-30  Jakub Jelinek  <jakub@redhat.com>

	PR c++/96994
	* call.c (build_over_call): If obj_arg is non-NULL, return INIT_EXPR
	setting obj_arg to call.

	* g++.dg/cpp2a/consteval18.C: New test.

--- gcc/cp/call.c.jj	2020-09-10 15:52:50.688207138 +0200
+++ gcc/cp/call.c	2020-09-29 20:39:55.003361651 +0200
@@ -9200,6 +9200,8 @@ build_over_call (struct z_candidate *can
 		}
 	    }
 	  call = cxx_constant_value (call, obj_arg);
+	  if (obj_arg && !error_operand_p (call))
+	    call = build2 (INIT_EXPR, void_type_node, obj_arg, call);
 	}
     }
   return call;
--- gcc/testsuite/g++.dg/cpp2a/consteval18.C.jj	2020-09-29 20:33:56.533596845 +0200
+++ gcc/testsuite/g++.dg/cpp2a/consteval18.C	2020-09-29 20:33:56.533596845 +0200
@@ -0,0 +1,26 @@
+// PR c++/96994
+// { dg-do run { target c++20 } }
+
+struct A { consteval A () { i = 1; } consteval A (int x) : i (x) {} int i = 0; };
+struct B { constexpr B () { i = 1; } constexpr B (int x) : i (x) {} int i = 0; };
+A const a;
+constexpr A b;
+B const c;
+A const constinit d;
+A const e = 2;
+constexpr A f = 3;
+B const g = 4;
+A const constinit h = 5;
+A i;
+B j;
+A k = 6;
+B l = 7;
+static_assert (b.i == 1 && f.i == 3);
+
+int
+main()
+{
+  if (a.i != 1 || c.i != 1 || d.i != 1 || e.i != 2 || g.i != 4 || h.i != 5
+      || i.i != 1 || j.i != 1 || k.i != 6 || l.i != 7)
+    __builtin_abort ();
+}


	Jakub


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] c++: Fix up default initialization with consteval default ctor [PR96994]
  2020-09-30  7:57   ` [PATCH v2] " Jakub Jelinek
@ 2020-09-30 20:18     ` Jason Merrill
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2020-09-30 20:18 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 9/30/20 3:57 AM, Jakub Jelinek wrote:
> On Fri, Sep 25, 2020 at 04:30:26PM -0400, Jason Merrill via Gcc-patches wrote:
>> On 9/15/20 3:57 AM, Jakub Jelinek wrote:
>>> The following testcase is miscompiled (in particular the a and i
>>> initialization).  The problem is that build_special_member_call due to
>>> the immediate constructors (but not evaluated in constant expression mode)
>>> doesn't create a CALL_EXPR, but returns a TARGET_EXPR with CONSTRUCTOR
>>> as the initializer for it,
>>
>> That seems like the bug; at the end of build_over_call, after you
>>
>>>         call = cxx_constant_value (call, obj_arg);
>>
>> You need to build an INIT_EXPR if obj_arg isn't a dummy.
> 
> That works.  obj_arg is NULL if it is a dummy from the earlier code.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?1

OK.

> 2020-09-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/96994
> 	* call.c (build_over_call): If obj_arg is non-NULL, return INIT_EXPR
> 	setting obj_arg to call.
> 
> 	* g++.dg/cpp2a/consteval18.C: New test.
> 
> --- gcc/cp/call.c.jj	2020-09-10 15:52:50.688207138 +0200
> +++ gcc/cp/call.c	2020-09-29 20:39:55.003361651 +0200
> @@ -9200,6 +9200,8 @@ build_over_call (struct z_candidate *can
>   		}
>   	    }
>   	  call = cxx_constant_value (call, obj_arg);
> +	  if (obj_arg && !error_operand_p (call))
> +	    call = build2 (INIT_EXPR, void_type_node, obj_arg, call);
>   	}
>       }
>     return call;
> --- gcc/testsuite/g++.dg/cpp2a/consteval18.C.jj	2020-09-29 20:33:56.533596845 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/consteval18.C	2020-09-29 20:33:56.533596845 +0200
> @@ -0,0 +1,26 @@
> +// PR c++/96994
> +// { dg-do run { target c++20 } }
> +
> +struct A { consteval A () { i = 1; } consteval A (int x) : i (x) {} int i = 0; };
> +struct B { constexpr B () { i = 1; } constexpr B (int x) : i (x) {} int i = 0; };
> +A const a;
> +constexpr A b;
> +B const c;
> +A const constinit d;
> +A const e = 2;
> +constexpr A f = 3;
> +B const g = 4;
> +A const constinit h = 5;
> +A i;
> +B j;
> +A k = 6;
> +B l = 7;
> +static_assert (b.i == 1 && f.i == 3);
> +
> +int
> +main()
> +{
> +  if (a.i != 1 || c.i != 1 || d.i != 1 || e.i != 2 || g.i != 4 || h.i != 5
> +      || i.i != 1 || j.i != 1 || k.i != 6 || l.i != 7)
> +    __builtin_abort ();
> +}
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-30 20:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15  7:57 [PATCH] c++: Fix up default initialization with consteval default ctor [PR96994] Jakub Jelinek
2020-09-25 12:49 ` Stephan Bergmann
2020-09-25 20:30 ` Jason Merrill
2020-09-30  7:57   ` [PATCH v2] " Jakub Jelinek
2020-09-30 20:18     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).