Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the receiver CPOs (set_value, set_error, set_stopped) #6

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

dietmarkuehl
Copy link
Owner

  • fixed the year of the copyright of some of the files
  • added missing std:: qualification to a tag_invoke in operation_state

Copy link
Collaborator

@miscco miscco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

The one thing I am missing is converting calls to set_value, which should also be possible and tested

@miscco
Copy link
Collaborator

miscco commented Feb 7, 2022

One obvious caveat that I forgot is that we should check that the arguments are recevier's. But that wont fly until we get the receiver concept done so...

@dietmarkuehl
Copy link
Owner Author

I’ll add tests checking for conversions later today.

With respect to the receiver concept: although currently it doesn’t (which I assume is a mistake) the receiver concept would depend on the completion signal CPOs (set_value, set_error, set_stopped) as well on other CPOs (eg. get_env). Making the CPOs check for receiver arguments would create a similar cyclic dependency as would be between operation_state and the start CPO. As over there, I think that would be misguided. The CPOs conceptually don’t have anything to do with receiver except that receiver uses them. I do know how that could be implemented but it still is wrong and I think it would be especially wrong in specification.

namespace _Set_stopped {
class _Cpo {
public:
template <class _Receiver>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do not go with receiver should we then constraint with movable-value?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe - that would need to go into the specification, though. The current specification just says "for some subexpression R" without adding any requirements on R. I believe the implementation allowing (and testing) the different lvalue overloads is as specified. An implementation not passing these tests does not match the specification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants