ACP: Implement Future for Option<F>

[nvzqz]

Proposal

Problem statement

When working with an Option<F: Future> in an async context, one may want to defer the handling of None. This is currently cumbersome/verbose to do.

Motivation, use-cases

let x = match f {
    Some(f) => Some(f.await),
    None => None,
};

It is not possible to shorten this to let x = f.map(|f| f.await) because the async context is not available in a closure.

Solution sketches

impl<F: Future> Future for Option<F> {
    type Output = Option<F::Output>;

    fn poll(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Self::Output> {
        match self.as_pin_mut() {
            Some(f) => f.poll(cx).map(Some),
            None => Poll::Ready(None),
        }
    }
}

This is implemented in rust-lang/rust#109691.

Links and related work

Prior art:

• OptionFuture
• impl Future for Either<L: Future, R: Future>

[Noratrieb]

cc @rust-lang/wg-async who might have opinions

[taiki-e]

futures has a future called OptionFuture that does the same thing, but there are some open discussions about how it should behave. https://github.com/rust-lang/futures-rs/issues?q=is%3Aissue+is%3Aopen+optionfuture

[nvzqz]

@taiki-e a benefit Option would have over OptionFuture is that properties of the inner future can be inspected before polling/consuming it. We could change OptionFuture to expose the underlying Option value via AsRef/AsMut conversions.

Option may also be able to implement traits like FusedFuture without bumping the MSRV by using where Option<F>: Future rather than where F: Future.

[taiki-e]

I did not mean "we shouldn't implement Future for Option because futures has something similar" but rather "there are open discussions in the futures's prior art about how it should behave".

[nvzqz]

Understood, thanks for elaborating. I agree that behavior questions should be resolved before committing to an implementation.

Here are my opinions:

• OptionFuture to automatically transition to None once Ready futures-rs#2457:
  • Transitioning to None may be unexpected behavior since there are some futures that are expected to return again. Arguably these types should instead implement Stream.
  • It would also be surprising from a functional perspective if this feature is to be considered as a map alternative.
  • Perhaps this can be resolved by instead recommending the Fuse future adapter?
• The other issues do not block this feature, since they are about how the futures crate would enhance Option's future-ness.

[nvzqz]

Another design question for this feature is "should Option instead implement IntoFuture for symmetry with its IntoIterator implementation?"

I think the answer should be "no" since iterating over Option is different than polling it. Iterating requires choices over mutability and ownership, whereas polling has only the single choice of Pin<&mut Self>.

[yoshuawuyts]

cc/ @rust-lang/wg-async

[nvzqz]

@oli-obk pointed out to me that similar functionality in an async context would be made possible with:

let x = try { Some(f?.await) };

Despite this, I think this feature is worth pursuing due to enabling:

• More flexibility in a generic context.
• Simplicity in a manual Future implementation that deals with Option fields.

[yoshuawuyts]

The problem impl Future for Option attempts to resolve strikes me as an instance of the the sandwich problem. As @nvzqz points out, it would not be an issue if Option::map was able to also take async function pointers:

let x = f.map(async |f| f.await);

This limitation does not just apply to Option::map, it also applies to other methods on Option which take function pointers, and in fact all other methods in the stdlib which take function pointers. If we consider the broader problem space, this does represent a divergence in usage patterns between sync and async Rust:

//! recommended `Option::map` usage patterns with this API

// sync rust
let x = t.map(|t| f(t));

// async rust
let x = t.await.map(|t| f(t));

This works okay as long as the function f used in .map is synchronous. If we want to invoke or pass an async fn into the Option::map method, this approach is no longer sufficient. Overall this particular API doesn't seem to me like it would be unreasonable to add. But I do want to call attention to the fact that the limitation on display here is not just exclusive Option::map method or the Option type. And that it doesn't address the full range of uses we have for Option::map. In order to do that, we will need to actually address the sandwich problem via the language.

[nvzqz]

I was going to bring up this morning the same thing! 😄

@thomcc pointed out to me that the map operation would be possible with keyword generics, but it is also blocked on async closures.

I bet that map-like operations happen disproportionately more than other operations over Option. To the point that Swift's ? semantics is essentially syntax sugar over map. And so I think that alone may be justification for Option to have Future semantics that essentially results in a map+await.

But I agree that it doesn't solve the situation with other Option operations. I also think the benefits in #197 (comment) give this enough merit to be worth worth pursuing regardless. Maybe there's a way to approach other operations similarly?

[traviscross]

We discussed this at some length today in the WG-async call. We came to the consensus that:

WG-async does not want this impl to exist and recommends this ACP be closed.

Overall, we felt this was a poor workaround for an async map (for reasons discussed in the thread above). We felt that to whatever degree that some solution other than that may be desired (e.g. for expediency) that it could be better addressed with a more precise tool such as a module-level function or a method on Option (but we are not advocating those necessarily).

In the sense that the impl proposed here would "reach through" the outer type into the future within, some of us thought this was analogous to the Try impls on Poll which some view as having been a mistake.

We discussed alternate semantics, such as having Option implement IntoFuture such that it would return the Ready future, but none of us were immediately able to propose use cases for that outside of testing code, and we were concerned about the desirability of any of the possible semantics for pin!(None.into_future()).poll(cx).

Work on async closures is proceeding at the moment, and we view that as the main path forward here.

[ppershing]

I have a feeling that returning Poll::Ready(None) might be detrimental for some use-cases. In our codebase we had use-cases where the sane default behavior was to return Poll::Pending unconditionally for Option::None.
Consider the following pattern where I have conditionally-enabled sources of events (note: code would not compile as it is).

let conn_primary: Option<TcpStream> = ... // enabled based on config
let conn_secondary: Option<TcpStream> = ... // enabled based on config

loop {
   select! {
      msg_primary = conn_primary.recv() => {...},
      msg_secondary = conn_secondary.recv() => {...},
   }
}

Returning Poll::Ready(None) on every iteration would basically starve the event loop if biased=true and first option is None. Even in non-biased scenario this would be just trashing cycles 50% of time.

As a consequence, having impl<F: Future> Future<Output=Option<F::Output>> for F might lead to abovementioned code to compile "easier" by suggesting users to e.g. map over the result which might be easy but wrong thing to do.

[dtolnay]

This feature completed an FCP to close in rust-lang/rust#109691. Thanks anyway for the proposal! And for the insightful comments both in favor and against, and the async WG's considerate review.