-
Notifications
You must be signed in to change notification settings - Fork 665
Description
OptionFuture
's design seems inconsistent to me.
The docs don't clearly explain what it's for:
A future representing a value which may or may not be present.
Which could describe any Future<Output = Option<_>>
.
Given that it's constructed from an Option<Future>
though, it seems clear that it's an adapter for Option<Future>
that can be awaited, yielding Some(output)
when the future is present, and None
when it's absent.
However, it also implements FusedFuture
(where F: FusedFuture
at least) so that OptionFuture::from(None).is_terminated() == true
which seems inconsistent with that interpretation. is_terminated
is documented as meaning:
Returns true if the underlying future should no longer be polled.
Which sort of means you're not supposed to await it if it's none?
And practically speaking, FusedFuture affects the behavior of select!
, which will skip over futures that are terminated (so it can safely be used in a loop). OptionFuture's current implementation will result in it getting skipped entirely for the None
case which is surprising if you're just expecting it to be an adapter that'll produce None
.
One might suggest that this None
-skipping behavior is desirable, as a solution for #2270. But for that usecase, why is the result wrapped in a Some()
? And why can you create an OptionFuture
from a !FusedFuture
which doesn't work this way?
A third option (pun not intended) is suggested by #2457 which is to have none mean terminated and have the inner transition to None after the future completes. Which would make OptionFuture::from(None)
very similar to Fuse::terminated()
, except polling/awaiting it yields None
rather than panicking.
Personally I think the adapter interpretation is correct, and OptionFuture
should not implement FusedFuture
. If that's desirable I'd be happy to send a PR. But I admit I haven't personally found a use for OptionFuture
in code I've written, so maybe I'm missing the point here?