在 2020/1/21 上午9:31, JunMa 写道:
在 2020/1/20 下午11:49, Nathan Sidwell 写道:
On 1/20/20 12:18 AM, JunMa wrote:
Hi
This patch adds lookup_awaitable_member, it outputs error messages when any of the await_ready/suspend/resume functions are missing in awaitable class.

This patch also add some error check on return value of build_co_await since we
may failed to build co_await_expr.

Bootstrap and test on X86_64, is it OK?

Regards
JunMa

gcc/cp
2020-01-20  Jun Ma <ju...@linux.alibaba.com>

         * coroutines.cc (lookup_awaitable_member): Lookup an awaitable member.          (build_co_await): Use lookup_awaitable_member instead of lookup_member.          (finish_co_yield_expr, finish_co_await_expr): Add error check on return
         value of build_co_await.

gcc/testsuite
2020-01-20  Jun Ma <ju...@linux.alibaba.com>

         * g++.dg/coroutines/coro1-missing-await-method.C: New test.


1) you're mixing two distinct changes, the one about the error message and the one about changing error_mark_node.

   tree op = build_co_await (kw, a, CO_AWAIT_SUSPEND_POINT);
-  TREE_SIDE_EFFECTS (op) = 1;
-  SET_EXPR_LOCATION (op, kw);
-
+  if (op != error_mark_node)
+    {
+      TREE_SIDE_EFFECTS (op) = 1;
+      SET_EXPR_LOCATION (op, kw);
+    }
   return op;
 }

these two fragments are ok, as a separate patch.


ok, I'll split it.
+/* Lookup an Awaitable member, which should be await_ready, await_suspend
+   or await_resume  */
+
+static tree
+lookup_awaitable_member (tree await_type, tree member_id, location_t loc)
+{
+  tree aw_memb
+    = lookup_member (await_type, member_id,
+             /*protect*/ 1, /*want_type*/ 0, tf_warning_or_error);
fwiw '/*foo=*/expr' is the canonical form -- i know Iain wasn't consistent, and it looks like I may have missed a few places in review.

ok.
+  if (aw_memb == NULL_TREE || aw_memb == error_mark_node)
+    {
+      error_at (loc, "no member named %qE in %qT", member_id, await_type);
+      return error_mark_node;
+    }
+  return aw_memb;
+}

This looks wrong.  lookup_member is being passed tf_warning_or_error, so it should already be emitting a diagnostic, for the cases where something is found, but is ambiguous or whatever.  So, I think you only want a diagnostic here when you get NULL_TREE back.

you are right, I follow with same code of lookup_promise_member, I'll update both.

Regards
JunMa
nathan

Hi nathan,
attachment is the updated patch which add error messages for lookup awaitable member.

Regards
JunMa

gcc/cp
2020-01-21  Jun Ma <ju...@linux.alibaba.com>

         * coroutines.cc (lookup_awaitable_member): Lookup an awaitable member.          (lookup_promise_method): Emit diagnostic when get NULL_TREE back only.          (build_co_await): Use lookup_awaitable_member instead of lookup_member.

gcc/testsuite
2020-01-21  Jun Ma <ju...@linux.alibaba.com>

         * g++.dg/coroutines/coro1-missing-await-method.C: New test.
---
 gcc/cp/coroutines.cc                          | 36 ++++++++++++-------
 .../coroutines/coro1-missing-await-method.C   | 21 +++++++++++
 .../coroutines/coro1-ret-int-yield-int.h      |  9 +++++
 3 files changed, 53 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index d3aacd7ad3b..43f03f7c49a 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -496,8 +496,8 @@ lookup_promise_method (tree fndecl, tree member_id, 
location_t loc,
   tree promise = get_coroutine_promise_type (fndecl);
   tree pm_memb
     = lookup_member (promise, member_id,
-                    /*protect*/ 1, /*want_type*/ 0, tf_warning_or_error);
-  if (musthave && (pm_memb == NULL_TREE || pm_memb == error_mark_node))
+                    /*protect=*/ 1, /*want_type=*/ 0, tf_warning_or_error);
+  if (musthave && pm_memb == NULL_TREE)
     {
       error_at (loc, "no member named %qE in %qT", member_id, promise);
       return error_mark_node;
@@ -505,6 +505,23 @@ lookup_promise_method (tree fndecl, tree member_id, 
location_t loc,
   return pm_memb;
 }
 
+/* Lookup an Awaitable member, which should be await_ready, await_suspend
+   or await_resume.  */
+
+static tree
+lookup_awaitable_member (tree await_type, tree member_id, location_t loc)
+{
+  tree aw_memb
+    = lookup_member (await_type, member_id,
+                    /*protect=*/ 1, /*want_type=*/ 0, tf_warning_or_error);
+  if (aw_memb == NULL_TREE)
+    {
+      error_at (loc, "no member named %qE in %qT", member_id, await_type);
+      return error_mark_node;
+    }
+  return aw_memb;
+}
+
 /* Here we check the constraints that are common to all keywords (since the
    presence of a coroutine keyword makes the function into a coroutine).  */
 
@@ -643,25 +660,18 @@ build_co_await (location_t loc, tree a, 
suspend_point_kind suspend_kind)
 
   /* Check for required awaitable members and their types.  */
   tree awrd_meth
-    = lookup_member (o_type, coro_await_ready_identifier,
-                    /* protect */ 1, /*want_type=*/0, tf_warning_or_error);
-
+    = lookup_awaitable_member (o_type, coro_await_ready_identifier, loc);
   if (!awrd_meth || awrd_meth == error_mark_node)
     return error_mark_node;
-
   tree awsp_meth
-    = lookup_member (o_type, coro_await_suspend_identifier,
-                    /* protect */ 1, /*want_type=*/0, tf_warning_or_error);
-
+    = lookup_awaitable_member (o_type, coro_await_suspend_identifier, loc);
   if (!awsp_meth || awsp_meth == error_mark_node)
     return error_mark_node;
 
   /* The type of the co_await is the return type of the awaitable's
-     co_resume(), so we need to look that up.  */
+     await_resume, so we need to look that up.  */
   tree awrs_meth
-    = lookup_member (o_type, coro_await_resume_identifier,
-                    /* protect */ 1, /*want_type=*/0, tf_warning_or_error);
-
+    = lookup_awaitable_member (o_type, coro_await_resume_identifier, loc);
   if (!awrs_meth || awrs_meth == error_mark_node)
     return error_mark_node;
 
diff --git a/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C 
b/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C
new file mode 100644
index 00000000000..c1869e0654c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/coro1-missing-await-method.C
@@ -0,0 +1,21 @@
+//  { dg-additional-options "-fsyntax-only -w" }
+#include "coro.h"
+
+#define MISSING_AWAIT_READY
+#define MISSING_AWAIT_SUSPEND
+#define MISSING_AWAIT_RESUME
+#include "coro1-ret-int-yield-int.h"
+
+coro1
+bar0 () // { dg-error {no member named 'await_suspend' in 
'coro1::suspend_always_prt'} }
+{
+  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;
+}
+
+int main (int ac, char *av[]) {
+  struct coro1 x0 = bar0 ();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/coro1-ret-int-yield-int.h 
b/gcc/testsuite/g++.dg/coroutines/coro1-ret-int-yield-int.h
index b961755e472..abf625869fa 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro1-ret-int-yield-int.h
+++ b/gcc/testsuite/g++.dg/coroutines/coro1-ret-int-yield-int.h
@@ -27,14 +27,20 @@ struct coro1 {
   // Some awaitables to use in tests.
   // With progress printing for debug.
   struct suspend_never_prt {
+#ifdef MISSING_AWAIT_READY
+#else
   bool await_ready() const noexcept { return true; }
+#endif
   void await_suspend(handle_type) const noexcept { PRINT ("susp-never-susp");}
   void await_resume() const noexcept { PRINT ("susp-never-resume");}
   };
 
   struct  suspend_always_prt {
   bool await_ready() const noexcept { return false; }
+#ifdef MISSING_AWAIT_SUSPEND
+#else
   void await_suspend(handle_type) const noexcept { PRINT ("susp-always-susp");}
+#endif
   void await_resume() const noexcept { PRINT ("susp-always-resume");}
   ~suspend_always_prt() { PRINT ("susp-always-dtor"); }
   };
@@ -46,7 +52,10 @@ struct coro1 {
     ~suspend_always_intprt() {}
     bool await_ready() const noexcept { return false; }
     void await_suspend(coro::coroutine_handle<>) const noexcept { PRINT 
("susp-always-susp-intprt");}
+#ifdef MISSING_AWAIT_RESUME
+#else
     int await_resume() const noexcept { PRINT ("susp-always-resume-intprt"); 
return x;}
+#endif
   };
   
   /* This returns the square of the int that it was constructed with.  */
-- 
2.19.1.3.ge56e4f7

Reply via email to