Conversation
williamspatrick
commented
Oct 11, 2022
williamspatrick
commented
- task: awaiter_context: remove unused parameter
- task: fix compile on gcc
- hello_coro: re-enable on gcc-12 or higher
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
GCC does not like the evaluation of `set_value_t(void)` in the
completion signature determination. Promote this to a
type-specialization using concepts instead of the std::conditional
to appease GCC.
The compile failure was as follows:
```
task.hpp:370:17: error: invalid parameter type ‘void’
370 | friend auto tag_invoke(std::execution::get_completion_signatures_t,
| ^~~~~~~~~~
task.hpp:370:17: error: in declaration ‘std::conditional_t<is_void_v<_Ty>, stdexec::completion_signatures<stdexec::__receivers::set_value_t(), stdexec::__receivers::set_error_t(std::__exception_ptr::exception_ptr), stdexec::__receivers::set_stopped_t()>, stdexec::completion_signatures<stdexec::__receivers::set_value_t(_Ty), stdexec::__receivers::set_error_t(std::__exception_ptr::exception_ptr), stdexec::__receivers::set_stopped_t()> > exec::__task::tag_invoke(stdexec::__get_completion_signatures::get_completion_signatures_t, const basic_task<_Ty, _Context>&, auto:82)’
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
The task code now works on GCC-12 but not earlier. Detect the version and re-enable when supported. Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
This is the same content as with #625, but with the example code only re-enabled on GCC-12+. Currently, lucteo/action-cxx-toolkit doesn't support GCC-12. Opened lucteo/action-cxx-toolkit#2 to start discussion on how to enable GCC-12. |
| std::execution::set_stopped_t()>; | ||
|
|
||
| friend auto tag_invoke(std::execution::get_completion_signatures_t, const basic_task&, auto) | ||
| -> std::conditional_t<std::is_void_v<_Ty>, __task_traits_t<>, __task_traits_t<_Ty>>; |
There was a problem hiding this comment.
I'm curious why you had to change the return type here.
There was a problem hiding this comment.
I tried to add the details to the commit message of the commit where I did this.
The original failure on GCC was:
task.hpp:370:17: error: invalid parameter type ‘void’
370 | friend auto tag_invoke(std::execution::get_completion_signatures_t,
| ^~~~~~~~~~
task.hpp:370:17: error: in declaration ‘std::conditional_t<is_void_v<_Ty>, stdexec::completion_signatures<stdexec::__receivers::set_value_t(), stdexec::__receivers::set_error_t(std::__exception_ptr::exception_ptr), stdexec::__receivers::set_stopped_t()>, stdexec::completion_signatures<stdexec::__receivers::set_value_t(_Ty), stdexec::__receivers::set_error_t(std::__exception_ptr::exception_ptr), stdexec::__receivers::set_stopped_t()> > exec::__task::tag_invoke(stdexec::__get_completion_signatures::get_completion_signatures_t, const basic_task<_Ty, _Context>&, auto:82)’
GCC is unhappy about the evaluation of __task_traits_t<void> even though it is inside the std::conditional_t. I promoted the return type to a template specialization so that the code never attempts to evaluate __task_traits_t<void> and only does __task_traits_t<> (when _Ty is void).
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| #if defined(__GNUC__) && !defined(__clang__) |
There was a problem hiding this comment.
If we have determined that coroutines are unusable on gcc-11 and earlier, then we should probably just set _STD_NO_COROUTINES to 1 in stdexec/coroutine.hpp and remove this check here. Line 25 below checks _STD_NO_COROUTINES.
There was a problem hiding this comment.
Actually, I don't think it is that co-routines are unusable on gcc-11 and earlier. With gcc-11, we get the following compile failure with something in task.hpp:
In file included from /github/workspace/examples/hello_coro.cpp:26:
/github/workspace/include/exec/task.hpp:121:43: error: redefinition of ‘struct exec::__task::__default_task_context_impl::__awaiter_context<_ParentPromise>’
121 | struct __default_task_context_impl::__awaiter_context<_ParentPromise> {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/github/workspace/include/exec/task.hpp:93:43: note: previous definition of ‘struct exec::__task::__default_task_context_impl::__awaiter_context<_ParentPromise>’
93 | struct __default_task_context_impl::__awaiter_context<_ParentPromise> {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/github/workspace/include/exec/task.hpp:134:43: error: redefinition of ‘struct exec::__task::__default_task_context_impl::__awaiter_context<_ParentPromise>’
134 | struct __default_task_context_impl::__awaiter_context<_ParentPromise> {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/github/workspace/include/exec/task.hpp:93:43: note: previous definition of ‘struct exec::__task::__default_task_context_impl::__awaiter_context<_ParentPromise>’
93 | struct __default_task_context_impl::__awaiter_context<_ParentPromise> {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I don't think setting _STD_NO_COROUTINES on gcc-11 is entirely appropriate. Any ideas on how we might resolve this issue? It isn't obvious to me what gcc is unhappy about.
There was a problem hiding this comment.
You're right. This is a gcc bug with concepts. I can work around it, and I've confirmed locally that gcc-11 can handle coroutines just fine. I'll make a PR and merge, then this PR is not needed.
|
Superseded by #631 |
2 participants
