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

Add core::stream::from_iter #81797

Merged
merged 1 commit into from
Aug 4, 2021
Merged

Add core::stream::from_iter #81797

merged 1 commit into from
Aug 4, 2021

Conversation

yoshuawuyts
Copy link
Member

@yoshuawuyts yoshuawuyts commented Feb 5, 2021

Tracking issue: #81798

This_ PR implements std::stream::from_iter, as outlined in the "Converting an Iterator to a Stream" section of the Stream RFC. This function enables converting an Iterator to a Stream by wrapping each item in the iterator with a Poll::Ready instance.

r? @tmandry

cc/ @rust-lang/libs @rust-lang/wg-async-foundations

Example

Being able to convert from an iterator into a stream is useful when refactoring from iterative loops into a more functional adapter-based style. This is fairly common when using more complex filter / map / find chains. In its basic form this conversion looks like this:

before

let mut output = vec![];
for item in my_vec {
    let out = do_io(item).await?;
    output.push(out);
}

after

use std::stream;

let output = stream::from_iter(my_vec.iter())
    .map(async |item| do_io(item).await)
    .collect()?;

Having a way to convert an Iterator to a Stream is essential in enabling this flow.

Implementation Notes

This PR makes use of unsafe {} to pin an item. Currently we're having conversations on the libs stream in Zulip how to bring pin-project in as a dependency to core so we can omit the unsafe {}.

This PR also includes a documentation block which references Stream::next which currently doesn't exist in the stdlib (originally included in the RFC and PR, but later omitted because of an unresolved issue). stream::from_iter can't stabilize before Stream does, and there's still a chance we may stabilize Stream with a next method. So this PR includes documentation referencing that method, which we can remove as part of stabilization if by any chance we don't have Stream::next.

Alternatives Considered

impl IntoStream for T: IntoIterator

An obvious question would be whether we could make it so every iterator can automatically be converted into a stream by calling into_stream on it. The answer is: "perhaps, but it could cause type issues". Types like std::collections may want to opt to create manual implementations for IntoStream and IntoIter, which wouldn't be possible if it was implemented through a catch-all trait.

Possibly an alternative such as impl IntoStream for T: Iterator could work, but it feels somewhat restrictive. In the end, converting an iterator to a stream is likely to be a bit of a niche case. And even then, adding a standalone function to convert an Iterator into a Stream would not be mutually exclusive with a blanket implementation.

Naming

The exact name can be debated in the period before stabilization. But I've chosen stream::from_iter rather than stream::iter because we are creating a stream from an iterator rather than iterating a stream. We also expect to add a stream counterpart to iter::from_fn later on (blocked on async closures), and having stream::from_fn and stream::from_iter would feel like a consistent pair. It also has prior art in async_std::stream::from_iter.

Future Directions

Stream conversions for collections

This is a building block towards implementing stream/stream_mut/into_stream methods for std::collections, std::vec, and more. This would allow even quicker refactorings from using loops to using iterator adapters by omitting the import altogether:

before

use std::stream;

let output = stream::from_iter(my_vec.iter())
    .map(async |item| do_io(item).await)
    .collect()?;

after

let output = my_vec
    .stream()
    .map(async |item| do_io(item).await)
    .collect()?;

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 5, 2021
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend seeing how the same function is implemented in futures. (that is the most widely used in the ecosystem.)

As for prior art, the same functions in futures, tokio, and futures-lite are called iter.

EDIT: This PR's implementation seems to be based on async-std and I opened an issue in async-std repo about it: async-rs/async-std#953

library/core/src/stream/from_iter.rs Outdated Show resolved Hide resolved
library/core/src/stream/from_iter.rs Show resolved Hide resolved
@JohnCSimon
Copy link
Member

failing build
@rustbot label: -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2021
@crlf0710
Copy link
Member

@yoshuawuyts Ping from triage! CI is still red here. Would you mind fixing that? Thanks!

@yoshuawuyts
Copy link
Member Author

Hey! Yeah I'm on vacation right now. I'll make sure to get around to this either next week or the week after!

I have a few outstanding PRs — just need to find a day to update them all 😅

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 29, 2021
@crlf0710
Copy link
Member

@yoshuawuyts Ping from triage! Any updates?

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2021
@bstrie bstrie added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 19, 2021
@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 5, 2021
@JohnCSimon
Copy link
Member

@yoshuawuyts Are you back from vacation?

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 22, 2021
@yoshuawuyts
Copy link
Member Author

yoshuawuyts commented Jun 23, 2021

@JohnCSimon heya, I've been busy with a move among other things. I'll be back to work starting July 15th!

edit: found some free time today, and repaired my local setup. I've updated the PR with all feedback provided, and it should be good again for review!

@rust-log-analyzer

This comment has been minimized.

@tmandry tmandry added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 30, 2021
@jackh726 jackh726 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 2, 2021
@dtolnay dtolnay reopened this Jul 27, 2021
@dtolnay
Copy link
Member

dtolnay commented Jul 27, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jul 27, 2021

📌 Commit 660f585 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 27, 2021
@bors
Copy link
Contributor

bors commented Jul 28, 2021

⌛ Testing commit 660f585 with merge b75dbad85bade79beb9f5c73b11dd9c463958c7f...

@bors
Copy link
Contributor

bors commented Jul 28, 2021

💔 Test failed - checks-actions

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 28, 2021
@dtolnay
Copy link
Member

dtolnay commented Aug 3, 2021

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 3, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 3, 2021
…lnay

Add `core::stream::from_iter`

_Tracking issue: https://github.com/rust-lang/rust/issues/81798_

This_ PR implements `std::stream::from_iter`, as outlined in the _"Converting an Iterator to a Stream"_ section of the [Stream RFC](https://github.com/nellshamrell/rfcs/blob/add-async-stream-rfc/text/0000-async-stream.md#converting-an-iterator-to-a-stream). This function enables converting an `Iterator` to a `Stream` by wrapping each item in the iterator with a `Poll::Ready` instance.

r? `@tmandry`

cc/ `@rust-lang/libs` `@rust-lang/wg-async-foundations`

## Example

Being able to convert from an iterator into a stream is useful when refactoring from iterative loops into a more functional adapter-based style. This is fairly common when using more complex `filter` / `map` / `find` chains. In its basic form this conversion looks like this:

**before**
```rust
let mut output = vec![];
for item in my_vec {
    let out = do_io(item).await?;
    output.push(out);
}
```
**after**
```rust
use std::stream;

let output = stream::from_iter(my_vec.iter())
    .map(async |item| do_io(item).await)
    .collect()?;
```

Having a way to convert an `Iterator` to a `Stream` is essential in enabling this flow.

## Implementation Notes

This PR makes use of `unsafe {}` to pin an item. Currently we're having conversations on the libs stream in Zulip how to bring `pin-project` in as a dependency to `core` so we can omit the `unsafe {}`.

This PR also includes a documentation block which references `Stream::next` which currently doesn't exist in the stdlib (originally included in the RFC and PR, but later omitted because of an unresolved issue). `stream::from_iter` can't stabilize before `Stream` does, and there's still a chance we may stabilize `Stream` with a `next` method. So this PR includes documentation referencing that method, which we can remove as part of stabilization if by any chance we don't have `Stream::next`.

## Alternatives Considered

### `impl IntoStream for T: IntoIterator`

An obvious question would be whether we could make it so every iterator can automatically be converted into a stream by calling `into_stream` on it. The answer is: "perhaps, but it could cause type issues". Types like `std::collections` may want to opt to create manual implementations for `IntoStream` and `IntoIter`, which wouldn't be possible if it was implemented through a catch-all trait.

Possibly an alternative such as `impl IntoStream for T: Iterator` could work, but it feels somewhat restrictive. In the end, converting an iterator to a stream is likely to be a bit of a niche case. And even then, **adding a standalone function to convert an `Iterator` into a `Stream` would not be mutually exclusive with a blanket implementation**.

### Naming

The exact name can be debated in the period before stabilization. But I've chosen `stream::from_iter` rather than `stream::iter` because we are _creating a stream from an iterator_ rather than _iterating a stream_. We also expect to add a stream counterpart to `iter::from_fn` later on (blocked on async closures), and having `stream::from_fn` and `stream::from_iter` would feel like a consistent pair. It also has prior art in `async_std::stream::from_iter`.

## Future Directions
### Stream conversions for collections

This is a building block towards implementing `stream/stream_mut/into_stream` methods for `std::collections`, `std::vec`, and more. This would allow even quicker refactorings from using loops to using iterator adapters by omitting the import altogether:

**before**
```rust
use std::stream;

let output = stream::from_iter(my_vec.iter())
    .map(async |item| do_io(item).await)
    .collect()?;
```
**after**
```rust
let output = my_vec
    .stream()
    .map(async |item| do_io(item).await)
    .collect()?;
```
@bors
Copy link
Contributor

bors commented Aug 3, 2021

⌛ Testing commit 660f585 with merge 669248f5fc704c5d7e527bc889c1f49937a6d5f3...

@JohnTitor
Copy link
Member

@bors retry rolled-up

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 3, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#81797 (Add `core::stream::from_iter`)
 - rust-lang#87267 (Remove space after negative sign in Literal to_string)
 - rust-lang#87663 (Rustdoc accessibility: use an icon for the [-]/[+] controls)
 - rust-lang#87720 (don't use .into() to convert types to identical types (clippy::useless_conversion))
 - rust-lang#87723 (Use .contains instead of manual reimplementation.)
 - rust-lang#87729 (Remove the aarch64 `crypto` target_feature)
 - rust-lang#87731 (Update cargo)
 - rust-lang#87734 (Test dropping union fields more)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Aug 4, 2021

⌛ Testing commit 660f585 with merge 2b8de6f...

@bors
Copy link
Contributor

bors commented Aug 4, 2021

☔ The latest upstream changes (presumably #87746) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 4, 2021
@bors bors merged commit ad74828 into rust-lang:master Aug 4, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 4, 2021
@yoshuawuyts yoshuawuyts deleted the stream_from_iter branch August 6, 2021 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet