Re: [PATCH] c++, coroutines: Account for overloaded promise return_value() [PR105301].
On Wed, Apr 20, 2022 at 4:19 AM Jason Merrill via Gcc-patches
wrote:
>
> On 4/18/22 10:03, Iain Sandoe wrote:
> > Whether it was intended or not, it is possible to define a coroutine promise
> > with multiple return_value() methods [which need not even have the same
> > type].
> >
> > We were not accounting for this possibility in the check to see whether both
> > return_value and return_void are specifier (which is prohibited by the
> > standard). Fixed thus and provided an adjusted diagnostic for the case that
> > multiple return_value() methods are present.
> >
> > tested on x86_64-darwin, OK for mainline? / Backports? (when?)
> > thanks,
> > Iain
> >
> > Signed-off-by: Iain Sandoe
> >
> > PR c++/105301
> >
> > gcc/cp/ChangeLog:
> >
> > * coroutines.cc (coro_promise_type_found_p): Account for possible
> > mutliple overloads of the promise return_value() method.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/coroutines/pr105301.C: New test.
> > ---
> > gcc/cp/coroutines.cc | 10 -
> > gcc/testsuite/g++.dg/coroutines/pr105301.C | 49 ++
> > 2 files changed, 57 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr105301.C
> >
> > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> > index dcc2284171b..d2a765cac11 100644
> > --- a/gcc/cp/coroutines.cc
> > +++ b/gcc/cp/coroutines.cc
> > @@ -513,8 +513,14 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
> > coro_info->promise_type);
> > inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_void)),
> > "% declared here");
> > - inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_val)),
> > - "% declared here");
> > + has_ret_val = BASELINK_FUNCTIONS (has_ret_val);
> > + const char *message = "% declared here";
> > + if (TREE_CODE (has_ret_val) == OVERLOAD)
> > + {
> > + has_ret_val = OVL_FIRST (has_ret_val);
> > + message = "% first declared here";
> > + }
>
> You could also use get_first_fn, but the patch is OK as is. I'm
> inclined to leave backports in coroutines.cc to your discretion, you
> probably have a better idea of how important they are.
Likewise. Please wait until after the 11.3 release.
Richard.
> > + inform (DECL_SOURCE_LOCATION (has_ret_val), message);
> > coro_info->coro_co_return_error_emitted = true;
> > return false;
> > }
> > diff --git a/gcc/testsuite/g++.dg/coroutines/pr105301.C
> > b/gcc/testsuite/g++.dg/coroutines/pr105301.C
> > new file mode 100644
> > index 000..33a0b03cf5d
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/coroutines/pr105301.C
> > @@ -0,0 +1,49 @@
> > +// { dg-additional-options "-fsyntax-only" }
> > +namespace std {
> > +template
> > +struct traits_sfinae_base {};
> > +
> > +template
> > +struct coroutine_traits : public traits_sfinae_base {};
> > +}
> > +
> > +template struct coro {};
> > +template
> > +struct std::coroutine_traits, Ps...> {
> > + using promise_type = Promise;
> > +};
> > +
> > +struct awaitable {
> > + bool await_ready() noexcept;
> > + template
> > + void await_suspend(F) noexcept;
> > + void await_resume() noexcept;
> > +} a;
> > +
> > +struct suspend_always {
> > + bool await_ready() noexcept { return false; }
> > + template
> > + void await_suspend(F) noexcept;
> > + void await_resume() noexcept {}
> > +};
> > +
> > +namespace std {
> > +template
> > +struct coroutine_handle {};
> > +}
> > +
> > +struct bad_promise_6 {
> > + coro get_return_object();
> > + suspend_always initial_suspend();
> > + suspend_always final_suspend() noexcept;
> > + void unhandled_exception();
> > + void return_void();
> > + void return_value(int) const;
> > + void return_value(int);
> > +};
> > +
> > +coro
> > +bad_implicit_return() // { dg-error {.aka 'bad_promise_6'. declares both
> > 'return_value' and 'return_void'} }
> > +{
> > + co_await a;
> > +}
>
Re: [PATCH] c++, coroutines: Account for overloaded promise return_value() [PR105301].
On 4/18/22 10:03, Iain Sandoe wrote:
Whether it was intended or not, it is possible to define a coroutine promise
with multiple return_value() methods [which need not even have the same type].
We were not accounting for this possibility in the check to see whether both
return_value and return_void are specifier (which is prohibited by the
standard). Fixed thus and provided an adjusted diagnostic for the case that
multiple return_value() methods are present.
tested on x86_64-darwin, OK for mainline? / Backports? (when?)
thanks,
Iain
Signed-off-by: Iain Sandoe
PR c++/105301
gcc/cp/ChangeLog:
* coroutines.cc (coro_promise_type_found_p): Account for possible
mutliple overloads of the promise return_value() method.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr105301.C: New test.
---
gcc/cp/coroutines.cc | 10 -
gcc/testsuite/g++.dg/coroutines/pr105301.C | 49 ++
2 files changed, 57 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr105301.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index dcc2284171b..d2a765cac11 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -513,8 +513,14 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
coro_info->promise_type);
inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_void)),
"% declared here");
- inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_val)),
- "% declared here");
+ has_ret_val = BASELINK_FUNCTIONS (has_ret_val);
+ const char *message = "% declared here";
+ if (TREE_CODE (has_ret_val) == OVERLOAD)
+ {
+ has_ret_val = OVL_FIRST (has_ret_val);
+ message = "% first declared here";
+ }
You could also use get_first_fn, but the patch is OK as is. I'm
inclined to leave backports in coroutines.cc to your discretion, you
probably have a better idea of how important they are.
+ inform (DECL_SOURCE_LOCATION (has_ret_val), message);
coro_info->coro_co_return_error_emitted = true;
return false;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr105301.C
b/gcc/testsuite/g++.dg/coroutines/pr105301.C
new file mode 100644
index 000..33a0b03cf5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr105301.C
@@ -0,0 +1,49 @@
+// { dg-additional-options "-fsyntax-only" }
+namespace std {
+template
+struct traits_sfinae_base {};
+
+template
+struct coroutine_traits : public traits_sfinae_base {};
+}
+
+template struct coro {};
+template
+struct std::coroutine_traits, Ps...> {
+ using promise_type = Promise;
+};
+
+struct awaitable {
+ bool await_ready() noexcept;
+ template
+ void await_suspend(F) noexcept;
+ void await_resume() noexcept;
+} a;
+
+struct suspend_always {
+ bool await_ready() noexcept { return false; }
+ template
+ void await_suspend(F) noexcept;
+ void await_resume() noexcept {}
+};
+
+namespace std {
+template
+struct coroutine_handle {};
+}
+
+struct bad_promise_6 {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend() noexcept;
+ void unhandled_exception();
+ void return_void();
+ void return_value(int) const;
+ void return_value(int);
+};
+
+coro
+bad_implicit_return() // { dg-error {.aka 'bad_promise_6'. declares both
'return_value' and 'return_void'} }
+{
+ co_await a;
+}
[PATCH] c++, coroutines: Account for overloaded promise return_value() [PR105301].
Whether it was intended or not, it is possible to define a coroutine promise
with multiple return_value() methods [which need not even have the same type].
We were not accounting for this possibility in the check to see whether both
return_value and return_void are specifier (which is prohibited by the
standard). Fixed thus and provided an adjusted diagnostic for the case that
multiple return_value() methods are present.
tested on x86_64-darwin, OK for mainline? / Backports? (when?)
thanks,
Iain
Signed-off-by: Iain Sandoe
PR c++/105301
gcc/cp/ChangeLog:
* coroutines.cc (coro_promise_type_found_p): Account for possible
mutliple overloads of the promise return_value() method.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr105301.C: New test.
---
gcc/cp/coroutines.cc | 10 -
gcc/testsuite/g++.dg/coroutines/pr105301.C | 49 ++
2 files changed, 57 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr105301.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index dcc2284171b..d2a765cac11 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -513,8 +513,14 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
coro_info->promise_type);
inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_void)),
"% declared here");
- inform (DECL_SOURCE_LOCATION (BASELINK_FUNCTIONS (has_ret_val)),
- "% declared here");
+ has_ret_val = BASELINK_FUNCTIONS (has_ret_val);
+ const char *message = "% declared here";
+ if (TREE_CODE (has_ret_val) == OVERLOAD)
+ {
+ has_ret_val = OVL_FIRST (has_ret_val);
+ message = "% first declared here";
+ }
+ inform (DECL_SOURCE_LOCATION (has_ret_val), message);
coro_info->coro_co_return_error_emitted = true;
return false;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr105301.C
b/gcc/testsuite/g++.dg/coroutines/pr105301.C
new file mode 100644
index 000..33a0b03cf5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr105301.C
@@ -0,0 +1,49 @@
+// { dg-additional-options "-fsyntax-only" }
+namespace std {
+template
+struct traits_sfinae_base {};
+
+template
+struct coroutine_traits : public traits_sfinae_base {};
+}
+
+template struct coro {};
+template
+struct std::coroutine_traits, Ps...> {
+ using promise_type = Promise;
+};
+
+struct awaitable {
+ bool await_ready() noexcept;
+ template
+ void await_suspend(F) noexcept;
+ void await_resume() noexcept;
+} a;
+
+struct suspend_always {
+ bool await_ready() noexcept { return false; }
+ template
+ void await_suspend(F) noexcept;
+ void await_resume() noexcept {}
+};
+
+namespace std {
+template
+struct coroutine_handle {};
+}
+
+struct bad_promise_6 {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend() noexcept;
+ void unhandled_exception();
+ void return_void();
+ void return_value(int) const;
+ void return_value(int);
+};
+
+coro
+bad_implicit_return() // { dg-error {.aka 'bad_promise_6'. declares both
'return_value' and 'return_void'} }
+{
+ co_await a;
+}
--
2.24.3 (Apple Git-128)
