Enlargening the function-specific data block is not great. I've considered changing the location of RETURN_STMT expressions to cover everything from the return expression to input_location after parsing the returned expr. The result of that is:
test.cc:38:3: error: a ‘return’ statement is not allowed in coroutine; did you mean ‘co_return’? 38 | return {}; | ^~~~~~~~~ test.cc:37:3: note: function was made a coroutine here 37 | co_return; | ^~~~~~~~~ ... so, not bad, but I'm not sure how intrusive such a change would be (haven't tried the testsuite). The current patch produces: test.cc:36:3: error: a ‘return’ statement is not allowed in coroutine; did you mean ‘co_return’? 36 | return {}; | ^~~~~~ test.cc:35:3: note: function was made a coroutine here 35 | co_return; | ^~~~~~~~~ Is there a better location to use here or is the current (latter) one OK? I haven't managed to found a nicer existing one. We also can't stash it in coroutine_info because a function might not have that at time we parse a return. Tested on x86_64-pc-linux-gnu. Have a lovely evening. ---------- >8 ---------- We now point out why a function is a coroutine. gcc/cp/ChangeLog: * coroutines.cc (coro_function_valid_p): Change how we diagnose returning coroutines. * cp-tree.h (struct language_function): Add first_return_loc field. Tracks the location of the first return encountered during parsing. (current_function_first_return_loc): New macro. Expands to the current functions' first_return_loc. * parser.cc (cp_parser_jump_statement): If parsing a RID_RETURN, save its location to current_function_first_return_loc. gcc/testsuite/ChangeLog: * g++.dg/coroutines/co-return-syntax-08-bad-return.C: Update to match new diagnostic. --- gcc/cp/coroutines.cc | 14 +++-- gcc/cp/cp-tree.h | 6 +++ gcc/cp/parser.cc | 4 ++ .../co-return-syntax-08-bad-return.C | 52 +++++++++++++++++-- 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 0f4dc42ec1c8..f32c7a2eec8d 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -968,11 +968,15 @@ 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%>?"); + coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl); + auto retloc = current_function_first_return_loc; + gcc_checking_assert (retloc && coro_info->first_coro_keyword); + + auto_diagnostic_group diaggrp; + error_at (retloc, "a %<return%> statement is not allowed in coroutine;" + " did you mean %<co_return%>?"); + inform (coro_info->first_coro_keyword, + "function was made a coroutine here"); return false; } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 911d1d7924cc..68c681150a1f 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -2123,6 +2123,8 @@ struct GTY(()) language_function { tree x_vtt_parm; tree x_return_value; + location_t first_return_loc; + BOOL_BITFIELD returns_value : 1; BOOL_BITFIELD returns_null : 1; BOOL_BITFIELD returns_abnormally : 1; @@ -2217,6 +2219,10 @@ struct GTY(()) language_function { #define current_function_return_value \ (cp_function_chain->x_return_value) +/* Location of the first 'return' stumbled upon during parsing. */ + +#define current_function_first_return_loc cp_function_chain->first_return_loc + /* In parser.cc. */ extern tree cp_literal_operator_id (const char *); diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index eb102dea8299..6cfe42f3bdd6 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -14957,6 +14957,10 @@ cp_parser_jump_statement (cp_parser* parser, tree &std_attrs) AGGR_INIT_EXPR_MUST_TAIL (ret_expr) = musttail_p; else set_musttail_on_return (expr, token->location, musttail_p); + + /* Save where we saw this keyword. */ + if (current_function_first_return_loc == UNKNOWN_LOCATION) + current_function_first_return_loc = token->location; } /* Build the return-statement, check co-return first, since type 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} } } -- 2.45.2