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

doc: calling TryStreamExt methods on !Unpin values #2200

Open
koivunej opened this issue Aug 26, 2020 · 0 comments
Open

doc: calling TryStreamExt methods on !Unpin values #2200

koivunej opened this issue Aug 26, 2020 · 0 comments
Labels
A-stream Area: futures::stream docs

Comments

@koivunej
Copy link

koivunej commented Aug 26, 2020

Ran into quite a confusing issue in a situation like:

async fn foo<'a, St>(bar: St) -> impl Stream<...> + Send + 'a
  where St: TryStream<Ok = _, Error = _> + Send + 'a
{
  async_stream::stream! {
    futures::pin_mut!(bar);
    
    // eventually
    bar.try_next().await;
  }
}

The error I received was E0277 "St cannot be unpinned" with a help of adding an + std::marker::Unpin bound on the where clause. Now this lead me to question everything I thought I understood about pinning until I was answered on #tokio-users that TryStream is not implemented for Pin<&mut T> where T: TryStream. I guess it's not implemented because of restrictions related to the relationship of Stream between TryStream and nothing cannot be done for those details right now, however I was thinking this should be written into the documentation of TryStreamExt methods and/or maybe TryStream trait docs.

Reviewing all Unpin bounded methods in TryStreamExt:

  • try_next
  • try_poll_next_unpin
  • compat
  • into_async_read

Of these only the into_async_read has the following paragraph:

Note that because into_async_read moves the stream, the Stream type must be Unpin. If you want to use into_async_read with a !Unpin stream, you'll first have to pin the stream. This can be done by boxing the stream using Box::pin or pinning it to the stack using the pin_mut! macro from the pin_utils crate.

The above paragraph appears 1:1 on StreamExt::next for example, and it's not too fitting for the TryStreamExt as the real answer would be either to Box::pin or convert to Stream before futures::pin_mut!. Also I am not sure if pin_utils should be mentioned since the pin_mut! is re-exported by futures (or even implemented, did not check).

I think a solution would be to:

  1. add paragraph about using TryStreamExt methods on !Unpin values at the trait level docs
    • something about Box::pin'ing the value or transforming it to Stream using into_stream() before futures::pin_mut!
  2. add links to trait level docs on the four methods which have T: Unpin bound

Any objections, ideas, could there be a better solution overall? I don't understand any/all details behind the Stream vs. TryStream and the reason to need to call into_stream().

@taiki-e taiki-e added this to the futures-0.4 milestone Dec 17, 2020
@taiki-e taiki-e added A-stream Area: futures::stream and removed futures-util labels Dec 17, 2020
arianvp added a commit to arianvp/wire-operator that referenced this issue Jul 31, 2021
no idea why I need to move anything to the heap at this point.
Everything is in the same freakin function. But futures:: Api wants me
to give something that implements the "Unpin" trait; whatever that is.

Guess we allocating on the heap now 🤷

Other people are confused about this too. See rust-lang/futures-rs#2200
arianvp added a commit to arianvp/wire-operator that referenced this issue Jul 31, 2021
no idea why I need to move anything to the heap at this point.
Everything is in the same function. But futures:: Api wants me
to give something that implements the "Unpin" trait; whatever that is.

Guess we allocating on the heap now 🤷

Other people are confused about this too. See rust-lang/futures-rs#2200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stream Area: futures::stream docs
Projects
None yet
Development

No branches or pull requests

2 participants