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::pending #91684

Closed
wants to merge 1 commit into from

Conversation

ibraheemdev
Copy link
Member

This PR adds core::stream::{pending, Pending}, analogous to core::future::{pending, Pending}.

Prior art: https://docs.rs/futures/latest/futures/stream/fn.pending.html

@rust-highfive
Copy link
Collaborator

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 9, 2021
@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Dec 9, 2021
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Do you think it's important that this be a distinct type, or would it be fine as impl Stream for the existing core::future::Pending struct?

@ibraheemdev
Copy link
Member Author

Using the same type would work, but would it live in core::future or core::stream then?

@taiki-e
Copy link
Member

taiki-e commented Dec 13, 2021

would it be fine as impl Stream for the existing core::future::Pending struct?

I guess that if we implement both Future and Stream, the FutureExt (futures, futures-lite, async-std) and StreamExt (futures, tokio, futures-lite, async-std) methods will conflict.

@taiki-e
Copy link
Member

taiki-e commented Dec 13, 2021

cc @rust-lang/wg-async-foundations

@ibraheemdev
Copy link
Member Author

the FutureExt (futures, futures-lite, async-std) and StreamExt (futures, tokio, futures-lite, async-std) methods will conflict.

That is a good point, I think this does have to be separate then.

Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

Adding this makes sense to me. There is precedence for this in async_std::stream::pending as well, and from experience it's convenient to be able to use stream::pending when authoring examples.

Note though that this should not be stabilized before the working group has been able to complete a feasibility study of async overloading, as that may mean that we may no longer have an std::stream submodule, and instead would likely fold the functionality of std::stream into std::iter, likely exposing this as std::iter::pending instead.

@eholk
Copy link
Contributor

eholk commented Dec 14, 2021

Looks good to me. It looks like there are a few implementations in the wild, so pulling this into std seems worthwhile.

@dtolnay dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 20, 2021
@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 25, 2022
@crlf0710
Copy link
Member

crlf0710 commented Feb 3, 2022

Will merge conflict with #93613.

@bors
Copy link
Contributor

bors commented Feb 18, 2022

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

@Dylan-DPC Dylan-DPC 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 19, 2022
@Dylan-DPC
Copy link
Member

@ibraheemdev this has some conflicts that if you can resolve we can push this forward

@JohnCSimon
Copy link
Member

@ibraheemdev
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this. Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Mar 20, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Mar 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. 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.