* [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-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
* 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
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).