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

Transwarp doesn't seem to be fully move aware #29

Closed
aheysse opened this issue Sep 29, 2020 · 3 comments
Closed

Transwarp doesn't seem to be fully move aware #29

aheysse opened this issue Sep 29, 2020 · 3 comments

Comments

@aheysse
Copy link

aheysse commented Sep 29, 2020

This could be more of a question than a bug, but is it intended to be able to package move only objects inside tasks? I've been looking at the code, and there seems to be some intent that transwarp is move aware, but it doesn't seem to be complete.

Minimum repo on VS2019 (v142) or Clang on godbolt:
transwarp::make_task(transwarp::root, []() { return std::make_unique<int>(); });

What happens is that because transwarp::task has both overloads for set_value:
virtual void set_value(const typename transwarp::decay<result_type>::type& value) = 0;
virtual void set_value(typename transwarp::decay<result_type>::type&& value) = 0;

...both functions get instantiated in task_impl_proxy. With movable only ResultTypes, the first function will still get instantiated, but it will be invalid C++ as the function's implementation will eventually attempt to make a copy.

Is it intended that this does not work?

Cheers

@bloomen
Copy link
Owner

bloomen commented Oct 4, 2020

Thanks for the question!

The values held by tasks have to be copyable so your value_type cannot be a unique_ptr.

The two set_value overloads simply provide the standard setter idiom for lvalue and rvalue references.

@aheysse
Copy link
Author

aheysse commented Oct 14, 2020

Thanks for answering. In theory it could be made to work with movable only types - would that be something you'd be interested in?

@bloomen
Copy link
Owner

bloomen commented Oct 19, 2020

I don't think that the current architecture and requirements can work with move-only types. One requirement of a transwarp graph is that it can be scheduled any number of times. Another is that task outputs can be shared by multiple children. It's not clear to me how you would satisfy these requirements with move-only types.

@aheysse aheysse closed this as completed Nov 2, 2020
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

No branches or pull requests

2 participants