From b256222910cfa4a9b2b477dff8954e51fdc36bb9 Mon Sep 17 00:00:00 2001 From: Patrick Palka Date: Wed, 8 Apr 2020 13:14:42 -0400 Subject: [PATCH] c++: Stray RESULT_DECLs in result of constexpr call [PR94034] When evaluating the initializer of 'a' in the following example struct A { A() = default; A(const A&); A *p = this; }; constexpr A foo() { return {}; } constexpr A a = foo(); the PLACEHOLDER_EXPR for 'this' in the aggregate initializer returned by foo gets resolved to the RESULT_DECL of foo. But due to guaranteed RVO, the 'this' should really be resolved to '&a'. Fixing this properly by immediately resolving 'this' and PLACEHOLDER_EXPRs to the ultimate object under construction would in general mean that we would no longer be able to cache constexpr calls for which RVO possibly applies, because the result of the call may now depend on the ultimate object under construction. So as a mostly correct stopgap solution that retains cachability of RVO'd constexpr calls, this patch fixes this issue by rewriting all occurrences of the RESULT_DECL in the result of a constexpr function call with the current object under construction, after the call returns. This means the 'this' pointer during construction of the temporary will still point to the temporary object instead of the ultimate object, but besides that this approach seems functionally equivalent to the proper approach. gcc/cp/ChangeLog: PR c++/94034 * constexpr.c (replace_result_decl_data): New struct. (replace_result_decl_data_r): New function. (replace_result_decl): New function. (cxx_eval_call_expression): Use it. * tree.c (build_aggr_init_expr): Set the location of the AGGR_INIT_EXPR to that of its initializer. gcc/testsuite/ChangeLog: PR c++/94034 * g++.dg/cpp0x/constexpr-empty15.C: New test. * g++.dg/cpp1y/constexpr-nsdmi6a.C: New test. * g++.dg/cpp1y/constexpr-nsdmi6b.C: New test. * g++.dg/cpp1y/constexpr-nsdmi7a.C: New test. * g++.dg/cpp1y/constexpr-nsdmi7b.C: New test. --- gcc/cp/ChangeLog | 10 ++++ gcc/cp/constexpr.c | 54 +++++++++++++++++++ gcc/cp/tree.c | 3 ++ gcc/testsuite/ChangeLog | 9 ++++ .../g++.dg/cpp0x/constexpr-empty15.C | 9 ++++ .../g++.dg/cpp1y/constexpr-nsdmi6a.C | 26 +++++++++ .../g++.dg/cpp1y/constexpr-nsdmi6b.C | 27 ++++++++++ .../g++.dg/cpp1y/constexpr-nsdmi7a.C | 49 +++++++++++++++++ .../g++.dg/cpp1y/constexpr-nsdmi7b.C | 48 +++++++++++++++++ 9 files changed, 235 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 7d742c15ba8..ce390931c2e 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,13 @@ +2020-04-14 Patrick Palka + + PR c++/94034 + * constexpr.c (replace_result_decl_data): New struct. + (replace_result_decl_data_r): New function. + (replace_result_decl): New function. + (cxx_eval_call_expression): Use it. + * tree.c (build_aggr_init_expr): Set the location of the AGGR_INIT_EXPR + to that of its initializer. + 2020-04-13 Marek Polacek PR c++/94588 diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index d8636ddb92f..c8e7d083f40 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -2029,6 +2029,52 @@ cxx_eval_dynamic_cast_fn (const constexpr_ctx *ctx, tree call, return cp_build_addr_expr (obj, complain); } +/* Data structure used by replace_result_decl and replace_result_decl_r. */ + +struct replace_result_decl_data +{ + /* The RESULT_DECL we want to replace. */ + tree decl; + /* The replacement for DECL. */ + tree replacement; + /* Whether we've performed any replacements. */ + bool changed; +}; + +/* Helper function for replace_result_decl, called through cp_walk_tree. */ + +static tree +replace_result_decl_r (tree *tp, int *walk_subtrees, void *data) +{ + replace_result_decl_data *d = (replace_result_decl_data *) data; + + if (*tp == d->decl) + { + *tp = unshare_expr (d->replacement); + d->changed = true; + *walk_subtrees = 0; + } + else if (TYPE_P (*tp)) + *walk_subtrees = 0; + + return NULL_TREE; +} + +/* Replace every occurrence of DECL, a RESULT_DECL, with (an unshared copy of) + REPLACEMENT within the reduced constant expression *TP. Returns true iff a + replacement was performed. */ + +static bool +replace_result_decl (tree *tp, tree decl, tree replacement) +{ + gcc_checking_assert (TREE_CODE (decl) == RESULT_DECL + && (same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (decl), TREE_TYPE (replacement)))); + replace_result_decl_data data = { decl, replacement, false }; + cp_walk_tree_without_duplicates (tp, replace_result_decl_r, &data); + return data.changed; +} + /* Subroutine of cxx_eval_constant_expression. Evaluate the call expression tree T in the context of OLD_CALL expression evaluation. */ @@ -2536,6 +2582,14 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, break; } } + + /* 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)) + && !is_empty_class (TREE_TYPE (res))) + if (replace_result_decl (&result, res, ctx->object)) + cacheable = false; } else /* Couldn't get a function copy to evaluate. */ diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 1d311b0fe61..8e4934c0009 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -669,6 +669,9 @@ build_aggr_init_expr (tree type, tree init) else rval = init; + if (location_t loc = EXPR_LOCATION (init)) + SET_EXPR_LOCATION (rval, loc); + return rval; } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 64ee01b943c..3fec84eb261 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2020-04-14 Patrick Palka + + PR c++/94034 + * g++.dg/cpp0x/constexpr-empty15.C: New test. + * g++.dg/cpp1y/constexpr-nsdmi6a.C: New test. + * g++.dg/cpp1y/constexpr-nsdmi6b.C: New test. + * g++.dg/cpp1y/constexpr-nsdmi7a.C: New test. + * g++.dg/cpp1y/constexpr-nsdmi7b.C: New test. + 2020-04-14 Jakub Jelinek PR tree-optimization/94573 diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C new file mode 100644 index 00000000000..97863d4e1a7 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty15.C @@ -0,0 +1,9 @@ +// { dg-do compile { target c++11 } } + +struct empty1 { }; +constexpr empty1 foo1() { return {}; } + +struct empty2 { }; +constexpr empty2 foo2(empty1) { return {}; } + +constexpr empty2 a = foo2(foo1()); diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C new file mode 100644 index 00000000000..bb844b952e2 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6a.C @@ -0,0 +1,26 @@ +// PR c++/94034 +// { dg-do compile { target c++14 } } + +struct A { + A *ap = this; +}; + +constexpr A foo() +{ + return {}; +} + +constexpr A bar() +{ + return foo(); +} + +void +baz() +{ + constexpr A a = foo(); // { dg-error ".A..& a... is not a constant expression" } + constexpr A b = bar(); // { dg-error ".A..& b... is not a constant expression" } +} + +constexpr A a = foo(); +constexpr A b = bar(); diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C new file mode 100644 index 00000000000..f847fe9809f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi6b.C @@ -0,0 +1,27 @@ +// PR c++/94034 +// { dg-do compile { target c++14 } } + +struct A { + A() = default; A(const A&); + A *ap = this; +}; + +constexpr A foo() +{ + return {}; +} + +constexpr A bar() +{ + return foo(); +} + +void +baz() +{ + constexpr A a = foo(); // { dg-error ".A..& a... is not a constant expression" } + constexpr A b = bar(); // { dg-error ".A..& b... is not a constant expression" } +} + +constexpr A a = foo(); +constexpr A b = bar(); diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C new file mode 100644 index 00000000000..5a40cd0b845 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7a.C @@ -0,0 +1,49 @@ +// PR c++/94034 +// { dg-do compile { target c++14 } } + +struct A +{ + A *p = this; + int n = 2; + int m = p->n++; +}; + +constexpr A +foo() +{ + return {}; +} + +constexpr A +bar() +{ + A a = foo(); + a.p->n = 5; + return a; +} + +static_assert(bar().n == 5, ""); + +constexpr int +baz() +{ + A b = foo(); + b.p->n = 10; + A c = foo(); + if (c.p->n != 3 || c.p->m != 2) + __builtin_abort(); + bar(); + return 0; +} + +static_assert(baz() == 0, ""); + +constexpr int +quux() +{ + const A d = foo(); + d.p->n++; // { dg-error "const object" } + return 0; +} + +static_assert(quux() == 0, ""); // { dg-error "non-constant" } diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C new file mode 100644 index 00000000000..86d8ab4e759 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C @@ -0,0 +1,48 @@ +// PR c++/94034 +// { dg-do compile { target c++14 } } + +struct A +{ + A() = default; A(const A&); + A *p = this; + int n = 2; + int m = p->n++; +}; + +constexpr A +foo() +{ + return {}; +} + +constexpr A +bar() +{ + A a = foo(); + a.p->n = 5; + return a; // { dg-error "non-.constexpr." } +} + +constexpr int +baz() +{ + A b = foo(); + b.p->n = 10; + A c = foo(); + if (c.p->n != 3 || c.p->m != 2) + __builtin_abort(); + foo(); + return 0; +} + +static_assert(baz() == 0, ""); + +constexpr int +quux() +{ + const A d = foo(); + d.p->n++; // { dg-error "const object" } + return 0; +} + +static_assert(quux() == 0, ""); // { dg-error "non-constant" } -- 2.30.2