From: Iain Sandoe <iain@sandoe.co.uk>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Nathan Sidwell <nathan@acm.org>
Subject: [PATCH] coroutines : Do not accept throwing final await expressions [PR95616].
Date: Thu, 4 Mar 2021 19:54:55 +0000 [thread overview]
Message-ID: <A02ED582-4D85-4C2D-ABC8-08EFB788D191@sandoe.co.uk> (raw)
Hi,
From the PR:
The wording of [dcl.fct.def.coroutine]/15 states:
* The expression co_await promise.final_suspend() shall not be
potentially-throwing ([except.spec]).
See http://eel.is/c++draft/dcl.fct.def.coroutine#15
and http://eel.is/c++draft/except.spec#6
ie. all of the following must be declared noexcept (if they form part of the await-expression):
- promise_type::final_suspend()
- finalSuspendObj.operator co_await()
- finalSuspendAwaiter.await_ready()
- finalSuspendAwaiter.await_suspend()
- finalSuspendAwaiter.await_resume()
- finalSuspedObj destructor
- finalSuspendAwaiter destructor
This implements the checks for these cases and rejects such code with
a diagnostic.
[ accepts invalid ]
tested on x86_64-darwin, x86_64-linux-gnu,
OK for master / 10.x?
thanks
Iain
gcc/cp/ChangeLog:
PR c++/95616
* coroutines.cc (coro_diagnose_throwing_fn): New helper.
(coro_diagnose_throwing_final_aw_expr): New helper.
(build_co_await): Diagnose throwing final await expression
components.
(build_init_or_final_await): Diagnose a throwing promise
final_suspend() call.
gcc/testsuite/ChangeLog:
PR c++/95616
* g++.dg/coroutines/pr95616-a.C: New test.
* g++.dg/coroutines/pr95616-b.C: New test.
* g++.dg/coroutines/pr95616-c.C: New test.
* g++.dg/coroutines/pr95616-d.C: New test.
* g++.dg/coroutines/pr95616-e.C: New test.
* g++.dg/coroutines/pr95616-f.C: New test.
* g++.dg/coroutines/pr95616-g.C: New test.
---
gcc/cp/coroutines.cc | 85 +++++++++++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95616-a.C | 51 +++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95616-b.C | 51 +++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95616-c.C | 51 +++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95616-d.C | 51 +++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95616-e.C | 51 +++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95616-f.C | 51 +++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95616-g.C | 51 +++++++++++++
8 files changed, 442 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95616-a.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95616-b.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95616-c.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95616-d.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95616-e.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95616-f.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95616-g.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index ca36c8b8b41..f79ac60dc77 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -793,6 +793,43 @@ get_awaitable_var (suspend_point_kind suspend_kind, tree v_type)
return ret;
}
+/* Helpers to diagnose missing noexcept on final await expressions. */
+
+static bool
+coro_diagnose_throwing_fn (tree fndecl)
+{
+ if (!TYPE_NOTHROW_P (TREE_TYPE (fndecl)))
+ {
+ location_t f_loc = cp_expr_loc_or_loc (fndecl,
+ DECL_SOURCE_LOCATION (fndecl));
+ error_at (f_loc, "the expression %qE is required to be non-throwing",
+ fndecl);
+ inform (f_loc, "must be declared with %<noexcept(true)%>");
+ return true;
+ }
+ return false;
+}
+
+static bool
+coro_diagnose_throwing_final_aw_expr (tree expr)
+{
+ tree t = TARGET_EXPR_INITIAL (expr);
+ tree fn = NULL_TREE;
+ if (TREE_CODE (t) == CALL_EXPR)
+ fn = CALL_EXPR_FN(t);
+ else if (TREE_CODE (t) == AGGR_INIT_EXPR)
+ fn = AGGR_INIT_EXPR_FN (t);
+ else if (TREE_CODE (t) == CONSTRUCTOR)
+ return false;
+ else
+ {
+ gcc_checking_assert (0 && "unhandled expression type");
+ return false;
+ }
+ fn = TREE_OPERAND (fn, 0);
+ return coro_diagnose_throwing_fn (fn);
+}
+
/* This performs [expr.await] bullet 3.3 and validates the interface obtained.
It is also used to build the initial and final suspend points.
@@ -815,6 +852,28 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
/* If no viable functions are found, o is a. */
if (!o || o == error_mark_node)
o = a;
+ else if (suspend_kind == FINAL_SUSPEND_POINT)
+ {
+ /* We found an overload for co_await(), diagnose throwing cases. */
+ if (TREE_CODE (o) == TARGET_EXPR
+ && coro_diagnose_throwing_final_aw_expr (o))
+ return error_mark_node;
+
+ /* We now know that the final suspend object is distinct from the
+ final awaiter, so check for a non-throwing DTOR where needed. */
+ tree a_type = TREE_TYPE (a);
+ if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (a_type))
+ {
+ tree dummy
+ = build_special_member_call (a, complete_dtor_identifier,
+ NULL, a_type, LOOKUP_NORMAL,
+ tf_none);
+ dummy = dummy ? TREE_OPERAND (CALL_EXPR_FN (dummy), 0)
+ : NULL_TREE;
+ if (dummy && coro_diagnose_throwing_fn (dummy))
+ return error_mark_node;
+ }
+ }
}
else
o = a; /* This is most likely about to fail anyway. */
@@ -958,6 +1017,27 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
if (!awrs_func || !awrs_call || awrs_call == error_mark_node)
return error_mark_node;
+ if (suspend_kind == FINAL_SUSPEND_POINT)
+ {
+ if (coro_diagnose_throwing_fn (awrd_func))
+ return error_mark_node;
+ if (coro_diagnose_throwing_fn (awsp_func))
+ return error_mark_node;
+ if (coro_diagnose_throwing_fn (awrs_func))
+ return error_mark_node;
+ if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (o_type))
+ {
+ tree dummy
+ = build_special_member_call (e_proxy, complete_dtor_identifier,
+ NULL, o_type, LOOKUP_NORMAL,
+ tf_none);
+ dummy = dummy ? TREE_OPERAND (CALL_EXPR_FN (dummy), 0)
+ : NULL_TREE;
+ if (dummy && coro_diagnose_throwing_fn (dummy))
+ return error_mark_node;
+ }
+ }
+
/* We now have three call expressions, in terms of the promise, handle and
'e' proxies. Save them in the await expression for later expansion. */
@@ -2538,6 +2618,11 @@ build_init_or_final_await (location_t loc, bool is_final)
= coro_build_promise_expression (current_function_decl, NULL, suspend_alt,
loc, NULL, /*musthave=*/true);
+ /* Check for noexcept on the final_suspend call. */
+ if (is_final && setup_call != error_mark_node
+ && coro_diagnose_throwing_final_aw_expr (setup_call))
+ return error_mark_node;
+
/* So build the co_await for this */
/* For initial/final suspends the call is "a" per [expr.await] 3.2. */
return build_co_await (loc, setup_call, (is_final ? FINAL_SUSPEND_POINT
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95616-a.C b/gcc/testsuite/g++.dg/coroutines/pr95616-a.C
new file mode 100644
index 00000000000..e500b6ea636
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95616-a.C
@@ -0,0 +1,51 @@
+// { dg-additional-options "-fsyntax-only" }
+
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std {
+ using namespace std::experimental;
+}
+#endif
+
+class promise;
+
+struct finalSuspendAwaiter {
+ int x;
+ finalSuspendAwaiter () : x(0) { }
+ finalSuspendAwaiter (int _x) : x(_x) { }
+ ~finalSuspendAwaiter() noexcept(true) { }
+ bool await_ready() const noexcept(true) { return false; }
+ void await_suspend(std::coroutine_handle<>) const noexcept(true) { }
+ int await_resume() const noexcept(true) { return x; }
+};
+
+struct finalSuspendObj {
+ int x;
+ finalSuspendObj () : x(0) { }
+ finalSuspendObj (int _x) : x(_x) { }
+ ~finalSuspendObj () noexcept(true) {}
+
+ finalSuspendAwaiter operator co_await() const & noexcept(true) {
+ return {x};
+ }
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+
+ finalSuspendObj final_suspend() { return {3}; } // NOTE: not declared noexcept
+ // { dg-error {the expression 'task::promise_type::final_suspend' is required to be non-throwing} "" { target *-*-* } .-1 }
+
+ void return_void() noexcept {}
+ void unhandled_exception() noexcept {}
+ };
+};
+
+// This should be ill-formed since final_suspend() is potentially throwing.
+task f() {
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95616-b.C b/gcc/testsuite/g++.dg/coroutines/pr95616-b.C
new file mode 100644
index 00000000000..8d0d0b318ca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95616-b.C
@@ -0,0 +1,51 @@
+// { dg-additional-options "-fsyntax-only" }
+
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std {
+ using namespace std::experimental;
+}
+#endif
+
+class promise;
+
+struct finalSuspendAwaiter {
+ int x;
+ finalSuspendAwaiter () : x(0) { }
+ finalSuspendAwaiter (int _x) : x(_x) { }
+ ~finalSuspendAwaiter() noexcept(false) { }
+ bool await_ready() const noexcept(false) { return false; }
+ void await_suspend(std::coroutine_handle<>) const noexcept(false) { }
+ int await_resume() const noexcept(false) { return x; }
+};
+
+struct finalSuspendObj {
+ int x;
+ finalSuspendObj () : x(0) { }
+ finalSuspendObj (int _x) : x(_x) { }
+ ~finalSuspendObj () noexcept(false) {}
+
+ finalSuspendAwaiter operator co_await() const & noexcept(false) {
+ // { dg-error {the expression 'finalSuspendObj::operator co_await' is required to be non-throwing} "" { target *-*-* } .-1 }
+ return {x};
+ }
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+
+ finalSuspendObj final_suspend() noexcept { return {3}; }
+
+ void return_void() noexcept {}
+ void unhandled_exception() noexcept {}
+ };
+};
+
+// This should be ill-formed since final_suspend() is potentially throwing.
+task f() {
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95616-c.C b/gcc/testsuite/g++.dg/coroutines/pr95616-c.C
new file mode 100644
index 00000000000..6ad251986ec
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95616-c.C
@@ -0,0 +1,51 @@
+// { dg-additional-options "-fsyntax-only" }
+
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std {
+ using namespace std::experimental;
+}
+#endif
+
+class promise;
+
+struct finalSuspendAwaiter {
+ int x;
+ finalSuspendAwaiter () : x(0) { }
+ finalSuspendAwaiter (int _x) : x(_x) { }
+ ~finalSuspendAwaiter() noexcept(false) { }
+ bool await_ready() const noexcept(false) { return false; }
+ void await_suspend(std::coroutine_handle<>) const noexcept(false) { }
+ int await_resume() const noexcept(false) { return x; }
+};
+
+struct finalSuspendObj {
+ int x;
+ finalSuspendObj () : x(0) { }
+ finalSuspendObj (int _x) : x(_x) { }
+ ~finalSuspendObj () noexcept(false) {}
+ // { dg-error {the expression 'finalSuspendObj::~finalSuspendObj' is required to be non-throwing} "" { target *-*-* } .-1 }
+
+ finalSuspendAwaiter operator co_await() const & noexcept(true) {
+ return {x};
+ }
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+
+ finalSuspendObj final_suspend() noexcept { return {3}; } // NOTE: not declared noexcept
+
+ void return_void() noexcept {}
+ void unhandled_exception() noexcept {}
+ };
+};
+
+// This should be ill-formed since final_suspend() is potentially throwing.
+task f() {
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95616-d.C b/gcc/testsuite/g++.dg/coroutines/pr95616-d.C
new file mode 100644
index 00000000000..7da1f6a9658
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95616-d.C
@@ -0,0 +1,51 @@
+// { dg-additional-options "-fsyntax-only" }
+
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std {
+ using namespace std::experimental;
+}
+#endif
+
+class promise;
+
+struct finalSuspendAwaiter {
+ int x;
+ finalSuspendAwaiter () : x(0) { }
+ finalSuspendAwaiter (int _x) : x(_x) { }
+ ~finalSuspendAwaiter() noexcept(false) { }
+ bool await_ready() const noexcept(false) { return false; }
+ // { dg-error {the expression 'finalSuspendAwaiter::await_ready' is required to be non-throwing} "" { target *-*-* } .-1 }
+ void await_suspend(std::coroutine_handle<>) const noexcept(false) { }
+ int await_resume() const noexcept(false) { return x; }
+};
+
+struct finalSuspendObj {
+ int x;
+ finalSuspendObj () : x(0) { }
+ finalSuspendObj (int _x) : x(_x) { }
+ ~finalSuspendObj () noexcept(true) {}
+
+ finalSuspendAwaiter operator co_await() const & noexcept(true) {
+ return {x};
+ }
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+
+ finalSuspendObj final_suspend() noexcept { return {3}; }
+
+ void return_void() noexcept {}
+ void unhandled_exception() noexcept {}
+ };
+};
+
+// This should be ill-formed since final_suspend() is potentially throwing.
+task f() {
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95616-e.C b/gcc/testsuite/g++.dg/coroutines/pr95616-e.C
new file mode 100644
index 00000000000..ef6a160a5c4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95616-e.C
@@ -0,0 +1,51 @@
+// { dg-additional-options "-fsyntax-only" }
+
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std {
+ using namespace std::experimental;
+}
+#endif
+
+class promise;
+
+struct finalSuspendAwaiter {
+ int x;
+ finalSuspendAwaiter () : x(0) { }
+ finalSuspendAwaiter (int _x) : x(_x) { }
+ ~finalSuspendAwaiter() noexcept(false) { }
+ bool await_ready() const noexcept(true) { return false; }
+ void await_suspend(std::coroutine_handle<>) const noexcept(false) { }
+ // { dg-error {the expression 'finalSuspendAwaiter::await_suspend' is required to be non-throwing} "" { target *-*-* } .-1 }
+ int await_resume() const noexcept(false) { return x; }
+};
+
+struct finalSuspendObj {
+ int x;
+ finalSuspendObj () : x(0) { }
+ finalSuspendObj (int _x) : x(_x) { }
+ ~finalSuspendObj () noexcept(true) {}
+
+ finalSuspendAwaiter operator co_await() const & noexcept(true) {
+ return {x};
+ }
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+
+ finalSuspendObj final_suspend() noexcept { return {3}; }
+
+ void return_void() noexcept {}
+ void unhandled_exception() noexcept {}
+ };
+};
+
+// This should be ill-formed since final_suspend() is potentially throwing.
+task f() {
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95616-f.C b/gcc/testsuite/g++.dg/coroutines/pr95616-f.C
new file mode 100644
index 00000000000..930c1a7e6a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95616-f.C
@@ -0,0 +1,51 @@
+// { dg-additional-options "-fsyntax-only" }
+
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std {
+ using namespace std::experimental;
+}
+#endif
+
+class promise;
+
+struct finalSuspendAwaiter {
+ int x;
+ finalSuspendAwaiter () : x(0) { }
+ finalSuspendAwaiter (int _x) : x(_x) { }
+ ~finalSuspendAwaiter() noexcept(false) { }
+ bool await_ready() const noexcept(true) { return false; }
+ void await_suspend(std::coroutine_handle<>) const noexcept(true) { }
+ int await_resume() const noexcept(false) { return x; }
+ // { dg-error {the expression 'finalSuspendAwaiter::await_resume' is required to be non-throwing} "" { target *-*-* } .-1 }
+};
+
+struct finalSuspendObj {
+ int x;
+ finalSuspendObj () : x(0) { }
+ finalSuspendObj (int _x) : x(_x) { }
+ ~finalSuspendObj () noexcept(true) {}
+
+ finalSuspendAwaiter operator co_await() const & noexcept(true) {
+ return {x};
+ }
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+
+ finalSuspendObj final_suspend() noexcept { return {3}; }
+
+ void return_void() noexcept {}
+ void unhandled_exception() noexcept {}
+ };
+};
+
+// This should be ill-formed since final_suspend() is potentially throwing.
+task f() {
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95616-g.C b/gcc/testsuite/g++.dg/coroutines/pr95616-g.C
new file mode 100644
index 00000000000..e7481711c5e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95616-g.C
@@ -0,0 +1,51 @@
+// { dg-additional-options "-fsyntax-only" }
+
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std {
+ using namespace std::experimental;
+}
+#endif
+
+class promise;
+
+struct finalSuspendAwaiter {
+ int x;
+ finalSuspendAwaiter () : x(0) { }
+ finalSuspendAwaiter (int _x) : x(_x) { }
+ ~finalSuspendAwaiter() noexcept(false) { }
+ // { dg-error {the expression 'finalSuspendAwaiter::~finalSuspendAwaiter' is required to be non-throwing} "" { target *-*-* } .-1 }
+ bool await_ready() const noexcept(true) { return false; }
+ void await_suspend(std::coroutine_handle<>) const noexcept(true) { }
+ int await_resume() const noexcept(true) { return x; }
+};
+
+struct finalSuspendObj {
+ int x;
+ finalSuspendObj () : x(0) { }
+ finalSuspendObj (int _x) : x(_x) { }
+ ~finalSuspendObj () noexcept(true) {}
+
+ finalSuspendAwaiter operator co_await() const & noexcept(true) {
+ return {x};
+ }
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+
+ finalSuspendObj final_suspend() noexcept { return {3}; }
+
+ void return_void() noexcept {}
+ void unhandled_exception() noexcept {}
+ };
+};
+
+// This should be ill-formed since final_suspend() is potentially throwing.
+task f() {
+ co_return;
+}
--
2.24.1
next reply other threads:[~2021-03-04 19:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-04 19:54 Iain Sandoe [this message]
2021-03-04 20:05 ` Nathan Sidwell
2021-03-05 17:03 ` Iain Sandoe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=A02ED582-4D85-4C2D-ABC8-08EFB788D191@sandoe.co.uk \
--to=iain@sandoe.co.uk \
--cc=gcc-patches@gcc.gnu.org \
--cc=nathan@acm.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).