[PATCH] c++, coroutines: Avoid expanding within templates [PR103868]

[Jason Merrill]

On 4/18/22 15:49, Eric Gallager via Gcc-patches wrote:
> On Mon, Apr 18, 2022 at 10:01 AM Iain Sandoe via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> From: Nathan Sidwell <nathan@acm.org>
>>
>> This is a forward-port of a patch by Nathan (against 10.x) which fixes an open
>> PR.
>>
>> We are ICEing because we ended up tsubst_copying something that had already
>> been tsubst, leading to an assert failure (mostly such repeated tsubsting is
>> harmless).

I wouldn't say "mostly".  It should always be avoided, it frequently 
causes problems.  Pretty much any time there's a class prvalue.

>> We had a non-dependent co_await in a non-dependent-type template fn, so we
>> processed it at definition time, and then reprocessed at instantiation time.
>> We fix this here by deferring substitution while processing templates.
>>
>> Additional observations (for a better future fix, in the GCC13 timescale):
>>
>> Exprs only have dependent type if at least one operand is dependent which was
>> what the current code was intending to do.  Coroutines have the additional
>> wrinkle, that the current fn's type is an implicit operand.
>>
>> So, if the coroutine function's type is not dependent, and the operand is not
>> dependent, we should determine the type of the co_await expression using the
>> DEPENDENT_EXPR wrapper machinery.  That allows us to determine the
>> subexpression type, but leave its operand unchanged and then instantiate it
>> later.

Sure, like what build_x_binary_op and the like do.

>> Tested on x86_64-darwin (it does also fix the original testcase, but that is
>> far too time-consuming for the testsuite).

The compiler change seems fine as a temporary workaround.  Is it not 
feasible to write a new short testcase that reproduces the problem, now 
that you understand it?

>> OK for master? / backports? (when?)
>> thanks,
>> Iain
>>
>>          PR c++/103868
>>
>> gcc/cp/ChangeLog:
>>
>>          * coroutines.cc (finish_co_await_expr): Do not process non-dependent
>>          coroutine expressions at template definition time.
>>          (finish_co_yield_expr): Likewise.
>>          (finish_co_return_stmt): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>>          * g++.dg/coroutines/pr103868.C: New test.
>>
>> Co-Authored-by: Iain Sandoe <iain@sandoe.co.uk>
>> ---
>>   gcc/cp/coroutines.cc                       |   18 +-
>>   gcc/testsuite/g++.dg/coroutines/pr103868.C | 7390 ++++++++++++++++++++
>>   2 files changed, 7396 insertions(+), 12 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr103868.C
>>
>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>> index cdf6503c4d3..a9ce6e050dd 100644
>> --- a/gcc/cp/coroutines.cc
>> +++ b/gcc/cp/coroutines.cc
>> @@ -1148,10 +1148,8 @@ finish_co_await_expr (location_t kw, tree expr)
>>        extraneous warnings during substitution.  */
>>     suppress_warning (current_function_decl, OPT_Wreturn_type);
>>
>> -  /* If we don't know the promise type, we can't proceed, build the
>> -     co_await with the expression unchanged.  */
>> -  tree functype = TREE_TYPE (current_function_decl);
>> -  if (dependent_type_p (functype) || type_dependent_expression_p (expr))
>> +  /* Defer processing when we have dependent types.  */
>> +  if (processing_template_decl)
>>       {
>>         tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, unknown_type_node, expr,
>>                                   NULL_TREE, NULL_TREE, NULL_TREE,
>> @@ -1222,10 +1220,8 @@ finish_co_yield_expr (location_t kw, tree expr)
>>        extraneous warnings during substitution.  */
>>     suppress_warning (current_function_decl, OPT_Wreturn_type);
>>
>> -  /* If we don't know the promise type, we can't proceed, build the
>> -     co_await with the expression unchanged.  */
>> -  tree functype = TREE_TYPE (current_function_decl);
>> -  if (dependent_type_p (functype) || type_dependent_expression_p (expr))
>> +  /* Defer processing when we have dependent types.  */
>> +  if (processing_template_decl)
>>       return build2_loc (kw, CO_YIELD_EXPR, unknown_type_node, expr, NULL_TREE);
>>
>>     if (!coro_promise_type_found_p (current_function_decl, kw))
>> @@ -1307,10 +1303,8 @@ finish_co_return_stmt (location_t kw, tree expr)
>>         && check_for_bare_parameter_packs (expr))
>>       return error_mark_node;
>>
>> -  /* If we don't know the promise type, we can't proceed, build the
>> -     co_return with the expression unchanged.  */
>> -  tree functype = TREE_TYPE (current_function_decl);
>> -  if (dependent_type_p (functype) || type_dependent_expression_p (expr))
>> +  /* Defer processing when we have dependent types.  */
>> +  if (processing_template_decl)
>>       {
>>         /* co_return expressions are always void type, regardless of the
>>           expression type.  */