fix(stdexec): added adaption to ensure successful build with stdexec#6846
Conversation
|
Can one of the admins verify this patch? |
hkaiser
added
type: enhancement
type: compatibility issue
category: senders/receivers
|
@isidorostsa Would you mind having a look? |
isidorostsa
left a comment
isidorostsa
left a comment
There was a problem hiding this comment.
Hey David, thank you for your PR!
The completions you added in the thread pool scheduler make sense.
Also, good job spotting the removed stuff :)
| meta::type<detail::make_helper<Sender, Env, AddlSignatures, | ||
| meta::func<SetValue>, meta::func1<SetError>, SendsStopped>>; | ||
| #else | ||
| HPX_CXX_EXPORT using exec::make_completion_signatures; |
There was a problem hiding this comment.
We don't appear to be using make_completion_signatures anywhere in the codebase, so we might as well avoid this dependency on exec
There was a problem hiding this comment.
Thank you so much! This part has been removed, shall I make a squash to the commits now or wait for the ci to be fixed. I suspect the failure is not generated from the codebase?
| { | ||
| return {*this}; | ||
| } | ||
|
|
There was a problem hiding this comment.
Do you think you should also add a non-const overload?
| { | ||
| return {*this}; | ||
| } | ||
|
|
There was a problem hiding this comment.
Do you think you should also add a non-const overload?
| )> | ||
| // clang-format on | ||
| constexpr auto query( | ||
| hpx::execution::experimental::get_completion_scheduler_t< |
There was a problem hiding this comment.
Nice spotting this too!
There was a problem hiding this comment.
I think both the query() and schedule() functions should only have const versions because scheduler is designed as a lightweight handle and these operations shouldn't change the states of the internal scheduler.
There was a problem hiding this comment.
Huh, I didn't know that, but it makes sense.
|
This pool request is good to go, @sususu5 I'll ping you once CI is fixed so you can rebase and then we will be good to merge. |
Thanks! |
sususu5
force-pushed
the
fix/stdexec_build_failure
branch
from
a5833f9 to
69c0246
Compare
|
@sususu5 please try rebasing onto master. The CIs should be ok now. |
sususu5
force-pushed
the
fix/stdexec_build_failure
branch
from
69c0246 to
41f905b
Compare
Thanks for the reminder. After checking again, there are still a few tests failing because the clang-tidy subprocess is being killed. |
sususu5
force-pushed
the
fix/stdexec_build_failure
branch
from
8dd9089 to
357cccc
Compare
Yes, CircleCI runs out of memory. Feel free to ignore those errors. |
Signed-off-by: David Hua <tkrd1205@gmail.com>
sususu5
force-pushed
the
fix/stdexec_build_failure
branch
from
357cccc to
a83d5ef
Compare
4 participants
Fixes #6842
Proposed Changes
stdexec::dependent_domainsince it has been deleted in the upstream repostdexec::make_completion_signaturestoexec::make_completion_signaturesto ensure consistencystdexec::transform_envsince it has been deleted in the upstream repoquery()andschedule()functions tothread_pool_policy_schedulerto meet the requirementAny background context you want to provide?
upstream repo
Checklist
Not all points below apply to all pull requests.