266: Changes Extend trait in order to allow streams that yield references r=yoshuawuyts a=sunjay
This is not ready to merge yet. I am mainly opening it so we can discuss a change I had to make to the `Extend` trait. cc @yoshuawuyts @stjepang (and anyone else interested)
## Before this can be merged
- [x] Discuss/Approve changes to `Extend` trait
- [x] Change to using `for_each` after #264 is merged
- [ ] (optional) Wait until a `copied()` method is added to `StreamExt` so that the `&char` impl can be finished.
- We can also just comment out or remove the impl that uses `copied` until that is added
## Changes To The Extend Trait
While writing the impls of the `Extend` trait for the `String` type, I noticed that certain impls weren't possible because there is no bound on `Extend` that guarantees that the type `A` being yielded from the stream actually lives long enough. We probably didn't run into this earlier because this usually isn't a problem for owned values since the compiler doesn't have to worry about whether they will out live the stream that they come from. I ran into this because of the `Extend` impls that consume streams of references.
The difference between the async `Extend` and the standard library `Extend` is that the async `Extend` returns a value that still references the input stream. That means that if `A` is any reference type, the compiler needs to be able to guarantee that `A` will be around as long as the `Future` returned from the trait method is around.
To fix this, I had to add the bound shown below:
```patch
pub trait Extend<A> {
/// Extends a collection with the contents of a stream.
fn stream_extend<'a, T: IntoStream<Item = A> + 'a>(
&'a mut self,
stream: T,
- ) -> Pin<Box<dyn Future<Output = ()> + 'a>>;
+ ) -> Pin<Box<dyn Future<Output = ()> + 'a>> where A: 'a;
}
```
This guarantees that each value of type `A` will last at least as long as our boxed future does. The bound had to be in a where clause on the method (and not on the declaration of `A` because the lifetime `'a` isn't in scope at the trait level. I don't think there are any negative consequences of using a where clause like this, but that's why I wanted to bring it up for discussion.
In addition to this, I had to ensure that when writing the `Extend` impls for `String` I appropriately bounded the lifetime of the references from the stream. You can see this in the code below with `where 'b: 'a`.
```rust
impl<'b> Extend<&'b str> for String {
fn stream_extend<'a, S: IntoStream<Item = &'b str> + 'a>(
&'a mut self,
stream: S,
) -> Pin<Box<dyn Future<Output = ()> + 'a>> where 'b: 'a {
//TODO: This can just be: stream.into_stream().for_each(move |s| self.push_str(s))
Box::pin(stream.into_stream().fold((), move |(), s| self.push_str(s)))
}
}
```
I should note that initially I tried to make it work with just the impl shown above, without modifying the `Extend` trait. This doesn't work because it would be a stricter bound than what is found in the trait itself. Rust does not allow stricter bounds like that because it could potentially cause unsoundness when dealing with generics.
Of course, I am totally open to being completely wrong in my understanding of how to resolve this issue. I tried to solve the problem with as minimal of a change as possible. Please let me know if you have some better ideas or other suggestions.
## `FromStream` impls for String
The purpose of adding these `Extend` impls is to continue my work from #129 in adding the rest of the `FromStream` impls. The `Extend` impls are used directly to add all of the `FromStream` impls for `String`. Just like with #207 and #265, this adds a new `string` module that is unstable just like the other modules added for `FromStream`.
Co-authored-by: Sunjay Varma <varma.sunjay@gmail.com>
241: Simplify extension traits using a macro r=yoshuawuyts a=stjepang
This PR would fix#235
Async methods in our extension traits are now written in the simpler `-> impl Future<Output = T> [ConcreteFuture<Self>]` style. At the same time, doc tests are used even when the `docs` feature is not enabled.
Co-authored-by: Stjepan Glavina <stjepang@gmail.com>
224: Re-export IO traits from futures r=stjepang a=stjepang
Sorry for the big PR!
Instead of providing our own traits `async_std::io::{Read, Write, Seek, BufRead}`, we now re-export `futures::io::{AsyncRead, AsyncWrite, AsyncSeek, AsyncRead}`. While re-exporting we rename them to strip away the "Async" prefix.
The documentation will display the contents of the original traits from the `futures` crate together with our own extension methods. There's a note in the docs saying the extenion methods become available only when `async_std::prelude::*` is imported.
Our extension traits are re-exported into the prelude, but are marked with `#[doc(hidden)]` so they're completely invisible to users.
The benefit of this is that people can now implement traits from `async_std::io` for their types and stay compatible with `futures`. This will also simplify some trait bounds in our APIs - for example, things like `where Self: futures_io::AsyncRead`.
At the same time, I cleaned up some trait bounds in our stream interfaces, but haven't otherwise fiddled with them much.
I intend to follow up with another PR doing the same change for `Stream` so that we re-export the stream trait from `futures`.
Co-authored-by: Stjepan Glavina <stjepang@gmail.com>
209: add feature guards for unstable features r=yoshuawuyts a=yoshuawuyts
Makes sure unstable features aren't accidentally usable without their corresponding flags. Thanks!
Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Co-authored-by: Yoshua Wuyts <yoshuawuyts+github@gmail.com>
