Fix future reduction with non copyable types #378
Conversation
Change-Id: I1210e106c9d50109a9b476a885d3f12f3f0faaf2
stlab/concurrency/future.hpp
Outdated
| return std::forward<R>(r); | ||
| } | ||
|
|
||
| static inline auto reduce(future<future<void>>&& r) -> future<void>; |
There was a problem hiding this comment.
I don't think static does anything useful here.
There was a problem hiding this comment.
Agree, I will propose a fix in the next patch set.
| @@ -1705,8 +1685,8 @@ auto when_any(E executor, F&& f, std::pair<I, I> range) { | |||
| /**************************************************************************************************/ | |||
|
|
|||
| template <typename E, typename F, typename... Args> | |||
| auto async(E executor, F&& f, Args&&... args) | |||
| -> future<detail::result_t<std::decay_t<F>, std::decay_t<Args>...>> { | |||
| auto async(E executor, F&& f, Args&&... args) -> future< | |||
There was a problem hiding this comment.
Let's discuss this as a breaking change on the (public) stlab slack - (let me know if you need an invite). Personally, I think I'm fine with it as I doubt there are any (or many) cases this would break and likely the break is minor but we might consider alternate names or using namespace versioning.
There was a problem hiding this comment.
Yes, this need to be discussed. I think need an invite, I can't sign in into stlab.slack.com.
stlab/concurrency/future.hpp
Outdated
| @@ -1739,6 +1719,10 @@ struct reduction_helper<future<void>> { | |||
| future<void> value; | |||
| }; | |||
|
|
|||
| static inline auto reduce(future<future<void>>&& r) -> future<void> { | |||
There was a problem hiding this comment.
Remove static and move this definition above - if this is circular we can use a tagged dispatch in a common - reduce to resolve or another technique.
There was a problem hiding this comment.
Agree, I will propose a fix in the next patch set.
Change-Id: I9b38a352dc6c3a6b9484c719c64619b64c2b9c3c
|
@sean-parent intends to pull this PR and integrate it with other items he has been fixing. He fixed issues related to future reduction with non-copyable types so this may actually be fixed already. |
Stumbled on an issue with future reduction and non copyable type.
I additionally removed the const ref qualified version of get_try from future of non copyable type interface because this method is dangerous and can easily be misused.