diff --git a/futures-util/src/future/first.rs b/futures-util/src/future/first.rs index 92510823d7..9fa14e3008 100644 --- a/futures-util/src/future/first.rs +++ b/futures-util/src/future/first.rs @@ -1,6 +1,6 @@ use crate::future::Either; use core::pin::Pin; -use futures_core::future::{FusedFuture, Future}; +use futures_core::future::Future; use futures_core::task::{Context, Poll}; use pin_utils::unsafe_pinned; @@ -12,6 +12,8 @@ pub struct First { future2: F2, } +impl Unpin for First {} + impl First { unsafe_pinned!(future1: F1); unsafe_pinned!(future2: F2); @@ -32,12 +34,9 @@ impl Future for First { } } -impl FusedFuture for First { - #[inline] - fn is_terminated(&self) -> bool { - self.future1.is_terminated() || self.future2.is_terminated() - } -} +// We don't provide FusedFuture, because the overhead of implementing it ( +// which requires a separate bool or Option field) is precisely the same as +// calling .fuse() /// Waits for either one of two differently-typed futures to complete. /// @@ -52,7 +51,7 @@ impl FusedFuture for First { /// wrapped version of them. /// /// Also note that if both this and the second future have the same -/// output type you can use the `Either::factor_first` method to +/// output type you can use the `Either::into_immer` method to /// conveniently extract out the value at the end. pub fn first(future1: F1, future2: F2) -> First { First { future1, future2 } diff --git a/futures-util/src/future/first_all.rs b/futures-util/src/future/first_all.rs index f679263478..13aa288108 100644 --- a/futures-util/src/future/first_all.rs +++ b/futures-util/src/future/first_all.rs @@ -1,6 +1,6 @@ use core::iter::FromIterator; use core::pin::Pin; -use futures_core::future::{FusedFuture, Future}; +use futures_core::future::Future; use futures_core::task::{Context, Poll}; /// Future for the [`first_all()`] function. @@ -32,26 +32,15 @@ impl Future for FirstAll { Poll::Pending => None, } }) { - Some(out) => { - // Safety: safe because vec clears in place - this.futures.clear(); - Poll::Ready(out) - } + Some(out) => Poll::Ready(out), None => Poll::Pending, } } } -impl FusedFuture for FirstAll { - #[inline] - fn is_terminated(&self) -> bool { - // Logic: it's possible for a future to independently become - // terminated, before it returns Ready, so we're not terminated unless - // *all* of our inner futures are terminated. When our own poll returns - // Ready, this vector is cleared, so the logic works correctly. - self.futures.iter().all(|fut| fut.is_terminated()) - } -} +// We don't provide FusedFuture, because the overhead of implementing it ( +// which requires clearing the vector after Ready is returned) is precisely +// the same as using .fuse() impl FromIterator for FirstAll { fn from_iter>(iter: T) -> Self { diff --git a/futures-util/src/future/first_ok.rs b/futures-util/src/future/first_ok.rs index 8be4369c5f..a25f6f1e12 100644 --- a/futures-util/src/future/first_ok.rs +++ b/futures-util/src/future/first_ok.rs @@ -23,17 +23,10 @@ impl Future for FirstOk { #[inline] fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { // Basic logic diagram: - // - If all existing futures are terminated, return Pending. This means - // someone polled after this future returned ready, or that this - // future will never return ready because a future spuriously - // terminated itself. - // - If a future returns Ok, clear the vector (this is safe because - // vec drops in place), then return that value. We clear the vector - // so that our FusedFuture impl, which checks `are all futures - // terminated`, works correctly. + // - If all existing futures are terminated, return Pending. + // - If a future returns Ok, return that value. // - If all existing futures BECOME terminated while polling them, and - // an error was returned, return the final error; otherwise return - // pending. + // an error was returned, return the final error. /// Helper enum to track our state as we poll each future enum State { @@ -64,19 +57,15 @@ impl Future for FirstOk { } let mut state = State::NoErrors; - let this = self.get_mut(); - for fut in this.futures.iter_mut() { + + for fut in self.get_mut().futures.iter_mut() { if !fut.is_terminated() { // Safety: we promise that the future is never moved out of the vec, // and that the vec never reallocates once FirstOk has been created // (specifically after the first poll) let pinned = unsafe { Pin::new_unchecked(fut) }; match pinned.try_poll(cx) { - Poll::Ready(Ok(out)) => { - // Safety: safe because vec clears in place - this.futures.clear(); - return Poll::Ready(Ok(out)); - } + Poll::Ready(Ok(out)) => return Poll::Ready(Ok(out)), Poll::Ready(Err(err)) => state.apply_error(err), Poll::Pending => state.apply_pending(), } @@ -85,17 +74,21 @@ impl Future for FirstOk { match state { SeenError(err) => Poll::Ready(Err(err)), - NoErrors | SeenPending => Poll::Pending, + SeenPending => Poll::Pending, + // This is unreachable unless every future in the vec returned + // is_terminated, which means that we must have returned Ready on + // a previous poll, or the vec is empty, which we disallow in the + // first_ok constructor, or that we were initialized with futures + // that have already returned Ready, which is possibly unsound + // (given !Unpin futures) but certainly breaks first_ok contract. + NoErrors => panic!("All futures in the FirstOk terminated without a result being found. Did you re-poll after Ready?"), } } } -impl FusedFuture for FirstOk { - #[inline] - fn is_terminated(&self) -> bool { - self.futures.iter().all(|fut| fut.is_terminated()) - } -} +// We don't provide FusedFuture, because the overhead of implementing it ( +// which requires clearing the vector after Ready is returned) is precisely +// the same as using .fuse() impl FromIterator for FirstOk { fn from_iter>(iter: T) -> Self { @@ -121,13 +114,27 @@ impl FromIterator for FirstOk { /// /// # Panics /// -/// This function will panic if the iterator specified contains no items. +/// This function will panic if the iterator specified contains no items, or +/// if any of the futures have already been terminated. pub fn first_ok(futures: I) -> FirstOk where I: IntoIterator, I::Item: FusedFuture + TryFuture, { - let futures = Vec::from_iter(futures); - assert!(!futures.is_empty(), "Need at least 1 future for first_ok"); + let futures: Vec<_> = futures + .into_iter() + .inspect(|fut| { + assert!( + !fut.is_terminated(), + "Can't call first_ok with a terminated future" + ) + }) + .collect(); + + assert!( + !futures.is_empty(), + "Need at least 1 non-terminated future for first_ok" + ); + FirstOk { futures } }