On Mon, 24 May 2021, Jason Merrill wrote:
> On 5/24/21 1:48 PM, Patrick Palka wrote:
> > In the testcase below, the initializer for C::b inside C's default
> > constructor is encoded as a TARGET_EXPR wrapping the CALL_EXPR f() in
> > C++17 mode. During massaging of this constexpr constructor,
> > build_target_expr_with_type called from bot_manip ends up trying to use
> > B's implicitly deleted copy constructor rather than preserving the
> > copy elision.
>
> > This patch makes bot_manip use force_target_expr instead of
> > build_target_expr_with_type so that it copies TARGET_EXPRs in a more
> > oblivious manner.
>
> Even with that change we should fix build_target_expr_with_type to handle
> CALL_EXPR properly; adding an extra copy is just wrong.
Sounds good.
>
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > for trunk?
> >
> > PR c++/100368
> >
> > gcc/cp/ChangeLog:
> >
> > * tree.c (build_target_expr_with_type): Simplify now that
> > bot_manip is no longer a caller.
> > (bot_manip): Use force_target_expr instead of
> > build_target_expr_with_type.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp1z/elide6.C: New test.
> > ---
> > gcc/cp/tree.c | 8 +++-----
> > gcc/testsuite/g++.dg/cpp1z/elide6.C | 16 ++++++++++++++++
> > 2 files changed, 19 insertions(+), 5 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp1z/elide6.C
> >
> > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> > index 72f498f4b3b..84b84621d35 100644
> > --- a/gcc/cp/tree.c
> > +++ b/gcc/cp/tree.c
> > @@ -848,12 +848,10 @@ build_target_expr_with_type (tree init, tree type,
> > tsubst_flags_t complain)
> > || init == error_mark_node)
> > return init;
> > else if (CLASS_TYPE_P (type) && type_has_nontrivial_copy_init (type)
> > - && !VOID_TYPE_P (TREE_TYPE (init))
> > && TREE_CODE (init) != COND_EXPR
> > && TREE_CODE (init) != CONSTRUCTOR
> > && TREE_CODE (init) != VA_ARG_EXPR)
> > - /* We need to build up a copy constructor call. A void initializer
> > - means we're being called from bot_manip.
>
> In general, a void initializer for a TARGET_EXPR means that the initialization
> is more complex than initializing the object from the value of the expression.
> The caller would need to handle making that initialization apply to the new
> TARGET_EXPR_SLOT (and bot_manip does). If we change bot_manip to not call this
> function, I think this function should reject void init.
I see, thanks. How does the following look for trunk? Bootstrapped and
regetested on x86_64-pc-linux-gnu.
-- >8 --
Subject: [PATCH] c++: constexpr and copy elision within mem init [PR100368]
In the testcase below, the member initializer b(f()) inside C's default
constructor is encoded as a TARGET_EXPR wrapping the CALL_EXPR f() in
C++17 mode. During massaging of this constexpr constructor,
build_target_expr_with_type called from bot_manip tries to add an extra
copy using B's implicitly deleted copy constructor rather than just
preserving the copy elision.
Since it's wrong to introduce an extra copy when initializing a
temporary from a CALL_EXPR, this patch makes build_target_expr_with_type
avoid calling force_rvalue in this case. Additionally, bot_manip should
be copying TARGET_EXPRs in a more oblivious manner, so this patch makes
bot_manip use force_target_expr instead of build_target_expr_with_type.
And since bot_manip is now no longer a caller, we can remove the void
initializer handling in build_target_expr_with_type and instead reject
such initializers.
PR c++/100368
gcc/cp/ChangeLog:
* tree.c (build_target_expr_with_type): Don't call force_rvalue
on CALL_EXPR initializer. Simplify now that bot_manip is no
longer a caller.
(bot_manip): Use force_target_expr instead of
build_target_expr_with_type.
gcc/testsuite/ChangeLog:
* g++.dg/cpp1z/elide6.C: New test.
---
gcc/cp/tree.c | 12 ++++++------
gcc/testsuite/g++.dg/cpp1z/elide6.C | 16 ++++++++++++++++
2 files changed, 22 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1z/elide6.C
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 72f498f4b3b..d97b220423d 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -843,17 +843,17 @@ tree
build_target_expr_with_type (tree init, tree type, tsubst_flags_t complain)
{
gcc_assert (!VOID_TYPE_P (type));
+ gcc_assert (!VOID_TYPE_P (TREE_TYPE (init)));
if (TREE_CODE (init) == TARGET_EXPR
|| init == error_mark_node)
return init;
else if (CLASS_TYPE_P (type) && type_has_nontrivial_copy_init (type)
- && !VOID_TYPE_P (TREE_TYPE (init))
&& TREE_CODE (init) != COND_EXPR
&& TREE_CODE (init) != CONSTRUCTOR
- && TREE_CODE (init) != VA_ARG_EXPR)
- /* We need to build up a copy constructor call. A void initializer
- means we're being called from bot_manip. COND_EXPR is a special
+ && TREE_CODE (init) != VA_ARG_EXPR
+ && TREE_CODE (init) != CALL_EXPR)
+ /* We need to build up a copy constructor call. COND_EXPR is a special
case because we already have copies on the arms and we don't want
another one here. A CONSTRUCTOR is aggregate initialization, which
is handled separately. A VA_ARG_EXPR is magic creation of an
@@ -3112,8 +3112,8 @@ bot_manip (tree* tp, int* walk_subtrees, void* data_)
AGGR_INIT_ZERO_FIRST (TREE_OPERAND (u, 1)) = true;
}
else
- u = build_target_expr_with_type (TREE_OPERAND (t, 1), TREE_TYPE (t),
- tf_warning_or_error);
+ u = force_target_expr (TREE_TYPE (t), TREE_OPERAND (t, 1),
+ tf_warning_or_error);
TARGET_EXPR_IMPLICIT_P (u) = TARGET_EXPR_IMPLICIT_P (t);
TARGET_EXPR_LIST_INIT_P (u) = TARGET_EXPR_LIST_INIT_P (t);
diff --git a/gcc/testsuite/g++.dg/cpp1z/elide6.C
b/gcc/testsuite/g++.dg/cpp1z/elide6.C
new file mode 100644
index 00000000000..399e1a9a1ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/elide6.C
@@ -0,0 +1,16 @@
+// PR c++/100368
+// { dg-do compile { target c++11 } }
+
+struct A {
+ A() = default;
+ A(const A&) = delete;
+};
+
+struct B { A a; }; // { dg-error "deleted" "" { target c++14_down } }
+
+constexpr B f() { return {}; }
+
+struct C {
+ constexpr C() : b(f()) {} // { dg-error "deleted" "" { target c++14_down } }
+ B b;
+};
--
2.32.0.rc0
