smol-rs / futures-lite

Futures, streams, and async I/O combinators.
Apache License 2.0
439 stars 25 forks source link

Implementing AsyncReadExt for &mut R or Take::by_ref #24

Closed ririsoft closed 3 years ago

ririsoft commented 3 years ago

Hello,

I am encountering the following issue:

use futures::io::{AsyncRead, Take, AsyncReadExt};

fn foo<T: AsyncRead>(s: &mut T) -> Take<&mut T> {
    s.take(5)
}
error[E0308]: mismatched types
 --> src/lib.rs:6:5
  |
5 | fn foo<T: AsyncRead>(s: &mut T) -> Take<&mut T> {
  |        - this type parameter
6 |     s.take(5)
  |     ^^^^^^^^^ expected `&mut T`, found type parameter `T`
  |
  = note: expected struct `futures::io::Take<&mut T>`
             found struct `futures::io::Take<T>`

I usually solves this by implementing the trait on &mut T, but this is not possible for AsyncReadExt (conflicting implementation).

Would you consider adding a helper function on Take, please, such:

impl<T> Take<T> {
    fn by_ref(&mut self) -> Take<&mut T> {...} 
}

Otherwise what workaround would you recommend, please ?

ghost commented 3 years ago

Sounds good! I think by_ref() could be implemented on AsyncReadExt, AsyncWriteExt, and StreamExt, similar to how it's done in the standard library.

r3v2d0g commented 3 years ago

@ririsoft An other way to solve your issue is to add an Unpin bound to T (AsyncRead is only implemented on &mut T if T: AsyncRead + Unpin, as you can see here: https://docs.rs/futures-util/0.3.5/futures_util/io/trait.AsyncRead.html#impl-AsyncRead-for-%26%27_%20mut%20T).

ririsoft commented 3 years ago

@ririsoft An other way to solve your issue is to add an Unpin bound to T (AsyncRead is only implemented on &mut T if T: AsyncRead + Unpin, as you can see here: https://docs.rs/futures-util/0.3.5/futures_util/io/trait.AsyncRead.html#impl-AsyncRead-for-%26%27_%20mut%20T).

Thanks a lot @r3v2d0g, you made my day. I should have noticed it myself, sorry about that !

@stjepang I also realize that what I had in mind such:

impl<T> Take<T> {
    fn by_ref(&mut self) -> Take<&mut T> {
        Take{
            inner: self.inner.by_ref(),
            limit: self.limit,
        }
   }
}

Is a mistake, because in such case we are loosing tracking of the limit in the parent Take. Sorry about this suggestion. At the end this issue can be rejected, unless you think a by_ref is useful for the traits.

ghost commented 3 years ago

I realized that by_ref() would have to require Unpin anyways, so this isn't that useful after all. Closing the issue but let me know if you disagree!