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

futures-util: FutureExt::now_or_never() #1747

Merged
merged 2 commits into from
Jul 25, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions futures-util/src/future/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,54 @@ pub trait FutureExt: Future {
{
Pin::new(self).poll(cx)
}

/// Evaluates and consumes the future, returning the resulting output if
/// the future is ready after the first call to `Future::poll`.
///
/// If `poll` instead returns `Poll::Pending`, `None` is returned.
///
/// This method is useful in cases where immediacy is more important than
/// waiting for a result. It is also convenient for quickly obtaining
/// the value of a future that is known to always resolve immediately.
///
/// # Examples
///
/// ```
/// # use futures::prelude::*;
/// use futures::{future::ready, future::pending};
/// let future_ready = ready("foobar");
/// let future_pending = pending::<&'static str>();
///
/// assert_eq!(future_ready.now_or_never(), Some("foobar"));
/// assert_eq!(future_pending.now_or_never(), None);
/// ```
///
/// In cases where it is absolutely known that a future should always
/// resolve immediately and never return `Poll::Pending`, this method can
/// be combined with `expect()`:
///
/// ```
/// # use futures::{prelude::*, future::ready};
/// let future_ready = ready("foobar");
///
/// assert_eq!(future_ready.now_or_never().expect("Future not ready"), "foobar");
/// ```
fn now_or_never(mut self) -> Option<Self::Output>
where Self: Sized
{
let noop_waker = crate::task::noop_waker();
let mut cx = Context::from_waker(&noop_waker);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have now_or_never return a Future instead it can make use of the Context already available. While I don't think it makes much of a difference right now, it would be more future proof in case more things are added to the context, like task-local storage.

This does have the downside of changing your proposed API so you'll have to use .await, even though it technically doesn't block.

Copy link
Contributor Author

@darconeous darconeous Jul 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would take away one of the things that I find most useful about this API proposal: now_or_never() allows you to easily resolve a future to a value (or not) when not inside of an async block. Usually if you are already in an async block you can .await for the value.

Perhaps I'm not really understanding the advantages of having it return a future instead. Are we imagining a case where a future would ultimately yield different output values depending on what Context was passed into it? Or would it be more of a cache thing?

Perhaps it would make sense to have both? Not sure what the appropriate names would be in that case. Open to suggestions.


// SAFETY: This is safe because this method consumes the future, so `poll` is
// only going to be called once. Thus it doesn't matter to us if the
// future is `Unpin` or not.
let pinned = unsafe { Pin::new_unchecked(&mut self) };

match pinned.poll(&mut cx) {
Poll::Ready(x) => Some(x),
_ => None,
}
}
}

// Just a helper function to ensure the futures we're returning all have the
Expand Down