diff mbox series

coroutines: Emit error for invalid promise return types [PR97438].

Message ID A848F0A6-F13A-4869-9504-CB489E6734D4@sandoe.co.uk
State New
Headers show
Series coroutines: Emit error for invalid promise return types [PR97438]. | expand

Commit Message

Iain Sandoe Oct. 18, 2020, 8:13 p.m. UTC 
Hi,

At one stage, use cases were proposed for allowing the promise
type to contain both return_value and return_void.  That was
not accepted into C++20; so we should reject it as per the PR.

Tested on x86_64-darwin, x86_64-linux-gnu,
OK for master?

(although this is technically an ‘accepts invalid’ not sure it’s serious
 enough to backport to 10.x - but I’m happy to do so if anyone thinks
 it should be).

thanks
Iain

gcc/cp/ChangeLog:

	PR c++/97438
	* coroutines.cc (struct coroutine_info): Add a field to
	record that we emitted a promise type error.
	(coro_promise_type_found_p): Check for the case that the
	promise type contains both return_void and return_value.
	Emit an error if so, with information about the wrong
	type methods.

gcc/testsuite/ChangeLog:

	PR c++/97438
	* g++.dg/coroutines/pr97438.C: New test.
---
 gcc/cp/coroutines.cc                      | 25 +++++++++++++++++++
 gcc/testsuite/g++.dg/coroutines/pr97438.C | 30 +++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr97438.C

Comments

Nathan Sidwell Oct. 19, 2020, 1:30 p.m. UTC | #1 
On 10/18/20 4:13 PM, Iain Sandoe wrote:
> Hi,
> 
> At one stage, use cases were proposed for allowing the promise
> type to contain both return_value and return_void.  That was
> not accepted into C++20; so we should reject it as per the PR.
> 
> Tested on x86_64-darwin, x86_64-linux-gnu,
> OK for master?
> 
> (although this is technically an ‘accepts invalid’ not sure it’s serious
>   enough to backport to 10.x - but I’m happy to do so if anyone thinks
>   it should be).

ok.  I don;t think it worth a speculative backport

nathan
diff mbox series

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index ba813454a0b..9b9141e51fd 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -94,6 +94,7 @@  struct GTY((for_user)) coroutine_info
   /* Flags to avoid repeated errors for per-function issues.  */
   bool coro_ret_type_error_emitted;
   bool coro_promise_error_emitted;
+  bool coro_co_return_error_emitted;
 };
 
 struct coroutine_info_hasher : ggc_ptr_hash<coroutine_info>
@@ -470,6 +471,30 @@  coro_promise_type_found_p (tree fndecl, location_t loc)
 	  return false;
 	}
 
+      /* Test for errors in the promise type that can be determined now.  */
+      tree has_ret_void = lookup_member (coro_info->promise_type,
+					 coro_return_void_identifier,
+					 /*protect=*/1, /*want_type=*/0,
+					 tf_none);
+      tree has_ret_val = lookup_member (coro_info->promise_type,
+					coro_return_value_identifier,
+					/*protect=*/1, /*want_type=*/0,
+					tf_none);
+      if (has_ret_void && has_ret_val)
+	{
+	  location_t ploc = DECL_SOURCE_LOCATION (fndecl);
+	  if (!coro_info->coro_co_return_error_emitted)
+	    error_at (ploc, "the coroutine promise type %qT declares both"
+		      " %<return_value%> and %<return_void%>",
+		      coro_info->promise_type);
+	  inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_void)),
+		  "%<return_void%> declared here");
+	  inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_val)),
+		  "%<return_value%> declared here");
+	  coro_info->coro_co_return_error_emitted = true;
+	  return false;
+	}
+
       /* Try to find the handle type for the promise.  */
       tree handle_type =
 	instantiate_coro_handle_for_promise_type (loc, coro_info->promise_type);
diff --git a/gcc/testsuite/g++.dg/coroutines/pr97438.C b/gcc/testsuite/g++.dg/coroutines/pr97438.C
new file mode 100644
index 00000000000..95376648ed7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr97438.C
@@ -0,0 +1,30 @@ 
+#if __has_include(<coroutine>)
+#include <coroutine>
+#else
+#include <experimental/coroutine>
+namespace std { using namespace experimental; }
+#endif
+
+struct dummy_coroutine {};
+
+namespace std {
+
+template<>
+class coroutine_traits<::dummy_coroutine> {
+public:
+    struct promise_type {
+        void return_value(int x) {  }
+        void return_void() {}
+        std::suspend_never initial_suspend() noexcept { return {}; }
+        std::suspend_never final_suspend() noexcept { return {}; }
+        dummy_coroutine get_return_object() { return {}; }
+        void unhandled_exception() {}
+    };
+};
+
+}
+
+dummy_coroutine
+foo() { // { dg-error {the coroutine promise type 'std::__n4861::coroutine_traits<dummy_coroutine>::promise_type' declares both 'return_value' and 'return_void'} }
+    co_return 17;
+}