[PATCH] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[PATCH] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Marek Polacek]

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

Re: [PATCH] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Jason Merrill]

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

[PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Marek Polacek]

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

Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Jason Merrill]

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

Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Marek Polacek]

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

Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Marek Polacek]

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

Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Marek Polacek]

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

Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Jason Merrill]

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

Re: [PATCH v2] c++: ICE with VEC_INIT_EXPR and defarg [PR106925]

[Jason Merrill]

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