rust-lang / futures-rs

Zero-cost asynchronous programming in Rust
https://rust-lang.github.io/futures-rs/
Apache License 2.0
5.34k stars 616 forks source link

Proposal: add `stream::OptionStream` #2737

Open dodomorandi opened 1 year ago

dodomorandi commented 1 year ago

I would like to propose the addition of a new abstraction: OptionStream. I would expect it to have the same Item as the underlying stream when it's built from an Option::Some variant, and to always return ~Poll::Pending~ Poll::Ready(None) (see my comment below) when it's built from Option::None.

The abstraction is useful when you need to enable/disable a particular stream (i.e. an IntervalStream) while keeping the same stream type.

~Here~ Here you can find a simple implementation, in case it could make things more clear.

taiki-e commented 1 year ago

Except that the behavior of the None case is different from the existing OptionFuture (and https://github.com/rust-lang/libs-team/issues/197), I think this is reasonable.

dodomorandi commented 1 year ago

Ok, I think I got it wrong the first time I thought about this, now I am pretty convinced that the stream should return a Poll::Ready(None) when it's internal variant is None, not Poll::Pending. But let me explain my reasoning.

In theory, the None variant of OptionFuture should compose like the future is not there, in reasonably ways. Let's take in fact what one would expect from the operations join and select (or race, if you consider the concepts behind futures-concurrency):

Let's see what happens instead when using the same approach for the stream, using the merge, the chain and the zip operations from futures-concurrency:

After this small analysis, it looks like the None variant of the OptionStream should return a Poll::Ready(None) (and not a Poll::Pending as I initially proposed). Let me know what you think :blush:.