-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Add core::stream::pending
#91684
Conversation
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this 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?
Using the same type would work, but would it live in |
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. |
cc @rust-lang/wg-async-foundations |
That is a good point, I think this does have to be separate then. |
There was a problem hiding this 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.
Looks good to me. It looks like there are a few implementations in the wild, so pulling this into std seems worthwhile. |
Will merge conflict with #93613. |
☔ The latest upstream changes (presumably #94121) made this pull request unmergeable. Please resolve the merge conflicts. |
@ibraheemdev this has some conflicts that if you can resolve we can push this forward |
@ibraheemdev @rustbot label: +S-inactive |
This PR adds
core::stream::{pending, Pending}
, analogous tocore::future::{pending, Pending}
.Prior art: https://docs.rs/futures/latest/futures/stream/fn.pending.html