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

Conversation

darconeous
Copy link
Contributor

This pull request introduces a new method to FutureExt: now_or_never().

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.

now_or_never() is a convenience method for FutureExt that 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.

I've used a version of this mechanism in my own code, but thought that it might be useful as an addition to FutureExt, hence this pull request.

Examples

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():

let future_ready = ready("foobar");

assert_eq!(future_ready.now_or_never().expect("Future not ready"), "foobar");

Name

It is named after the english phrase "It's now or never", which the Macmillan Dictionary defines¹ as such:

used for saying that if something is not done immediately, there will not be another chance to do it in the future

The name could easily be changed if the phrase "now or never" is considered too idiomatic. For example, ready_or_not() has some nice symmetries with Poll::Ready, but loses the sense of finality.

`now_or_never()` is a convenience method for `FutureExt` that
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.

It is named after the english phrase "It's *now or never*", which the
Macmillan Dictionary defines[¹] as such:

> used for saying that if something is not done immediately, there
> will not be another chance to do it in the future

[¹]: https://www.macmillandictionary.com/us/dictionary/american/it-s-now-or-never
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.

@cramertj
Copy link
Member

Neat! It'd also be handy to have a Pin<&mut Self> version which polls with a no-op context just to check if the value is already ready.

@darconeous
Copy link
Contributor Author

Neat! It'd also be handy to have a Pin<&mut Self> version which polls with a no-op context just to check if the value is already ready.

I actually considered adding two other methods:

  • A Poll::ready() method, that would return an Option<X>; similar in spirit to Result::ok() and Result::err().
  • A Default implementation for Context that would construct a new no-op context instance with noop_waker().

That way you could do something like this (using poll_unpin here, but you could imagine it with poll):

let future_ready = ready("foobar");
let future_pending = pending::<&'static str>();

assert_eq!(future_ready.poll_unpin(Default::default()).ready(), Some("foobar"));
assert_eq!(future_pending.poll_unpin(Default::default()).ready(), None);

I ended up not writing that up because I figured it was too obvious for someone else not to have considered adding, and was thus unlikely to pass muster (either because it represents an undesirable design pattern or perhaps for some other reason I can't think of). I'd be happy to consider submitting those changes as separate pull requests if anyone on the team thinks it would be useful.

For me personally, having such things doesn't solve any problem that I have encountered that wasn't better addressed with now_or_never(). But maybe that is a failure of imagination on my part.

On the other hand, I think now_or_never() in its present form represents a clear win.

@darconeous
Copy link
Contributor Author

I'd be happy to submit a Pin<&mut Self> version though if you think it would be useful. The only thing that is currently tripping me up about it is what to call it. peek_poll(), maybe? poll_no_context()? poll_damnit()?

@cramertj
Copy link
Member

peek_poll()

Seems fine to me! 🤷‍♀️

In any case, that doesn't have to block this PR.

@cramertj cramertj merged commit a32ea8e into rust-lang:master Jul 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants