From ed8198461735f9b5b3c2cbe50f9913690ce4b4ca Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Tue, 2 Mar 2021 10:12:58 +0000 Subject: [PATCH] coroutines : Avoid generating empty statements [PR96749]. In the compiler-only idiom: " a = (target expr creats temp, op uses temp) " the target expression variable needs to be promoted to a frame one (if the expression has a suspend point). However, the only uses of the var are in the second part of the compound expression - and we were creating an empty statement corresponding to the (now unused) first arm. This then produces the spurious warnings noted. Fixed by avoiding generation of a separate variable nest for isolated target expressions (or similarly isolated co_awaits used in a function call). gcc/cp/ChangeLog: PR c++/96749 * coroutines.cc (flatten_await_stmt): Allow for the case where a target expression variable only has uses in the second part of a compound expression. (maybe_promote_temps): Avoid emiting empty statements. gcc/testsuite/ChangeLog: PR c++/96749 * g++.dg/coroutines/pr96749-1.C: New test. * g++.dg/coroutines/pr96749-2.C: New test. --- gcc/cp/coroutines.cc | 64 ++++++++++++++++++++--------- gcc/testsuite/g++.dg/coroutines/pr96749-1.C | 42 +++++++++++++++++++ gcc/testsuite/g++.dg/coroutines/pr96749-2.C | 37 +++++++++++++++++ 3 files changed, 123 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr96749-1.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr96749-2.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index c5aeb66..7124315 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2955,7 +2955,9 @@ flatten_await_stmt (var_nest_node *n, hash_set *promoted, break; case TARGET_EXPR: { - /* We have a temporary; promote it. */ + /* We have a temporary; promote it, but allow for the idiom in code + generated by the compiler like + a = (target_expr produces temp, op uses temp). */ tree init = t; temps_used->add (init); tree var_type = TREE_TYPE (init); @@ -2976,20 +2978,35 @@ flatten_await_stmt (var_nest_node *n, hash_set *promoted, } else init = build2 (INIT_EXPR, var_type, var, init); - var_nest_node *ins - = new var_nest_node (var, init, n->prev, n); - /* We have to replace the target expr... */ - *v.entry = var; - /* ... and any uses of its var. */ - proxy_replace pr = {TREE_OPERAND (t, 0), var}; - cp_walk_tree (&n->init, replace_proxy, &pr, NULL); - /* Compiler-generated temporaries can also have uses in following - arms of compound expressions, which will be listed in 'replace_in' - if present. */ - if (replace_in) - cp_walk_tree (replace_in, replace_proxy, &pr, NULL); - flatten_await_stmt (ins, promoted, temps_used, NULL); - flatten_await_stmt (n, promoted, temps_used, NULL); + /* Simplify for the case that we have an init containing the temp + alone. */ + if (t == n->init && n->var == NULL_TREE) + { + n->var = var; + proxy_replace pr = {TREE_OPERAND (t, 0), var}; + cp_walk_tree (&init, replace_proxy, &pr, NULL); + n->init = init; + if (replace_in) + cp_walk_tree (replace_in, replace_proxy, &pr, NULL); + flatten_await_stmt (n, promoted, temps_used, NULL); + } + else + { + var_nest_node *ins + = new var_nest_node (var, init, n->prev, n); + /* We have to replace the target expr... */ + *v.entry = var; + /* ... and any uses of its var. */ + proxy_replace pr = {TREE_OPERAND (t, 0), var}; + cp_walk_tree (&n->init, replace_proxy, &pr, NULL); + /* Compiler-generated temporaries can also have uses in + following arms of compound expressions, which will be listed + in 'replace_in' if present. */ + if (replace_in) + cp_walk_tree (replace_in, replace_proxy, &pr, NULL); + flatten_await_stmt (ins, promoted, temps_used, NULL); + flatten_await_stmt (n, promoted, temps_used, NULL); + } return; } break; @@ -3178,7 +3195,6 @@ maybe_promote_temps (tree *stmt, void *d) gcc_checking_assert (root->next == NULL); tree vlist = NULL_TREE; var_nest_node *t = root; - gcc_checking_assert (!t->var); /* We build the bind scope expression from the bottom-up. EXPR_LIST holds the inner expression nest at the current cleanup level (becoming the final expression list when we've exhausted the @@ -3214,9 +3230,12 @@ maybe_promote_temps (tree *stmt, void *d) add_stmt (cl); /* push this onto the level above. */ } else if (expr_list) - add_stmt (expr_list); - else - gcc_unreachable (); + { + if (TREE_CODE (expr_list) != STATEMENT_LIST) + add_stmt (expr_list); + else if (!tsi_end_p (tsi_start (expr_list))) + add_stmt (expr_list); + } } else { @@ -3225,7 +3244,12 @@ maybe_promote_temps (tree *stmt, void *d) else finish_expr_stmt (t->init); if (expr_list) - add_stmt (expr_list); + { + if (TREE_CODE (expr_list) != STATEMENT_LIST) + add_stmt (expr_list); + else if (!tsi_end_p (tsi_start (expr_list))) + add_stmt (expr_list); + } } expr_list = pop_stmt_list (new_list); var_nest_node *old = t; diff --git a/gcc/testsuite/g++.dg/coroutines/pr96749-1.C b/gcc/testsuite/g++.dg/coroutines/pr96749-1.C new file mode 100644 index 0000000..941a64e --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr96749-1.C @@ -0,0 +1,42 @@ +// { dg-additional-options "-Wall" } + +#include + +template struct promise; +template struct task { + using promise_type = promise<_Tp>; + bool await_ready() noexcept { return false; } + void await_suspend(std::coroutine_handle<> awaiter) noexcept { m_a = awaiter; } + auto await_resume() { return _Tp(); } + std::coroutine_handle<> m_a; +}; + +template struct promise { + auto get_return_object() noexcept { return task<_Tp>(); } + auto initial_suspend() noexcept { return std::suspend_always(); } + auto final_suspend() noexcept { return std::suspend_always(); } + void return_value(_Tp value) noexcept { m_v = value; } + void unhandled_exception() noexcept {} + _Tp m_v; +}; + +task test_coro(void) { + int r = 0; +#if 1 + // this code causes the unexpected warning below + r += co_await task(); +#else + // this code causes no warning + auto b = co_await task(); + r += b; +#endif + co_return r; + // test1.cpp: In function ‘task test_coro(int)’: + // test1.cpp:36:1: warning: statement has no effect [-Wunused-value] + // 36 | } + // | ^ +} + +int main(void) { + return 0; +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr96749-2.C b/gcc/testsuite/g++.dg/coroutines/pr96749-2.C new file mode 100644 index 0000000..43052b5 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr96749-2.C @@ -0,0 +1,37 @@ +// { dg-additional-options "-Wall" } + +#include + +#if 1 +// with a struct, GCC emits "statement has no effect" +struct S {}; +#else +// no warning with built-in types +using S = int; +#endif + +S Func1(int); + +struct C { + auto operator co_await() { + struct Awaitable final { + bool await_ready() const { return true; } + std::coroutine_handle<> await_suspend(std::coroutine_handle<>) { return {}; } + int await_resume() { return 42; } + }; + return Awaitable{}; + } +}; + +struct Task { + struct promise_type { + auto initial_suspend() { return std::suspend_always{}; } + auto final_suspend() noexcept { return std::suspend_always{}; } + void get_return_object() {} + void unhandled_exception() {} + }; +}; + +Task Func2() { + Func1(co_await C()); +} -- 2.7.4