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

[question] is pin-project on SelectAll necessary? #2724

Closed
yshui opened this issue Mar 20, 2023 · 3 comments
Closed

[question] is pin-project on SelectAll necessary? #2724

yshui opened this issue Mar 20, 2023 · 3 comments

Comments

@yshui
Copy link
Contributor

yshui commented Mar 20, 2023

pin_project! {
/// An unbounded set of streams
///
/// This "combinator" provides the ability to maintain a set of streams
/// and drive them all to completion.
///
/// Streams are pushed into this set and their realized values are
/// yielded as they become ready. Streams will only be polled when they
/// generate notifications. This allows to coordinate a large number of streams.
///
/// Note that you can create a ready-made `SelectAll` via the
/// `select_all` function in the `stream` module, or you can start with an
/// empty set with the `SelectAll::new` constructor.
#[must_use = "streams do nothing unless polled"]
pub struct SelectAll<St> {
#[pin]
inner: FuturesUnordered<StreamFuture<St>>,
}
}

FuturesUnordered itself is Unpin, did I miss something?

@yshui
Copy link
Contributor Author

yshui commented Mar 20, 2023

For the same reason, I think SelectAll doesn't need St: Unpin to implement Stream

@taiki-e
Copy link
Member

taiki-e commented Mar 22, 2023

Yeah, I don't think pin projection is needed here. #2431 removed the need for it but forgot to remove it.

For the same reason, I think SelectAll doesn't need St: Unpin to implement Stream

IIRC, it is required by StreamFuture.

@yshui
Copy link
Contributor Author

yshui commented Mar 23, 2023

it is required by StreamFuture.

Indeed! sorry I missed that 😅

yshui added a commit to yshui/futures-rs that referenced this issue Mar 23, 2023
taiki-e pushed a commit that referenced this issue Mar 30, 2023
taiki-e pushed a commit that referenced this issue Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants