On Tue, Mar 29, 2022 at 10:19:47PM -0400, Jason Merrill wrote:
> On 3/25/22 18:16, Marek Polacek wrote:
> > The attached 93280 test no longer ICEs but looks like it was never added to
> > the
> > testsuite. The 104583 test, modified so that it closely resembles 93280,
> > still
> > ICEs.
> >
> > The problem is that in 104583 we have a value-init from {} (the line A
> > a{};),
> > so this code in convert_like_internal
> >
> > 7960 /* If we're initializing from {}, it's value-initialization.
> > */
> > 7961 if (BRACE_ENCLOSED_INITIALIZER_P (expr)
> > 7962 && CONSTRUCTOR_NELTS (expr) == 0
> > 7963 && TYPE_HAS_DEFAULT_CONSTRUCTOR (totype)
> > 7964 && !processing_template_decl)
> > 7965 {
> > 7966 bool direct = CONSTRUCTOR_IS_DIRECT_INIT (expr);
> > ...
> > 7974 TARGET_EXPR_DIRECT_INIT_P (expr) = direct;
> >
> > sets TARGET_EXPR_DIRECT_INIT_P. This does not happen in 93280 where we
> > initialize from {0}.
> >
> > In 104583, when gimplifying, the d = {}; line, we have
> >
> > d = {.a=TARGET_EXPR <D.2474, <<< Unknown tree: aggr_init_expr
> > 4
> > __ct_comp
> > D.2474
> > (struct A *) <<< Unknown tree: void_cst >>> >>>>}
> >
> > where the TARGET_EXPR is the one with TARGET_EXPR_DIRECT_INIT_P set. In
> > gimplify_init_ctor_preeval we do
> >
> > 4724 FOR_EACH_VEC_SAFE_ELT (v, ix, ce)
> > 4725 gimplify_init_ctor_preeval (&ce->value, pre_p, post_p, data);
> >
> > so we gimplify the TARGET_EXPR, crashing at
> >
> > 744 case TARGET_EXPR:
> > 745 /* A TARGET_EXPR that expresses direct-initialization should
> > have
> > been
> > 746 elided by cp_gimplify_init_expr. */
> > 747 gcc_checking_assert (!TARGET_EXPR_DIRECT_INIT_P (*expr_p));
> >
> > but there is no INIT_EXPR so cp_gimplify_init_expr was never called!
> >
> > Now, the fix for c++/93280
> > <https://gcc.gnu.org/pipermail/gcc-patches/2020-January/538414.html>
> > says "let's only set TARGET_EXPR_DIRECT_INIT_P when we're using the DMI in
> > a constructor." and the comment talks about the full initialization. Is
> > is accurate to say that our TARGET_EXPR does not represent the full
> > initialization, because it only initializes the 'a' subobject? If so,
> > then maybe get_nsdmi should clear TARGET_EXPR_DIRECT_INIT_P when in_ctor
> > is false.
> >
> > I've compared the 93280.s and 104583.s files, they differ only in one
> > movl $0, so there are no extra calls and similar.
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > PR c++/93280
> > PR c++/104583
> >
> > gcc/cp/ChangeLog:
> >
> > * init.cc (get_nsdmi): Set TARGET_EXPR_DIRECT_INIT_P to in_ctor.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp0x/nsdmi-list7.C: New test.
> > * g++.dg/cpp0x/nsdmi-list8.C: New test.
> > ---
> > gcc/cp/init.cc | 8 ++++----
> > gcc/testsuite/g++.dg/cpp0x/nsdmi-list7.C | 17 +++++++++++++++++
> > gcc/testsuite/g++.dg/cpp0x/nsdmi-list8.C | 17 +++++++++++++++++
> > 3 files changed, 38 insertions(+), 4 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-list7.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-list8.C
> >
> > diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> > index 08767679dd4..fd32a8bd90f 100644
> > --- a/gcc/cp/init.cc
> > +++ b/gcc/cp/init.cc
> > @@ -679,10 +679,10 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t
> > complain)
> > if (simple_target)
> > init = TARGET_EXPR_INITIAL (init);
> > init = break_out_target_exprs (init, /*loc*/true);
> > - if (in_ctor && init && TREE_CODE (init) == TARGET_EXPR)
> > - /* This expresses the full initialization, prevent perform_member_init
> > from
> > - calling another constructor (58162). */
> > - TARGET_EXPR_DIRECT_INIT_P (init) = true;
> > + if (init && TREE_CODE (init) == TARGET_EXPR)
> > + /* If this expresses the full initialization, prevent
> > perform_member_init
>
> Maybe "In a constructor, this expresses..." ? The added "if" suggests a
> test that I don't see in the code. OK with that change.
Ah, the "if" was meant to be about the value of in_ctor. I've changed the
wording and pushed the patch. Thanks!
Marek
