-
Notifications
You must be signed in to change notification settings - Fork 634
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
Tighter contract for FusedFuture #2111
Comments
- Removed FusedIterator; .fuse() is equivalent. - FirstOk is more explicit about its use of FusedIterator. - This design will have to wait for rust-lang#2111 to be resolved; currently the contract of FusedFuture is not strict enough for our needs.
Hm, there are definitely things today that would return |
Follow-up: I'd be open to adding a new trait for this; my concern is that All that being said, I understand the need to not break backwards compatibility without sufficient cause, so I'd be totally open to a new trait called something like |
We can't propagate implementation properly until the (min) specialization is stable, so I think it's preferable to remove FusedFuture in the next major(minor) version. (i.e., always use |
Closing in favor of #2207, which propose the removal of FusedFuture. (See #2207 (comment) for a detailed description.) |
- Removed FusedIterator; .fuse() is equivalent. - FirstOk is more explicit about its use of FusedIterator. - This design will have to wait for rust-lang#2111 to be resolved; currently the contract of FusedFuture is not strict enough for our needs.
- Removed FusedIterator; .fuse() is equivalent. - FirstOk is more explicit about its use of FusedIterator. - This design will have to wait for rust-lang#2111 to be resolved; currently the contract of FusedFuture is not strict enough for our needs.
- Removed FusedIterator; .fuse() is equivalent. - FirstOk is more explicit about its use of FusedIterator. - This design will have to wait for rust-lang#2111 to be resolved; currently the contract of FusedFuture is not strict enough for our needs.
The current contract for
FusedFuture
is not strict enough to be useful in practice. I'm running into this issue while writingfirst_ok
, as described in #2110/#2031. For reference, this is a future that polls a list of futures and returns the firstOk
result, dropping the rest, or if none returnOk
, the lastErr
result. It's similar toselect_ok
, except that it doesn't return the incomplete futures after running.My original thought was that, to avoid the overhead of manually tracking which futures have completed, I could use
FusedFuture
, and allow my consumers to usefut.fuse()
to fuse futures that don't already implement it. However,FusedFuture::is_terminated
is allowed to returntrue
even if it hasn't yet returned aready
.I'd like to propose either strengthening the current contract or adding a new trait or new method so that
is_terminated
(or a separate, new method, perhapsis_completed
) ONLY and ALWAYS returns true if the future has polledReady
. The existing functionality could be retained as a separate method, as it could be used for optimizing pollers. A contract change would be breaking, but would probably lead to less confusion long-term than introducing a new trait, because of existing associations withFusedIterator
.See #1894 for related discussion.
The text was updated successfully, but these errors were encountered: