| Message ID | 20240731110326.3615564-1-arsen@aarsen.me |
|---|---|
| State | New |
| Headers | show |
| Series | c++/coroutines: only defer expanding co_{await, return, yield} if dependent [PR112341] | expand |
On 7/31/24 6:54 AM, Arsen Arsenović wrote: > Tested on x86_64-pc-linux-gnu. OK for trunk? > > TIA, have a lovely day. > ---------- >8 ---------- > By doing so, we can get diagnostics in template decls when we know we > can. For instance, in the following: > > awaitable g(); > template<typename> > task f() > { > co_await g(); > co_yield 1; > co_return "foo"; > } > > ... the coroutine promise type in each statement is always > std::coroutine_handle<task>::promise_type, and all of the operands are > not type-dependent, so we can always compute the resulting types (and > expected types) of these expressions and statements. > > Also, when we do not know the type of the CO_AWAIT_EXPR or > CO_YIELD_EXPR, we now return NULL_TREE as the type rather than > unknown_type_node. This is more correct, since the type is not unknown, > it just isn't determined yet. This also means we can remove the > CO_AWAIT_EXPR and CO_YIELD_EXPR special-cases from > type_dependent_expression_p. > > PR c++/112341 - error: insufficient contextual information to determine type on co_await result in function template > > gcc/cp/ChangeLog: > > PR c++/112341 > * coroutines.cc (struct coroutine_info): Also cache the > traits type. > (ensure_coro_initialized): New function. Makes sure we have > initialized the coroutine state successfully, or informs the > caller should it fail to do so. Extracted from > coro_promise_type_found_p. > (coro_get_traits_class): New function. Gets the (cached) > coroutine traits type for a given coroutine. Extracted from > coro_promise_type_found_p and refactored to cache the result. > (coro_promise_type_found_p): Use the two functions above. > (build_template_co_await_expr): New function. Builds a > CO_AWAIT_EXPR representing a CO_AWAIT_EXPR in a template > declaration. > (build_co_await): Use the above if processing_template_decl, and > give it a propert type. > (defer_expansion_p): New function. Returns true iff its > argument is a type-dependent expression OR the current functions > traits class is type dependent. > (finish_co_await_expr): Defer expansion only in the case > defer_expasnion_p returns true. > (finish_co_yield_expr): Ditto. > (finish_co_return_stmt): Ditto. > * pt.cc (type_dependent_expression_p): Do not treat > CO_AWAIT/CO_YIELD specially. > > gcc/testsuite/ChangeLog: > > PR c++/112341 > * g++.dg/coroutines/pr112341-2.C: New test. > * g++.dg/coroutines/pr112341-3.C: New test. > * g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C: New > test. > * g++.dg/coroutines/pr112341.C: New test. > --- > gcc/cp/coroutines.cc | 142 ++++++++++++++---- > gcc/cp/pt.cc | 5 - > gcc/testsuite/g++.dg/coroutines/pr112341-2.C | 25 +++ > gcc/testsuite/g++.dg/coroutines/pr112341-3.C | 65 ++++++++ > gcc/testsuite/g++.dg/coroutines/pr112341.C | 21 +++ > .../torture/co-yield-03-tmpl-nondependent.C | 140 +++++++++++++++++ > 6 files changed, 361 insertions(+), 37 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341-2.C > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341-3.C > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341.C > create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 127a1c06b56e..9494cb499454 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -85,6 +85,7 @@ struct GTY((for_user)) coroutine_info > tree actor_decl; /* The synthesized actor function. */ > tree destroy_decl; /* The synthesized destroy function. */ > tree promise_type; /* The cached promise type for this function. */ > + tree traits_type; /* The cached traits type for this function. */ > tree handle_type; /* The cached coroutine handle for this function. */ > tree self_h_proxy; /* A handle instance that is used as the proxy for the > one that will eventually be allocated in the coroutine > @@ -429,11 +430,12 @@ find_promise_type (tree traits_class) > return promise_type; > } > > +/* Perform initialization of the coroutine processor state, if not done > + before. */ > + > static bool > -coro_promise_type_found_p (tree fndecl, location_t loc) > +ensure_coro_initialized (location_t loc) > { > - gcc_assert (fndecl != NULL_TREE); > - > if (!coro_initialized) > { > /* Trees we only need to create once. > @@ -466,6 +468,33 @@ coro_promise_type_found_p (tree fndecl, location_t loc) > > coro_initialized = true; > } > + return true; > +} > + > +/* Try to get the coroutine traits class. */ > +static tree > +coro_get_traits_class (tree fndecl, location_t loc) > +{ > + gcc_assert (fndecl != NULL_TREE); > + > + if (!ensure_coro_initialized (loc)) > + /* We can't continue. */ > + return error_mark_node; > + > + coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl); > + auto& traits_type = coro_info->traits_type; > + if (!traits_type) > + traits_type = instantiate_coro_traits (fndecl, loc); > + return traits_type; > +} > + > +static bool > +coro_promise_type_found_p (tree fndecl, location_t loc) > +{ > + gcc_assert (fndecl != NULL_TREE); > + > + if (!ensure_coro_initialized (loc)) > + return false; > > /* Save the coroutine data on the side to avoid the overhead on every > function decl tree. */ > @@ -480,7 +509,7 @@ coro_promise_type_found_p (tree fndecl, location_t loc) > /* Get the coroutine traits template class instance for the function > signature we have - coroutine_traits <R, ...> */ > > - tree templ_class = instantiate_coro_traits (fndecl, loc); > + tree templ_class = coro_get_traits_class (fndecl, loc); > > /* Find the promise type for that. */ > coro_info->promise_type = find_promise_type (templ_class); > @@ -914,6 +943,19 @@ coro_diagnose_throwing_final_aw_expr (tree expr) > return coro_diagnose_throwing_fn (fn); > } > > +/* Build a co_await suitable for later expansion. */ > + > +tree > +build_template_co_await_expr (location_t kw, tree expr, tree type, tree kind) For consistency with other build*, let's put type before expr. > +{ > + tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, type, expr, > + NULL_TREE, NULL_TREE, NULL_TREE, > + kind); Typically in templates we use build_min_nt_loc for the dependent case, and build_min_non_dep for the non-dependent case...though I suppose that distinction is primarily about handling TREE_SIDE_EFFECTS, which is less significant when we're always setting it to true anyway. So I guess this is fine. > + TREE_SIDE_EFFECTS (aw_expr) = true; > + return aw_expr; > +} > + > + > /* This performs [expr.await] bullet 3.3 and validates the interface obtained. > It is also used to build the initial and final suspend points. > > @@ -922,11 +964,24 @@ coro_diagnose_throwing_final_aw_expr (tree expr) > A, the original yield/await expr, is found at source location LOC. > > We will be constructing a CO_AWAIT_EXPR for a suspend point of one of > - the four suspend_point_kind kinds. This is indicated by SUSPEND_KIND. */ > + the four suspend_point_kind kinds. This is indicated by SUSPEND_KIND. > + > + In the case that we're processing a template declaration, we can't save > + actual awaiter expressions as the frame type will differ between > + instantiations, but we can generate them to type-check them and compute the > + resulting expression type. In those cases, we will return a > + template-appropriate CO_AWAIT_EXPR and throw away the rest of the results. > + Such an expression requires the original co_await operand unaltered. Pass > + it as ORIG_OPERAND. If it is the same as 'a', you can pass error_mark_node > + (the default) to use 'a' as the value. */ Why error_mark_node rather than NULL_TREE as the default? I'd prefer to reserve error_mark_node for actual errors. > static tree > -build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) > +build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind, > + tree orig_operand = error_mark_node) > { > + if (orig_operand == error_mark_node) > + orig_operand = a; > + > /* Try and overload of operator co_await, .... */ > tree o; > if (MAYBE_CLASS_TYPE_P (TREE_TYPE (a))) > @@ -1131,11 +1186,13 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) > if (REFERENCE_REF_P (e_proxy)) > e_proxy = TREE_OPERAND (e_proxy, 0); > > + tree awrs_type = TREE_TYPE (TREE_TYPE (awrs_func)); > + tree suspend_kind_cst = build_int_cst (integer_type_node, > + (int) suspend_kind); > tree await_expr = build5_loc (loc, CO_AWAIT_EXPR, > - TREE_TYPE (TREE_TYPE (awrs_func)), > + awrs_type, > a, e_proxy, o, awaiter_calls, > - build_int_cst (integer_type_node, > - (int) suspend_kind)); > + suspend_kind_cst); > TREE_SIDE_EFFECTS (await_expr) = true; > if (te) > { > @@ -1144,7 +1201,26 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) > await_expr = te; > } > SET_EXPR_LOCATION (await_expr, loc); > - return convert_from_reference (await_expr); > + > + if (processing_template_decl) > + /* FIXME: Can we reuse the results above rather than re-make at template > + instantiating? The "obvious" solution of checking if we have a fully > + built expression and recurring into the operands and 'e' call > + expressions seems to cause PR103868 again. */ In general all that we keep is types and name lookup results, everything else gets rebuilt at instantiation time, I wouldn't try to do anything different for co_await. > + return build_template_co_await_expr (loc, orig_operand, awrs_type, > + suspend_kind_cst); > + return convert_from_reference (await_expr); > +} > + > +/* Returns true iff EXPR or the current functions traits class are dependent. > + In those cases, we can't decide what the type of our expressions is, so we > + defer expansion entirely. */ > + > +static bool > +defer_expansion_p (tree expr, location_t loc) This name doesn't suggest it's specific to coroutines. Maybe coro_dependent_p? I'm also skeptical about passing in loc here; we shouldn't need to give a diagnostic by this point. Relatedly, it looks like none of the callers of coro_get_traits_class deal with error_mark_node return. If you aren't going to handle that, maybe coro_get_traits_class should abort instead of returning it. Or call ensure_coro_initialized somewhere else. Jason
Jason Merrill <jason@redhat.com> writes: > On 7/31/24 6:54 AM, Arsen Arsenović wrote: >> Tested on x86_64-pc-linux-gnu. OK for trunk? >> TIA, have a lovely day. >> ---------- >8 ---------- >> By doing so, we can get diagnostics in template decls when we know we >> can. For instance, in the following: >> awaitable g(); >> template<typename> >> task f() >> { >> co_await g(); >> co_yield 1; >> co_return "foo"; >> } >> ... the coroutine promise type in each statement is always >> std::coroutine_handle<task>::promise_type, and all of the operands are >> not type-dependent, so we can always compute the resulting types (and >> expected types) of these expressions and statements. >> Also, when we do not know the type of the CO_AWAIT_EXPR or >> CO_YIELD_EXPR, we now return NULL_TREE as the type rather than >> unknown_type_node. This is more correct, since the type is not unknown, >> it just isn't determined yet. This also means we can remove the >> CO_AWAIT_EXPR and CO_YIELD_EXPR special-cases from >> type_dependent_expression_p. >> PR c++/112341 - error: insufficient contextual information to determine type >> on co_await result in function template >> gcc/cp/ChangeLog: >> PR c++/112341 >> * coroutines.cc (struct coroutine_info): Also cache the >> traits type. >> (ensure_coro_initialized): New function. Makes sure we have >> initialized the coroutine state successfully, or informs the >> caller should it fail to do so. Extracted from >> coro_promise_type_found_p. >> (coro_get_traits_class): New function. Gets the (cached) >> coroutine traits type for a given coroutine. Extracted from >> coro_promise_type_found_p and refactored to cache the result. >> (coro_promise_type_found_p): Use the two functions above. >> (build_template_co_await_expr): New function. Builds a >> CO_AWAIT_EXPR representing a CO_AWAIT_EXPR in a template >> declaration. >> (build_co_await): Use the above if processing_template_decl, and >> give it a propert type. >> (defer_expansion_p): New function. Returns true iff its >> argument is a type-dependent expression OR the current functions >> traits class is type dependent. >> (finish_co_await_expr): Defer expansion only in the case >> defer_expasnion_p returns true. >> (finish_co_yield_expr): Ditto. >> (finish_co_return_stmt): Ditto. >> * pt.cc (type_dependent_expression_p): Do not treat >> CO_AWAIT/CO_YIELD specially. >> gcc/testsuite/ChangeLog: >> PR c++/112341 >> * g++.dg/coroutines/pr112341-2.C: New test. >> * g++.dg/coroutines/pr112341-3.C: New test. >> * g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C: New >> test. >> * g++.dg/coroutines/pr112341.C: New test. >> --- >> gcc/cp/coroutines.cc | 142 ++++++++++++++---- >> gcc/cp/pt.cc | 5 - >> gcc/testsuite/g++.dg/coroutines/pr112341-2.C | 25 +++ >> gcc/testsuite/g++.dg/coroutines/pr112341-3.C | 65 ++++++++ >> gcc/testsuite/g++.dg/coroutines/pr112341.C | 21 +++ >> .../torture/co-yield-03-tmpl-nondependent.C | 140 +++++++++++++++++ >> 6 files changed, 361 insertions(+), 37 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341-2.C >> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341-3.C >> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341.C >> create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C >> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc >> index 127a1c06b56e..9494cb499454 100644 >> --- a/gcc/cp/coroutines.cc >> +++ b/gcc/cp/coroutines.cc >> @@ -85,6 +85,7 @@ struct GTY((for_user)) coroutine_info >> tree actor_decl; /* The synthesized actor function. */ >> tree destroy_decl; /* The synthesized destroy function. */ >> tree promise_type; /* The cached promise type for this function. */ >> + tree traits_type; /* The cached traits type for this function. */ >> tree handle_type; /* The cached coroutine handle for this function. */ >> tree self_h_proxy; /* A handle instance that is used as the proxy for the >> one that will eventually be allocated in the coroutine >> @@ -429,11 +430,12 @@ find_promise_type (tree traits_class) >> return promise_type; >> } >> +/* Perform initialization of the coroutine processor state, if not done >> + before. */ >> + >> static bool >> -coro_promise_type_found_p (tree fndecl, location_t loc) >> +ensure_coro_initialized (location_t loc) >> { >> - gcc_assert (fndecl != NULL_TREE); >> - >> if (!coro_initialized) >> { >> /* Trees we only need to create once. >> @@ -466,6 +468,33 @@ coro_promise_type_found_p (tree fndecl, location_t loc) >> coro_initialized = true; >> } >> + return true; >> +} >> + >> +/* Try to get the coroutine traits class. */ >> +static tree >> +coro_get_traits_class (tree fndecl, location_t loc) >> +{ >> + gcc_assert (fndecl != NULL_TREE); >> + >> + if (!ensure_coro_initialized (loc)) >> + /* We can't continue. */ >> + return error_mark_node; >> + >> + coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl); >> + auto& traits_type = coro_info->traits_type; >> + if (!traits_type) >> + traits_type = instantiate_coro_traits (fndecl, loc); >> + return traits_type; >> +} >> + >> +static bool >> +coro_promise_type_found_p (tree fndecl, location_t loc) >> +{ >> + gcc_assert (fndecl != NULL_TREE); >> + >> + if (!ensure_coro_initialized (loc)) >> + return false; >> /* Save the coroutine data on the side to avoid the overhead on every >> function decl tree. */ >> @@ -480,7 +509,7 @@ coro_promise_type_found_p (tree fndecl, location_t loc) >> /* Get the coroutine traits template class instance for the function >> signature we have - coroutine_traits <R, ...> */ >> - tree templ_class = instantiate_coro_traits (fndecl, loc); >> + tree templ_class = coro_get_traits_class (fndecl, loc); >> /* Find the promise type for that. */ >> coro_info->promise_type = find_promise_type (templ_class); >> @@ -914,6 +943,19 @@ coro_diagnose_throwing_final_aw_expr (tree expr) >> return coro_diagnose_throwing_fn (fn); >> } >> +/* Build a co_await suitable for later expansion. */ >> + >> +tree >> +build_template_co_await_expr (location_t kw, tree expr, tree type, tree kind) > > For consistency with other build*, let's put type before expr. Sure, done. >> +{ >> + tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, type, expr, >> + NULL_TREE, NULL_TREE, NULL_TREE, >> + kind); > > Typically in templates we use build_min_nt_loc for the dependent case, and > build_min_non_dep for the non-dependent case...though I suppose that > distinction is primarily about handling TREE_SIDE_EFFECTS, which is less > significant when we're always setting it to true anyway. So I guess this is > fine. I don't think we can use the _nt variant here since the type is variable, no? >> + TREE_SIDE_EFFECTS (aw_expr) = true; >> + return aw_expr; >> +} >> + >> + >> /* This performs [expr.await] bullet 3.3 and validates the interface obtained. >> It is also used to build the initial and final suspend points. >> @@ -922,11 +964,24 @@ coro_diagnose_throwing_final_aw_expr (tree expr) >> A, the original yield/await expr, is found at source location LOC. >> We will be constructing a CO_AWAIT_EXPR for a suspend point of one of >> - the four suspend_point_kind kinds. This is indicated by SUSPEND_KIND. */ >> + the four suspend_point_kind kinds. This is indicated by SUSPEND_KIND. >> + >> + In the case that we're processing a template declaration, we can't save >> + actual awaiter expressions as the frame type will differ between >> + instantiations, but we can generate them to type-check them and compute the >> + resulting expression type. In those cases, we will return a >> + template-appropriate CO_AWAIT_EXPR and throw away the rest of the results. >> + Such an expression requires the original co_await operand unaltered. Pass >> + it as ORIG_OPERAND. If it is the same as 'a', you can pass error_mark_node >> + (the default) to use 'a' as the value. */ > > Why error_mark_node rather than NULL_TREE as the default? I'd prefer to > reserve error_mark_node for actual errors. Hm, unsure why I did that, and I see no reason to keep error_mark_node, so I'll swap it out for NULL_TREE. >> static tree >> -build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) >> +build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind, >> + tree orig_operand = error_mark_node) >> { >> + if (orig_operand == error_mark_node) >> + orig_operand = a; >> + >> /* Try and overload of operator co_await, .... */ >> tree o; >> if (MAYBE_CLASS_TYPE_P (TREE_TYPE (a))) >> @@ -1131,11 +1186,13 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) >> if (REFERENCE_REF_P (e_proxy)) >> e_proxy = TREE_OPERAND (e_proxy, 0); >> + tree awrs_type = TREE_TYPE (TREE_TYPE (awrs_func)); >> + tree suspend_kind_cst = build_int_cst (integer_type_node, >> + (int) suspend_kind); >> tree await_expr = build5_loc (loc, CO_AWAIT_EXPR, >> - TREE_TYPE (TREE_TYPE (awrs_func)), >> + awrs_type, >> a, e_proxy, o, awaiter_calls, >> - build_int_cst (integer_type_node, >> - (int) suspend_kind)); >> + suspend_kind_cst); >> TREE_SIDE_EFFECTS (await_expr) = true; >> if (te) >> { >> @@ -1144,7 +1201,26 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) >> await_expr = te; >> } >> SET_EXPR_LOCATION (await_expr, loc); >> - return convert_from_reference (await_expr); >> + >> + if (processing_template_decl) >> + /* FIXME: Can we reuse the results above rather than re-make at template >> + instantiating? The "obvious" solution of checking if we have a fully >> + built expression and recurring into the operands and 'e' call >> + expressions seems to cause PR103868 again. */ > > In general all that we keep is types and name lookup results, everything else > gets rebuilt at instantiation time, I wouldn't try to do anything different for > co_await. OK. I've dropped the FIXME. >> + return build_template_co_await_expr (loc, orig_operand, awrs_type, >> + suspend_kind_cst); >> + return convert_from_reference (await_expr); >> +} >> + >> +/* Returns true iff EXPR or the current functions traits class are dependent. >> + In those cases, we can't decide what the type of our expressions is, so we >> + defer expansion entirely. */ >> + >> +static bool >> +defer_expansion_p (tree expr, location_t loc) > > This name doesn't suggest it's specific to coroutines. Maybe coro_dependent_p? Yes, that's a better name, I've renamed it. > I'm also skeptical about passing in loc here; we shouldn't need to give a > diagnostic by this point. > > Relatedly, it looks like none of the callers of coro_get_traits_class deal with > error_mark_node return. If you aren't going to handle that, maybe > coro_get_traits_class should abort instead of returning it. Or call > ensure_coro_initialized somewhere else. Hm, I think a better design is indeed to extract the ensure_coro_initialized call and make coro_initialized a precondition of coro_dependent_p. This means that coro_get_traits_class returns NULL_TREE as the only possible error return. I think I'll also extract getting the traits class a level up so that the predicate is only busy with checking whether the types are dependent, meaning there's no need to contrive the promise_type being NULL_TREE into either a true or false return from coro_dependent_p (currently true, because of dependent_type_p (NULL_TREE) == true). Working on those changes now. Thanks for the comments.
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 127a1c06b56e..9494cb499454 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -85,6 +85,7 @@ struct GTY((for_user)) coroutine_info tree actor_decl; /* The synthesized actor function. */ tree destroy_decl; /* The synthesized destroy function. */ tree promise_type; /* The cached promise type for this function. */ + tree traits_type; /* The cached traits type for this function. */ tree handle_type; /* The cached coroutine handle for this function. */ tree self_h_proxy; /* A handle instance that is used as the proxy for the one that will eventually be allocated in the coroutine @@ -429,11 +430,12 @@ find_promise_type (tree traits_class) return promise_type; } +/* Perform initialization of the coroutine processor state, if not done + before. */ + static bool -coro_promise_type_found_p (tree fndecl, location_t loc) +ensure_coro_initialized (location_t loc) { - gcc_assert (fndecl != NULL_TREE); - if (!coro_initialized) { /* Trees we only need to create once. @@ -466,6 +468,33 @@ coro_promise_type_found_p (tree fndecl, location_t loc) coro_initialized = true; } + return true; +} + +/* Try to get the coroutine traits class. */ +static tree +coro_get_traits_class (tree fndecl, location_t loc) +{ + gcc_assert (fndecl != NULL_TREE); + + if (!ensure_coro_initialized (loc)) + /* We can't continue. */ + return error_mark_node; + + coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl); + auto& traits_type = coro_info->traits_type; + if (!traits_type) + traits_type = instantiate_coro_traits (fndecl, loc); + return traits_type; +} + +static bool +coro_promise_type_found_p (tree fndecl, location_t loc) +{ + gcc_assert (fndecl != NULL_TREE); + + if (!ensure_coro_initialized (loc)) + return false; /* Save the coroutine data on the side to avoid the overhead on every function decl tree. */ @@ -480,7 +509,7 @@ coro_promise_type_found_p (tree fndecl, location_t loc) /* Get the coroutine traits template class instance for the function signature we have - coroutine_traits <R, ...> */ - tree templ_class = instantiate_coro_traits (fndecl, loc); + tree templ_class = coro_get_traits_class (fndecl, loc); /* Find the promise type for that. */ coro_info->promise_type = find_promise_type (templ_class); @@ -914,6 +943,19 @@ coro_diagnose_throwing_final_aw_expr (tree expr) return coro_diagnose_throwing_fn (fn); } +/* Build a co_await suitable for later expansion. */ + +tree +build_template_co_await_expr (location_t kw, tree expr, tree type, tree kind) +{ + tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, type, expr, + NULL_TREE, NULL_TREE, NULL_TREE, + kind); + TREE_SIDE_EFFECTS (aw_expr) = true; + return aw_expr; +} + + /* This performs [expr.await] bullet 3.3 and validates the interface obtained. It is also used to build the initial and final suspend points. @@ -922,11 +964,24 @@ coro_diagnose_throwing_final_aw_expr (tree expr) A, the original yield/await expr, is found at source location LOC. We will be constructing a CO_AWAIT_EXPR for a suspend point of one of - the four suspend_point_kind kinds. This is indicated by SUSPEND_KIND. */ + the four suspend_point_kind kinds. This is indicated by SUSPEND_KIND. + + In the case that we're processing a template declaration, we can't save + actual awaiter expressions as the frame type will differ between + instantiations, but we can generate them to type-check them and compute the + resulting expression type. In those cases, we will return a + template-appropriate CO_AWAIT_EXPR and throw away the rest of the results. + Such an expression requires the original co_await operand unaltered. Pass + it as ORIG_OPERAND. If it is the same as 'a', you can pass error_mark_node + (the default) to use 'a' as the value. */ static tree -build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) +build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind, + tree orig_operand = error_mark_node) { + if (orig_operand == error_mark_node) + orig_operand = a; + /* Try and overload of operator co_await, .... */ tree o; if (MAYBE_CLASS_TYPE_P (TREE_TYPE (a))) @@ -1131,11 +1186,13 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) if (REFERENCE_REF_P (e_proxy)) e_proxy = TREE_OPERAND (e_proxy, 0); + tree awrs_type = TREE_TYPE (TREE_TYPE (awrs_func)); + tree suspend_kind_cst = build_int_cst (integer_type_node, + (int) suspend_kind); tree await_expr = build5_loc (loc, CO_AWAIT_EXPR, - TREE_TYPE (TREE_TYPE (awrs_func)), + awrs_type, a, e_proxy, o, awaiter_calls, - build_int_cst (integer_type_node, - (int) suspend_kind)); + suspend_kind_cst); TREE_SIDE_EFFECTS (await_expr) = true; if (te) { @@ -1144,7 +1201,26 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) await_expr = te; } SET_EXPR_LOCATION (await_expr, loc); - return convert_from_reference (await_expr); + + if (processing_template_decl) + /* FIXME: Can we reuse the results above rather than re-make at template + instantiating? The "obvious" solution of checking if we have a fully + built expression and recurring into the operands and 'e' call + expressions seems to cause PR103868 again. */ + return build_template_co_await_expr (loc, orig_operand, awrs_type, + suspend_kind_cst); + return convert_from_reference (await_expr); +} + +/* Returns true iff EXPR or the current functions traits class are dependent. + In those cases, we can't decide what the type of our expressions is, so we + defer expansion entirely. */ + +static bool +defer_expansion_p (tree expr, location_t loc) +{ + return type_dependent_expression_p (expr) + || dependent_type_p (coro_get_traits_class (current_function_decl, loc)); } tree @@ -1166,20 +1242,17 @@ finish_co_await_expr (location_t kw, tree expr) extraneous warnings during substitution. */ suppress_warning (current_function_decl, OPT_Wreturn_type); - /* Defer expansion when we are processing a template. - FIXME: 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. */ - if (processing_template_decl) - { - tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, unknown_type_node, expr, - NULL_TREE, NULL_TREE, NULL_TREE, - integer_zero_node); - TREE_SIDE_EFFECTS (aw_expr) = true; - return aw_expr; - } + /* Defer expansion when we are processing a template, unless the traits type + and expression would not be dependent. In the case that the types are + not dependent but we are processing a template declaration, we will do + most of the computation but throw away the results (except for the + await_resume type). Otherwise, if our co_await is type-dependent + (i.e. the promise type or operand type is dependent), we can't do much, + and just return early with a NULL_TREE type (indicating that we cannot + compute the type yet). */ + if (defer_expansion_p (expr, kw)) + return build_template_co_await_expr (kw, expr, NULL_TREE, + integer_zero_node); /* We must be able to look up the "await_transform" method in the scope of the promise type, and obtain its return type. */ @@ -1216,7 +1289,7 @@ finish_co_await_expr (location_t kw, tree expr) } /* Now we want to build co_await a. */ - return build_co_await (kw, a, CO_AWAIT_SUSPEND_POINT); + return build_co_await (kw, a, CO_AWAIT_SUSPEND_POINT, expr); } /* Take the EXPR given and attempt to build: @@ -1243,10 +1316,15 @@ finish_co_yield_expr (location_t kw, tree expr) extraneous warnings during substitution. */ suppress_warning (current_function_decl, OPT_Wreturn_type); - /* Defer expansion when we are processing a template; see FIXME in the - co_await code. */ - if (processing_template_decl) - return build2_loc (kw, CO_YIELD_EXPR, unknown_type_node, expr, NULL_TREE); + /* Defer expansion when we are processing a template; see note in + finish_co_await_expr. Also note that a yield is equivalent to + + co_await p.yield_value(EXPR) + + If either p or EXPR are type-dependent, then the whole expression is + certainly type-dependent, and we can't proceed. */ + if (defer_expansion_p (expr, kw)) + return build2_loc (kw, CO_YIELD_EXPR, NULL_TREE, expr, NULL_TREE); if (!coro_promise_type_found_p (current_function_decl, kw)) /* We must be able to look up the "yield_value" method in the scope of @@ -1327,9 +1405,9 @@ finish_co_return_stmt (location_t kw, tree expr) && check_for_bare_parameter_packs (expr)) return error_mark_node; - /* Defer expansion when we are processing a template; see FIXME in the - co_await code. */ - if (processing_template_decl) + /* Defer expansion when we must and are processing a template; see note in + finish_co_await_expr. */ + if (defer_expansion_p (expr, kw)) { /* co_return expressions are always void type, regardless of the expression type. */ diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 39f7e8a4e688..77fa5907c3d3 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -28651,11 +28651,6 @@ type_dependent_expression_p (tree expression) if (TREE_CODE (expression) == SCOPE_REF) return false; - /* CO_AWAIT/YIELD_EXPR with unknown type is always dependent. */ - if (TREE_CODE (expression) == CO_AWAIT_EXPR - || TREE_CODE (expression) == CO_YIELD_EXPR) - return true; - if (BASELINK_P (expression)) { if (BASELINK_OPTYPE (expression) diff --git a/gcc/testsuite/g++.dg/coroutines/pr112341-2.C b/gcc/testsuite/g++.dg/coroutines/pr112341-2.C new file mode 100644 index 000000000000..54c7d851ae8d --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr112341-2.C @@ -0,0 +1,25 @@ +// https://gcc.gnu.org/PR112341 +#include <coroutine> +struct A { int j; }; +struct B { + bool await_ready(); + bool await_suspend(std::coroutine_handle<>); + A await_resume(); +}; +struct C { + struct promise_type { + std::suspend_always initial_suspend(); + std::suspend_always final_suspend() noexcept; + B yield_value(auto); + void unhandled_exception(); + C get_return_object(); + void return_value(A); + }; +}; +template<typename> +C f() { + // Make sure we verify types we can still. + (co_await B()).i; // { dg-error "'struct A' has no member named 'i'" } + (co_yield 123).i; // { dg-error "'struct A' has no member named 'i'" } + co_return 123; // { dg-error "cannot convert 'int' to 'A'" } +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr112341-3.C b/gcc/testsuite/g++.dg/coroutines/pr112341-3.C new file mode 100644 index 000000000000..79beab88416a --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr112341-3.C @@ -0,0 +1,65 @@ +// https://gcc.gnu.org/PR112341 +#include <coroutine> +struct A { int j; }; +struct B { + bool await_ready(); + bool await_suspend(std::coroutine_handle<>); + A await_resume(); +}; +struct C { + struct promise_type { + std::suspend_always initial_suspend(); + std::suspend_always final_suspend() noexcept; + void unhandled_exception(); + C get_return_object(); + void return_void(); + std::suspend_never yield_value(int x); + }; +}; + +// Similar to the above, but with a template yield_value and return_value. +struct D { + struct promise_type { + std::suspend_always initial_suspend(); + std::suspend_always final_suspend() noexcept; + void unhandled_exception(); + D get_return_object(); + void return_value(auto); + std::suspend_never yield_value(auto); + }; +}; + +template<typename> +C f() { + co_return; +} + +template<typename> +C g() +{ + co_yield 123; +} + +template<typename> +D f1() { + co_return 123; +} + +template<typename> +D g1() +{ + co_yield 123; +} + +void +g() { + f<int>(); + f<bool>(); + g<int>(); + g<bool>(); + + f1<int>(); + f1<bool>(); + g1<int>(); + g1<bool>(); +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr112341.C b/gcc/testsuite/g++.dg/coroutines/pr112341.C new file mode 100644 index 000000000000..ffb1fec87c02 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr112341.C @@ -0,0 +1,21 @@ +// https://gcc.gnu.org/PR112341 +#include <coroutine> +struct A { int j; }; +struct B { + bool await_ready(); + bool await_suspend(std::coroutine_handle<>); + A await_resume(); +}; +struct C { + struct promise_type { + std::suspend_always initial_suspend(); + std::suspend_always final_suspend() noexcept; + void unhandled_exception(); + C get_return_object(); + void return_void(); + B yield_value(auto) { return {}; } + }; +}; +C f1(auto) { (co_await B()).j; } +C f2(auto) { (co_yield 0).j; } +void g() { f1(0); f2(0); } diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C b/gcc/testsuite/g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C new file mode 100644 index 000000000000..8e91d9557d2f --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C @@ -0,0 +1,140 @@ +// { dg-do run } + +// Test co_yield in templated code where the promise type is not dependent. + +#include "../coro.h" + +template <typename T> +struct looper { + + struct promise_type { + T value; + promise_type() { PRINT ("Created Promise"); } + ~promise_type() { PRINT ("Destroyed Promise"); } + + auto get_return_object () { + PRINT ("get_return_object: handle from promise"); + return handle_type::from_promise (*this); + } + + auto initial_suspend () { + PRINT ("get initial_suspend (always)"); + return suspend_always_prt{}; + } + + auto final_suspend () noexcept { + PRINT ("get final_suspend (always)"); + return suspend_always_prt{}; + } + + void return_value (T v) { + PRINTF ("return_value () %lf\n", (double)v); + value = v; + } + + auto yield_value (T v) { + PRINTF ("yield_value () %lf and suspend always\n", (double)v); + value = v; + return suspend_always_prt{}; + } + + T get_value (void) { return value; } + + void unhandled_exception() { PRINT ("** unhandled exception"); } + }; + + using handle_type = coro::coroutine_handle<looper::promise_type>; + handle_type handle; + + looper () : handle(0) {} + looper (handle_type _handle) + : handle(_handle) { + PRINT("Created coro1 object from handle"); + } + looper (const looper &) = delete; // no copying + looper (looper &&s) : handle(s.handle) { + s.handle = nullptr; + PRINT("looper mv ctor "); + } + looper &operator = (looper &&s) { + handle = s.handle; + s.handle = nullptr; + PRINT("looper op= "); + return *this; + } + ~looper() { + PRINT("Destroyed coro1"); + if ( handle ) + handle.destroy(); + } + + struct suspend_never_prt { + bool await_ready() const noexcept { return true; } + void await_suspend(handle_type) const noexcept { PRINT ("susp-never-susp"); } + void await_resume() const noexcept { PRINT ("susp-never-resume");} + }; + + /* NOTE: this has a DTOR to test that pathway. */ + struct suspend_always_prt { + bool await_ready() const noexcept { return false; } + void await_suspend(handle_type) const noexcept { PRINT ("susp-always-susp"); } + void await_resume() const noexcept { PRINT ("susp-always-resume"); } + ~suspend_always_prt() { PRINT ("susp-always-DTOR"); } + }; + +}; + +// Contrived to avoid non-scalar state across the yield. +template <typename> +looper<int> f () noexcept +{ + for (int i = 5; i < 10 ; ++i) + { + PRINTF ("f: about to yield %d\n", i); + co_yield (int) i; + } + + PRINT ("f: about to return 6174"); + co_return 6174; +} + +// contrived, only going to work for an int. +int main () +{ + PRINT ("main: create int looper"); + auto f_coro = f<int> (); + + if (f_coro.handle.done()) + { + PRINT ("main: said we were done, but we hadn't started!"); + abort(); + } + + PRINT ("main: OK -- looping"); + int y, test = 5; + do { + f_coro.handle.resume(); + if (f_coro.handle.done()) + break; + y = f_coro.handle.promise().get_value(); + if (y != test) + { + PRINTF ("main: failed for test %d, got %d\n", test, y); + abort(); + } + test++; + } while (test < 20); + + y = f_coro.handle.promise().get_value(); + if ( y != 6174 ) + abort (); + + PRINT ("main: apparently got 6174"); + if (!f_coro.handle.done()) + { + PRINT ("main: apparently not done..."); + abort (); + } + PRINT ("main: returning"); + return 0; +}
Tested on x86_64-pc-linux-gnu. OK for trunk? TIA, have a lovely day. ---------- >8 ---------- By doing so, we can get diagnostics in template decls when we know we can. For instance, in the following: awaitable g(); template<typename> task f() { co_await g(); co_yield 1; co_return "foo"; } ... the coroutine promise type in each statement is always std::coroutine_handle<task>::promise_type, and all of the operands are not type-dependent, so we can always compute the resulting types (and expected types) of these expressions and statements. Also, when we do not know the type of the CO_AWAIT_EXPR or CO_YIELD_EXPR, we now return NULL_TREE as the type rather than unknown_type_node. This is more correct, since the type is not unknown, it just isn't determined yet. This also means we can remove the CO_AWAIT_EXPR and CO_YIELD_EXPR special-cases from type_dependent_expression_p. PR c++/112341 - error: insufficient contextual information to determine type on co_await result in function template gcc/cp/ChangeLog: PR c++/112341 * coroutines.cc (struct coroutine_info): Also cache the traits type. (ensure_coro_initialized): New function. Makes sure we have initialized the coroutine state successfully, or informs the caller should it fail to do so. Extracted from coro_promise_type_found_p. (coro_get_traits_class): New function. Gets the (cached) coroutine traits type for a given coroutine. Extracted from coro_promise_type_found_p and refactored to cache the result. (coro_promise_type_found_p): Use the two functions above. (build_template_co_await_expr): New function. Builds a CO_AWAIT_EXPR representing a CO_AWAIT_EXPR in a template declaration. (build_co_await): Use the above if processing_template_decl, and give it a propert type. (defer_expansion_p): New function. Returns true iff its argument is a type-dependent expression OR the current functions traits class is type dependent. (finish_co_await_expr): Defer expansion only in the case defer_expasnion_p returns true. (finish_co_yield_expr): Ditto. (finish_co_return_stmt): Ditto. * pt.cc (type_dependent_expression_p): Do not treat CO_AWAIT/CO_YIELD specially. gcc/testsuite/ChangeLog: PR c++/112341 * g++.dg/coroutines/pr112341-2.C: New test. * g++.dg/coroutines/pr112341-3.C: New test. * g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C: New test. * g++.dg/coroutines/pr112341.C: New test. --- gcc/cp/coroutines.cc | 142 ++++++++++++++---- gcc/cp/pt.cc | 5 - gcc/testsuite/g++.dg/coroutines/pr112341-2.C | 25 +++ gcc/testsuite/g++.dg/coroutines/pr112341-3.C | 65 ++++++++ gcc/testsuite/g++.dg/coroutines/pr112341.C | 21 +++ .../torture/co-yield-03-tmpl-nondependent.C | 140 +++++++++++++++++ 6 files changed, 361 insertions(+), 37 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341-2.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341-3.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr112341.C create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-yield-03-tmpl-nondependent.C