| Message ID | 20210107151015.1921720-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: ICE with constexpr call that returns a PMF [PR98551] | expand |
On 1/7/21 10:10 AM, Patrick Palka wrote: > We shouldn't do replace_result_decl after evaluating a call that returns > a PMF because PMF temporaries aren't wrapped in a TARGET_EXPR (and so we > can't trust ctx->object), and PMF initializers can't be self-referential > anyway, so replace_result_decl would always be a no-op. This fixes an > ICE from the sanity check in replace_result_decl in the below testcase > during cxx_eval_call_expression of the call f() in the initializer g(f()). > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? > > gcc/cp/ChangeLog: > > PR c++/98551 > * constexpr.c (cxx_eval_call_expression): Don't call > replace_result_decl when the result is a PMF. > > gcc/testsuite/ChangeLog: > > PR c++/98551 > * g++.dg/cpp0x/constexpr-pmf2.C: New test. > --- > gcc/cp/constexpr.c | 1 + > gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C | 9 +++++++++ > 2 files changed, 10 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 0c12f608d36..a7272d49d0d 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -2788,6 +2788,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > current object under construction. */ > if (!*non_constant_p && ctx->object > && AGGREGATE_TYPE_P (TREE_TYPE (res)) > + && !TYPE_PTRMEMFUNC_P (TREE_TYPE (res)) It ought to work to change AGGREGATE_TYPE_P to CLASS_TYPE_P; we can't return an array, and a vector can't contain a pointer to itself. Alternately, we could change the same-type assert in replace_result_decl to a test and return false if different. OK with either change. > && !is_empty_class (TREE_TYPE (res))) > if (replace_result_decl (&result, res, ctx->object)) > cacheable = false; > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C > new file mode 100644 > index 00000000000..a76e712afe1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C > @@ -0,0 +1,9 @@ > +// PR c++/98551 > +// { dg-do compile { target c++11 } } > + > +struct A {}; > +struct B { int t(); }; > +using pmf = decltype(&B::t); > +constexpr pmf f() { return &B::t; } > +constexpr A g(pmf) { return {}; }; > +constexpr A x = g(f()); >
On Thu, 7 Jan 2021, Jason Merrill wrote: > On 1/7/21 10:10 AM, Patrick Palka wrote: > > We shouldn't do replace_result_decl after evaluating a call that returns > > a PMF because PMF temporaries aren't wrapped in a TARGET_EXPR (and so we > > can't trust ctx->object), and PMF initializers can't be self-referential > > anyway, so replace_result_decl would always be a no-op. This fixes an > > ICE from the sanity check in replace_result_decl in the below testcase > > during cxx_eval_call_expression of the call f() in the initializer g(f()). > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk? > > > > gcc/cp/ChangeLog: > > > > PR c++/98551 > > * constexpr.c (cxx_eval_call_expression): Don't call > > replace_result_decl when the result is a PMF. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/98551 > > * g++.dg/cpp0x/constexpr-pmf2.C: New test. > > --- > > gcc/cp/constexpr.c | 1 + > > gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C | 9 +++++++++ > > 2 files changed, 10 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > index 0c12f608d36..a7272d49d0d 100644 > > --- a/gcc/cp/constexpr.c > > +++ b/gcc/cp/constexpr.c > > @@ -2788,6 +2788,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, > > tree t, > > current object under construction. */ > > if (!*non_constant_p && ctx->object > > && AGGREGATE_TYPE_P (TREE_TYPE (res)) > > + && !TYPE_PTRMEMFUNC_P (TREE_TYPE (res)) > > It ought to work to change AGGREGATE_TYPE_P to CLASS_TYPE_P; we can't return > an array, and a vector can't contain a pointer to itself. > > Alternately, we could change the same-type assert in replace_result_decl to a > test and return false if different. > > OK with either change. Sounds good. I went with changing AGGREGATE_TYPE_P to CLASS_TYPE_P and leaving alone the assert in replace_result_decl for now: -- >8 -- Subject: [PATCH] c++: ICE with constexpr call that returns a PMF [PR98551] We shouldn't do replace_result_decl after evaluating a call that returns a PMF because PMF temporaries aren't wrapped in a TARGET_EXPR (and so we can't trust ctx->object), and PMF initializers can't be self-referential anyway, so replace_result_decl would always be a no-op. To that end, this patch changes the relevant AGGREGATE_TYPE_P test to CLASS_TYPE_P, which should rule out PMFs (as well as arrays, which we can't return and therefore won't see here). This fixes an ICE from the sanity check in replace_result_decl in the below testcase during constexpr evaluation of the call f() in the initializer g(f()). gcc/cp/ChangeLog: PR c++/98551 * constexpr.c (cxx_eval_call_expression): Check CLASS_TYPE_P instead of AGGREGATE_TYPE_P before calling replace_result_decl. gcc/testsuite/ChangeLog: PR c++/98551 * g++.dg/cpp0x/constexpr-pmf2.C: New test. --- gcc/cp/constexpr.c | 2 +- gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 4a5e6384da6..9dddc53ca52 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -2790,7 +2790,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, /* Rewrite all occurrences of the function's RESULT_DECL with the current object under construction. */ if (!*non_constant_p && ctx->object - && AGGREGATE_TYPE_P (TREE_TYPE (res)) + && CLASS_TYPE_P (TREE_TYPE (res)) && !is_empty_class (TREE_TYPE (res))) if (replace_result_decl (&result, res, ctx->object)) cacheable = false; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C new file mode 100644 index 00000000000..a76e712afe1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C @@ -0,0 +1,9 @@ +// PR c++/98551 +// { dg-do compile { target c++11 } } + +struct A {}; +struct B { int t(); }; +using pmf = decltype(&B::t); +constexpr pmf f() { return &B::t; } +constexpr A g(pmf) { return {}; }; +constexpr A x = g(f());
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 0c12f608d36..a7272d49d0d 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -2788,6 +2788,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, current object under construction. */ if (!*non_constant_p && ctx->object && AGGREGATE_TYPE_P (TREE_TYPE (res)) + && !TYPE_PTRMEMFUNC_P (TREE_TYPE (res)) && !is_empty_class (TREE_TYPE (res))) if (replace_result_decl (&result, res, ctx->object)) cacheable = false; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C new file mode 100644 index 00000000000..a76e712afe1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-pmf2.C @@ -0,0 +1,9 @@ +// PR c++/98551 +// { dg-do compile { target c++11 } } + +struct A {}; +struct B { int t(); }; +using pmf = decltype(&B::t); +constexpr pmf f() { return &B::t; } +constexpr A g(pmf) { return {}; }; +constexpr A x = g(f());