Remove Stream impl from VecDeque in futures-core?

[yoshuawuyts]

In futures-core the Stream trait is implemented for VecDeque, which is the only type from std:: collections this is done for.

This is a problem for us because in async-std because of how we've setup some blanket impls, we can't implement IntoStream for VecDeque.

Given this implementation is a bit of an odd one out, I'd like to propose we either move from futures-core to futures, or remove it all together.

Thanks heaps!

References

• [tracking] streams async-rs/async-std#129
• https://docs.rs/futures-core-preview/0.3.0-alpha.18/futures_core/stream/trait.Stream.html#implementors

[taiki-e]

we either move from futures-core to futures

This is not possible because VecDeque has generics.

remove it all together.

async-rs/async-std#213 is an error because "may be added impl Stream for Vec<T> in the future". I don't think this will fix the problem.

[taiki-e]

So, I don't think there is a way to resolve async-rs/async-std#213 other than adding IntoStream trait to futures-core.

[taiki-e]

Personally, I am not against adding IntoStream. (For example, it could replace some of the places where we are currently using iter(vec![...]) with vec![...].)

Also, if I remember correctly, someone said that for-await might need IntoStream.

cc @cramertj @withoutboats @Nemo157

[yoshuawuyts]

So, I don't think there is a way to resolve async-rs/async-std#213 other than adding IntoStream trait to futures-core.

Oh yes so Rayon has something very comparable working, and we're now investigating why it doesn't work.

This issue is intended as separate from async-rs/async-std#123, looking forward to the roadblocks for implementing async-rs/async-std#129. If you know more on why async-rs/async-std#123 isn't working however, I'd love to hear more! People suspect it may be a compiler error? But it sounds like you may have it a bit clearer?

edit: I just read the comments on async-rs/async-std#213. My bad; thanks for the detailed explanation @taiki-e!

[Nemo157]

It works in rayon because they define ParallelIterator and IntoParallelIterator in the same crate,

I think I've mentioned IntoStream as a parallel to IntoIterator for async-for in the past. It doesn't seem like something that is necessary, current/previous async-for experiments have just worked directly by taking a Stream by-value. Unlike synchronous loops wanting to asyncly iterate over a collection doesn't seem a common usecase outside testing (which would be a good reason to remove impl Stream for VecDeque anyway); that makes being able to add implementations like impl IntoStream for &_ much less relevant (since that's only possible on synchronous collections, actual async streams require mutable access to poll).

@yoshuawuyts have you seen any usecases that IntoStream actually enables other than keeping consistency with std APIs (which might be enough of a usecase itself)?

[yoshuawuyts]

have you seen any usecases that IntoStream actually enables other than keeping consistency with std APIs (which might be enough of a usecase itself)?

I realize futures::stream::StreamExt::collect is a thing that exists, but in general IntoStream and FromStream allow defining a trait bound that resembles stdlib and feels more flexible than Default + Extend.

Also having FromStream for e.g. ranges would be a rather nice construct, removing many of the uses for stream::iter. I agree this is particularly nice for demos, but I think having the flexibility to iterate over std collections is something that would remove a rough edge from general async programming too.

---

(since that's only possible on synchronous collections, actual async streams require mutable access to poll)

Could you say more about this?

[Nemo157]

The closest idea I have to an async collection (something that can be iterated multiple times and give back the same elements, and probably randomly accessed via index; cf. an IO/in-memory stream that only gives you each value once) is something like a proxy to a remote collection with a limited local cache. In that case you would be accessing some underlying IO stream whenever you were getting access to a value, so even if you only wanted a shared reference to value you would need unique access to the underlying IO to populate the cache (along with ensuring the cache is not invalidated while the reference is live). The proxy could hide this internally via locking, but that would be unnecessary overhead if the usecases could all be supported by requiring unique access to the proxy.

But, after detailing that I've argued myself into there being an additional usecase for IntoStream, while the proxy couldn't impl IntoStream for &ProxyCollection since it needs unique access, it could impl IntoStream for &mut ProxyCollection and return proxies to the collection slots (allowing overwriting the value stored at that index, assuming it is a semantically valid thing to do) while also impl IntoStream for ProxyCollection returning by-value proxies to the objects, allowing editing the object but not overwriting the value stored at the index.

(And I do think that having IntoStream able to be implemented for synchronous collections for simpler flat_map etc. usage is probably an advantage (although I'm now noticing that doesn't exist)).

[cramertj]

The impl in question was removed in #1930, so I'm removing this from the 0.3 milestone as I believe that we've made all necessary breaking changes.