Re: [PATCH Coroutines 1/2] Add error messages for missing methods of awaitable class
在 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
* 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
* 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
* 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
* 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
Re: [PATCH Coroutines 1/2] Add error messages for missing methods of awaitable class
On 1/20/20 10:38 PM, JunMa wrote:
在 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
* 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
* 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.
thanks this is ok. Could you remove the space in '/*protect=*/ 1' and
similar? This appear to be one place where our coding style has fewer
spaces than expected!
nathan
--
Nathan Sidwell
Re: [PATCH Coroutines 1/2] Add error messages for missing methods of awaitable class
在 2020/1/21 下午8:06, Nathan Sidwell 写道:
On 1/20/20 10:38 PM, JunMa wrote:
在 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
* 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
* 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.
thanks this is ok. Could you remove the space in '/*protect=*/ 1' and
similar? This appear to be one place where our coding style has fewer
spaces than expected!
ok, I'll update it.
Regards
JunMa
nathan
