On 8/7/24 7:31 PM, Arsen Arsenović wrote:
Enlargening the function-specific data block is not great.
Indeed, I think it would be better to search DECL_SAVED_TREE for a
RETURN_STMT once we've decided to give an error.
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?
The latter seems fine.
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} }
}
