From 5afd90c5f36bf45291ca09ef3791f4a574e90d5d Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 20 Oct 2020 09:33:20 +0200 Subject: [PATCH] c++: Fix constexpr dtors vs invisible ref [PR97388] For arguments passed by invisible reference, in the IL until genericization we have the source types on the callee side and while on the caller side we already pass references to the actual argument slot in the caller, we undo that in cxx_bind_parameters_in_call's if (TREE_ADDRESSABLE (type)) /* Undo convert_for_arg_passing work here. */ x = convert_from_reference (x); This works fine most of the time, except when the type also has constexpr destructor; in that case the destructor is invoked in the caller and thus the unsharing we do to make sure that the callee doesn't modify caller's values is in that case undesirable, it prevents the changes done in the callee propagating to the caller which should see them for the constexpr dtor evaluation. The following patch fixes that. While it could be perhaps done for all TREE_ADDRESSABLE types, I don't see the need to change the behavior if there is no constexpr non-trivial dtor. Jason: And we need to avoid memoizing the call, because a later equivalent call also needs to modify its argument. And we don't need to unshare constructors when we aren't memoizing the call, because we already unshared them when evaluating the TARGET_EXPR representing the copy-initialization of the argument. 2020-10-20 Jakub Jelinek Jason Merrill PR c++/97388 * constexpr.c (cxx_bind_parameters_in_call): Set non_constant_args if the parameter type has a non-trivial destructor. (cxx_eval_call_expression): Only unshare arguments if we're memoizing this evaluation. * g++.dg/cpp2a/constexpr-dtor5.C: New test. * g++.dg/cpp2a/constexpr-dtor6.C: New test. * g++.dg/cpp2a/constexpr-dtor7.C: New test. --- gcc/cp/constexpr.c | 19 +++++++---- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor5.C | 35 +++++++++++++++++++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor6.C | 36 ++++++++++++++++++++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor7.C | 19 +++++++++++ 4 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor5.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor6.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-dtor7.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 37c836a8a7b..c959b532ec3 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1602,6 +1602,11 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx, tree t, arg = adjust_temp_type (type, arg); if (!TREE_CONSTANT (arg)) *non_constant_args = true; + else if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)) + /* The destructor needs to see any modifications the callee makes + to the argument. */ + *non_constant_args = true; + /* For virtual calls, adjust the this argument, so that it is the object on which the method is called, rather than one of its bases. */ @@ -2626,14 +2631,14 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, problems with verify_gimple. */ arg = unshare_expr_without_location (arg); TREE_VEC_ELT (bound, i) = arg; + + /* And then unshare again so the callee doesn't change the + argument values in the hash table. XXX Could we unshare + lazily in cxx_eval_store_expression? */ + arg = unshare_constructor (arg); + if (TREE_CODE (arg) == CONSTRUCTOR) + vec_safe_push (ctors, arg); } - /* Don't share a CONSTRUCTOR that might be changed. This is not - redundant with the unshare just above; we also don't want to - change the argument values in the hash table. XXX Could we - unshare lazily in cxx_eval_store_expression? */ - arg = unshare_constructor (arg); - if (TREE_CODE (arg) == CONSTRUCTOR) - vec_safe_push (ctors, arg); ctx->global->values.put (remapped, arg); remapped = DECL_CHAIN (remapped); } diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor5.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor5.C new file mode 100644 index 00000000000..1739afbcc68 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor5.C @@ -0,0 +1,35 @@ +// PR c++/97388 +// { dg-do compile { target c++20 } } + +struct S { + int m; + constexpr S () : m(1) {} + constexpr ~S () noexcept (false) { if (m == 1) { throw; } } +}; + +constexpr bool +foo (S v) +{ + v.m = 2; + return true; +} + +constexpr bool +bar () +{ + return foo (S ()); +} + +constexpr bool +baz () +{ + foo (S ()); + return foo (S ()); +} + +static_assert (foo (S ())); +static_assert (bar ()); +static_assert (baz ()); +constexpr bool x = foo (S ()); +constexpr bool y = bar (); +constexpr bool z = baz (); diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor6.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor6.C new file mode 100644 index 00000000000..ce783a1df88 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor6.C @@ -0,0 +1,36 @@ +// PR c++/97388 +// { dg-do compile { target c++20 } } + +struct S { + int *s; + constexpr S () : s(new int ()) {} + constexpr S (S &&x) noexcept : s(x.s) { x.s = nullptr; } + constexpr ~S () noexcept { delete s; } +}; + +constexpr bool +foo (S v) +{ + auto x = static_cast (v); + return true; +} + +constexpr bool +bar () +{ + return foo (S ()); +} + +constexpr bool +baz () +{ + foo (S ()); + return foo (S ()); +} + +static_assert (foo (S ())); +static_assert (bar ()); +static_assert (baz ()); +constexpr bool x = foo (S ()); +constexpr bool y = bar (); +constexpr bool z = baz (); diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor7.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor7.C new file mode 100644 index 00000000000..463eaca0539 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor7.C @@ -0,0 +1,19 @@ +// PR c++/97388 +// { dg-do compile { target c++20 } } + +struct S { + int *s; + constexpr S () : s(new int) {} // { dg-error "is not a constant expression because allocated storage has not been deallocated" } + S (const S &) = delete; + S &operator= (const S &) = delete; + constexpr ~S () { delete s; } +}; + +constexpr bool +foo (S v) +{ + v.s = nullptr; + return true; +} + +static_assert (foo (S ())); // { dg-error "non-constant condition for static assertion" } -- 2.30.2