Hi Jason,

>>+      error_at (loc, "%sawaitable type %qT is not a structure",
>>+             extra, o_type);

>Generally identifiers should be incorporated with %qs, and relying on the %s 
>to provide a space doesn't seem very i8n-friendly.  Better, I think, to handle 
>the case with no identifier as a separate diagnostic.

Fixed.

>It looks like there's no test for the initial_suspend case?

Added one, retested on x86_64-darwin, powerpc64le linux, OK for trunk?
thanks,
Iain

--- 8< ---

At present, we can issue diagnostics about missing or malformed
awaiter or promise methods when we encounter their uses in the
body of a user's function.  We might then re-issue the same
diagnostics when processing the initial or final await expressions.

This change avoids such duplication, and also attempts to
identify issues with the initial or final expressions specifically
since diagnostics for those do not have any useful line number.

gcc/cp/ChangeLog:

        * coroutines.cc (build_co_await): Identify diagnostics
        for initial and final await expressions.
        (cp_coroutine_transform::wrap_original_function_body): Do
        not handle initial and final await expressions here ...
        (cp_coroutine_transform::apply_transforms): ... handle them
        here and avoid duplicate diagnostics.
        * coroutines.h: Declare inital and final await expressions
        in the transform class.

gcc/testsuite/ChangeLog:

        * g++.dg/coroutines/coro1-missing-await-method.C: Adjust for
        improved diagnostics.
        * g++.dg/coroutines/pr104051.C: Move to...
        * g++.dg/coroutines/pr104051-0.C: ...here.
        * g++.dg/coroutines/pr104051-1.C: New test.

Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
 gcc/cp/coroutines.cc                          | 24 +++++++++++++++----
 gcc/cp/coroutines.h                           |  3 +++
 .../coroutines/coro1-missing-await-method.C   |  4 ++--
 .../coroutines/{pr104051.C => pr104051-0.C}   |  2 +-
 gcc/testsuite/g++.dg/coroutines/pr104051-1.C  | 23 ++++++++++++++++++
 5 files changed, 49 insertions(+), 7 deletions(-)
 rename gcc/testsuite/g++.dg/coroutines/{pr104051.C => pr104051-0.C} (92%)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr104051-1.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index d482f52fefa..18c0a4812c4 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1277,8 +1277,14 @@ build_co_await (location_t loc, tree a, 
suspend_point_kind suspend_kind,
 
   if (TREE_CODE (o_type) != RECORD_TYPE)
     {
-      error_at (loc, "awaitable type %qT is not a structure",
-               o_type);
+      if (suspend_kind == FINAL_SUSPEND_POINT)
+       error_at (loc, "%qs awaitable type %qT is not a structure",
+                 "final_suspend()", o_type);
+      else if (suspend_kind == INITIAL_SUSPEND_POINT)
+       error_at (loc, "%qs awaitable type %qT is not a structure",
+                 "initial_suspend()", o_type);
+      else
+       error_at (loc, "awaitable type %qT is not a structure", o_type);
       return error_mark_node;
     }
 
@@ -4329,7 +4335,6 @@ cp_coroutine_transform::wrap_original_function_body ()
   /* Wrap the function body in a try {} catch (...) {} block, if exceptions
      are enabled.  */
   tree var_list = NULL_TREE;
-  tree initial_await = build_init_or_final_await (fn_start, false);
 
   /* [stmt.return.coroutine] / 3
      If p.return_void() is a valid expression, flowing off the end of a
@@ -4527,7 +4532,8 @@ cp_coroutine_transform::wrap_original_function_body ()
   zero_resume = build2_loc (fn_end, MODIFY_EXPR, act_des_fn_ptr_type,
                            resume_fn_ptr, zero_resume);
   finish_expr_stmt (zero_resume);
-  finish_expr_stmt (build_init_or_final_await (fn_end, true));
+  finish_expr_stmt (final_await);
+
   BIND_EXPR_BODY (update_body) = pop_stmt_list (BIND_EXPR_BODY (update_body));
   BIND_EXPR_VARS (update_body) = nreverse (var_list);
   BLOCK_VARS (top_block) = BIND_EXPR_VARS (update_body);
@@ -5266,6 +5272,16 @@ cp_coroutine_transform::apply_transforms ()
     = coro_build_actor_or_destroy_function (orig_fn_decl, act_des_fn_type,
                                            frame_ptr_type, false);
 
+  /* Avoid repeating diagnostics about promise or awaiter fails.  */
+  if (!seen_error ())
+    {
+      iloc_sentinel stable_input_loc (fn_start);
+      initial_await = build_init_or_final_await (fn_start, false);
+      input_location = fn_end;
+      if (initial_await && initial_await != error_mark_node)
+       final_await = build_init_or_final_await (fn_end, true);
+    }
+
   /* Transform the function body as per [dcl.fct.def.coroutine] / 5.  */
   wrap_original_function_body ();
 
diff --git a/gcc/cp/coroutines.h b/gcc/cp/coroutines.h
index cb5d5572733..7613e292c58 100644
--- a/gcc/cp/coroutines.h
+++ b/gcc/cp/coroutines.h
@@ -127,6 +127,9 @@ private:
   bool inline_p = false;
   bool valid_coroutine = false;
 
+  tree initial_await = error_mark_node;
+  tree final_await = error_mark_node;
+  
   void analyze_fn_parms ();
   void wrap_original_function_body ();
   bool build_ramp_function ();
diff --git a/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C 
b/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C
index 93b6159216f..c6a3188ee35 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C
@@ -7,13 +7,13 @@
 #include "coro1-ret-int-yield-int.h"
 
 coro1
-bar0 () // { dg-error {no member named 'await_suspend' in 
'coro1::suspend_always_prt'} }
+bar0 ()
 {
   co_await coro1::suspend_never_prt{}; // { dg-error {no member named 
'await_ready' in 'coro1::suspend_never_prt'} }
   co_yield 5; // { dg-error {no member named 'await_suspend' in 
'coro1::suspend_always_prt'} }
   co_await coro1::suspend_always_intprt(5); // { dg-error {no member named 
'await_resume' in 'coro1::suspend_always_intprt'} }
   co_return 0;
-} // { dg-error {no member named 'await_suspend' in 
'coro1::suspend_always_prt'} }
+}
 
 int main (int ac, char *av[]) {
   struct coro1 x0 = bar0 ();
diff --git a/gcc/testsuite/g++.dg/coroutines/pr104051.C 
b/gcc/testsuite/g++.dg/coroutines/pr104051-0.C
similarity index 92%
rename from gcc/testsuite/g++.dg/coroutines/pr104051.C
rename to gcc/testsuite/g++.dg/coroutines/pr104051-0.C
index cd69877361d..bf878b2acbb 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr104051.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr104051-0.C
@@ -27,4 +27,4 @@ template <typename T> struct task {
 task<std::vector<int>> foo() {
   while ((co_await foo()).empty())
     ;
-} // { dg-error {awaitable type 'bool' is not a structure} }
+} // { dg-error {'final_suspend\(\)' awaitable type 'bool' is not a structure} 
}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr104051-1.C 
b/gcc/testsuite/g++.dg/coroutines/pr104051-1.C
new file mode 100644
index 00000000000..35b5e2c700d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr104051-1.C
@@ -0,0 +1,23 @@
+// { dg-additional-options "-fsyntax-only" }
+// { dg-skip-if "requires hosted libstdc++ for vector" { ! hostedlib } }
+#include <coroutine>
+template <typename>
+struct promise {
+  auto get_return_object() {
+    return std::coroutine_handle<promise>::from_promise(*this);
+  }
+  auto initial_suspend() { return 42.0; }
+  auto final_suspend() noexcept { return true; }
+  void unhandled_exception();
+  void return_void ();
+};
+template <typename T> struct task {
+  using promise_type = promise<T>;
+  task(std::coroutine_handle<promise<T>>);
+  bool await_ready();
+  std::coroutine_handle<> await_suspend(std::coroutine_handle<>);
+  T await_resume();
+};
+task<double> foo() { // { dg-error {'initial_suspend\(\)' awaitable type 
'double' is not a structure} }
+  co_return;
+} 
-- 
2.39.2 (Apple Git-143)

Reply via email to