Message ID | 20240821180101.3976132-2-arsen@aarsen.me |
---|---|
State | New |
Headers | show |
Series | [v3,1/2] c++: improve location of parsed RETURN_EXPRs | expand |
On 8/21/24 1:52 PM, Arsen Arsenović wrote: > We now point out why a function is a coroutine, and where (the first > return) is in the function. OK. > gcc/cp/ChangeLog: > > * coroutines.cc (struct coroutine_info): Rename > first_coro_keyword -> first_coro_expr. The former name is no > longer accurate. > (coro_promise_type_found_p): Adjust accordingly. > (coro_function_valid_p): Change how we diagnose 'return' > statements in coroutines to also point out where a function was > made a coroutine, and where 'return' was used. > (find_coro_traits_template_decl): Rename kw parameter into loc, > since it might not refer to a keyword always. > (instantiate_coro_traits): Ditto. > (find_coro_handle_template_decl): Ditto. > (get_handle_type_address): Ditto. > (get_handle_type_from_address): Ditto. > (instantiate_coro_handle_for_promise_type): Ditto. > (build_template_co_await_expr): Ditto. > (finish_co_await_expr): Ditto. > (finish_co_yield_expr): Ditto. > (finish_co_return_stmt): Ditto. > > gcc/testsuite/ChangeLog: > > * g++.dg/coroutines/co-return-syntax-08-bad-return.C: Update to > match new diagnostic. Test more keyword combinations. > --- > gcc/cp/coroutines.cc | 127 ++++++++++-------- > .../co-return-syntax-08-bad-return.C | 52 ++++++- > 2 files changed, 119 insertions(+), 60 deletions(-) > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index f7791cbfb9a6..81096784b4d7 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -93,8 +93,8 @@ struct GTY((for_user)) coroutine_info > tree promise_proxy; /* Likewise, a proxy promise instance. */ > tree from_address; /* handle_type from_address function. */ > tree return_void; /* The expression for p.return_void() if it exists. */ > - location_t first_coro_keyword; /* The location of the keyword that made this > - function into a coroutine. */ > + location_t first_coro_expr; /* The location of the expression that turned > + this funtion into a coroutine. */ > /* Flags to avoid repeated errors for per-function issues. */ > bool coro_ret_type_error_emitted; > bool coro_promise_error_emitted; > @@ -285,7 +285,7 @@ static GTY(()) tree void_coro_handle_address; > Lookup the coroutine_traits template decl. */ > > static tree > -find_coro_traits_template_decl (location_t kw) > +find_coro_traits_template_decl (location_t loc) > { > /* If we are missing fundamental information, such as the traits, (or the > declaration found is not a type template), then don't emit an error for > @@ -300,7 +300,7 @@ find_coro_traits_template_decl (location_t kw) > { > if (!traits_error_emitted) > { > - gcc_rich_location richloc (kw); > + gcc_rich_location richloc (loc); > error_at (&richloc, "coroutines require a traits template; cannot" > " find %<%E::%E%>", std_node, coro_traits_identifier); > inform (&richloc, "perhaps %<#include <coroutine>%> is missing"); > @@ -315,7 +315,7 @@ find_coro_traits_template_decl (location_t kw) > /* Instantiate Coroutine traits for the function signature. */ > > static tree > -instantiate_coro_traits (tree fndecl, location_t kw) > +instantiate_coro_traits (tree fndecl, location_t loc) > { > /* [coroutine.traits.primary] > So now build up a type list for the template <typename _R, typename...>. > @@ -358,7 +358,7 @@ instantiate_coro_traits (tree fndecl, location_t kw) > > if (traits_class == error_mark_node) > { > - error_at (kw, "cannot instantiate %<coroutine traits%>"); > + error_at (loc, "cannot instantiate %<coroutine traits%>"); > return NULL_TREE; > } > > @@ -368,7 +368,7 @@ instantiate_coro_traits (tree fndecl, location_t kw) > /* [coroutine.handle] */ > > static tree > -find_coro_handle_template_decl (location_t kw) > +find_coro_handle_template_decl (location_t loc) > { > /* As for the coroutine traits, this error is per TU, so only emit > it once. */ > @@ -380,7 +380,7 @@ find_coro_handle_template_decl (location_t kw) > || !DECL_CLASS_TEMPLATE_P (handle_decl)) > { > if (!coro_handle_error_emitted) > - error_at (kw, "coroutines require a handle class template;" > + error_at (loc, "coroutines require a handle class template;" > " cannot find %<%E::%E%>", std_node, coro_handle_identifier); > coro_handle_error_emitted = true; > return NULL_TREE; > @@ -394,21 +394,21 @@ find_coro_handle_template_decl (location_t kw) > void*. If that is not the case, signals an error and returns NULL_TREE. */ > > static tree > -get_handle_type_address (location_t kw, tree handle_type) > +get_handle_type_address (location_t loc, tree handle_type) > { > tree addr_getter = lookup_member (handle_type, coro_address_identifier, 1, > 0, tf_warning_or_error); > if (!addr_getter || addr_getter == error_mark_node) > { > qualified_name_lookup_error (handle_type, coro_address_identifier, > - error_mark_node, kw); > + error_mark_node, loc); > return NULL_TREE; > } > > if (!BASELINK_P (addr_getter) > || TREE_CODE (TREE_TYPE (addr_getter)) != METHOD_TYPE) > { > - error_at (kw, "%qE must be a non-overloaded method", addr_getter); > + error_at (loc, "%qE must be a non-overloaded method", addr_getter); > return NULL_TREE; > } > > @@ -421,14 +421,14 @@ get_handle_type_address (location_t kw, tree handle_type) > /* Check that from_addr has the argument list (). */ > if (arg != void_list_node) > { > - error_at (kw, "%qE must take no arguments", addr_getter); > + error_at (loc, "%qE must take no arguments", addr_getter); > return NULL_TREE; > } > > tree ret_t = TREE_TYPE (fn_t); > if (!same_type_p (ret_t, ptr_type_node)) > { > - error_at (kw, "%qE must return %qT, not %qT", > + error_at (loc, "%qE must return %qT, not %qT", > addr_getter, ptr_type_node, ret_t); > return NULL_TREE; > } > @@ -442,20 +442,20 @@ get_handle_type_address (location_t kw, tree handle_type) > NULL_TREE. */ > > static tree > -get_handle_type_from_address (location_t kw, tree handle_type) > +get_handle_type_from_address (location_t loc, tree handle_type) > { > tree from_addr = lookup_member (handle_type, coro_from_address_identifier, 1, > 0, tf_warning_or_error); > if (!from_addr || from_addr == error_mark_node) > { > qualified_name_lookup_error (handle_type, coro_from_address_identifier, > - error_mark_node, kw); > + error_mark_node, loc); > return NULL_TREE; > } > if (!BASELINK_P (from_addr) > || TREE_CODE (TREE_TYPE (from_addr)) != FUNCTION_TYPE) > { > - error_at (kw, "%qE must be a non-overloaded static function", from_addr); > + error_at (loc, "%qE must be a non-overloaded static function", from_addr); > return NULL_TREE; > } > > @@ -466,14 +466,14 @@ get_handle_type_from_address (location_t kw, tree handle_type) > || !same_type_p (TREE_VALUE (arg), ptr_type_node) > || TREE_CHAIN (arg) != void_list_node) > { > - error_at (kw, "%qE must take a single %qT", from_addr, ptr_type_node); > + error_at (loc, "%qE must take a single %qT", from_addr, ptr_type_node); > return NULL_TREE; > } > > tree ret_t = TREE_TYPE (fn_t); > if (!same_type_p (ret_t, handle_type)) > { > - error_at (kw, "%qE must return %qT, not %qT", > + error_at (loc, "%qE must return %qT, not %qT", > from_addr, handle_type, ret_t); > return NULL_TREE; > } > @@ -482,7 +482,7 @@ get_handle_type_from_address (location_t kw, tree handle_type) > } > > static tree > -instantiate_coro_handle_for_promise_type (location_t kw, tree promise_type) > +instantiate_coro_handle_for_promise_type (location_t loc, tree promise_type) > { > /* So now build up a type list for the template, one entry, the promise. */ > tree targ = make_tree_vec (1); > @@ -495,7 +495,7 @@ instantiate_coro_handle_for_promise_type (location_t kw, tree promise_type) > > if (handle_type == error_mark_node) > { > - error_at (kw, "cannot instantiate a %<coroutine handle%> for" > + error_at (loc, "cannot instantiate a %<coroutine handle%> for" > " promise type %qT", promise_type); > return NULL_TREE; > } > @@ -674,8 +674,8 @@ coro_promise_type_found_p (tree fndecl, location_t loc) > = build_lang_decl (VAR_DECL, coro_promise_id, > coro_info->promise_type); > > - /* Note where we first saw a coroutine keyword. */ > - coro_info->first_coro_keyword = loc; > + /* Note where we were when we made this function into a coroutine. */ > + coro_info->first_coro_expr = loc; > } > > return true; > @@ -968,11 +968,24 @@ coro_function_valid_p (tree fndecl) > > if (current_function_returns_value || current_function_returns_null) > { > - /* TODO: record or extract positions of returns (and the first coro > - keyword) so that we can add notes to the diagnostic about where > - the bad keyword is and what made the function into a coro. */ > - error_at (f_loc, "a %<return%> statement is not allowed in coroutine;" > - " did you mean %<co_return%>?"); > + auto coro_info = get_coroutine_info (fndecl); > + gcc_checking_assert (coro_info->first_coro_expr); > + > + walk_tree_fn retf = [] (tree* pt, int*, void*) > + { > + if (*pt && TREE_CODE (*pt) == RETURN_EXPR) > + return *pt; > + return NULL_TREE; > + }; > + > + tree ret = cp_walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl), > + retf, nullptr); > + auto ret_loc = EXPR_LOCATION (ret); > + auto_diagnostic_group diaggrp; > + error_at (ret_loc, "a %<return%> statement is not allowed in coroutine;" > + " did you mean %<co_return%>?"); > + inform (coro_info->first_coro_expr, > + "function was made a coroutine here"); > return false; > } > > @@ -1051,9 +1064,9 @@ coro_diagnose_throwing_final_aw_expr (tree expr) > /* Build a co_await suitable for later expansion. */ > > tree > -build_template_co_await_expr (location_t kw, tree type, tree expr, tree kind) > +build_template_co_await_expr (location_t loc, tree type, tree expr, tree kind) > { > - tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, type, expr, > + tree aw_expr = build5_loc (loc, CO_AWAIT_EXPR, type, expr, > NULL_TREE, NULL_TREE, NULL_TREE, > kind); > TREE_SIDE_EFFECTS (aw_expr) = true; > @@ -1326,12 +1339,12 @@ coro_dependent_p (tree expr, tree traits_class) > } > > tree > -finish_co_await_expr (location_t kw, tree expr) > +finish_co_await_expr (location_t loc, tree expr) > { > if (!expr || error_operand_p (expr)) > return error_mark_node; > > - if (!coro_common_keyword_context_valid_p (current_function_decl, kw, > + if (!coro_common_keyword_context_valid_p (current_function_decl, loc, > "co_await")) > return error_mark_node; > > @@ -1345,11 +1358,11 @@ finish_co_await_expr (location_t kw, tree expr) > suppress_warning (current_function_decl, OPT_Wreturn_type); > > /* Prepare for coroutine transformations. */ > - if (!ensure_coro_initialized (kw)) > + if (!ensure_coro_initialized (loc)) > return error_mark_node; > > /* Get our traits class. */ > - tree traits_class = coro_get_traits_class (current_function_decl, kw); > + tree traits_class = coro_get_traits_class (current_function_decl, loc); > > /* 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 > @@ -1360,12 +1373,12 @@ finish_co_await_expr (location_t kw, tree expr) > and just return early with a NULL_TREE type (indicating that we cannot > compute the type yet). */ > if (coro_dependent_p (expr, traits_class)) > - return build_template_co_await_expr (kw, NULL_TREE, expr, > + return build_template_co_await_expr (loc, NULL_TREE, expr, > 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. */ > - if (!coro_promise_type_found_p (current_function_decl, kw)) > + if (!coro_promise_type_found_p (current_function_decl, loc)) > return error_mark_node; > > /* [expr.await] 3.2 > @@ -1373,7 +1386,7 @@ finish_co_await_expr (location_t kw, tree expr) > 'await_transform()'. */ > tree at_meth > = lookup_promise_method (current_function_decl, > - coro_await_transform_identifier, kw, > + coro_await_transform_identifier, loc, > /*musthave=*/false); > if (at_meth == error_mark_node) > return error_mark_node; > @@ -1398,7 +1411,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, expr); > + return build_co_await (loc, a, CO_AWAIT_SUSPEND_POINT, expr); > } > > /* Take the EXPR given and attempt to build: > @@ -1406,13 +1419,13 @@ finish_co_await_expr (location_t kw, tree expr) > per [expr.yield] para 1. */ > > tree > -finish_co_yield_expr (location_t kw, tree expr) > +finish_co_yield_expr (location_t loc, tree expr) > { > if (!expr || error_operand_p (expr)) > return error_mark_node; > > /* Check the general requirements and simple syntax errors. */ > - if (!coro_common_keyword_context_valid_p (current_function_decl, kw, > + if (!coro_common_keyword_context_valid_p (current_function_decl, loc, > "co_yield")) > return error_mark_node; > > @@ -1426,11 +1439,11 @@ finish_co_yield_expr (location_t kw, tree expr) > suppress_warning (current_function_decl, OPT_Wreturn_type); > > /* Prepare for coroutine transformations. */ > - if (!ensure_coro_initialized (kw)) > + if (!ensure_coro_initialized (loc)) > return error_mark_node; > > /* Get our traits class. */ > - tree traits_class = coro_get_traits_class (current_function_decl, kw); > + tree traits_class = coro_get_traits_class (current_function_decl, loc); > > /* Defer expansion when we are processing a template; see note in > finish_co_await_expr. Also note that a yield is equivalent to > @@ -1440,9 +1453,9 @@ finish_co_yield_expr (location_t kw, tree expr) > If either p or EXPR are type-dependent, then the whole expression is > certainly type-dependent, and we can't proceed. */ > if (coro_dependent_p (expr, traits_class)) > - return build2_loc (kw, CO_YIELD_EXPR, NULL_TREE, expr, NULL_TREE); > + return build2_loc (loc, CO_YIELD_EXPR, NULL_TREE, expr, NULL_TREE); > > - if (!coro_promise_type_found_p (current_function_decl, kw)) > + if (!coro_promise_type_found_p (current_function_decl, loc)) > /* We must be able to look up the "yield_value" method in the scope of > the promise type, and obtain its return type. */ > return error_mark_node; > @@ -1455,7 +1468,7 @@ finish_co_yield_expr (location_t kw, tree expr) > vec<tree, va_gc> *args = make_tree_vector_single (expr); > tree yield_call > = coro_build_promise_expression (current_function_decl, NULL, > - coro_yield_value_identifier, kw, > + coro_yield_value_identifier, loc, > &args, /*musthave=*/true); > release_tree_vector (args); > > @@ -1463,7 +1476,7 @@ finish_co_yield_expr (location_t kw, tree expr) > Noting that for co_yield, there is no evaluation of any potential > promise transform_await(), so we call build_co_await directly. */ > > - tree op = build_co_await (kw, yield_call, CO_YIELD_SUSPEND_POINT); > + tree op = build_co_await (loc, yield_call, CO_YIELD_SUSPEND_POINT); > if (op != error_mark_node) > { > if (REFERENCE_REF_P (op)) > @@ -1474,11 +1487,11 @@ finish_co_yield_expr (location_t kw, tree expr) > if (TREE_CODE (op) == TARGET_EXPR) > { > tree t = TREE_OPERAND (op, 1); > - t = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (t), expr, t); > + t = build2_loc (loc, CO_YIELD_EXPR, TREE_TYPE (t), expr, t); > TREE_OPERAND (op, 1) = t; > } > else > - op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op); > + op = build2_loc (loc, CO_YIELD_EXPR, TREE_TYPE (op), expr, op); > TREE_SIDE_EFFECTS (op) = 1; > op = convert_from_reference (op); > } > @@ -1493,7 +1506,7 @@ finish_co_yield_expr (location_t kw, tree expr) > in with the actual frame version when the function is transformed. */ > > tree > -finish_co_return_stmt (location_t kw, tree expr) > +finish_co_return_stmt (location_t loc, tree expr) > { > if (expr) > STRIP_ANY_LOCATION_WRAPPER (expr); > @@ -1503,7 +1516,7 @@ finish_co_return_stmt (location_t kw, tree expr) > > /* If it fails the following test, the function is not permitted to be a > coroutine, so the co_return statement is erroneous. */ > - if (!coro_common_keyword_context_valid_p (current_function_decl, kw, > + if (!coro_common_keyword_context_valid_p (current_function_decl, loc, > "co_return")) > return error_mark_node; > > @@ -1518,11 +1531,11 @@ finish_co_return_stmt (location_t kw, tree expr) > suppress_warning (current_function_decl, OPT_Wreturn_type); > > /* Prepare for coroutine transformations. */ > - if (!ensure_coro_initialized (kw)) > + if (!ensure_coro_initialized (loc)) > return error_mark_node; > > /* Get our traits class. */ > - tree traits_class = coro_get_traits_class (current_function_decl, kw); > + tree traits_class = coro_get_traits_class (current_function_decl, loc); > > if (processing_template_decl > && check_for_bare_parameter_packs (expr)) > @@ -1534,13 +1547,13 @@ finish_co_return_stmt (location_t kw, tree expr) > { > /* co_return expressions are always void type, regardless of the > expression type. */ > - expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node, > + expr = build2_loc (loc, CO_RETURN_EXPR, void_type_node, > expr, NULL_TREE); > expr = maybe_cleanup_point_expr_void (expr); > return add_stmt (expr); > } > > - if (!coro_promise_type_found_p (current_function_decl, kw)) > + if (!coro_promise_type_found_p (current_function_decl, loc)) > return error_mark_node; > > /* Suppress -Wreturn-type for co_return, we need to check indirectly > @@ -1565,7 +1578,7 @@ finish_co_return_stmt (location_t kw, tree expr) > tree co_ret_call = error_mark_node; > if (expr == NULL_TREE || VOID_TYPE_P (TREE_TYPE (expr))) > co_ret_call > - = get_coroutine_return_void_expr (current_function_decl, kw, true); > + = get_coroutine_return_void_expr (current_function_decl, loc, true); > else > { > /* [class.copy.elision] / 3. > @@ -1583,17 +1596,17 @@ finish_co_return_stmt (location_t kw, tree expr) > releasing_vec args = make_tree_vector_single (arg); > co_ret_call > = coro_build_promise_expression (current_function_decl, NULL, > - coro_return_value_identifier, kw, > + coro_return_value_identifier, loc, > &args, /*musthave=*/true); > } > > /* Makes no sense for a co-routine really. */ > if (TREE_THIS_VOLATILE (current_function_decl)) > - warning_at (kw, 0, > + warning_at (loc, 0, > "function declared %<noreturn%> has a" > " %<co_return%> statement"); > > - expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node, expr, co_ret_call); > + expr = build2_loc (loc, CO_RETURN_EXPR, void_type_node, expr, co_ret_call); > expr = maybe_cleanup_point_expr_void (expr); > return add_stmt (expr); > } > diff --git a/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C > index 148ee4543e87..1e5d9e7a65a1 100644 > --- a/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C > +++ b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C > @@ -27,6 +27,7 @@ struct Coro { > auto final_suspend () noexcept { return coro::suspend_always{}; } > void return_void () { } > void unhandled_exception() { } > + auto yield_value (auto) noexcept { return coro::suspend_always{}; } > }; > }; > > @@ -34,10 +35,55 @@ extern int x; > > // Diagnose disallowed "return" in coroutine. > Coro > -bar () // { dg-error {a 'return' statement is not allowed} } > +bar () > { > if (x) > - return Coro(); > + return Coro(); // { dg-error {a 'return' statement is not allowed} } > else > - co_return; > + co_return; // { dg-note "function was made a coroutine here" } > +} > + > +Coro > +bar1 () > +{ > + if (x) > + return Coro(); // { dg-error {a 'return' statement is not allowed} } > + else > + co_await std::suspend_never{}; // { dg-note "function was made a coroutine here" } > +} > + > +Coro > +bar2 () > +{ > + if (x) > + return Coro(); // { dg-error {a 'return' statement is not allowed} } > + else > + co_yield 123; // { dg-note "function was made a coroutine here" } > +} > + > +Coro > +bar3 () > +{ > + if (x) > + co_return; // { dg-note "function was made a coroutine here" } > + else > + return Coro(); // { dg-error {a 'return' statement is not allowed} } > +} > + > +Coro > +bar4 () > +{ > + if (x) > + co_await std::suspend_never{}; // { dg-note "function was made a coroutine here" } > + else > + return Coro(); // { dg-error {a 'return' statement is not allowed} } > +} > + > +Coro > +bar5 () > +{ > + if (x) > + co_yield 123; // { dg-note "function was made a coroutine here" } > + else > + return Coro(); // { dg-error {a 'return' statement is not allowed} } > }
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index f7791cbfb9a6..81096784b4d7 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -93,8 +93,8 @@ struct GTY((for_user)) coroutine_info tree promise_proxy; /* Likewise, a proxy promise instance. */ tree from_address; /* handle_type from_address function. */ tree return_void; /* The expression for p.return_void() if it exists. */ - location_t first_coro_keyword; /* The location of the keyword that made this - function into a coroutine. */ + location_t first_coro_expr; /* The location of the expression that turned + this funtion into a coroutine. */ /* Flags to avoid repeated errors for per-function issues. */ bool coro_ret_type_error_emitted; bool coro_promise_error_emitted; @@ -285,7 +285,7 @@ static GTY(()) tree void_coro_handle_address; Lookup the coroutine_traits template decl. */ static tree -find_coro_traits_template_decl (location_t kw) +find_coro_traits_template_decl (location_t loc) { /* If we are missing fundamental information, such as the traits, (or the declaration found is not a type template), then don't emit an error for @@ -300,7 +300,7 @@ find_coro_traits_template_decl (location_t kw) { if (!traits_error_emitted) { - gcc_rich_location richloc (kw); + gcc_rich_location richloc (loc); error_at (&richloc, "coroutines require a traits template; cannot" " find %<%E::%E%>", std_node, coro_traits_identifier); inform (&richloc, "perhaps %<#include <coroutine>%> is missing"); @@ -315,7 +315,7 @@ find_coro_traits_template_decl (location_t kw) /* Instantiate Coroutine traits for the function signature. */ static tree -instantiate_coro_traits (tree fndecl, location_t kw) +instantiate_coro_traits (tree fndecl, location_t loc) { /* [coroutine.traits.primary] So now build up a type list for the template <typename _R, typename...>. @@ -358,7 +358,7 @@ instantiate_coro_traits (tree fndecl, location_t kw) if (traits_class == error_mark_node) { - error_at (kw, "cannot instantiate %<coroutine traits%>"); + error_at (loc, "cannot instantiate %<coroutine traits%>"); return NULL_TREE; } @@ -368,7 +368,7 @@ instantiate_coro_traits (tree fndecl, location_t kw) /* [coroutine.handle] */ static tree -find_coro_handle_template_decl (location_t kw) +find_coro_handle_template_decl (location_t loc) { /* As for the coroutine traits, this error is per TU, so only emit it once. */ @@ -380,7 +380,7 @@ find_coro_handle_template_decl (location_t kw) || !DECL_CLASS_TEMPLATE_P (handle_decl)) { if (!coro_handle_error_emitted) - error_at (kw, "coroutines require a handle class template;" + error_at (loc, "coroutines require a handle class template;" " cannot find %<%E::%E%>", std_node, coro_handle_identifier); coro_handle_error_emitted = true; return NULL_TREE; @@ -394,21 +394,21 @@ find_coro_handle_template_decl (location_t kw) void*. If that is not the case, signals an error and returns NULL_TREE. */ static tree -get_handle_type_address (location_t kw, tree handle_type) +get_handle_type_address (location_t loc, tree handle_type) { tree addr_getter = lookup_member (handle_type, coro_address_identifier, 1, 0, tf_warning_or_error); if (!addr_getter || addr_getter == error_mark_node) { qualified_name_lookup_error (handle_type, coro_address_identifier, - error_mark_node, kw); + error_mark_node, loc); return NULL_TREE; } if (!BASELINK_P (addr_getter) || TREE_CODE (TREE_TYPE (addr_getter)) != METHOD_TYPE) { - error_at (kw, "%qE must be a non-overloaded method", addr_getter); + error_at (loc, "%qE must be a non-overloaded method", addr_getter); return NULL_TREE; } @@ -421,14 +421,14 @@ get_handle_type_address (location_t kw, tree handle_type) /* Check that from_addr has the argument list (). */ if (arg != void_list_node) { - error_at (kw, "%qE must take no arguments", addr_getter); + error_at (loc, "%qE must take no arguments", addr_getter); return NULL_TREE; } tree ret_t = TREE_TYPE (fn_t); if (!same_type_p (ret_t, ptr_type_node)) { - error_at (kw, "%qE must return %qT, not %qT", + error_at (loc, "%qE must return %qT, not %qT", addr_getter, ptr_type_node, ret_t); return NULL_TREE; } @@ -442,20 +442,20 @@ get_handle_type_address (location_t kw, tree handle_type) NULL_TREE. */ static tree -get_handle_type_from_address (location_t kw, tree handle_type) +get_handle_type_from_address (location_t loc, tree handle_type) { tree from_addr = lookup_member (handle_type, coro_from_address_identifier, 1, 0, tf_warning_or_error); if (!from_addr || from_addr == error_mark_node) { qualified_name_lookup_error (handle_type, coro_from_address_identifier, - error_mark_node, kw); + error_mark_node, loc); return NULL_TREE; } if (!BASELINK_P (from_addr) || TREE_CODE (TREE_TYPE (from_addr)) != FUNCTION_TYPE) { - error_at (kw, "%qE must be a non-overloaded static function", from_addr); + error_at (loc, "%qE must be a non-overloaded static function", from_addr); return NULL_TREE; } @@ -466,14 +466,14 @@ get_handle_type_from_address (location_t kw, tree handle_type) || !same_type_p (TREE_VALUE (arg), ptr_type_node) || TREE_CHAIN (arg) != void_list_node) { - error_at (kw, "%qE must take a single %qT", from_addr, ptr_type_node); + error_at (loc, "%qE must take a single %qT", from_addr, ptr_type_node); return NULL_TREE; } tree ret_t = TREE_TYPE (fn_t); if (!same_type_p (ret_t, handle_type)) { - error_at (kw, "%qE must return %qT, not %qT", + error_at (loc, "%qE must return %qT, not %qT", from_addr, handle_type, ret_t); return NULL_TREE; } @@ -482,7 +482,7 @@ get_handle_type_from_address (location_t kw, tree handle_type) } static tree -instantiate_coro_handle_for_promise_type (location_t kw, tree promise_type) +instantiate_coro_handle_for_promise_type (location_t loc, tree promise_type) { /* So now build up a type list for the template, one entry, the promise. */ tree targ = make_tree_vec (1); @@ -495,7 +495,7 @@ instantiate_coro_handle_for_promise_type (location_t kw, tree promise_type) if (handle_type == error_mark_node) { - error_at (kw, "cannot instantiate a %<coroutine handle%> for" + error_at (loc, "cannot instantiate a %<coroutine handle%> for" " promise type %qT", promise_type); return NULL_TREE; } @@ -674,8 +674,8 @@ coro_promise_type_found_p (tree fndecl, location_t loc) = build_lang_decl (VAR_DECL, coro_promise_id, coro_info->promise_type); - /* Note where we first saw a coroutine keyword. */ - coro_info->first_coro_keyword = loc; + /* Note where we were when we made this function into a coroutine. */ + coro_info->first_coro_expr = loc; } return true; @@ -968,11 +968,24 @@ coro_function_valid_p (tree fndecl) if (current_function_returns_value || current_function_returns_null) { - /* TODO: record or extract positions of returns (and the first coro - keyword) so that we can add notes to the diagnostic about where - the bad keyword is and what made the function into a coro. */ - error_at (f_loc, "a %<return%> statement is not allowed in coroutine;" - " did you mean %<co_return%>?"); + auto coro_info = get_coroutine_info (fndecl); + gcc_checking_assert (coro_info->first_coro_expr); + + walk_tree_fn retf = [] (tree* pt, int*, void*) + { + if (*pt && TREE_CODE (*pt) == RETURN_EXPR) + return *pt; + return NULL_TREE; + }; + + tree ret = cp_walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl), + retf, nullptr); + auto ret_loc = EXPR_LOCATION (ret); + auto_diagnostic_group diaggrp; + error_at (ret_loc, "a %<return%> statement is not allowed in coroutine;" + " did you mean %<co_return%>?"); + inform (coro_info->first_coro_expr, + "function was made a coroutine here"); return false; } @@ -1051,9 +1064,9 @@ coro_diagnose_throwing_final_aw_expr (tree expr) /* Build a co_await suitable for later expansion. */ tree -build_template_co_await_expr (location_t kw, tree type, tree expr, tree kind) +build_template_co_await_expr (location_t loc, tree type, tree expr, tree kind) { - tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, type, expr, + tree aw_expr = build5_loc (loc, CO_AWAIT_EXPR, type, expr, NULL_TREE, NULL_TREE, NULL_TREE, kind); TREE_SIDE_EFFECTS (aw_expr) = true; @@ -1326,12 +1339,12 @@ coro_dependent_p (tree expr, tree traits_class) } tree -finish_co_await_expr (location_t kw, tree expr) +finish_co_await_expr (location_t loc, tree expr) { if (!expr || error_operand_p (expr)) return error_mark_node; - if (!coro_common_keyword_context_valid_p (current_function_decl, kw, + if (!coro_common_keyword_context_valid_p (current_function_decl, loc, "co_await")) return error_mark_node; @@ -1345,11 +1358,11 @@ finish_co_await_expr (location_t kw, tree expr) suppress_warning (current_function_decl, OPT_Wreturn_type); /* Prepare for coroutine transformations. */ - if (!ensure_coro_initialized (kw)) + if (!ensure_coro_initialized (loc)) return error_mark_node; /* Get our traits class. */ - tree traits_class = coro_get_traits_class (current_function_decl, kw); + tree traits_class = coro_get_traits_class (current_function_decl, loc); /* 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 @@ -1360,12 +1373,12 @@ finish_co_await_expr (location_t kw, tree expr) and just return early with a NULL_TREE type (indicating that we cannot compute the type yet). */ if (coro_dependent_p (expr, traits_class)) - return build_template_co_await_expr (kw, NULL_TREE, expr, + return build_template_co_await_expr (loc, NULL_TREE, expr, 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. */ - if (!coro_promise_type_found_p (current_function_decl, kw)) + if (!coro_promise_type_found_p (current_function_decl, loc)) return error_mark_node; /* [expr.await] 3.2 @@ -1373,7 +1386,7 @@ finish_co_await_expr (location_t kw, tree expr) 'await_transform()'. */ tree at_meth = lookup_promise_method (current_function_decl, - coro_await_transform_identifier, kw, + coro_await_transform_identifier, loc, /*musthave=*/false); if (at_meth == error_mark_node) return error_mark_node; @@ -1398,7 +1411,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, expr); + return build_co_await (loc, a, CO_AWAIT_SUSPEND_POINT, expr); } /* Take the EXPR given and attempt to build: @@ -1406,13 +1419,13 @@ finish_co_await_expr (location_t kw, tree expr) per [expr.yield] para 1. */ tree -finish_co_yield_expr (location_t kw, tree expr) +finish_co_yield_expr (location_t loc, tree expr) { if (!expr || error_operand_p (expr)) return error_mark_node; /* Check the general requirements and simple syntax errors. */ - if (!coro_common_keyword_context_valid_p (current_function_decl, kw, + if (!coro_common_keyword_context_valid_p (current_function_decl, loc, "co_yield")) return error_mark_node; @@ -1426,11 +1439,11 @@ finish_co_yield_expr (location_t kw, tree expr) suppress_warning (current_function_decl, OPT_Wreturn_type); /* Prepare for coroutine transformations. */ - if (!ensure_coro_initialized (kw)) + if (!ensure_coro_initialized (loc)) return error_mark_node; /* Get our traits class. */ - tree traits_class = coro_get_traits_class (current_function_decl, kw); + tree traits_class = coro_get_traits_class (current_function_decl, loc); /* Defer expansion when we are processing a template; see note in finish_co_await_expr. Also note that a yield is equivalent to @@ -1440,9 +1453,9 @@ finish_co_yield_expr (location_t kw, tree expr) If either p or EXPR are type-dependent, then the whole expression is certainly type-dependent, and we can't proceed. */ if (coro_dependent_p (expr, traits_class)) - return build2_loc (kw, CO_YIELD_EXPR, NULL_TREE, expr, NULL_TREE); + return build2_loc (loc, CO_YIELD_EXPR, NULL_TREE, expr, NULL_TREE); - if (!coro_promise_type_found_p (current_function_decl, kw)) + if (!coro_promise_type_found_p (current_function_decl, loc)) /* We must be able to look up the "yield_value" method in the scope of the promise type, and obtain its return type. */ return error_mark_node; @@ -1455,7 +1468,7 @@ finish_co_yield_expr (location_t kw, tree expr) vec<tree, va_gc> *args = make_tree_vector_single (expr); tree yield_call = coro_build_promise_expression (current_function_decl, NULL, - coro_yield_value_identifier, kw, + coro_yield_value_identifier, loc, &args, /*musthave=*/true); release_tree_vector (args); @@ -1463,7 +1476,7 @@ finish_co_yield_expr (location_t kw, tree expr) Noting that for co_yield, there is no evaluation of any potential promise transform_await(), so we call build_co_await directly. */ - tree op = build_co_await (kw, yield_call, CO_YIELD_SUSPEND_POINT); + tree op = build_co_await (loc, yield_call, CO_YIELD_SUSPEND_POINT); if (op != error_mark_node) { if (REFERENCE_REF_P (op)) @@ -1474,11 +1487,11 @@ finish_co_yield_expr (location_t kw, tree expr) if (TREE_CODE (op) == TARGET_EXPR) { tree t = TREE_OPERAND (op, 1); - t = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (t), expr, t); + t = build2_loc (loc, CO_YIELD_EXPR, TREE_TYPE (t), expr, t); TREE_OPERAND (op, 1) = t; } else - op = build2_loc (kw, CO_YIELD_EXPR, TREE_TYPE (op), expr, op); + op = build2_loc (loc, CO_YIELD_EXPR, TREE_TYPE (op), expr, op); TREE_SIDE_EFFECTS (op) = 1; op = convert_from_reference (op); } @@ -1493,7 +1506,7 @@ finish_co_yield_expr (location_t kw, tree expr) in with the actual frame version when the function is transformed. */ tree -finish_co_return_stmt (location_t kw, tree expr) +finish_co_return_stmt (location_t loc, tree expr) { if (expr) STRIP_ANY_LOCATION_WRAPPER (expr); @@ -1503,7 +1516,7 @@ finish_co_return_stmt (location_t kw, tree expr) /* If it fails the following test, the function is not permitted to be a coroutine, so the co_return statement is erroneous. */ - if (!coro_common_keyword_context_valid_p (current_function_decl, kw, + if (!coro_common_keyword_context_valid_p (current_function_decl, loc, "co_return")) return error_mark_node; @@ -1518,11 +1531,11 @@ finish_co_return_stmt (location_t kw, tree expr) suppress_warning (current_function_decl, OPT_Wreturn_type); /* Prepare for coroutine transformations. */ - if (!ensure_coro_initialized (kw)) + if (!ensure_coro_initialized (loc)) return error_mark_node; /* Get our traits class. */ - tree traits_class = coro_get_traits_class (current_function_decl, kw); + tree traits_class = coro_get_traits_class (current_function_decl, loc); if (processing_template_decl && check_for_bare_parameter_packs (expr)) @@ -1534,13 +1547,13 @@ finish_co_return_stmt (location_t kw, tree expr) { /* co_return expressions are always void type, regardless of the expression type. */ - expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node, + expr = build2_loc (loc, CO_RETURN_EXPR, void_type_node, expr, NULL_TREE); expr = maybe_cleanup_point_expr_void (expr); return add_stmt (expr); } - if (!coro_promise_type_found_p (current_function_decl, kw)) + if (!coro_promise_type_found_p (current_function_decl, loc)) return error_mark_node; /* Suppress -Wreturn-type for co_return, we need to check indirectly @@ -1565,7 +1578,7 @@ finish_co_return_stmt (location_t kw, tree expr) tree co_ret_call = error_mark_node; if (expr == NULL_TREE || VOID_TYPE_P (TREE_TYPE (expr))) co_ret_call - = get_coroutine_return_void_expr (current_function_decl, kw, true); + = get_coroutine_return_void_expr (current_function_decl, loc, true); else { /* [class.copy.elision] / 3. @@ -1583,17 +1596,17 @@ finish_co_return_stmt (location_t kw, tree expr) releasing_vec args = make_tree_vector_single (arg); co_ret_call = coro_build_promise_expression (current_function_decl, NULL, - coro_return_value_identifier, kw, + coro_return_value_identifier, loc, &args, /*musthave=*/true); } /* Makes no sense for a co-routine really. */ if (TREE_THIS_VOLATILE (current_function_decl)) - warning_at (kw, 0, + warning_at (loc, 0, "function declared %<noreturn%> has a" " %<co_return%> statement"); - expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node, expr, co_ret_call); + expr = build2_loc (loc, CO_RETURN_EXPR, void_type_node, expr, co_ret_call); expr = maybe_cleanup_point_expr_void (expr); return add_stmt (expr); } diff --git a/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C index 148ee4543e87..1e5d9e7a65a1 100644 --- a/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C +++ b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C @@ -27,6 +27,7 @@ struct Coro { auto final_suspend () noexcept { return coro::suspend_always{}; } void return_void () { } void unhandled_exception() { } + auto yield_value (auto) noexcept { return coro::suspend_always{}; } }; }; @@ -34,10 +35,55 @@ extern int x; // Diagnose disallowed "return" in coroutine. Coro -bar () // { dg-error {a 'return' statement is not allowed} } +bar () { if (x) - return Coro(); + return Coro(); // { dg-error {a 'return' statement is not allowed} } else - co_return; + co_return; // { dg-note "function was made a coroutine here" } +} + +Coro +bar1 () +{ + if (x) + return Coro(); // { dg-error {a 'return' statement is not allowed} } + else + co_await std::suspend_never{}; // { dg-note "function was made a coroutine here" } +} + +Coro +bar2 () +{ + if (x) + return Coro(); // { dg-error {a 'return' statement is not allowed} } + else + co_yield 123; // { dg-note "function was made a coroutine here" } +} + +Coro +bar3 () +{ + if (x) + co_return; // { dg-note "function was made a coroutine here" } + else + return Coro(); // { dg-error {a 'return' statement is not allowed} } +} + +Coro +bar4 () +{ + if (x) + co_await std::suspend_never{}; // { dg-note "function was made a coroutine here" } + else + return Coro(); // { dg-error {a 'return' statement is not allowed} } +} + +Coro +bar5 () +{ + if (x) + co_yield 123; // { dg-note "function was made a coroutine here" } + else + return Coro(); // { dg-error {a 'return' statement is not allowed} } }