> On 11 Jun 2025, at 17:51, Jason Merrill <ja...@redhat.com> wrote:
>
> On 6/9/25 3:49 PM, Iain Sandoe wrote:
>> 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.
>
> Rather than print just "initial_suspend" or "final_suspend", you might %D the
> called promise member function?
I took a look at this, it seems we’d have to fish the information out of
the target expr (additionally there could be a co_await operator involved)
I think we can improve the informational additions to the coroutines
diagnostics in several places - perhaps take a pass through doing that
once the functionality issues are sorted out.
> Up to you, the patch is OK as is.
I applied as is, that makes some progress, and expect to revisit more
generally as above.
thanks
Iain
>
>> 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;
>> +}
>
