Exposing combinators

[Darksonn]

We have considered exposing the combinators of streams as well as AsyncRead and such. This issue tracks whether we want to do this, however the general opinion has been that they shouldn't be public in v1.0, which is not far away (#2718), even if they are temporarily made public in v0.2.

In particular the return types of the following methods would become public:

• StreamExt::chain
• StreamExt::filter
• StreamExt::filter_map
• StreamExt::fuse
• StreamExt::map
• StreamExt::merge
• StreamExt::skip
• StreamExt::skip_while
• StreamExt::take
• StreamExt::take_while
• StreamExt::timeout
• AsyncReadExt::chain

To avoid cluttering the stream module, we could create a tokio::stream::adapters or tokio::stream::combinators module.

A quick overview of pros & cons:

Pros:

• The return type of combinators can be stored in structs easily.
• When users build their own combinators that build on ones we provide, they don't have to reimplement the code in our combinator.
• The standard library makes Iterator and Read combinators public.

Cons:

• Making more types public can clutter the documentation.

It is worth noting that we already have some public combinators, e.g. io::Take and io::StreamReader, as well as many public leaf-IO resources such as io::Empty that are in some sense similar to combinators. We should consider moving these into new modules if we do that.

[hawkw]

IMO, the main benefit for keeping these types private is to reserve the ability to replace them with async fns if/when that becomes possible. I'm less concerned about cluttering the documentation with combinator types, as we could also add #[doc(hidden)] attributes, or export them all in a submodule so that they don't show up in the main stream module.

If reserving the ability to make them async fns someday is the primary reason to keep them private, we should then think about whether this is likely to happen. If/when async fn trait methods are possible on stable, will we want to rewrite these combinators to be async fns?

It's worth noting that if we want to reserve the ability to use async fns for these methods eventually, that is still a breaking change with the current API surface. Currently, the various Stream combinators will all automatically impl Unpin if all their type parameters do, while an async fn-based implementation will never be Unpin. Therefore, using async fn would be a breaking change.

If being able to replace these combinators with async fns is something we care about, we should probably also add PhantomPinned to them in 0.3, making them !Unpin. If we don't make them !Unpin, we are essentially committing to not replacing them with async fn for the next three years after when Tokio 1.0 is released. From my perspective, if we don't make the combinators !Unpin, then we should just go ahead and export the types publicly, especially since many users seem to want this.

[Darksonn]

Note that this issue is specifically about combinators, and that nothing in the list of methods actually returns a Future.

[hawkw]

Whoops! In that case, the motivation for not exposing the combinators that I mentioned doesn't really apply at all.

[alce]

As far as I understand, there are at least two things to consider:

• Stabilization of the Stream trait in std. It looks like the trait will include the next method and Next future and could happen relatively quickly.

• The potential availability of generators, which will produce !Unpin streams. At this point probably most streams will be !Unpin. As @hawkw suggests, we could force combinators to not implement Unpin . This means streams would need to be pinned before iteration.

Another possibility can be not to use combinators at all and return Pin<Box<dyn Stream<...>>> in lieu of impl Stream<..> although it seems a little heavy handed.

Edit: ...on second thought the Pin<Box<...>> thing doesn't make sense, does it?

[carllerche]

This is not critical to solve for 0.3. I am going to punt.

[osa1]

In the meantime, is there anything users can do to e.g. pass return value of StreamExt::fuse to a function? Because the type is not public, we currently can't implement a function that takes return value of StreamExt::fuse as argument. Do we have any workarounds for this?

[Darksonn]

Sure, you can use generics to accept any stream.

fn takes_stream<S: Stream<Item = ...>>(s: S) {
    ...
}

In the specific case of fuse, you could also use the analogous method from the futures crate.

[chapa]

Hi!

Until combinators of streams are public, is it possible to write a function that returns a Throttle?

pub fn foo() -> /* what do I put here ? */ {
    stream::iter(vec![1, 2, 3]).throttle(Duration::from_secs(1))
}

If I try to use a generic type:

pub fn foo<S: Stream<Item = i32>>() -> S {
    stream::iter(vec![1, 2, 3]).throttle(Duration::from_secs(1))
}

I get the error:

pub fn foo<S: Stream<Item = i32>>() -> S {
           - this type parameter       - expected `S` because of return type
    stream::iter(vec![1, 2, 3]).throttle(Duration::from_secs(1))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type parameter `S`, found struct `stream::throttle::Throttle`

I don't understand, why isn't Throttle a S?

[nytopop]

@chapa

I don't understand, why isn't Throttle a S?

Because S is decided by whoever calls foo, not foo. For example, someone might attempt to call it in such a way that S is not the return type of stream::iter, as in foo::<SomethingElse>().

Returning an impl Trait should work in this case:

pub fn foo() -> impl Stream<Item = i32> {
    stream::iter(vec![1, 2, 3]).throttle(Duration::from_secs(1))
}

[Darksonn]

Yeah, the throttle stream may not be an S, since the caller might have chosen S to be some other stream type than Throttle.

[chapa]

Ok thank you, impl Trait solves the problem in that case indeed. Actually I should have explain my real problem instead of trying to find a simpler equivalent that is not so equivalent :sweat_smile:

I wanted a trait with a method that returns a Stream but I couldn't have impl Stream as return type because "impl Trait not allowed outside of function and inherent method return types". And since I can't use generics on the method (as we saw here), I ended up with an associated type :

pub trait MyTrait {
    type Stream: Stream<Item = i32>;

    fn foo(&self) -> Self::Stream;
}

But now I'm stuck on trait implementations, I would have liked to do something like this :

pub struct MyStruct {}

impl MyTrait for MyStruct {
    type Stream = impl Stream<Item = i32>;

    fn foo(&self) -> Self::Stream {
        stream::iter(vec![1, 2, 3]).throttle(Duration::from_secs(1))
    }
}

But impl Trait in type aliases is unstable and needs the type_alias_impl_trait feature.

Is there another way to have a trait's method returning a Stream ? (without enabling unstable features)

[asonix]

Rather than returning an impl Stream you could try returning a Box<dyn Stream> or a Pin<Box<dyn Stream>> and suffer the allocation until it's possible to return impl Foo in traits

[chapa]

It works well with a Pin<Box<dyn Stream>>, thank you! I'm not sure if I prefer this or using the unstable feature, but now I know it's possible without it.

[carllerche]

Exposing combinators is not a breaking change. I am going to remove the 1.0 tag.

[talchas]

Is it a good idea in the first place to have these combinators which cause conflicts with futures::stream::StreamExt but don't replace futures entirely? timeout and throttle obviously are tokio-specific, and all, any, merge at least don't exist at the moment in futures. The rest however cause name conflicts the moment you need any of the combinators in futures that don't exist in tokio.

[carllerche]

Tokio aims to provide everything commonly needed to implement an async app with Rust.

[NeoLegends]

This this a decision you intend to stick to or is that something you might want to reconsider in the future? Reason being I presume most projects will rely on both futures and tokio at the same time and it might just make the adoption story easier for users if those conflicts didn't arise.

That said, that's a decision that can be made when time comes to decide whether to expose the combinators, or not.

[Darksonn]

Note that with the introduction of the async-stream crate, exposing the various combinator types in that crate is something we can experiment with, without having to commit to that for Tokio 1.0.

[ahornby]

Hi, I'd like to be able to call Chain::into_inner() to get back to the two component parts, what's the recommended route to be able to do that?

[alcjzk]

Why not outsource this decision to the users of the crate, by having combinators re-exported in a module(s) behind a feature flag? That way those who need access to these types can choose said access with the cost of some extra namespace clutter.

I don't really see why this would need to be an API level restriction.

[Darksonn]

I am open to exposing the combinators in tokio-stream, but I don't think it makes sense to hide them behind a feature flag.

[tazjin]

This is required for cases where a concrete combinator type must be named for whatever reason (e.g. during a trait implementation where there is nothing else to "attach" a type parameter to).