diff mbox series

[2/2] cp+coroutines: teach convert_to_void to diagnose discarded co_awaits

Message ID 20240724011619.2013098-2-arsen@aarsen.me
State New
Headers show
Series [1/2] cp/coroutines: do not rewrite parameters in unevaluated contexts | expand

Commit Message

Arsen Arsenović July 23, 2024, 11:41 p.m. UTC 
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.

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

Comments

Jason Merrill July 24, 2024, 7:29 p.m. UTC | #1 
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?

> 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?

> +co_await_get_resume_call (tree await_expr)
> +{
> +  gcc_checking_assert (TREE_CODE (await_expr) == CO_AWAIT_EXPR);
> +  tree vec = TREE_OPERAND (await_expr, 3);
> +  if (!vec)
> +    return nullptr;
> +  return &TREE_VEC_ELT (vec, 2);
> +}
> +
> +
>   /* These functions assumes that the caller has verified that the state for
>      the decl has been initialized, we try to minimize work here.  */
>   
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 856699de82f..c9ae8950bd1 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8763,6 +8763,9 @@ extern tree coro_get_actor_function		(tree);
>   extern tree coro_get_destroy_function		(tree);
>   extern tree coro_get_ramp_function		(tree);
>   
> +extern tree* co_await_get_resume_call		(tree await_expr);
> +
> +
>   /* contracts.cc */
>   extern tree make_postcondition_variable		(cp_expr);
>   extern tree make_postcondition_variable		(cp_expr, tree);
> diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
> index d95e01c118c..7b4bd8a9dc4 100644
> --- a/gcc/cp/cvt.cc
> +++ b/gcc/cp/cvt.cc
> @@ -1502,6 +1502,14 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
>   	maybe_warn_nodiscard (expr, implicit);
>         break;
>   
> +    case CO_AWAIT_EXPR:
> +      {
> +	auto awr = co_await_get_resume_call (expr);
> +	if (awr && *awr)
> +	  *awr = convert_to_void (*awr, implicit, complain);
> +	break;
> +      }
> +
>       default:;
>       }
>     expr = resolve_nondeduced_context (expr, complain);
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110171-1.C b/gcc/testsuite/g++.dg/coroutines/pr110171-1.C
> new file mode 100644
> index 00000000000..d8aff582487
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr110171-1.C
> @@ -0,0 +1,34 @@
> +// { dg-do compile }
> +#include <coroutine>
> +
> +struct must_check_result
> +{
> +    bool await_ready() { return false; }
> +    void await_suspend(std::coroutine_handle<>) {}
> +    [[nodiscard]] bool await_resume() { return {}; }
> +};
> +
> +struct task {};
> +
> +namespace std
> +{
> +    template<typename... Args>
> +    struct coroutine_traits<task, Args...>
> +    {
> +        struct promise_type
> +        {
> +            task get_return_object() { return {}; }
> +            suspend_always initial_suspend() noexcept { return {}; }
> +            suspend_always final_suspend() noexcept { return {}; }
> +            void return_void() {}
> +            void unhandled_exception() {}
> +        };
> +    };
> +}
> +
> +task example(auto)
> +{
> +    co_await must_check_result(); // { dg-warning "-Wunused-result" }
> +}
> +
> +void f() { example(1); }
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110171.C b/gcc/testsuite/g++.dg/coroutines/pr110171.C
> new file mode 100644
> index 00000000000..4b82e23656c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr110171.C
> @@ -0,0 +1,32 @@
> +// { dg-do compile }
> +#include <coroutine>
> +
> +struct must_check_result
> +{
> +    bool await_ready() { return false; }
> +    void await_suspend(std::coroutine_handle<>) {}
> +    [[nodiscard]] bool await_resume() { return {}; }
> +};
> +
> +struct task {};
> +
> +namespace std
> +{
> +    template<typename... Args>
> +    struct coroutine_traits<task, Args...>
> +    {
> +        struct promise_type
> +        {
> +            task get_return_object() { return {}; }
> +            suspend_always initial_suspend() noexcept { return {}; }
> +            suspend_always final_suspend() noexcept { return {}; }
> +            void return_void() {}
> +            void unhandled_exception() {}
> +        };
> +    };
> +}
> +
> +task example()
> +{
> +    co_await must_check_result(); // { dg-warning "-Wunused-result" }
> +}
Arsen Arsenović July 24, 2024, 8:20 p.m. UTC | #2 
Hi Jason,

Thanks for the review.

Jason Merrill <jason@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);
	break;
      }

>> +co_await_get_resume_call (tree await_expr)
>> +{
>> +  gcc_checking_assert (TREE_CODE (await_expr) == CO_AWAIT_EXPR);
>> +  tree vec = TREE_OPERAND (await_expr, 3);
>> +  if (!vec)
>> +    return nullptr;
>> +  return &TREE_VEC_ELT (vec, 2);
>> +}
>> +
>> +
>>   /* These functions assumes that the caller has verified that the state for
>>      the decl has been initialized, we try to minimize work here.  */
>>   diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>> index 856699de82f..c9ae8950bd1 100644
>> --- a/gcc/cp/cp-tree.h
>> +++ b/gcc/cp/cp-tree.h
>> @@ -8763,6 +8763,9 @@ extern tree coro_get_actor_function		(tree);
>>   extern tree coro_get_destroy_function		(tree);
>>   extern tree coro_get_ramp_function		(tree);
>>   +extern tree* co_await_get_resume_call		(tree await_expr);
>> +
>> +
>>   /* contracts.cc */
>>   extern tree make_postcondition_variable		(cp_expr);
>>   extern tree make_postcondition_variable		(cp_expr, tree);
>> diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
>> index d95e01c118c..7b4bd8a9dc4 100644
>> --- a/gcc/cp/cvt.cc
>> +++ b/gcc/cp/cvt.cc
>> @@ -1502,6 +1502,14 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
>>   	maybe_warn_nodiscard (expr, implicit);
>>         break;
>>   +    case CO_AWAIT_EXPR:
>> +      {
>> +	auto awr = co_await_get_resume_call (expr);
>> +	if (awr && *awr)
>> +	  *awr = convert_to_void (*awr, implicit, complain);
>> +	break;
>> +      }
>> +
>>       default:;
>>       }
>>     expr = resolve_nondeduced_context (expr, complain);
>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110171-1.C b/gcc/testsuite/g++.dg/coroutines/pr110171-1.C
>> new file mode 100644
>> index 00000000000..d8aff582487
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/coroutines/pr110171-1.C
>> @@ -0,0 +1,34 @@
>> +// { dg-do compile }
>> +#include <coroutine>
>> +
>> +struct must_check_result
>> +{
>> +    bool await_ready() { return false; }
>> +    void await_suspend(std::coroutine_handle<>) {}
>> +    [[nodiscard]] bool await_resume() { return {}; }
>> +};
>> +
>> +struct task {};
>> +
>> +namespace std
>> +{
>> +    template<typename... Args>
>> +    struct coroutine_traits<task, Args...>
>> +    {
>> +        struct promise_type
>> +        {
>> +            task get_return_object() { return {}; }
>> +            suspend_always initial_suspend() noexcept { return {}; }
>> +            suspend_always final_suspend() noexcept { return {}; }
>> +            void return_void() {}
>> +            void unhandled_exception() {}
>> +        };
>> +    };
>> +}
>> +
>> +task example(auto)
>> +{
>> +    co_await must_check_result(); // { dg-warning "-Wunused-result" }
>> +}
>> +
>> +void f() { example(1); }
>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110171.C b/gcc/testsuite/g++.dg/coroutines/pr110171.C
>> new file mode 100644
>> index 00000000000..4b82e23656c
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/coroutines/pr110171.C
>> @@ -0,0 +1,32 @@
>> +// { dg-do compile }
>> +#include <coroutine>
>> +
>> +struct must_check_result
>> +{
>> +    bool await_ready() { return false; }
>> +    void await_suspend(std::coroutine_handle<>) {}
>> +    [[nodiscard]] bool await_resume() { return {}; }
>> +};
>> +
>> +struct task {};
>> +
>> +namespace std
>> +{
>> +    template<typename... Args>
>> +    struct coroutine_traits<task, Args...>
>> +    {
>> +        struct promise_type
>> +        {
>> +            task get_return_object() { return {}; }
>> +            suspend_always initial_suspend() noexcept { return {}; }
>> +            suspend_always final_suspend() noexcept { return {}; }
>> +            void return_void() {}
>> +            void unhandled_exception() {}
>> +        };
>> +    };
>> +}
>> +
>> +task example()
>> +{
>> +    co_await must_check_result(); // { dg-warning "-Wunused-result" }
>> +}
Jason Merrill July 24, 2024, 8:47 p.m. UTC | #3 
On 7/24/24 4:20 PM, Arsen Arsenović wrote:
> Hi Jason,
> 
> Thanks for the review.
> 
> Jason Merrill <jason@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
Arsen Arsenović July 24, 2024, 8:52 p.m. UTC | #4 
Jason Merrill <jason@redhat.com> writes:

> Ah, of course, I was overlooking the assignment.  The patch is OK.

Thanks.  Here's a range diff with a few changes to the commits, chiefly
in the commit messages.  If you agree, I can push with these changes
applied:

1:  32f810cca55 ! 1:  d2e74525965 cp/coroutines: do not rewrite parameters in unevaluated contexts
    @@ Commit message
         parameters.  Prevent this by simply skipping rewrites in unevaluated
         contexts.  Those won't mind the value not being present anyway.
     
    +    This prevents an ICE during parameter substitution.  In the testcase
    +    from the PR, the rewriting machinery finds a param in the body of the
    +    coroutine, which it did not previously encounter while processing the
    +    coroutine declaration, and that does not have a DECL_VALUE_EXPR, and
    +    fails.
    +
         gcc/cp/ChangeLog:
     
                 PR c++/111728
    @@ gcc/cp/coroutines.cc: rewrite_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_
      
     +  if (unevaluated_p (TREE_CODE (*stmt)))
     +    {
    -+      *do_subtree = 0; // Nothing to do.
    ++      /* No odr-uses in unevaluated operands.  */
    ++      *do_subtree = 0;
     +      return NULL_TREE;
     +    }
     +
2:  a16f1d34047 ! 2:  adc77c732f5 cp+coroutines: teach convert_to_void to diagnose discarded co_awaits
    @@ Commit message
         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.
    +    This patch teaches convert_to_void how to discard 'through' a
    +    CO_AWAIT_EXPR. When we discard a CO_AWAIT_EXPR, we can also just discard
    +    the await_resume() call conveniently embedded within it.  This results
    +    in a [[nodiscard]] diagnostic that the PR noted was missing.
     
         gcc/cp/ChangeLog:
     
Thanks again, have a lovely evening.
Jason Merrill July 25, 2024, 2:01 p.m. UTC | #5 
On 7/24/24 4:52 PM, Arsen Arsenović wrote:
> Jason Merrill <jason@redhat.com> writes:
> 
>> Ah, of course, I was overlooking the assignment.  The patch is OK.
> 
> Thanks.  Here's a range diff with a few changes to the commits, chiefly
> in the commit messages.  If you agree, I can push with these changes
> applied:

Looks good, thanks.

> 1:  32f810cca55 ! 1:  d2e74525965 cp/coroutines: do not rewrite parameters in unevaluated contexts
>      @@ Commit message
>           parameters.  Prevent this by simply skipping rewrites in unevaluated
>           contexts.  Those won't mind the value not being present anyway.
>       
>      +    This prevents an ICE during parameter substitution.  In the testcase
>      +    from the PR, the rewriting machinery finds a param in the body of the
>      +    coroutine, which it did not previously encounter while processing the
>      +    coroutine declaration, and that does not have a DECL_VALUE_EXPR, and
>      +    fails.
>      +
>           gcc/cp/ChangeLog:
>       
>                   PR c++/111728
>      @@ gcc/cp/coroutines.cc: rewrite_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_
>        
>       +  if (unevaluated_p (TREE_CODE (*stmt)))
>       +    {
>      -+      *do_subtree = 0; // Nothing to do.
>      ++      /* No odr-uses in unevaluated operands.  */
>      ++      *do_subtree = 0;
>       +      return NULL_TREE;
>       +    }
>       +
> 2:  a16f1d34047 ! 2:  adc77c732f5 cp+coroutines: teach convert_to_void to diagnose discarded co_awaits
>      @@ Commit message
>           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.
>      +    This patch teaches convert_to_void how to discard 'through' a
>      +    CO_AWAIT_EXPR. When we discard a CO_AWAIT_EXPR, we can also just discard
>      +    the await_resume() call conveniently embedded within it.  This results
>      +    in a [[nodiscard]] diagnostic that the PR noted was missing.
>       
>           gcc/cp/ChangeLog:
>       
> Thanks again, have a lovely evening.
diff mbox series

Patch

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*
+co_await_get_resume_call (tree await_expr)
+{
+  gcc_checking_assert (TREE_CODE (await_expr) == CO_AWAIT_EXPR);
+  tree vec = TREE_OPERAND (await_expr, 3);
+  if (!vec)
+    return nullptr;
+  return &TREE_VEC_ELT (vec, 2);
+}
+
+
 /* These functions assumes that the caller has verified that the state for
    the decl has been initialized, we try to minimize work here.  */
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 856699de82f..c9ae8950bd1 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8763,6 +8763,9 @@  extern tree coro_get_actor_function		(tree);
 extern tree coro_get_destroy_function		(tree);
 extern tree coro_get_ramp_function		(tree);
 
+extern tree* co_await_get_resume_call		(tree await_expr);
+
+
 /* contracts.cc */
 extern tree make_postcondition_variable		(cp_expr);
 extern tree make_postcondition_variable		(cp_expr, tree);
diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
index d95e01c118c..7b4bd8a9dc4 100644
--- a/gcc/cp/cvt.cc
+++ b/gcc/cp/cvt.cc
@@ -1502,6 +1502,14 @@  convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
 	maybe_warn_nodiscard (expr, implicit);
       break;
 
+    case CO_AWAIT_EXPR:
+      {
+	auto awr = co_await_get_resume_call (expr);
+	if (awr && *awr)
+	  *awr = convert_to_void (*awr, implicit, complain);
+	break;
+      }
+
     default:;
     }
   expr = resolve_nondeduced_context (expr, complain);
diff --git a/gcc/testsuite/g++.dg/coroutines/pr110171-1.C b/gcc/testsuite/g++.dg/coroutines/pr110171-1.C
new file mode 100644
index 00000000000..d8aff582487
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr110171-1.C
@@ -0,0 +1,34 @@ 
+// { dg-do compile }
+#include <coroutine>
+
+struct must_check_result
+{
+    bool await_ready() { return false; }
+    void await_suspend(std::coroutine_handle<>) {}
+    [[nodiscard]] bool await_resume() { return {}; }
+};
+
+struct task {};
+
+namespace std
+{
+    template<typename... Args>
+    struct coroutine_traits<task, Args...>
+    {
+        struct promise_type
+        {
+            task get_return_object() { return {}; }
+            suspend_always initial_suspend() noexcept { return {}; }
+            suspend_always final_suspend() noexcept { return {}; }
+            void return_void() {}
+            void unhandled_exception() {}
+        };
+    };
+}
+
+task example(auto)
+{
+    co_await must_check_result(); // { dg-warning "-Wunused-result" }
+}
+
+void f() { example(1); }
diff --git a/gcc/testsuite/g++.dg/coroutines/pr110171.C b/gcc/testsuite/g++.dg/coroutines/pr110171.C
new file mode 100644
index 00000000000..4b82e23656c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr110171.C
@@ -0,0 +1,32 @@ 
+// { dg-do compile }
+#include <coroutine>
+
+struct must_check_result
+{
+    bool await_ready() { return false; }
+    void await_suspend(std::coroutine_handle<>) {}
+    [[nodiscard]] bool await_resume() { return {}; }
+};
+
+struct task {};
+
+namespace std
+{
+    template<typename... Args>
+    struct coroutine_traits<task, Args...>
+    {
+        struct promise_type
+        {
+            task get_return_object() { return {}; }
+            suspend_always initial_suspend() noexcept { return {}; }
+            suspend_always final_suspend() noexcept { return {}; }
+            void return_void() {}
+            void unhandled_exception() {}
+        };
+    };
+}
+
+task example()
+{
+    co_await must_check_result(); // { dg-warning "-Wunused-result" }
+}