* [PATCH] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] @ 2022-10-11 20:00 Marek Polacek 2022-10-11 20:28 ` Jason Merrill 0 siblings, 1 reply; 9+ messages in thread From: Marek Polacek @ 2022-10-11 20:00 UTC (permalink / raw) To: GCC Patches, Jason Merrill Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr while processing the default argument in this test. At this point start_preparsed_function hasn't yet set current_function_decl. expand_vec_init_expr then leads to maybe_splice_retval_cleanup which checks DECL_CONSTRUCTOR_P (current_function_decl) without checking that c_f_d is non-null first. It seems correct that c_f_d is null here, so it seems to me that maybe_splice_retval_cleanup should check c_f_d as in the following patch. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12? PR c++/106925 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Check current_function_decl. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/initlist-defarg3.C: New test. --- gcc/cp/except.cc | 3 +++ gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C | 13 +++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc index b8a85ed0572..9f77289b9ca 100644 --- a/gcc/cp/except.cc +++ b/gcc/cp/except.cc @@ -1327,6 +1327,9 @@ maybe_splice_retval_cleanup (tree compound_stmt) && current_binding_level->level_chain->kind == sk_function_parms); if ((function_body || current_binding_level->kind == sk_try) + /* When we're processing a default argument, c_f_d may not have been + set. */ + && current_function_decl && !DECL_CONSTRUCTOR_P (current_function_decl) && !DECL_DESTRUCTOR_P (current_function_decl) && current_retval_sentinel) diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C new file mode 100644 index 00000000000..5c3e886b306 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C @@ -0,0 +1,13 @@ +// PR c++/106925 +// { dg-do compile { target c++11 } } + +struct Foo; +template <int _Nm> struct __array_traits { typedef Foo _Type[_Nm]; }; +template <int _Nm> struct array { + typename __array_traits<_Nm>::_Type _M_elems; +}; +template <int size> struct MyVector { array<size> data{}; }; +struct Foo { + float a{0}; +}; +void foo(MyVector<1> = MyVector<1>()); base-commit: 23c3cbaed36f6d2f3a7a64f6ebda69329723514b -- 2.37.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-11 20:00 [PATCH] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] Marek Polacek @ 2022-10-11 20:28 ` Jason Merrill 2022-10-12 16:27 ` [PATCH v2] " Marek Polacek 0 siblings, 1 reply; 9+ messages in thread From: Jason Merrill @ 2022-10-11 20:28 UTC (permalink / raw) To: Marek Polacek, GCC Patches On 10/11/22 16:00, Marek Polacek wrote: > Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr > while processing the default argument in this test. Hmm, why are we calling cxx_eval_vec_init during parsing of the default argument? In particular, any expansion that depends on the enclosing function context should be deferred until the default arg is used by a call. But it's certainly true that the "function_body" test is wrong in this situation; you might move the c_f_d test into the calculation of that variable. The patch is OK with that change, but please also answer my question above. > At this point > start_preparsed_function hasn't yet set current_function_decl. > expand_vec_init_expr then leads to maybe_splice_retval_cleanup which > checks DECL_CONSTRUCTOR_P (current_function_decl) without checking that > c_f_d is non-null first. It seems correct that c_f_d is null here, so > it seems to me that maybe_splice_retval_cleanup should check c_f_d as > in the following patch. > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12? > > PR c++/106925 > > gcc/cp/ChangeLog: > > * except.cc (maybe_splice_retval_cleanup): Check current_function_decl. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/initlist-defarg3.C: New test. > --- > gcc/cp/except.cc | 3 +++ > gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C | 13 +++++++++++++ > 2 files changed, 16 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C > > diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc > index b8a85ed0572..9f77289b9ca 100644 > --- a/gcc/cp/except.cc > +++ b/gcc/cp/except.cc > @@ -1327,6 +1327,9 @@ maybe_splice_retval_cleanup (tree compound_stmt) > && current_binding_level->level_chain->kind == sk_function_parms); > > if ((function_body || current_binding_level->kind == sk_try) > + /* When we're processing a default argument, c_f_d may not have been > + set. */ > + && current_function_decl > && !DECL_CONSTRUCTOR_P (current_function_decl) > && !DECL_DESTRUCTOR_P (current_function_decl) > && current_retval_sentinel) > diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C > new file mode 100644 > index 00000000000..5c3e886b306 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C > @@ -0,0 +1,13 @@ > +// PR c++/106925 > +// { dg-do compile { target c++11 } } > + > +struct Foo; > +template <int _Nm> struct __array_traits { typedef Foo _Type[_Nm]; }; > +template <int _Nm> struct array { > + typename __array_traits<_Nm>::_Type _M_elems; > +}; > +template <int size> struct MyVector { array<size> data{}; }; > +struct Foo { > + float a{0}; > +}; > +void foo(MyVector<1> = MyVector<1>()); > > base-commit: 23c3cbaed36f6d2f3a7a64f6ebda69329723514b ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-11 20:28 ` Jason Merrill @ 2022-10-12 16:27 ` Marek Polacek 2022-10-12 16:47 ` Jason Merrill 0 siblings, 1 reply; 9+ messages in thread From: Marek Polacek @ 2022-10-12 16:27 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Tue, Oct 11, 2022 at 04:28:11PM -0400, Jason Merrill wrote: > On 10/11/22 16:00, Marek Polacek wrote: > > Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr > > while processing the default argument in this test. > > Hmm, why are we calling cxx_eval_vec_init during parsing of the default > argument? In particular, any expansion that depends on the enclosing > function context should be deferred until the default arg is used by a call. I think this is part of the semantic constraints checking [dcl.fct.default]/5 talks about, as in, this doesn't compile even though the default argument is not executed: struct S { S() = delete; }; void foo (S = S()) { } In the test below we parse '= MyVector<1>()' and end up calling mark_used on the implicit "constexpr MyVector<1>::MyVector() noexcept (<uninstantiated>)" ctor. mark_used calls maybe_instantiate_noexcept. Since the ctor has a DEFERRED_NOEXCEPT, we have to figure out if the ctor should be noexcept or not using get_defaulted_eh_spec. That means walking the members of MyVector. Thus we reach /* Core 1351: If the field has an NSDMI that could throw, the default constructor is noexcept(false). */ and call get_nsdmi on 'data'. There we digest its initializer which is {}. massage_init_elt calls digest_init_r on the {} and produces TARGET_EXPR <D.2518, <<< Unknown tree: vec_init_expr D.2518 {} >>>> and the subsequent fold_non_dependent_init leads to cxx_eval_vec_init -> expand_vec_init_expr. I think this is all correct except that the fold_non_dependent_init is somewhat questionable to me; do we really have to fold in order to say if the NSDMI init can throw? Sure, we need to digest the {}, maybe the field's ctors can throw, but I don't know about the folding. > But it's certainly true that the "function_body" test is wrong in this > situation; you might move the c_f_d test into the calculation of that > variable. The patch is OK with that change, but please also answer my > question above. I like that. Before I go ahead and apply, please let me know if the answer above is satisfying. -- >8 -- Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr while processing the default argument in this test. At this point start_preparsed_function hasn't yet set current_function_decl. expand_vec_init_expr then leads to maybe_splice_retval_cleanup which checks DECL_CONSTRUCTOR_P (current_function_decl) without checking that c_f_d is non-null first. It seems correct that c_f_d is null here, so it seems to me that maybe_splice_retval_cleanup should check c_f_d as in the following patch. PR c++/106925 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Check current_function_decl. Make the bool const. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/initlist-defarg3.C: New test. --- gcc/cp/except.cc | 7 +++++-- gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc index b8a85ed0572..a9114a5f7a5 100644 --- a/gcc/cp/except.cc +++ b/gcc/cp/except.cc @@ -1322,9 +1322,12 @@ maybe_splice_retval_cleanup (tree compound_stmt) { /* If we need a cleanup for the return value, add it in at the same level as pushdecl_outermost_localscope. And also in try blocks. */ - bool function_body + const bool function_body = (current_binding_level->level_chain - && current_binding_level->level_chain->kind == sk_function_parms); + && current_binding_level->level_chain->kind == sk_function_parms + /* When we're processing a default argument, c_f_d may not have been + set. */ + && current_function_decl); if ((function_body || current_binding_level->kind == sk_try) && !DECL_CONSTRUCTOR_P (current_function_decl) diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C new file mode 100644 index 00000000000..5c3e886b306 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C @@ -0,0 +1,13 @@ +// PR c++/106925 +// { dg-do compile { target c++11 } } + +struct Foo; +template <int _Nm> struct __array_traits { typedef Foo _Type[_Nm]; }; +template <int _Nm> struct array { + typename __array_traits<_Nm>::_Type _M_elems; +}; +template <int size> struct MyVector { array<size> data{}; }; +struct Foo { + float a{0}; +}; +void foo(MyVector<1> = MyVector<1>()); base-commit: fbf423309e103b54f7c9d39b2f7870b9bedfe9d2 -- 2.37.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-12 16:27 ` [PATCH v2] " Marek Polacek @ 2022-10-12 16:47 ` Jason Merrill 2022-10-12 17:12 ` Marek Polacek 0 siblings, 1 reply; 9+ messages in thread From: Jason Merrill @ 2022-10-12 16:47 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 10/12/22 12:27, Marek Polacek wrote: > On Tue, Oct 11, 2022 at 04:28:11PM -0400, Jason Merrill wrote: >> On 10/11/22 16:00, Marek Polacek wrote: >>> Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr >>> while processing the default argument in this test. >> >> Hmm, why are we calling cxx_eval_vec_init during parsing of the default >> argument? In particular, any expansion that depends on the enclosing >> function context should be deferred until the default arg is used by a call. > > I think this is part of the semantic constraints checking [dcl.fct.default]/5 > talks about, as in, this doesn't compile even though the default argument is > not executed: > > struct S { > S() = delete; > }; > void foo (S = S()) { } > > In the test below we parse '= MyVector<1>()' and end up calling mark_used > on the implicit "constexpr MyVector<1>::MyVector() noexcept (<uninstantiated>)" > ctor. mark_used calls maybe_instantiate_noexcept. Since the ctor has > a DEFERRED_NOEXCEPT, we have to figure out if the ctor should be noexcept > or not using get_defaulted_eh_spec. That means walking the members of > MyVector. Thus we reach > /* Core 1351: If the field has an NSDMI that could throw, the > default constructor is noexcept(false). */ Maybe we need a cp_unevaluated here? The operand of noexcept should be unevaluated. > and call get_nsdmi on 'data'. There we digest its initializer which is {}. > massage_init_elt calls digest_init_r on the {} and produces > TARGET_EXPR <D.2518, <<< Unknown tree: vec_init_expr > D.2518 > {} >>>> > and the subsequent fold_non_dependent_init leads to cxx_eval_vec_init > -> expand_vec_init_expr. > > I think this is all correct except that the fold_non_dependent_init is > somewhat questionable to me; do we really have to fold in order to say > if the NSDMI init can throw? Sure, we need to digest the {}, maybe > the field's ctors can throw, but I don't know about the folding. And we can check cp_unevaluated_operand to avoid the fold_non_dependent_init? >> But it's certainly true that the "function_body" test is wrong in this >> situation; you might move the c_f_d test into the calculation of that >> variable. The patch is OK with that change, but please also answer my >> question above. > > I like that. Before I go ahead and apply, please let me know if the answer > above is satisfying. > > -- >8 -- > Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr > while processing the default argument in this test. At this point > start_preparsed_function hasn't yet set current_function_decl. > expand_vec_init_expr then leads to maybe_splice_retval_cleanup which > checks DECL_CONSTRUCTOR_P (current_function_decl) without checking that > c_f_d is non-null first. It seems correct that c_f_d is null here, so > it seems to me that maybe_splice_retval_cleanup should check c_f_d as > in the following patch. > > PR c++/106925 > > gcc/cp/ChangeLog: > > * except.cc (maybe_splice_retval_cleanup): Check current_function_decl. > Make the bool const. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/initlist-defarg3.C: New test. > --- > gcc/cp/except.cc | 7 +++++-- > gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C | 13 +++++++++++++ > 2 files changed, 18 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C > > diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc > index b8a85ed0572..a9114a5f7a5 100644 > --- a/gcc/cp/except.cc > +++ b/gcc/cp/except.cc > @@ -1322,9 +1322,12 @@ maybe_splice_retval_cleanup (tree compound_stmt) > { > /* If we need a cleanup for the return value, add it in at the same level as > pushdecl_outermost_localscope. And also in try blocks. */ > - bool function_body > + const bool function_body > = (current_binding_level->level_chain > - && current_binding_level->level_chain->kind == sk_function_parms); > + && current_binding_level->level_chain->kind == sk_function_parms > + /* When we're processing a default argument, c_f_d may not have been > + set. */ > + && current_function_decl); > > if ((function_body || current_binding_level->kind == sk_try) > && !DECL_CONSTRUCTOR_P (current_function_decl) > diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C > new file mode 100644 > index 00000000000..5c3e886b306 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/initlist-defarg3.C > @@ -0,0 +1,13 @@ > +// PR c++/106925 > +// { dg-do compile { target c++11 } } > + > +struct Foo; > +template <int _Nm> struct __array_traits { typedef Foo _Type[_Nm]; }; > +template <int _Nm> struct array { > + typename __array_traits<_Nm>::_Type _M_elems; > +}; > +template <int size> struct MyVector { array<size> data{}; }; > +struct Foo { > + float a{0}; > +}; > +void foo(MyVector<1> = MyVector<1>()); > > base-commit: fbf423309e103b54f7c9d39b2f7870b9bedfe9d2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-12 16:47 ` Jason Merrill @ 2022-10-12 17:12 ` Marek Polacek 2022-10-12 18:23 ` Marek Polacek 0 siblings, 1 reply; 9+ messages in thread From: Marek Polacek @ 2022-10-12 17:12 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Wed, Oct 12, 2022 at 12:47:21PM -0400, Jason Merrill wrote: > On 10/12/22 12:27, Marek Polacek wrote: > > On Tue, Oct 11, 2022 at 04:28:11PM -0400, Jason Merrill wrote: > > > On 10/11/22 16:00, Marek Polacek wrote: > > > > Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr > > > > while processing the default argument in this test. > > > > > > Hmm, why are we calling cxx_eval_vec_init during parsing of the default > > > argument? In particular, any expansion that depends on the enclosing > > > function context should be deferred until the default arg is used by a call. > > > > I think this is part of the semantic constraints checking [dcl.fct.default]/5 > > talks about, as in, this doesn't compile even though the default argument is > > not executed: > > > > struct S { > > S() = delete; > > }; > > void foo (S = S()) { } > > In the test below we parse '= MyVector<1>()' and end up calling mark_used > > on the implicit "constexpr MyVector<1>::MyVector() noexcept (<uninstantiated>)" > > ctor. mark_used calls maybe_instantiate_noexcept. Since the ctor has > > a DEFERRED_NOEXCEPT, we have to figure out if the ctor should be noexcept > > or not using get_defaulted_eh_spec. That means walking the members of > > MyVector. Thus we reach > > /* Core 1351: If the field has an NSDMI that could throw, the > > default constructor is noexcept(false). */ > > Maybe we need a cp_unevaluated here? The operand of noexcept should be > unevaluated. That wouldn't help since get_nsdmi specifically does "cp_evaluated ev;", so... > > and call get_nsdmi on 'data'. There we digest its initializer which is {}. > > massage_init_elt calls digest_init_r on the {} and produces > > TARGET_EXPR <D.2518, <<< Unknown tree: vec_init_expr > > D.2518 > > {} >>>> > > and the subsequent fold_non_dependent_init leads to cxx_eval_vec_init > > -> expand_vec_init_expr. > > > > I think this is all correct except that the fold_non_dependent_init is > > somewhat questionable to me; do we really have to fold in order to say > > if the NSDMI init can throw? Sure, we need to digest the {}, maybe > > the field's ctors can throw, but I don't know about the folding. > > And we can check cp_unevaluated_operand to avoid the > fold_non_dependent_init? ...we'd still fold. I'm not sure if we want a LOOKUP_ flag that says "we're just checking if we can throw, don't fold". Marek ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-12 17:12 ` Marek Polacek @ 2022-10-12 18:23 ` Marek Polacek 2022-10-13 13:58 ` Marek Polacek 2022-10-13 14:44 ` Jason Merrill 0 siblings, 2 replies; 9+ messages in thread From: Marek Polacek @ 2022-10-12 18:23 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Wed, Oct 12, 2022 at 01:12:57PM -0400, Marek Polacek wrote: > On Wed, Oct 12, 2022 at 12:47:21PM -0400, Jason Merrill wrote: > > On 10/12/22 12:27, Marek Polacek wrote: > > > On Tue, Oct 11, 2022 at 04:28:11PM -0400, Jason Merrill wrote: > > > > On 10/11/22 16:00, Marek Polacek wrote: > > > > > Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr > > > > > while processing the default argument in this test. > > > > > > > > Hmm, why are we calling cxx_eval_vec_init during parsing of the default > > > > argument? In particular, any expansion that depends on the enclosing > > > > function context should be deferred until the default arg is used by a call. > > > > > > I think this is part of the semantic constraints checking [dcl.fct.default]/5 > > > talks about, as in, this doesn't compile even though the default argument is > > > not executed: > > > > > > struct S { > > > S() = delete; > > > }; > > > void foo (S = S()) { } > > > In the test below we parse '= MyVector<1>()' and end up calling mark_used > > > on the implicit "constexpr MyVector<1>::MyVector() noexcept (<uninstantiated>)" > > > ctor. mark_used calls maybe_instantiate_noexcept. Since the ctor has > > > a DEFERRED_NOEXCEPT, we have to figure out if the ctor should be noexcept > > > or not using get_defaulted_eh_spec. That means walking the members of > > > MyVector. Thus we reach > > > /* Core 1351: If the field has an NSDMI that could throw, the > > > default constructor is noexcept(false). */ > > > > Maybe we need a cp_unevaluated here? The operand of noexcept should be > > unevaluated. > > That wouldn't help since get_nsdmi specifically does "cp_evaluated ev;", > so... > > > > and call get_nsdmi on 'data'. There we digest its initializer which is {}. > > > massage_init_elt calls digest_init_r on the {} and produces > > > TARGET_EXPR <D.2518, <<< Unknown tree: vec_init_expr > > > D.2518 > > > {} >>>> > > > and the subsequent fold_non_dependent_init leads to cxx_eval_vec_init > > > -> expand_vec_init_expr. > > > > > > I think this is all correct except that the fold_non_dependent_init is > > > somewhat questionable to me; do we really have to fold in order to say > > > if the NSDMI init can throw? Sure, we need to digest the {}, maybe > > > the field's ctors can throw, but I don't know about the folding. > > > > And we can check cp_unevaluated_operand to avoid the > > fold_non_dependent_init? > > ...we'd still fold. I'm not sure if we want a LOOKUP_ flag that says > "we're just checking if we can throw, don't fold". Eh, a new flag is overkill. Maybe don't do cp_evaluated in get_nsdmi if we're called from walk_field_subobs would be worth a try? Marek ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-12 18:23 ` Marek Polacek @ 2022-10-13 13:58 ` Marek Polacek 2022-10-13 14:45 ` Jason Merrill 2022-10-13 14:44 ` Jason Merrill 1 sibling, 1 reply; 9+ messages in thread From: Marek Polacek @ 2022-10-13 13:58 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Wed, Oct 12, 2022 at 02:23:40PM -0400, Marek Polacek wrote: > On Wed, Oct 12, 2022 at 01:12:57PM -0400, Marek Polacek wrote: > > On Wed, Oct 12, 2022 at 12:47:21PM -0400, Jason Merrill wrote: > > > On 10/12/22 12:27, Marek Polacek wrote: > > > > On Tue, Oct 11, 2022 at 04:28:11PM -0400, Jason Merrill wrote: > > > > > On 10/11/22 16:00, Marek Polacek wrote: > > > > > > Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr > > > > > > while processing the default argument in this test. > > > > > > > > > > Hmm, why are we calling cxx_eval_vec_init during parsing of the default > > > > > argument? In particular, any expansion that depends on the enclosing > > > > > function context should be deferred until the default arg is used by a call. > > > > > > > > I think this is part of the semantic constraints checking [dcl.fct.default]/5 > > > > talks about, as in, this doesn't compile even though the default argument is > > > > not executed: > > > > > > > > struct S { > > > > S() = delete; > > > > }; > > > > void foo (S = S()) { } > > > > In the test below we parse '= MyVector<1>()' and end up calling mark_used > > > > on the implicit "constexpr MyVector<1>::MyVector() noexcept (<uninstantiated>)" > > > > ctor. mark_used calls maybe_instantiate_noexcept. Since the ctor has > > > > a DEFERRED_NOEXCEPT, we have to figure out if the ctor should be noexcept > > > > or not using get_defaulted_eh_spec. That means walking the members of > > > > MyVector. Thus we reach > > > > /* Core 1351: If the field has an NSDMI that could throw, the > > > > default constructor is noexcept(false). */ > > > > > > Maybe we need a cp_unevaluated here? The operand of noexcept should be > > > unevaluated. > > > > That wouldn't help since get_nsdmi specifically does "cp_evaluated ev;", > > so... > > > > > > and call get_nsdmi on 'data'. There we digest its initializer which is {}. > > > > massage_init_elt calls digest_init_r on the {} and produces > > > > TARGET_EXPR <D.2518, <<< Unknown tree: vec_init_expr > > > > D.2518 > > > > {} >>>> > > > > and the subsequent fold_non_dependent_init leads to cxx_eval_vec_init > > > > -> expand_vec_init_expr. > > > > > > > > I think this is all correct except that the fold_non_dependent_init is > > > > somewhat questionable to me; do we really have to fold in order to say > > > > if the NSDMI init can throw? Sure, we need to digest the {}, maybe > > > > the field's ctors can throw, but I don't know about the folding. > > > > > > And we can check cp_unevaluated_operand to avoid the > > > fold_non_dependent_init? > > > > ...we'd still fold. I'm not sure if we want a LOOKUP_ flag that says > > "we're just checking if we can throw, don't fold". > > Eh, a new flag is overkill. Maybe don't do cp_evaluated in get_nsdmi if > we're called from walk_field_subobs would be worth a try? FWIW, my experiments with cp_unevaluated_operand failed because then we'd miss warnings as in g++.dg/ext/cond5.C which warns from the get_defaulted_eh_spec context -- so I'd have no way to distinguish that from the test in this PR. Should we just go back to my patch? Marek ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-13 13:58 ` Marek Polacek @ 2022-10-13 14:45 ` Jason Merrill 0 siblings, 0 replies; 9+ messages in thread From: Jason Merrill @ 2022-10-13 14:45 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 10/13/22 09:58, Marek Polacek wrote: > On Wed, Oct 12, 2022 at 02:23:40PM -0400, Marek Polacek wrote: >> On Wed, Oct 12, 2022 at 01:12:57PM -0400, Marek Polacek wrote: >>> On Wed, Oct 12, 2022 at 12:47:21PM -0400, Jason Merrill wrote: >>>> On 10/12/22 12:27, Marek Polacek wrote: >>>>> On Tue, Oct 11, 2022 at 04:28:11PM -0400, Jason Merrill wrote: >>>>>> On 10/11/22 16:00, Marek Polacek wrote: >>>>>>> Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr >>>>>>> while processing the default argument in this test. >>>>>> >>>>>> Hmm, why are we calling cxx_eval_vec_init during parsing of the default >>>>>> argument? In particular, any expansion that depends on the enclosing >>>>>> function context should be deferred until the default arg is used by a call. >>>>> >>>>> I think this is part of the semantic constraints checking [dcl.fct.default]/5 >>>>> talks about, as in, this doesn't compile even though the default argument is >>>>> not executed: >>>>> >>>>> struct S { >>>>> S() = delete; >>>>> }; >>>>> void foo (S = S()) { } >>>>> In the test below we parse '= MyVector<1>()' and end up calling mark_used >>>>> on the implicit "constexpr MyVector<1>::MyVector() noexcept (<uninstantiated>)" >>>>> ctor. mark_used calls maybe_instantiate_noexcept. Since the ctor has >>>>> a DEFERRED_NOEXCEPT, we have to figure out if the ctor should be noexcept >>>>> or not using get_defaulted_eh_spec. That means walking the members of >>>>> MyVector. Thus we reach >>>>> /* Core 1351: If the field has an NSDMI that could throw, the >>>>> default constructor is noexcept(false). */ >>>> >>>> Maybe we need a cp_unevaluated here? The operand of noexcept should be >>>> unevaluated. >>> >>> That wouldn't help since get_nsdmi specifically does "cp_evaluated ev;", >>> so... >>> >>>>> and call get_nsdmi on 'data'. There we digest its initializer which is {}. >>>>> massage_init_elt calls digest_init_r on the {} and produces >>>>> TARGET_EXPR <D.2518, <<< Unknown tree: vec_init_expr >>>>> D.2518 >>>>> {} >>>> >>>>> and the subsequent fold_non_dependent_init leads to cxx_eval_vec_init >>>>> -> expand_vec_init_expr. >>>>> >>>>> I think this is all correct except that the fold_non_dependent_init is >>>>> somewhat questionable to me; do we really have to fold in order to say >>>>> if the NSDMI init can throw? Sure, we need to digest the {}, maybe >>>>> the field's ctors can throw, but I don't know about the folding. >>>> >>>> And we can check cp_unevaluated_operand to avoid the >>>> fold_non_dependent_init? >>> >>> ...we'd still fold. I'm not sure if we want a LOOKUP_ flag that says >>> "we're just checking if we can throw, don't fold". >> >> Eh, a new flag is overkill. Maybe don't do cp_evaluated in get_nsdmi if >> we're called from walk_field_subobs would be worth a try? > > FWIW, my experiments with cp_unevaluated_operand failed because then we'd > miss warnings as in g++.dg/ext/cond5.C which warns from the > get_defaulted_eh_spec context -- so I'd have no way to distinguish that > from the test in this PR. Should we just go back to my patch? Your patch is still OK. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] 2022-10-12 18:23 ` Marek Polacek 2022-10-13 13:58 ` Marek Polacek @ 2022-10-13 14:44 ` Jason Merrill 1 sibling, 0 replies; 9+ messages in thread From: Jason Merrill @ 2022-10-13 14:44 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 10/12/22 14:23, Marek Polacek wrote: > On Wed, Oct 12, 2022 at 01:12:57PM -0400, Marek Polacek wrote: >> On Wed, Oct 12, 2022 at 12:47:21PM -0400, Jason Merrill wrote: >>> On 10/12/22 12:27, Marek Polacek wrote: >>>> On Tue, Oct 11, 2022 at 04:28:11PM -0400, Jason Merrill wrote: >>>>> On 10/11/22 16:00, Marek Polacek wrote: >>>>>> Since r12-8066, in cxx_eval_vec_init we perform expand_vec_init_expr >>>>>> while processing the default argument in this test. >>>>> >>>>> Hmm, why are we calling cxx_eval_vec_init during parsing of the default >>>>> argument? In particular, any expansion that depends on the enclosing >>>>> function context should be deferred until the default arg is used by a call. >>>> >>>> I think this is part of the semantic constraints checking [dcl.fct.default]/5 >>>> talks about, as in, this doesn't compile even though the default argument is >>>> not executed: >>>> >>>> struct S { >>>> S() = delete; >>>> }; >>>> void foo (S = S()) { } >>>> In the test below we parse '= MyVector<1>()' and end up calling mark_used >>>> on the implicit "constexpr MyVector<1>::MyVector() noexcept (<uninstantiated>)" >>>> ctor. mark_used calls maybe_instantiate_noexcept. Since the ctor has >>>> a DEFERRED_NOEXCEPT, we have to figure out if the ctor should be noexcept >>>> or not using get_defaulted_eh_spec. That means walking the members of >>>> MyVector. Thus we reach >>>> /* Core 1351: If the field has an NSDMI that could throw, the >>>> default constructor is noexcept(false). */ >>> >>> Maybe we need a cp_unevaluated here? The operand of noexcept should be >>> unevaluated. >> >> That wouldn't help since get_nsdmi specifically does "cp_evaluated ev;", >> so... >> >>>> and call get_nsdmi on 'data'. There we digest its initializer which is {}. >>>> massage_init_elt calls digest_init_r on the {} and produces >>>> TARGET_EXPR <D.2518, <<< Unknown tree: vec_init_expr >>>> D.2518 >>>> {} >>>> >>>> and the subsequent fold_non_dependent_init leads to cxx_eval_vec_init >>>> -> expand_vec_init_expr. >>>> >>>> I think this is all correct except that the fold_non_dependent_init is >>>> somewhat questionable to me; do we really have to fold in order to say >>>> if the NSDMI init can throw? Sure, we need to digest the {}, maybe >>>> the field's ctors can throw, but I don't know about the folding. >>> >>> And we can check cp_unevaluated_operand to avoid the >>> fold_non_dependent_init? >> >> ...we'd still fold. I'm not sure if we want a LOOKUP_ flag that says >> "we're just checking if we can throw, don't fold". > > Eh, a new flag is overkill. Maybe don't do cp_evaluated in get_nsdmi if > we're called from walk_field_subobs would be worth a try? It seems that we treat DMI instantiations as evaluated even if they're triggered from unevaluated context so sharing lambdas between different uses of the DMI works properly. I don't think this is worth messing with at this point; thanks for satisfying my curiosity. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-13 14:45 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-11 20:00 [PATCH] c++: ICE with VEC_INIT_EXPR and defarg [PR106925] Marek Polacek 2022-10-11 20:28 ` Jason Merrill 2022-10-12 16:27 ` [PATCH v2] " Marek Polacek 2022-10-12 16:47 ` Jason Merrill 2022-10-12 17:12 ` Marek Polacek 2022-10-12 18:23 ` Marek Polacek 2022-10-13 13:58 ` Marek Polacek 2022-10-13 14:45 ` Jason Merrill 2022-10-13 14:44 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).