aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIain Sandoe <iain@sandoe.co.uk>2021-03-02 10:12:58 +0000
committerIain Sandoe <iain@sandoe.co.uk>2021-03-15 15:48:14 +0000
commited8198461735f9b5b3c2cbe50f9913690ce4b4ca (patch)
treebc006f141a585245c004dab651a040252b7998be
parent6f4b0ff2b1fbd58669ae130387c7535110300c52 (diff)
downloadgcc-ed8198461735f9b5b3c2cbe50f9913690ce4b4ca.zip
gcc-ed8198461735f9b5b3c2cbe50f9913690ce4b4ca.tar.gz
gcc-ed8198461735f9b5b3c2cbe50f9913690ce4b4ca.tar.bz2
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.
-rw-r--r--gcc/cp/coroutines.cc64
-rw-r--r--gcc/testsuite/g++.dg/coroutines/pr96749-1.C42
-rw-r--r--gcc/testsuite/g++.dg/coroutines/pr96749-2.C37
3 files changed, 123 insertions, 20 deletions
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<tree> *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<tree> *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 <coroutine>
+
+template <typename _Tp> struct promise;
+template <typename _Tp> 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 <typename _Tp> 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<int> test_coro(void) {
+ int r = 0;
+#if 1
+ // this code causes the unexpected warning below
+ r += co_await task<int>();
+#else
+ // this code causes no warning
+ auto b = co_await task<int>();
+ r += b;
+#endif
+ co_return r;
+ // test1.cpp: In function ‘task<int> 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 <coroutine>
+
+#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());
+}