Implement Ready::into_inner()

[daxpedda]

Tracking issue: #101196.

This implements a method to unwrap the value inside a Ready outside an async context.
See https://docs.rs/futures/0.3.24/futures/future/struct.Ready.html#method.into_inner for previous work.

This was discussed in Zulip beforehand:

An example I'm hitting right now:
I have a cross-platform library that provides a functions that returns a Future. The only reason why it returns a Future is because the WASM platform requires it, but the native doesn't, to make a cross-platform API that is equal for all I just return a Ready on the native targets.

Now I would like to expose native-only functions that aren't async, that users can use to avoid having to deal with async when they are targeting native. With into_inner that's easily solvable now.

I want to point out that some internal restructuring could be used to solve that problem too, but in this case it's not that simple, the library uses internal traits that return the Future already and playing around with that would introduce unnecessary cfg in a lot more places. So it is really only a quality-of-life feature.

[rust-highfive]

r? @scottmcm

(rust-highfive has picked a reviewer for you, use r? to override)

[scottmcm]

Something named into_inner that can panic is surprising enough that I'll spin this over to someone from libs-api even for nightly. (And I'm generally not a good reviewer for async-related stuff.)

r? libs-api

[conradludgate]

Ah that's a very good point. I forgot that Ready was Option based. I would offer up the term take to return an option

[daxpedda]

I modeled this very much after futures::future::Ready::into_inner().

Generally speaking in the async world I have encountered the attitude that doing anything with a Future that was already polled to completion by returning Poll::Ready will panic. You can see this as well in the Future implementation of Ready, it will panic if polled after it already returned Poll::Ready. I'm not in favor of this mechanic, but I would argue that this is a weakness of the Future trait that we can't fix anymore (not that I have an idea how to fix this).

Either way, personally I would prefer a method that will actually panic in this case. I don't oppose adding a take method that returns an Option, but I would argue that it will probably remain unused. I would propose if we introduce a take method, that we might want to rename into_inner into unwrap?

[robjtede]

Firstly, in my experience of this particular future, I've never needed a take method but into_inner is used frequently.

Given the panic behavior, unwrap does seems appropriate; though, I don't see any precedent in std apart outside of Result/Option. One might presume that if an unwrap is provided then expect, unwrap_or, etc. would also exist. I believe this is an argument in favor of take so that Option methods are not duplicated. End of the day, .take().unwrap() is not much longer than .into_inner() to be any bother.

[joshtriplett]

Seems relatively reasonable to me. It does seem like few cases will know in advance they have a Ready rather than an impl Future, apart from internal usage within a library. But nonetheless, as an unstable API this seems fine to experiment with. And I agree that expect seems right here.

@bors r+

[bors]

📌 Commit 5ed1787 has been approved by joshtriplett

It is now in the queue for this repository.