Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update stream rfc #15

Merged
merged 26 commits into from
Jul 29, 2020
Merged
Changes from 6 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7ede7b7
update terminology to lending stream, rather than detached stream
nellshamrell Jun 30, 2020
3ccfcb0
adds more information on IntoStream trait
nellshamrell Jun 30, 2020
b5f7031
expands on FromStream trait
nellshamrell Jun 30, 2020
f6cd705
add more info on converting between lending streams and non-lending s…
nellshamrell Jun 30, 2020
a6a69a0
even more information about converting Streams to LendingStreams
nellshamrell Jun 30, 2020
81a2d51
adds in information about the next method
nellshamrell Jul 2, 2020
7c95855
Update rfc-drafts/stream.md
nellshamrell Jul 7, 2020
9f6aa05
Update rfc-drafts/stream.md
nellshamrell Jul 7, 2020
1750343
Update rfc-drafts/stream.md
nellshamrell Jul 7, 2020
0050a3e
Update rfc-drafts/stream.md
nellshamrell Jul 7, 2020
9d48ced
Update rfc-drafts/stream.md
nellshamrell Jul 7, 2020
8d19261
Update rfc-drafts/stream.md
nellshamrell Jul 7, 2020
06205c4
adds in info on IntoStream
nellshamrell Jul 7, 2020
d646a7e
adds in more info about generators
nellshamrell Jul 7, 2020
c0bbd43
Update rfc-drafts/stream.md
nellshamrell Jul 9, 2020
8cf7b37
Update rfc-drafts/stream.md
nellshamrell Jul 9, 2020
df39f12
Update rfc-drafts/stream.md
nellshamrell Jul 9, 2020
a13dd48
Update rfc-drafts/stream.md
nellshamrell Jul 9, 2020
e398679
Update rfc-drafts/stream.md
nellshamrell Jul 9, 2020
d41c12b
Update rfc-drafts/stream.md
nellshamrell Jul 9, 2020
ddec9e0
a few clarifications
nellshamrell Jul 9, 2020
4bcc194
fixes name of crate
nellshamrell Jul 10, 2020
6d06d3f
Update rfc-drafts/stream.md
nellshamrell Jul 10, 2020
35efa81
removes unnecessary bullet points
nellshamrell Jul 10, 2020
642935a
Update rfc-drafts/stream.md
nellshamrell Jul 14, 2020
b9c8bff
adds more info based on feedback
nellshamrell Jul 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
221 changes: 202 additions & 19 deletions rfc-drafts/stream.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,52 @@ where
}
```

## Next method/struct

We should also implement a next method, similar to [the implementation in the futures crate](https://docs.rs/futures-util/0.3.5/src/futures_util/stream/stream/next.rs.html#10-12).
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

```rust
/// Future for the [`next`](super::StreamExt::next) method.
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Debug)]
#[must_use = "futures do nothing unless you `.await` or poll them"]
pub struct Next<'a, St: ?Sized> {
stream: &'a mut St,
}
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

impl<St: ?Sized + Unpin> Unpin for Next<'_, St> {}

impl<'a, St: ?Sized + Stream + Unpin> Next<'a, St> {
pub(super) fn new(stream: &'a mut St) -> Self {
Next { stream }
}
}

impl<St: ?Sized + FusedStream + Unpin> FusedFuture for Next<'_, St> {
fn is_terminated(&self) -> bool {
self.stream.is_terminated()
}
}

nellshamrell marked this conversation as resolved.
Show resolved Hide resolved
impl<St: ?Sized + Stream + Unpin> Future for Next<'_, St> {
type Output = Option<St::Item>;

fn poll(
mut self: Pin<&mut Self>,
cx: &mut Context<'_>,
) -> Poll<Self::Output> {
self.stream.poll_next_unpin(cx)
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved
}
}
```

This would allow a user to await on a future:

```rust
while let Some(v) = stream.next().await {

}
```

# Reference-level explanation
[reference-level-explanation]: #reference-level-explanation

Expand Down Expand Up @@ -235,27 +281,137 @@ Designing such a migration feature is out of scope for this RFC.

### IntoStream

**Iterators**

Iterators have an `IntoIterator` that is used with `for` loops to convert items of other types to an iterator.

```rust
pub trait IntoIterator where
<Self::IntoIter as Iterator>::Item == Self::Item,
{
type Item;

type IntoIter: Iterator;

fn into_iter(self) -> Self::IntoIter;
}
```

Examples taken from the Rust docs on [for loops and into_iter]](https://doc.rust-lang.org/std/iter/index.html#for-loops-and-intoiterator)
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

* `for x in iter` uses `impl IntoIterator for T`

```rust
let values = vec![1, 2, 3, 4, 5];

for x in values {
println!("{}", x);
}
```

Desugars to:

```rust
let values = vec![1, 2, 3, 4, 5];
{
let result = match IntoIterator::into_iter(values) {
mut iter => loop {
let next;
match iter.next() {
Some(val) => next = val,
None => break,
};
let x = next;
let () = { println!("{}", x); };
},
};
result
}
```
* `for x in &iter` uses `impl IntoIterator for &T`
* `for x in &mut iter` uses `impl IntoIterator for &mut T`

**Streams**

We may want a trait similar to this for `Stream`. The `IntoStream` trait would provide a way to convert something into a `Stream`.

This trait could look like this:

[TO BE ADDED]
```rust
pub trait IntoStream where
<Self::IntoStream as Stream>::Item == Self::Item,
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved
{
type Item;

type IntoStream: Stream;

fn into_stream(self) -> Self::IntoStream;
}
```
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

### FromStream

**Iterators**

Iterators have an `FromIterator` that is used to convert iterators into another type.

```rust
pub trait FromIterator<A> {

fn from_iter<T>(iter: T) -> Self
where
T: IntoIterator<Item = A>;
}
```

It should be noted that this trait is rarely used directly, instead used through Iterator's collect method ([source](https://doc.rust-lang.org/std/iter/trait.FromIterator.html)).

```rust
pub trait Interator {
fn collect<B>(self) -> B
where
B: FromIterator<Self::Item>,
{ ... }
}
```

Examples taken from the Rust docs on [iter and collect]](https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.collect)
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved


```rust
let a = [1, 2, 3];

let doubled: Vec<i32> = a.iter()
.map(|&x| x * 2)
.collect();

```

**Streams**

We may want a trait similar to this for `Stream`. The `FromStream` trait would provide way to convert a `Stream` into another type.

This trait could look like this:

[TO BE ADDED]
```rust
pub trait FromStream<A> {

fn from_stream<T>(iter: T) -> Self
where
T: IntoStream<Item = A>;
}
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved
```

We could potentially include a collect method for Stream as well.

```rust
pub trait Stream {
fn collect<B>(self) -> B
where
B: FromStream<Self::Item>,
{ ... }
}
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved
```

## Other Traits

Expand Down Expand Up @@ -289,25 +445,29 @@ for elem in stream { ... }

Designing this extension is out of scope for this RFC. However, it could be prototyped using procedural macros today.

## "Attached" streams
## "Lending" streams

There has been much discussion around attached/detached streams.
There has been much discussion around lending streams (also referred to as attached streams).

### Definitions

[Source](https://smallcultfollowing.com/babysteps/blog/2019/12/10/async-interview-2-cramertj-part-2/#the-need-for-streaming-streams-and-iterators)

In a **detached** stream, the `Item` that gets returned by `Stream` is "detached" from self. This means it can be stored and moved about independently from `self`.

In an **attached** stream, the `Item` that gets returned by `Stream` may be borrowed from `self`. It can only be used as long as the `self` reference remains live.
In an **lending** stream (also known as an "attached" stream), the `Item` that gets returned by `Stream` may be borrowed from `self`. It can only be used as long as the `self` reference remains live.
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

In a **non-lending** stream (also known as a "detached" stream), the `Item` that gets returned by `Stream` is "detached" from self. This means it can be stored and moved about independently from `self`.

This RFC does not cover the addition of lending streams (streams as implemented through this RFC are all non-lending streams).
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

This RFC does not cover the addition of attached/detached owned/borrowed streams.
We can add the `Stream` trait to the standard library now and delay
adding in this distinction between two types of streams. The advantage of this
is it would allow us to copy the `Stream` trait from `futures` largely 'as is'.
The disadvantage of this is functions that consume streams would first be written
to work with `Stream`, and then potentially have to be rewritten later to work with
`AttachedStream`s.
adding in this distinction between the two types of streams - lending and
non-lending. The advantage of this is it would allow us to copy the `Stream`
trait from `futures` largely 'as is'.

The disadvantage of this is functions that consume streams would
first be written to work with `Stream`, and then potentially have
to be rewritten later to work with `LendingStream`s.

### Current Stream Trait

Expand All @@ -327,10 +487,10 @@ pub trait Stream {
This trait, like `Iterator`, always gives ownership of each item back to its caller. This offers flexibility -
such as the ability to spawn off futures processing each item in parallel.

### Potential Attached Stream Trait
### Potential Lending Stream Trait

```rust
impl<S> AttachedStream for S
impl<S> LendingStream for S
where
S: Stream,
{
Expand All @@ -346,19 +506,42 @@ where
```

This is a "conversion" trait such that anything which implements `Stream` can also implement
`Attached Stream`.
`Lending Stream`.

This trait captures the case we re-use internal buffers. This would be less flexible for
consumers, but potentially more efficient. Types could implement the `AttachedStream`
consumers, but potentially more efficient. Types could implement the `LendingStream`
where they need to re-use an internal buffer and `Stream` if they do not. There is room for both.

We would also need to pursue the same design for iterators - whether through adding two traits
or one new trait with a "conversion" from the old trait.

This also brings up the question of whether we should allow conversion in the opposite way - if
every "Detached" stream can become an attached one, should _some_ detached streams be able to
become attached ones? These use cases need more thought, which is part of the reason
it is out of the scope of this particular RFC.
every non-lending stream can become a lending one, should _some_ non-lending streams be able to
become lending ones?
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

We can say that, as the Rust language stands today, we cannot cleanly convert impl Stream to impl LendingStream due to a coherence conflict.
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

If you have other impls like:

```rust
impl<T> Stream for Box<T> where T: Stream
```

and

```rust
impl<T> LendingStream for Box<T> where T: LendingStream
```

There is a coherence conflict for `Box<impl Stream>`, so presumably it will fail the coherence rules.

[More examples are available here](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a667a7560f8dc97ab82a780e27dfc9eb).
nellshamrell marked this conversation as resolved.
Show resolved Hide resolved

Resolving this would require either an explicit “wrapper” step or else some form of language extension.

It should be noted that the same applies to Iterator, it is not unique to Stream.

These use cases for lending/non-lending streams need more thought, which is part of the reason it is out of the scope of this particular RFC.

## Generator syntax
[generator syntax]: #generator-syntax
Expand Down