On Thu, May 09, 2024 at 03:47:54PM -0400, Jason Merrill wrote:
> On 5/9/24 12:04, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > Here we crash on a cp_gimplify_expr/TARGET_EXPR assert:
> > 
> >        /* A TARGET_EXPR that expresses direct-initialization should have 
> > been
> >           elided by cp_gimplify_init_expr.  */
> >        gcc_checking_assert (!TARGET_EXPR_DIRECT_INIT_P (*expr_p));
> > 
> > the TARGET_EXPR in question is created for the NSDMI in:
> > 
> >    class Vector { int m_size; };
> >    struct S {
> >      const Vector &vec{};
> >    };
> > 
> > where we first need to create a Vector{} temporary, and then bind the
> > vec reference to it.  The temporary is represented by a TARGET_EXPR
> > and it cannot be elided.  When we create an object of type S, we get
> > 
> >    D.2848 = {.vec=(const struct Vector &) &TARGET_EXPR <D.2840, 
> > {.m_size=0}>}
> > 
> > where the TARGET_EXPR is no longer direct-initializing anything.
> 
> Seems like the problem is in convert_like_internal:
> 
> >             bool direct = CONSTRUCTOR_IS_DIRECT_INIT (expr);
> >             if (abstract_virtuals_error (NULL_TREE, totype, complain))
> >               return error_mark_node;
> >             expr = build_value_init (totype, complain);
> >             expr = get_target_expr (expr, complain);
> >             if (expr != error_mark_node)
> >               {
> >                 TARGET_EXPR_LIST_INIT_P (expr) = true;
> > =>              TARGET_EXPR_DIRECT_INIT_P (expr) = direct;
> >               }
> 
> My patch for 50930 assumed that if a CONSTRUCTOR represents syntactic
> direct-initialization, a resulting TARGET_EXPR is itself the direct
> initializer, but that isn't the case here; the temporary is
> copy-initialized.
> 
> We could calculate direct-initializanity from cand->flags, but perhaps we
> can just stop trying to set TARGET_EXPR_DIRECT_INIT_P here at all? We don't
> do that for other list-initialization in ck_user, I don't know why I thought
> it was needed for {} specifically.  It doesn't seem to be needed for the
> 50930 testcase.

...and it doesn't seem to be needed for any other test.  Then not setting
the flag in the first place is better than resetting it later, sure.

I thought about leaving a gcc_checking_assert (!TARGET_EXPR_DIRECT_INIT_P)
in cp_build_addr_expr_1, but since we already have that assert when
gimplifying, I dropped it.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/branches?

-- >8 --
Here we crash on a cp_gimplify_expr/TARGET_EXPR assert:

      /* A TARGET_EXPR that expresses direct-initialization should have been
         elided by cp_gimplify_init_expr.  */
      gcc_checking_assert (!TARGET_EXPR_DIRECT_INIT_P (*expr_p));

the TARGET_EXPR in question is created for the NSDMI in:

  class Vector { int m_size; };
  struct S {
    const Vector &vec{};
  };

where we first need to create a Vector{} temporary, and then bind the
vec reference to it.  The temporary is represented by a TARGET_EXPR
and it cannot be elided.  When we create an object of type S, we get

  D.2848 = {.vec=(const struct Vector &) &TARGET_EXPR <D.2840, {.m_size=0}>}

where the TARGET_EXPR is no longer direct-initializing anything.

Fixed by not setting TARGET_EXPR_DIRECT_INIT_P in convert_like_internal/ck_user.

        PR c++/114854

gcc/cp/ChangeLog:

        * call.cc (convert_like_internal) <case ck_user>: Don't set
        TARGET_EXPR_DIRECT_INIT_P.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp1y/nsdmi-aggr22.C: New test.
---
 gcc/cp/call.cc                            |  6 +-----
 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr22.C | 12 ++++++++++++
 2 files changed, 13 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr22.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index e058da7735f..ed68eb3c568 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -8597,16 +8597,12 @@ convert_like_internal (conversion *convs, tree expr, 
tree fn, int argnum,
            && TYPE_HAS_DEFAULT_CONSTRUCTOR (totype)
            && !processing_template_decl)
          {
-           bool direct = CONSTRUCTOR_IS_DIRECT_INIT (expr);
            if (abstract_virtuals_error (NULL_TREE, totype, complain))
              return error_mark_node;
            expr = build_value_init (totype, complain);
            expr = get_target_expr (expr, complain);
            if (expr != error_mark_node)
-             {
-               TARGET_EXPR_LIST_INIT_P (expr) = true;
-               TARGET_EXPR_DIRECT_INIT_P (expr) = direct;
-             }
+             TARGET_EXPR_LIST_INIT_P (expr) = true;
            return expr;
          }
 
diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr22.C 
b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr22.C
new file mode 100644
index 00000000000..a4f9ae19ca9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr22.C
@@ -0,0 +1,12 @@
+// PR c++/114854
+// { dg-do compile { target c++14 } }
+
+struct Vector {
+  int m_size;
+};
+struct S {
+  const Vector &vec{};
+};
+
+void spawn(S);
+void test() { spawn({}); }

base-commit: 0a99ad5c52caa06c113b1889bbe6634812b89be5
-- 
2.45.0

Reply via email to