| Message ID | ZkNrr2vDreGpelgu@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] c++: ICE with reference NSDMI [PR114854] | expand |
On 5/14/24 09:48, Marek Polacek wrote: > 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? OK. > -- >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
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({}); }