From c7100843831147a034fe37d231c54ac53ceace45 Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Thu, 7 May 2020 19:48:31 +0100 Subject: [PATCH] coroutines: Improve error recovery [PR94817, PR94829]. When we have completely missing key information (e.g. the coroutine_traits) or a partially transformed function body, we need to try and balance returning useful information about failures with the possibility that some part of the diagnostics machinery or following code will not be able to handle the state. The PRs (and revised testcase) point to cases where that processing has failed. This revises the process to avoid special handling for the ramp, and falls back on the same code used for regular function fails. There are test-cases (in addition to the ones for the PRs) that now cover all early exit points [where the transforms are considered to have failed in a manner that does not allow compilation to continue]. gcc/cp/ChangeLog: 2020-05-07 Iain Sandoe PR c++/94817 PR c++/94829 * coroutines.cc (morph_fn_to_coro): Set unformed outline functions to error_mark_node. For early error returns suppress warnings about missing ramp return values. Fix reinstatement of the function body on pre-existing initial error. * decl.c (finish_function): Use the normal error path for fails in the ramp function, do not try to compile the helpers if the transform fails. gcc/testsuite/ChangeLog: 2020-05-07 Iain Sandoe PR c++/94817 PR c++/94829 * g++.dg/coroutines/coro-missing-final-suspend.C: New test. * g++.dg/coroutines/coro-missing-initial-suspend.C: New test. * g++.dg/coroutines/coro-missing-promise-yield.C: Check for continuation of compilation. * g++.dg/coroutines/coro-missing-promise.C: Likewise. * g++.dg/coroutines/coro-missing-ret-value.C: Likewise * g++.dg/coroutines/coro-missing-ret-void.C: Likewise * g++.dg/coroutines/coro-missing-ueh-3.C: Likewise * g++.dg/coroutines/pr94817.C: New test. * g++.dg/coroutines/pr94829.C: New test. --- gcc/cp/ChangeLog | 12 +++++ gcc/cp/coroutines.cc | 31 +++++++++--- gcc/cp/decl.c | 20 ++++---- gcc/testsuite/ChangeLog | 15 ++++++ .../coroutines/coro-missing-final-suspend.C | 19 +++++++ .../coroutines/coro-missing-initial-suspend.C | 19 +++++++ .../coroutines/coro-missing-promise-yield.C | 6 +++ .../g++.dg/coroutines/coro-missing-promise.C | 6 +++ .../coroutines/coro-missing-ret-value.C | 6 +++ .../g++.dg/coroutines/coro-missing-ret-void.C | 6 +++ .../g++.dg/coroutines/coro-missing-ueh-3.C | 6 +++ .../coroutines/coro1-ret-int-yield-int.h | 6 +++ gcc/testsuite/g++.dg/coroutines/pr94817.C | 10 ++++ gcc/testsuite/g++.dg/coroutines/pr94829.C | 49 +++++++++++++++++++ 14 files changed, 195 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/coro-missing-final-suspend.C create mode 100644 gcc/testsuite/g++.dg/coroutines/coro-missing-initial-suspend.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94817.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94829.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 031741e2264..0c19a6fe47c 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,15 @@ +2020-05-07 Iain Sandoe + + PR c++/94817 + PR c++/94829 + * coroutines.cc (morph_fn_to_coro): Set unformed outline + functions to error_mark_node. For early error returns suppress + warnings about missing ramp return values. Fix reinstatement + of the function body on pre-existing initial error. + * decl.c (finish_function): Use the normal error path for fails + in the ramp function, do not try to compile the helpers if the + transform fails. + 2020-05-07 Marek Polacek PR c++/94590 - Detect long double -> double narrowing. diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index fc319a58cc2..8f0f2d5960d 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -3540,14 +3540,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) { gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); + *resumer = error_mark_node; + *destroyer = error_mark_node; if (!coro_function_valid_p (orig)) - return false; + { + /* For early errors, we do not want a diagnostic about the missing + ramp return value, since the user cannot fix this - a 'return' is + not allowed in a coroutine. */ + TREE_NO_WARNING (orig) = true; + return false; + } /* We can't validly get here with an empty statement list, since there's no way for the FE to decide it's a coroutine in the absence of any code. */ tree fnbody = pop_stmt_list (DECL_SAVED_TREE (orig)); - if (fnbody == NULL_TREE) - return false; + gcc_checking_assert (fnbody != NULL_TREE); /* We don't have the locus of the opening brace - it's filled in later (and there doesn't really seem to be any easy way to get at it). @@ -3561,7 +3568,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) if (body_start == NULL_TREE || body_start == error_mark_node) { DECL_SAVED_TREE (orig) = push_stmt_list (); - append_to_statement_list (DECL_SAVED_TREE (orig), &fnbody); + append_to_statement_list (fnbody, &DECL_SAVED_TREE (orig)); + /* Suppress warnings about the missing return value. */ + TREE_NO_WARNING (orig) = true; return false; } @@ -3630,13 +3639,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) the coroutine promise. */ tree initial_await = build_init_or_final_await (fn_start, false); if (initial_await == error_mark_node) - return false; + { + /* Suppress warnings about the missing return value. */ + TREE_NO_WARNING (orig) = true; + return false; + } /* The type of the frame var for this is the type of its temp proxy. */ tree initial_suspend_type = TREE_TYPE (TREE_OPERAND (initial_await, 1)); tree final_await = build_init_or_final_await (fn_start, true); if (final_await == error_mark_node) - return false; + { + /* Suppress warnings about the missing return value. */ + TREE_NO_WARNING (orig) = true; + return false; + } /* The type of the frame var for this is the type of its temp proxy. */ tree final_suspend_type = TREE_TYPE (TREE_OPERAND (final_await, 1)); @@ -4219,6 +4236,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) { BIND_EXPR_BODY (ramp_bind) = pop_stmt_list (ramp_body); DECL_SAVED_TREE (orig) = newbody; + /* Suppress warnings about the missing return value. */ + TREE_NO_WARNING (orig) = true; return false; } diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index cc12e005154..73a06a60786 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -16886,6 +16886,7 @@ finish_function (bool inline_p) bool coro_p = flag_coroutines && !processing_template_decl && DECL_COROUTINE_P (fndecl); + bool coro_emit_helpers = false; /* When we get some parse errors, we can end up without a current_function_decl, so cope. */ @@ -16914,18 +16915,16 @@ finish_function (bool inline_p) if (coro_p) { - if (!morph_fn_to_coro (fndecl, &resumer, &destroyer)) - { - DECL_SAVED_TREE (fndecl) = pop_stmt_list (DECL_SAVED_TREE (fndecl)); - poplevel (1, 0, 1); - DECL_SAVED_TREE (fndecl) = error_mark_node; - return fndecl; - } + /* Only try to emit the coroutine outlined helper functions if the + transforms succeeded. Otherwise, treat errors in the same way as + a regular function. */ + coro_emit_helpers = morph_fn_to_coro (fndecl, &resumer, &destroyer); /* We should handle coroutine IFNs in middle end lowering. */ cfun->coroutine_component = true; - if (use_eh_spec_block (fndecl)) + /* Do not try to process the ramp's EH unless outlining succeeded. */ + if (coro_emit_helpers && use_eh_spec_block (fndecl)) finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fndecl)), current_eh_spec_block); @@ -17174,8 +17173,9 @@ finish_function (bool inline_p) && !DECL_OMP_DECLARE_REDUCTION_P (fndecl)) cp_genericize (fndecl); - /* Emit the resumer and destroyer functions now. */ - if (coro_p) + /* Emit the resumer and destroyer functions now, providing that we have + not encountered some fatal error. */ + if (coro_emit_helpers) { emit_coro_helper (resumer); emit_coro_helper (destroyer); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 6712c1dfbb2..61670c97729 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,18 @@ +2020-05-07 Iain Sandoe + + PR c++/94817 + PR c++/94829 + * g++.dg/coroutines/coro-missing-final-suspend.C: New test. + * g++.dg/coroutines/coro-missing-initial-suspend.C: New test. + * g++.dg/coroutines/coro-missing-promise-yield.C: Check for + continuation of compilation. + * g++.dg/coroutines/coro-missing-promise.C: Likewise. + * g++.dg/coroutines/coro-missing-ret-value.C: Likewise + * g++.dg/coroutines/coro-missing-ret-void.C: Likewise + * g++.dg/coroutines/coro-missing-ueh-3.C: Likewise + * g++.dg/coroutines/pr94817.C: New test. + * g++.dg/coroutines/pr94829.C: New test. + 2020-05-07 Marek Polacek PR c++/94590 - Detect long double -> double narrowing. diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-final-suspend.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-final-suspend.C new file mode 100644 index 00000000000..6a0878c1269 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-final-suspend.C @@ -0,0 +1,19 @@ +// { dg-additional-options "-fsyntax-only -w" } +#include "coro.h" + +// Check diagnostic return from missing promise initial suspend entry. + +#define MISSING_FINAL_SUSPEND +#include "coro1-ret-int-yield-int.h" + +coro1 +my_coro () // { dg-error {no member named 'final_suspend' in} } +{ + co_return 0; +} + +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-initial-suspend.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-initial-suspend.C new file mode 100644 index 00000000000..c5b88ff434a --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-initial-suspend.C @@ -0,0 +1,19 @@ +// { dg-additional-options "-fsyntax-only -w" } +#include "coro.h" + +// Check diagnostic return from missing promise initial suspend entry. + +#define MISSING_INITIAL_SUSPEND +#include "coro1-ret-int-yield-int.h" + +coro1 +my_coro () // { dg-error {no member named 'initial_suspend' in} } +{ + co_return 0; +} + +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise-yield.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise-yield.C index d489c3953ac..a85a4d54bbe 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise-yield.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise-yield.C @@ -27,6 +27,12 @@ bar () co_return 0; } +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; + int main (int ac, char *av[]) { MissingPromiseYield x = bar (); return 0; diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C index e75f2002db0..834cf1209cd 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C @@ -16,3 +16,9 @@ bar () co_yield 22; // { dg-error {unable to find the promise type for this coroutine} } co_return 0; } + +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-value.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-value.C index f238c4b9a35..f745fa9c852 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-value.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-value.C @@ -28,6 +28,12 @@ bar () co_return 6174; // { dg-error {no member named 'return_value' in} } } +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; + int main (int ac, char *av[]) { MissingRetValue x = bar (); return 0; diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-void.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-void.C index c9f84e59020..8639a9d4f76 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-void.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-ret-void.C @@ -28,6 +28,12 @@ bar () co_return; // { dg-error "no member named .return_void. in" } } +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; + int main (int ac, char *av[]) { MissingRetVoid x = bar (); return 0; diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-ueh-3.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-ueh-3.C index d775d8a630e..36bd7da4991 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-missing-ueh-3.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-ueh-3.C @@ -12,6 +12,12 @@ bar () co_return; } +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; + int main (int ac, char *av[]) { MissingUEH x = bar (); 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 67ac197fee4..7cf50525924 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 @@ -99,14 +99,20 @@ struct coro1 { return handle_type::from_promise (*this); } +#ifdef MISSING_INITIAL_SUSPEND +#else auto initial_suspend () { PRINT ("get initial_suspend (always)"); return suspend_always_prt{}; } +#endif +#ifdef MISSING_FINAL_SUSPEND +#else auto final_suspend () { PRINT ("get final_suspend (always)"); return suspend_always_prt{}; } +#endif #ifdef USE_AWAIT_TRANSFORM diff --git a/gcc/testsuite/g++.dg/coroutines/pr94817.C b/gcc/testsuite/g++.dg/coroutines/pr94817.C new file mode 100644 index 00000000000..8f667bb942b --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr94817.C @@ -0,0 +1,10 @@ + +void no_coroutine_traits() { + co_await 4; // { dg-error {coroutines require a traits template\; cannot find 'std::coroutine_traits'} } +} + +// check we have not messed up continuation of the compilation. +template +struct void_t_imp { + using type = void; +}; diff --git a/gcc/testsuite/g++.dg/coroutines/pr94829.C b/gcc/testsuite/g++.dg/coroutines/pr94829.C new file mode 100644 index 00000000000..289c2802273 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr94829.C @@ -0,0 +1,49 @@ + +namespace std::experimental { +template struct coroutine_traits { + using promise_type = typename R::promise_type; +}; + +template struct coroutine_handle; +template <> struct coroutine_handle { + static coroutine_handle from_address(void *) noexcept; + coroutine_handle() = default; + template + coroutine_handle(coroutine_handle) noexcept; +}; +template struct coroutine_handle : coroutine_handle { + coroutine_handle() = default; + static coroutine_handle from_address(void *) noexcept; +}; +} + +struct suspend_always { + bool await_ready() noexcept; + void await_suspend(std::experimental::coroutine_handle<>) noexcept; + void await_resume() noexcept; +}; + +struct Task { + struct promise_type { + Task get_return_object(); + void return_void() {} + suspend_always initial_suspend() noexcept; + suspend_always final_suspend() noexcept; + void unhandled_exception() noexcept; + }; +}; + +template auto SyncAwait(_AwrT &&A) { + if (!A.await_ready()) { + auto AwaitAsync = [&]() -> Task { + try { (void)(co_await A); } catch (...) {} // { dg-error {coroutines require a traits template; cannot find 'std::coroutine_traits'} } + }; + Task t = AwaitAsync(); + } + return A.await_resume(); +} + +void f() { + suspend_always test; + SyncAwait(test); +} -- 2.30.2