public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [pushed] c++: Fix DMI with lambda 'this' capture [PR94205]
@ 2020-04-01  5:16 Jason Merrill
  2020-04-01 14:47 ` Patrick Palka
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Merrill @ 2020-04-01  5:16 UTC (permalink / raw)
  To: gcc-patches

We represent 'this' in a default member initializer with a PLACEHOLDER_EXPR.
Normally in constexpr evaluation when we encounter one it refers to
ctx->ctor, but when we're creating a temporary of class type, that replaces
ctx->ctor, so a PLACEHOLDER_EXPR that refers to the type of the member being
initialized needs to be replaced before that happens.

This patch fixes the testcase below, but not the original testcase from the PR,
which is still broken due to PR94219.

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

gcc/cp/ChangeLog
2020-03-31  Jason Merrill  <jason@redhat.com>

	PR c++/94205
	* constexpr.c (cxx_eval_constant_expression) [TARGET_EXPR]: Call
	replace_placeholders.
	* typeck2.c (store_init_value): Fix arguments to
	fold_non_dependent_expr.
---
 gcc/cp/constexpr.c                            |  6 ++++++
 gcc/cp/tree.c                                 |  2 +-
 gcc/cp/typeck2.c                              |  2 +-
 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C | 20 +++++++++++++++++++
 gcc/testsuite/g++.dg/cpp1z/lambda-this4.C     | 13 ++++++++++++
 5 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this4.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index e85b3c113f0..91f0c3ba269 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5553,6 +5553,12 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	tree init = TARGET_EXPR_INITIAL (t);
 	if ((AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type)))
 	  {
+	    if (ctx->object)
+	      /* If the initializer contains any PLACEHOLDER_EXPR, we need to
+		 resolve them before we create a new CONSTRUCTOR for the
+		 temporary.  */
+	      init = replace_placeholders (init, ctx->object);
+
 	    /* We're being expanded without an explicit target, so start
 	       initializing a new object; expansion with an explicit target
 	       strips the TARGET_EXPR before we get here.  */
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index a2172dea0d8..5eb0dcd717a 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -3239,7 +3239,7 @@ replace_placeholders_r (tree* t, int* walk_subtrees, void* data_)
    a PLACEHOLDER_EXPR has been encountered.  */
 
 tree
-replace_placeholders (tree exp, tree obj, bool *seen_p)
+replace_placeholders (tree exp, tree obj, bool *seen_p /*= NULL*/)
 {
   /* This is only relevant for C++14.  */
   if (cxx_dialect < cxx14)
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index bff4ddbcf81..cf1cb5ace66 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -871,7 +871,7 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags)
     {
       bool const_init;
       tree oldval = value;
-      value = fold_non_dependent_expr (value);
+      value = fold_non_dependent_expr (value, tf_warning_or_error, true, decl);
       if (DECL_DECLARED_CONSTEXPR_P (decl)
 	  || (DECL_IN_AGGR_P (decl)
 	      && DECL_INITIALIZED_IN_CLASS_P (decl)))
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C
new file mode 100644
index 00000000000..c51f734a134
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C
@@ -0,0 +1,20 @@
+// PR c++/94205
+// { dg-do compile { target c++14 } }
+
+struct S
+{
+  struct A
+  {
+    S *p;
+    constexpr A(S* p): p(p) {}
+    constexpr operator int() { p->i = 5; return 6; }
+  };
+  int i;
+  int a = A(this);
+};
+
+
+constexpr S s = {};
+
+#define SA(X) static_assert((X),#X)
+SA(s.i == 5 && s.a == 6);
diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this4.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this4.C
new file mode 100644
index 00000000000..5d968791491
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this4.C
@@ -0,0 +1,13 @@
+// PR c++/94205
+// { dg-do compile { target c++17 } }
+
+struct S
+{
+  int i;
+  int a = [this] { this->i = 5; return 6; } ();
+};
+
+
+constexpr S s = {};
+
+static_assert (s.i == 5 && s.a == 6);

base-commit: f14b41d27124601284347a10d496362c8b4b8e1c
-- 
2.18.1


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

* Re: [pushed] c++: Fix DMI with lambda 'this' capture [PR94205]
  2020-04-01  5:16 [pushed] c++: Fix DMI with lambda 'this' capture [PR94205] Jason Merrill
@ 2020-04-01 14:47 ` Patrick Palka
  2020-04-02  4:35   ` Jason Merrill
  0 siblings, 1 reply; 3+ messages in thread
From: Patrick Palka @ 2020-04-01 14:47 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, ppalka

On Wed, 1 Apr 2020, Jason Merrill wrote:

> We represent 'this' in a default member initializer with a PLACEHOLDER_EXPR.
> Normally in constexpr evaluation when we encounter one it refers to
> ctx->ctor, but when we're creating a temporary of class type, that replaces
> ctx->ctor, so a PLACEHOLDER_EXPR that refers to the type of the member being
> initialized needs to be replaced before that happens.
> 
> This patch fixes the testcase below, but not the original testcase from the PR,
> which is still broken due to PR94219.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk and 9.
> 
> gcc/cp/ChangeLog
> 2020-03-31  Jason Merrill  <jason@redhat.com>
> 
> 	PR c++/94205
> 	* constexpr.c (cxx_eval_constant_expression) [TARGET_EXPR]: Call
> 	replace_placeholders.
> 	* typeck2.c (store_init_value): Fix arguments to
> 	fold_non_dependent_expr.
> ---
>  gcc/cp/constexpr.c                            |  6 ++++++
>  gcc/cp/tree.c                                 |  2 +-
>  gcc/cp/typeck2.c                              |  2 +-
>  gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C | 20 +++++++++++++++++++
>  gcc/testsuite/g++.dg/cpp1z/lambda-this4.C     | 13 ++++++++++++
>  5 files changed, 41 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this4.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index e85b3c113f0..91f0c3ba269 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -5553,6 +5553,12 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>  	tree init = TARGET_EXPR_INITIAL (t);
>  	if ((AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type)))
>  	  {
> +	    if (ctx->object)
> +	      /* If the initializer contains any PLACEHOLDER_EXPR, we need to
> +		 resolve them before we create a new CONSTRUCTOR for the
> +		 temporary.  */
> +	      init = replace_placeholders (init, ctx->object);

I think I'm running into an issue due to this call to replace_placeholders.
After this change, the following (compiled with -std=c++17) ICEs

    struct S
    {
      int a = [this] { return 6; } ();
    };

    S
    foo()
    {
      return {};
    }

with

    internal compiler error: in gimplify_var_or_parm_decl, at gimplify.c:2830

Unsharing 'init' before before doing replace_placeholders seems to fix
this ICE.

(This came up while working on PR94034, which seems like it may admit a
fix that's similar to this one for PR94205.)

> +
>  	    /* We're being expanded without an explicit target, so start
>  	       initializing a new object; expansion with an explicit target
>  	       strips the TARGET_EXPR before we get here.  */
> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> index a2172dea0d8..5eb0dcd717a 100644
> --- a/gcc/cp/tree.c
> +++ b/gcc/cp/tree.c
> @@ -3239,7 +3239,7 @@ replace_placeholders_r (tree* t, int* walk_subtrees, void* data_)
>     a PLACEHOLDER_EXPR has been encountered.  */
>  
>  tree
> -replace_placeholders (tree exp, tree obj, bool *seen_p)
> +replace_placeholders (tree exp, tree obj, bool *seen_p /*= NULL*/)
>  {
>    /* This is only relevant for C++14.  */
>    if (cxx_dialect < cxx14)
> diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
> index bff4ddbcf81..cf1cb5ace66 100644
> --- a/gcc/cp/typeck2.c
> +++ b/gcc/cp/typeck2.c
> @@ -871,7 +871,7 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags)
>      {
>        bool const_init;
>        tree oldval = value;
> -      value = fold_non_dependent_expr (value);
> +      value = fold_non_dependent_expr (value, tf_warning_or_error, true, decl);
>        if (DECL_DECLARED_CONSTEXPR_P (decl)
>  	  || (DECL_IN_AGGR_P (decl)
>  	      && DECL_INITIALIZED_IN_CLASS_P (decl)))
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C
> new file mode 100644
> index 00000000000..c51f734a134
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C
> @@ -0,0 +1,20 @@
> +// PR c++/94205
> +// { dg-do compile { target c++14 } }
> +
> +struct S
> +{
> +  struct A
> +  {
> +    S *p;
> +    constexpr A(S* p): p(p) {}
> +    constexpr operator int() { p->i = 5; return 6; }
> +  };
> +  int i;
> +  int a = A(this);
> +};
> +
> +
> +constexpr S s = {};
> +
> +#define SA(X) static_assert((X),#X)
> +SA(s.i == 5 && s.a == 6);
> diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this4.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this4.C
> new file mode 100644
> index 00000000000..5d968791491
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this4.C
> @@ -0,0 +1,13 @@
> +// PR c++/94205
> +// { dg-do compile { target c++17 } }
> +
> +struct S
> +{
> +  int i;
> +  int a = [this] { this->i = 5; return 6; } ();
> +};
> +
> +
> +constexpr S s = {};
> +
> +static_assert (s.i == 5 && s.a == 6);
> 
> base-commit: f14b41d27124601284347a10d496362c8b4b8e1c
> -- 
> 2.18.1
> 
> 


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

* Re: [pushed] c++: Fix DMI with lambda 'this' capture [PR94205]
  2020-04-01 14:47 ` Patrick Palka
@ 2020-04-02  4:35   ` Jason Merrill
  0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2020-04-02  4:35 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches

On 4/1/20 10:47 AM, Patrick Palka wrote:
> On Wed, 1 Apr 2020, Jason Merrill wrote:
> 
>> We represent 'this' in a default member initializer with a PLACEHOLDER_EXPR.
>> Normally in constexpr evaluation when we encounter one it refers to
>> ctx->ctor, but when we're creating a temporary of class type, that replaces
>> ctx->ctor, so a PLACEHOLDER_EXPR that refers to the type of the member being
>> initialized needs to be replaced before that happens.
>>
>> This patch fixes the testcase below, but not the original testcase from the PR,
>> which is still broken due to PR94219.
>>
>> Tested x86_64-pc-linux-gnu, applying to trunk and 9.
>>
>> gcc/cp/ChangeLog
>> 2020-03-31  Jason Merrill  <jason@redhat.com>
>>
>> 	PR c++/94205
>> 	* constexpr.c (cxx_eval_constant_expression) [TARGET_EXPR]: Call
>> 	replace_placeholders.
>> 	* typeck2.c (store_init_value): Fix arguments to
>> 	fold_non_dependent_expr.
>> ---
>>   gcc/cp/constexpr.c                            |  6 ++++++
>>   gcc/cp/tree.c                                 |  2 +-
>>   gcc/cp/typeck2.c                              |  2 +-
>>   gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C | 20 +++++++++++++++++++
>>   gcc/testsuite/g++.dg/cpp1z/lambda-this4.C     | 13 ++++++++++++
>>   5 files changed, 41 insertions(+), 2 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi2.C
>>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this4.C
>>
>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>> index e85b3c113f0..91f0c3ba269 100644
>> --- a/gcc/cp/constexpr.c
>> +++ b/gcc/cp/constexpr.c
>> @@ -5553,6 +5553,12 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>>   	tree init = TARGET_EXPR_INITIAL (t);
>>   	if ((AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type)))
>>   	  {
>> +	    if (ctx->object)
>> +	      /* If the initializer contains any PLACEHOLDER_EXPR, we need to
>> +		 resolve them before we create a new CONSTRUCTOR for the
>> +		 temporary.  */
>> +	      init = replace_placeholders (init, ctx->object);
> 
> I think I'm running into an issue due to this call to replace_placeholders.
> After this change, the following (compiled with -std=c++17) ICEs
> 
>      struct S
>      {
>        int a = [this] { return 6; } ();
>      };
> 
>      S
>      foo()
>      {
>        return {};
>      }
> 
> with
> 
>      internal compiler error: in gimplify_var_or_parm_decl, at gimplify.c:2830
> 
> Unsharing 'init' before before doing replace_placeholders seems to fix
> this ICE.

That sounds good, thanks.

Jason


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

end of thread, other threads:[~2020-04-02  4:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01  5:16 [pushed] c++: Fix DMI with lambda 'this' capture [PR94205] Jason Merrill
2020-04-01 14:47 ` Patrick Palka
2020-04-02  4:35   ` 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).