On 7/24/24 4:20 PM, Arsen Arsenović wrote:
Hi Jason,
Thanks for the review.
Jason Merrill <ja...@redhat.com> writes:
On 7/23/24 7:41 PM, Arsen Arsenović wrote:
co_await expressions are nearly calls to Awaitable::await_resume, and,
as such, should inherit its nodiscard. A discarded co_await expression
should, hence, act as if its call to await_resume was discarded.
CO_AWAIT_EXPR trees do conveniently contain the expression for calling
await_resume in them, so we can discard it.
gcc/cp/ChangeLog:
PR c++/110171
* coroutines.cc (co_await_get_resume_call): New function.
Returns the await_resume expression of a given co_await.
* cp-tree.h (co_await_get_resume_call): New function.
* cvt.cc (convert_to_void): Handle CO_AWAIT_EXPRs and call
maybe_warn_nodiscard on their resume exprs.
gcc/testsuite/ChangeLog:
PR c++/110171
* g++.dg/coroutines/pr110171-1.C: New test.
* g++.dg/coroutines/pr110171.C: New test.
---
This patch teaches convert_to_void how to discard 'through' a
CO_AWAIT_EXPR. CO_AWAIT_EXPR nodes (most of the time) already contain
their relevant await_resume() call embedded within them, so, when we
discard a CO_AWAIT_EXPR, we can also just discard the await_resume()
call embedded within it. This results in a [[nodiscard]] diagnostic
that the PR noted was missing.
Again you have two different versions of the patch rationale?
The duplication is for no particular reason - I just have a habit of
always filling out the email body besides just the commit message. In
the future, I'll move most text into the commit message, and I'll merge
all discussion from both versions into the commit message and post them
for review before pushing (for this patch and the previous).
As with the previous patch, regression-tested on x86_64-pc-linux-gnu.
OK for trunk?
TIA.
gcc/cp/coroutines.cc | 13 ++++++++
gcc/cp/cp-tree.h | 3 ++
gcc/cp/cvt.cc | 8 +++++
gcc/testsuite/g++.dg/coroutines/pr110171-1.C | 34 ++++++++++++++++++++
gcc/testsuite/g++.dg/coroutines/pr110171.C | 32 ++++++++++++++++++
5 files changed, 90 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110171-1.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110171.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index fb8f24e6c61..05486c2fb19 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -596,6 +596,19 @@ coro_get_destroy_function (tree decl)
return NULL_TREE;
}
+/* Given a CO_AWAIT_EXPR AWAIT_EXPR, return its resume call. */
+
+tree*
Why return tree* rather than tree?
So that we can save the result of convert_to_void on the resume
expression back into the CO_AWAIT_EXPR:
case CO_AWAIT_EXPR:
{
auto awr = co_await_get_resume_call (expr);
if (awr && *awr)
*awr = convert_to_void (*awr, implicit, complain);
Ah, of course, I was overlooking the assignment. The patch is OK.
Jason
