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.

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index c5aeb66..7124315 100644 (file)
--- 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 (file)
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 (file)
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());
+}