Tested on x86_64-pc-linux-gnu. I have blinking tsan test results again,
but I think they're bogus (I'll re-test on physical hardware before
pushing if needed).
I'm somewhat curious of we should do a similar change WRT RETURN_EXPRs
in the C FE (currently, the C FE uses the operand location for its
RETURN_EXPR locations, or the return kw if missing, which I suspect is
why malloc-CWE-401-example.c fails in C today, which appears confirmed
by -Wsystem-headers de-suppressing it).
It also might be worth splitting this patch into two (change RETURN_EXPR
location, change diagnostic in coroutines.cc), now that I think about
it. I'll do that before pushing or sending a v3.
One thing to consider is the argument to finish_co_return_stmt. It is
called kw, but I changed its location to be stmt_loc. It is down the
line passed to coro_promise_type_found_p which saves it as
first_coro_keyword. I was thinking to maybe pass both the full
statement location (to use in the built expr) and kw location (to use
for first_coro_keyword). Iain, what do you think of that?
Otherwise, OK for trunk?
Thanks, have a lovely evening.
---------- >8 ----------
We now point out why a function is a coroutine, and produce better
location information for parsed RETURN_EXPRs.
gcc/cp/ChangeLog:
* coroutines.cc (coro_function_valid_p): Change how we diagnose
'return' statements in coroutines.
* cp-tree.h (finish_return_stmt): Add a location parameter
defaulting to input_location.
* semantics.cc (finish_return_stmt): Use the new stmt_loc
parameter in place of input_location.
* parser.cc (cp_parser_jump_statement): Improve return and
co_return locations so that they span their entire expressions.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/co-return-syntax-08-bad-return.C: Update to
match new diagnostic. Test more keyword combinations.
* c-c++-common/analyzer/inlining-4-multiline.c: Adjust locations
in diagnostics.
* c-c++-common/analyzer/malloc-paths-9-noexcept.c: Ditto.
* c-c++-common/analyzer/malloc-CWE-401-example.c: Accept the
warning on line 34 (fixed false negative).
---
gcc/cp/coroutines.cc | 23 ++++++--
gcc/cp/cp-tree.h | 2 +-
gcc/cp/parser.cc | 9 +++-
gcc/cp/semantics.cc | 4 +-
.../analyzer/inlining-4-multiline.c | 6 ---
.../analyzer/malloc-CWE-401-example.c | 1 +
.../analyzer/malloc-paths-9-noexcept.c | 10 ++--
.../co-return-syntax-08-bad-return.C | 52 +++++++++++++++++--
8 files changed, 83 insertions(+), 24 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 0f4dc42ec1c8..4e751b01eaba 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -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_keyword);
+
+ 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_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..46f53c363613 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7790,7 +7790,7 @@ extern void finish_while_stmt (tree);
extern tree begin_do_stmt (void);
extern void finish_do_body (tree);
extern void finish_do_stmt (tree, tree, bool, tree, bool);
-extern tree finish_return_stmt (tree);
+extern tree finish_return_stmt (tree, location_t =
input_location);
extern tree begin_for_scope (tree *);
extern tree begin_for_stmt (tree, tree);
extern void finish_init_stmt (tree);
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index eb102dea8299..8574299276a9 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14959,14 +14959,19 @@ cp_parser_jump_statement (cp_parser* parser, tree
&std_attrs)
set_musttail_on_return (expr, token->location, musttail_p);
}
+ /* A location spanning the whole statement (up to ';'). */
+ auto stmt_loc = make_location (token->location,
+ token->location,
+ input_location);
+
/* Build the return-statement, check co-return first, since type
deduction is not valid there. */
if (keyword == RID_CO_RETURN)
- statement = finish_co_return_stmt (token->location, expr);
+ statement = finish_co_return_stmt (stmt_loc, expr);
else if (FNDECL_USED_AUTO (current_function_decl) && in_discarded_stmt)
/* Don't deduce from a discarded return statement. */;
else
- statement = finish_return_stmt (expr);
+ statement = finish_return_stmt (expr, stmt_loc);
/* Look for the final `;'. */
cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
}
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 669da4ad9698..94445cca9b6a 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -1400,7 +1400,7 @@ finish_do_stmt (tree cond, tree do_stmt, bool ivdep, tree
unroll,
indicated. */
tree
-finish_return_stmt (tree expr)
+finish_return_stmt (tree expr, location_t stmt_loc)
{
tree r;
bool no_warning;
@@ -1423,7 +1423,7 @@ finish_return_stmt (tree expr)
verify_sequence_points (expr);
}
- r = build_stmt (input_location, RETURN_EXPR, expr);
+ r = build_stmt (stmt_loc, RETURN_EXPR, expr);
RETURN_EXPR_LOCAL_ADDR_P (r) = dangling;
if (no_warning)
suppress_warning (r, OPT_Wreturn_type);
diff --git a/gcc/testsuite/c-c++-common/analyzer/inlining-4-multiline.c
b/gcc/testsuite/c-c++-common/analyzer/inlining-4-multiline.c
index 5c971c581ae4..235b715cff96 100644
--- a/gcc/testsuite/c-c++-common/analyzer/inlining-4-multiline.c
+++ b/gcc/testsuite/c-c++-common/analyzer/inlining-4-multiline.c
@@ -109,15 +109,9 @@ outer (int flag)
|
'const char* inner(int)': event 5 (depth 3)
|
- |
- | #define NULL
- |
- | |
- | (5) ...to here
{ dg-end-multiline-output "" { target c++ } } */
/* { dg-begin-multiline-output "" }
| return NULL;
- | ^~~~
|
<-------------+
|
diff --git a/gcc/testsuite/c-c++-common/analyzer/malloc-CWE-401-example.c
b/gcc/testsuite/c-c++-common/analyzer/malloc-CWE-401-example.c
index cfb5e86260c1..659c38adf2f0 100644
--- a/gcc/testsuite/c-c++-common/analyzer/malloc-CWE-401-example.c
+++ b/gcc/testsuite/c-c++-common/analyzer/malloc-CWE-401-example.c
@@ -32,6 +32,7 @@ char* getBlock(int fd) {
if (read(fd, buf, BLOCK_SIZE) != BLOCK_SIZE) {
return NULL; /* TODO: should complain that "buf" is leaked on this path.
*/
+ // { dg-warning "CWE-401" "" { target c++ } {.-1} }
}
return buf;
}
diff --git a/gcc/testsuite/c-c++-common/analyzer/malloc-paths-9-noexcept.c
b/gcc/testsuite/c-c++-common/analyzer/malloc-paths-9-noexcept.c
index 57d25f436a08..156bfb2c5300 100644
--- a/gcc/testsuite/c-c++-common/analyzer/malloc-paths-9-noexcept.c
+++ b/gcc/testsuite/c-c++-common/analyzer/malloc-paths-9-noexcept.c
@@ -374,7 +374,7 @@ int test_3 (int x, int y)
{ dg-end-multiline-output "" { target c } } */
/* { dg-begin-multiline-output "" }
NN | return *ptr;
- | ^~~
+ | ^~~~~~~~~~~
'int test_3(int, int)': events 1-7
NN | int *ptr = (int *)malloc (sizeof (int));
| ~~~~~~~^~~~~~~~~~~~~~
@@ -400,8 +400,8 @@ int test_3 (int x, int y)
| (5) following 'false' branch (when 'y == 0')...
......
NN | return *ptr;
- | ~~~
- | |
- | (6) ...to here
- | (7) 'ptr' leaks here; was allocated at (1)
+ | ~~~~~~~~~~~
+ | | |
+ | | (6) ...to here
+ | (7) 'ptr' leaks here; was allocated at (1)
{ dg-end-multiline-output "" { target c++ } } */
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
