io::Cursor is a bit out of place

[alexcrichton]

I think this may not wish to use io::Cursor pervasively. This use case doesn't have much to do with std::io. The std version also stores a u64 which isn't needed for bytes's use case I believe.

[alexcrichton]

I also just twitch a bit whenever I see io::Cursor, I sort of regret adding it to std, it doesn't seem to really pull its weight.

[carllerche]

Makes sense. I mostly opted for Cursor to not add another type.

Another option would be to bring back ByteBuf or something like that?

[alexcrichton]

I think a local "cursor type" (w/e the name) may be right here, but I wouldn't add it just yet (I don't know what it should look like)

[est31]

I also just twitch a bit whenever I see io::Cursor, I sort of regret adding it to std, it doesn't seem to really pull its weight.

A bit off topic, but note that I love it being in std, you can easily use it to Read over a buffer. Very interesting when you only offer a io::Read based API and people want to use it with their memory buffered data (best use case is tests).

[carllerche]

Using Cursor here is definitely a mistake on my part...

std implements Read and Write for &[u8] and &mut [u8] respectively. We probably want to follow that pattern and implement Buf / BufMut for &[u8] and &mut [u8].

[tarcieri]

+1 on this. io::Cursor complicates no_std usage. I had to vendor it in to my PR to add no_std support: #135

Perhaps I should change that PR to just stop using io::Cursor entirely?

[tarcieri]

I opened a PR which vendors std::io::Cursor as buf::Cursor (while also reducing the position counter size to u32): #146

I'm not sure that's entirely ideal but I don't see any other path forward here. I need something like it to make progress on #135

[seanmonstar]

So to clarify what will happen in v0.5:

• impl Buf for Bytes directly (remove IntoBuf of Cursor<Self>)
• impl Buf for BytesMut directly (remove IntoBuf of Cursor<Self>)
• impl IntoBuf for &Bytes to return &[u8]
• impl IntoBuf for &BytesMut to return &[u8]
• impl IntoIterator for Bytes to use slice::Iter
• impl IntoIterator for BytesMut to use vec::IntoIter

Look right?

[carllerche]

Looks bout right.

[YetAnotherMinion]

I am looking at implementing IntoBuf for BytesMut, to solve the tokio-io problem. I am trying to understand what needs to happen

Am I reading the issues correctly that you want to get rid of Cursor. Does the replacement implementation you have in mind look like adding a position field to Bytes and BytesMut structs?

[YetAnotherMinion]

Never mind, I think I figured out how to do it using the fields of Inner.

The only problem I ran into is the removal of set_position. My understanding is that using only the elements of Inner, advancing the buffer would either be permanent or affect all copies. Is the ability to seek backwards used by anyone?.

With the current design, it would be impossible to seek backwards when in KIND_INLINE mode anyway because that information is thrown away. The documentation on advance indicates the first bytes are dropped, so the only reason we could rewind is because of extra functionality io::Cursor provided (using extra storage to avoid modifying the original impl Buf)

Update: I got the tests to pass, but it reveals in my opinion a dubious iterator implementation for Buf that consumes the Buf, emptying it of data. This went unnoticed before because Cursor was wrapping the underlying storage.

diff --git a/src/buf/iter.rs b/src/buf/iter.rs
index 9345c05..389c252 100644
--- a/src/buf/iter.rs
+++ b/src/buf/iter.rs
@@ -76,14 +76,14 @@ impl<T> Iter<T> {
     /// ```rust
     /// use bytes::{Buf, IntoBuf, BytesMut};
     ///
-    /// let buf = BytesMut::from(&b"abc"[..]).into_buf();
+    /// let buf = BytesMut::from(&b"abc"[..]);
     /// let mut iter = buf.iter();
     ///
     /// assert_eq!(iter.next(), Some(b'a'));
     ///
-    /// iter.get_mut().set_position(0);
+    /// iter.get_mut().advance(1);
     ///
-    /// assert_eq!(iter.next(), Some(b'a'));
+    /// assert_eq!(iter.next(), Some(b'c'));
     /// ```
     pub fn get_mut(&mut self) -> &mut T {
         &mut self.inner
diff --git a/src/bytes.rs b/src/bytes.rs
index 4650235..979f3fc 100644
--- a/src/bytes.rs
+++ b/src/bytes.rs
@@ -1516,6 +1516,26 @@ impl BytesMut {
     }
 }
 
+impl Buf for BytesMut {
+    #[inline]
+    fn remaining(&self) -> usize {
+        self.len()
+    }
+
+    fn bytes(&self) -> &[u8] {
+        &(self.inner.as_ref())
+    }
+
+    //fn bytes_vec<'b>(&'b self, dst: &mut [IoVec<'b>]) -> usize {
+    //    (**self).bytes_vec(dst)
+    //}
+
+    fn advance(&mut self, cnt: usize) {
+        assert!(cnt <= self.inner.as_ref().len(), "cannot advance past `remaining`");
+        unsafe { self.inner.set_start(cnt); }
+    }
+}
+
 impl BufMut for BytesMut {
     #[inline]
     fn remaining_mut(&self) -> usize {
@@ -1561,22 +1581,6 @@ impl BufMut for BytesMut {
     }
 }
 
-impl IntoBuf for BytesMut {
-    type Buf = Cursor<Self>;
-
-    fn into_buf(self) -> Self::Buf {
-        Cursor::new(self)
-    }
-}
-
-impl<'a> IntoBuf for &'a BytesMut {
-    type Buf = Cursor<&'a BytesMut>;
-
-    fn into_buf(self) -> Self::Buf {
-        Cursor::new(self)
-    }
-}
-
 impl AsRef<[u8]> for BytesMut {
     #[inline]
     fn as_ref(&self) -> &[u8] {
@@ -1741,19 +1745,19 @@ impl Clone for BytesMut {
 
 impl IntoIterator for BytesMut {
     type Item = u8;
-    type IntoIter = Iter<Cursor<BytesMut>>;
+    type IntoIter = Iter<BytesMut>;
 
     fn into_iter(self) -> Self::IntoIter {
-        self.into_buf().iter()
+        self.iter()
     }
 }
 
-impl<'a> IntoIterator for &'a BytesMut {
+impl<'a> IntoIterator for &'a mut BytesMut {
     type Item = u8;
-    type IntoIter = Iter<Cursor<&'a BytesMut>>;
+    type IntoIter = Iter<&'a mut BytesMut>;
 
     fn into_iter(self) -> Self::IntoIter {
-        self.into_buf().iter()
+        self.iter()
     }
 }
 
diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs
index ccc89fa..67ff12c 100644
--- a/tests/test_bytes.rs
+++ b/tests/test_bytes.rs
@@ -181,11 +181,11 @@ fn split_off_to_loop() {
         }
         {
             let mut bytes = BytesMut::from(&s[..]);
-            let off = bytes.split_off(i);
+            let mut off = bytes.split_off(i);
             assert_eq!(i, bytes.len());
             let mut sum = Vec::new();
-            sum.extend(&bytes);
-            sum.extend(&off);
+            sum.extend(&mut bytes);
+            sum.extend(&mut off);
             assert_eq!(&s[..], &sum[..]);
         }
         {
@@ -199,11 +199,11 @@ fn split_off_to_loop() {
         }
         {
             let mut bytes = BytesMut::from(&s[..]);
-            let off = bytes.split_to(i);
+            let mut off = bytes.split_to(i);
             assert_eq!(i, off.len());
             let mut sum = Vec::new();
-            sum.extend(&off);
-            sum.extend(&bytes);
+            sum.extend(&mut off);
+            sum.extend(&mut bytes);
             assert_eq!(&s[..], &sum[..]);
         }
     }