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

future<T> and friends shall work for movable only Ts #865

Closed
K-ballo opened this issue Sep 21, 2013 · 9 comments

Comments

Projects
None yet
4 participants
@K-ballo
Copy link
Member

commented Sep 21, 2013

Currently future<T> makes several wrong assumptions for T. get() returns copies instead of references, it default constructs instances, etc. Related functionalities like then also makes copies of T.

future<T> and friends shall work for movable only Ts (including T&).

@sithhell

This comment has been minimized.

Copy link
Member

commented Sep 21, 2013

I am not sure if that's the way to go. Not all types are movable. For some
moving just doesn't make sense.

@K-ballo

This comment has been minimized.

Copy link
Member Author

commented Sep 21, 2013

I should clarify the wording here. future<T> should only assume that T is MoveConstructible or otherwise CopyConstructible, so that T(boost::move(t)) constructs a new T by either move or copy.

@sithhell

This comment has been minimized.

Copy link
Member

commented Jan 14, 2014

Is this fixed now?

@K-ballo

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2014

There's a minor change left, async needs to be changed slightly to support future<R&>

@ghost ghost assigned K-ballo Jan 14, 2014

@hkaiser

This comment has been minimized.

Copy link
Member

commented Jan 14, 2014

Semantically, I understand what it means to return a future<R&> from a local async. However we should find proper semantics for this in remote contexts as well, if possible.

@eschnett

This comment has been minimized.

Copy link
Contributor

commented Jan 14, 2014

I assume this should be the same as passing A& to a remote routine, i.e. having a routine that accepts an A& argument that is called via async. I would expect this to have the same semantics as passing A* or returning R*. In particular, I expect HPX to make copies of the respective objects, and object lifetime is a mess: It would be the responsibility of the callee to deallocate the A& arguments, and the responsibility of the routine calling future.get() to deallocate the result. Of course, these routines would need to detect that such copies were made (which wouldn't be made with a local async).

Another possible semantics for remote calls with references would be to disallow them...

@hkaiser

This comment has been minimized.

Copy link
Member

commented Jan 14, 2014

Remote calls to async (actions) do not support having T& arguments (for the reasons you outlined), you can only have T const& as arguments. Neither do they support receiving pointers.

If we want to enable those we need to resolve the object lifetime issues. I'm not sure if that's possible.

@K-ballo

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2014

Remote calls should follow value semantics in both arguments and result, since it's not possible to keep identities across localities.

Action parameters would have to be compatible with T&&, since the remote function will be invoked with rvalues, leaving out non-const T& as an option. An attempt to pass an actual lvalue reference to an action would require using boost::ref(), which should fail to compile as boost::reference_wrapper shouldn't be serializable.

The result type of an action should be the decayed result type of the function that will be invoked. That would mean there would be no future<R&> returned from actions, unless explicitly specified as a result type. We do deal with those differently, I guess it would make sense to extend that support so that the result is decayed after the future is ready and before it is returned.

If this constrains aren't enforced by the implementation already, we should enforce them explicitly.

@hkaiser hkaiser modified the milestones: 0.9.9, 0.9.8 Mar 15, 2014

@K-ballo

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2014

The remaining change to async was applied in 9506f89

@K-ballo K-ballo closed this Jun 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.