Merge #266

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>

src/lib.rs

@@ -68,6 +68,7 @@ cfg_if! {
         mod vec;
         mod result;
         mod option;
+        mod string;
     }
 }
 

src/stream/extend.rs

@@ -33,7 +33,9 @@ pub trait Extend<A> {
     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;
 }
 
 impl Extend<()> for () {

src/string/extend.rs

@@ -0,0 +1,67 @@
+use std::borrow::Cow;
+use std::pin::Pin;
+
+use crate::prelude::*;
+use crate::stream::{Extend, IntoStream};
+
+impl Extend<char> for String {
+    fn stream_extend<'a, S: IntoStream<Item = char> + 'a>(
+        &'a mut self,
+        stream: S,
+    ) -> Pin<Box<dyn Future<Output = ()> + 'a>> {
+        let stream = stream.into_stream();
+        //TODO: Add this back in when size_hint is added to Stream/StreamExt
+        // let (lower_bound, _) = stream.size_hint();
+        // self.reserve(lower_bound);
+
+        Box::pin(stream.for_each(move |c| self.push(c)))
+    }
+}
+
+impl<'b> Extend<&'b char> for String {
+    fn stream_extend<'a, S: IntoStream<Item = &'b char> + 'a>(
+        &'a mut self,
+        //TODO: Remove the underscore when uncommenting the body of this impl
+        _stream: S,
+    ) -> Pin<Box<dyn Future<Output = ()> + 'a>>
+    where
+        'b: 'a,
+    {
+        //TODO: This can be uncommented when `copied` is added to Stream/StreamExt
+        //Box::pin(stream.into_stream().copied())
+        unimplemented!()
+    }
+}
+
+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,
+    {
+        Box::pin(stream.into_stream().for_each(move |s| self.push_str(s)))
+    }
+}
+
+impl Extend<String> for String {
+    fn stream_extend<'a, S: IntoStream<Item = String> + 'a>(
+        &'a mut self,
+        stream: S,
+    ) -> Pin<Box<dyn Future<Output = ()> + 'a>> {
+        Box::pin(stream.into_stream().for_each(move |s| self.push_str(&s)))
+    }
+}
+
+impl<'b> Extend<Cow<'b, str>> for String {
+    fn stream_extend<'a, S: IntoStream<Item = Cow<'b, str>> + 'a>(
+        &'a mut self,
+        stream: S,
+    ) -> Pin<Box<dyn Future<Output = ()> + 'a>>
+    where
+        'b: 'a,
+    {
+        Box::pin(stream.into_stream().for_each(move |s| self.push_str(&s)))
+    }
+}

src/string/from_stream.rs

@@ -0,0 +1,104 @@
+use std::borrow::Cow;
+use std::pin::Pin;
+
+use crate::stream::{Extend, FromStream, IntoStream};
+
+impl FromStream<char> for String {
+    #[inline]
+    fn from_stream<'a, S: IntoStream<Item = char>>(
+        stream: S,
+    ) -> Pin<Box<dyn core::future::Future<Output = Self> + 'a>>
+    where
+        <S as IntoStream>::IntoStream: 'a,
+    {
+        let stream = stream.into_stream();
+
+        Box::pin(async move {
+            pin_utils::pin_mut!(stream);
+
+            let mut out = String::new();
+            out.stream_extend(stream).await;
+            out
+        })
+    }
+}
+
+impl<'b> FromStream<&'b char> for String {
+    #[inline]
+    fn from_stream<'a, S: IntoStream<Item = &'b char>>(
+        stream: S,
+    ) -> Pin<Box<dyn core::future::Future<Output = Self> + 'a>>
+    where
+        <S as IntoStream>::IntoStream: 'a,
+    {
+        let stream = stream.into_stream();
+
+        Box::pin(async move {
+            pin_utils::pin_mut!(stream);
+
+            let mut out = String::new();
+            out.stream_extend(stream).await;
+            out
+        })
+    }
+}
+
+impl<'b> FromStream<&'b str> for String {
+    #[inline]
+    fn from_stream<'a, S: IntoStream<Item = &'b str>>(
+        stream: S,
+    ) -> Pin<Box<dyn core::future::Future<Output = Self> + 'a>>
+    where
+        <S as IntoStream>::IntoStream: 'a,
+    {
+        let stream = stream.into_stream();
+
+        Box::pin(async move {
+            pin_utils::pin_mut!(stream);
+
+            let mut out = String::new();
+            out.stream_extend(stream).await;
+            out
+        })
+    }
+}
+
+impl FromStream<String> for String {
+    #[inline]
+    fn from_stream<'a, S: IntoStream<Item = String>>(
+        stream: S,
+    ) -> Pin<Box<dyn core::future::Future<Output = Self> + 'a>>
+    where
+        <S as IntoStream>::IntoStream: 'a,
+    {
+        let stream = stream.into_stream();
+
+        Box::pin(async move {
+            pin_utils::pin_mut!(stream);
+
+            let mut out = String::new();
+            out.stream_extend(stream).await;
+            out
+        })
+    }
+}
+
+impl<'b> FromStream<Cow<'b, str>> for String {
+    #[inline]
+    fn from_stream<'a, S: IntoStream<Item = Cow<'b, str>>>(
+        stream: S,
+    ) -> Pin<Box<dyn core::future::Future<Output = Self> + 'a>>
+    where
+        <S as IntoStream>::IntoStream: 'a,
+    {
+        let stream = stream.into_stream();
+
+        Box::pin(async move {
+            pin_utils::pin_mut!(stream);
+
+            let mut out = String::new();
+            out.stream_extend(stream).await;
+            out
+        })
+    }
+}

src/string/mod.rs

@@ -0,0 +1,9 @@
+//! The Rust core string library
+//!
+//! This library provides a UTF-8 encoded, growable string.
+
+mod extend;
+mod from_stream;
+
+#[doc(inline)]
+pub use std::string::String;