From fa1daac3ae1dcb07dffe3a41a041dffd6edf177b Mon Sep 17 00:00:00 2001 From: Anthony Ramine <123095+nox@users.noreply.github.com> Date: Tue, 28 May 2024 10:14:02 +0200 Subject: [PATCH] Change Bytes::make_mut to impl From for BytesMut (closes #709) (#710) >::make_mut returns a &mut T, such an API is doable for Bytes too and thus we should reserve Bytes::make_mut for that. Furthermore, it would be helpful to use From as a trait bound in some cases with other traits such as Hyper's body trait, where Hyper gives you Bytes values. Finally, making it impl From for BytesMut means the API is more easily discoverable as it appears on both Bytes and BytesMut. --- src/bytes.rs | 44 ++++++++++++++++++----------------- src/bytes/promotable.rs | 0 tests/test_bytes.rs | 40 +++++++++++++++---------------- tests/test_bytes_odd_alloc.rs | 22 +++++++++--------- 4 files changed, 54 insertions(+), 52 deletions(-) create mode 100644 src/bytes/promotable.rs diff --git a/src/bytes.rs b/src/bytes.rs index e23d9a81f..e0c33b3e6 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -525,32 +525,12 @@ impl Bytes { /// ``` pub fn try_into_mut(self) -> Result { if self.is_unique() { - Ok(self.make_mut()) + Ok(self.into()) } else { Err(self) } } - /// Convert self into `BytesMut`. - /// - /// If `self` is unique for the entire original buffer, this will return a - /// `BytesMut` with the contents of `self` without copying. - /// If `self` is not unique for the entire original buffer, this will make - /// a copy of `self` subset of the original buffer in a new `BytesMut`. - /// - /// # Examples - /// - /// ``` - /// use bytes::{Bytes, BytesMut}; - /// - /// let bytes = Bytes::from(b"hello".to_vec()); - /// assert_eq!(bytes.make_mut(), BytesMut::from(&b"hello"[..])); - /// ``` - pub fn make_mut(self) -> BytesMut { - let bytes = ManuallyDrop::new(self); - unsafe { (bytes.vtable.to_mut)(&bytes.data, bytes.ptr, bytes.len) } - } - #[inline] pub(crate) unsafe fn with_vtable( ptr: *const u8, @@ -932,6 +912,28 @@ impl From> for Bytes { } } +impl From for BytesMut { + /// Convert self into `BytesMut`. + /// + /// If `bytes` is unique for the entire original buffer, this will return a + /// `BytesMut` with the contents of `bytes` without copying. + /// If `bytes` is not unique for the entire original buffer, this will make + /// a copy of `bytes` subset of the original buffer in a new `BytesMut`. + /// + /// # Examples + /// + /// ``` + /// use bytes::{Bytes, BytesMut}; + /// + /// let bytes = Bytes::from(b"hello".to_vec()); + /// assert_eq!(BytesMut::from(bytes), BytesMut::from(&b"hello"[..])); + /// ``` + fn from(bytes: Bytes) -> Self { + let bytes = ManuallyDrop::new(bytes); + unsafe { (bytes.vtable.to_mut)(&bytes.data, bytes.ptr, bytes.len) } + } +} + impl From for Bytes { fn from(s: String) -> Bytes { Bytes::from(s.into_bytes()) diff --git a/src/bytes/promotable.rs b/src/bytes/promotable.rs new file mode 100644 index 000000000..e69de29bb diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index 2f283af2f..3ac429832 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -1174,29 +1174,29 @@ fn shared_is_unique() { } #[test] -fn test_bytes_make_mut_static() { +fn test_bytesmut_from_bytes_static() { let bs = b"1b23exfcz3r"; // Test STATIC_VTABLE.to_mut - let bytes_mut = Bytes::from_static(bs).make_mut(); + let bytes_mut = BytesMut::from(Bytes::from_static(bs)); assert_eq!(bytes_mut, bs[..]); } #[test] -fn test_bytes_make_mut_bytes_mut_vec() { +fn test_bytesmut_from_bytes_bytes_mut_vec() { let bs = b"1b23exfcz3r"; let bs_long = b"1b23exfcz3r1b23exfcz3r"; // Test case where kind == KIND_VEC let mut bytes_mut: BytesMut = bs[..].into(); - bytes_mut = bytes_mut.freeze().make_mut(); + bytes_mut = BytesMut::from(bytes_mut.freeze()); assert_eq!(bytes_mut, bs[..]); bytes_mut.extend_from_slice(&bs[..]); assert_eq!(bytes_mut, bs_long[..]); } #[test] -fn test_bytes_make_mut_bytes_mut_shared() { +fn test_bytesmut_from_bytes_bytes_mut_shared() { let bs = b"1b23exfcz3r"; // Set kind to KIND_ARC so that after freeze, Bytes will use bytes_mut.SHARED_VTABLE @@ -1207,17 +1207,17 @@ fn test_bytes_make_mut_bytes_mut_shared() { let b2 = b1.clone(); // shared.is_unique() = False - let mut b1m = b1.make_mut(); + let mut b1m = BytesMut::from(b1); assert_eq!(b1m, bs[..]); b1m[0] = b'9'; // shared.is_unique() = True - let b2m = b2.make_mut(); + let b2m = BytesMut::from(b2); assert_eq!(b2m, bs[..]); } #[test] -fn test_bytes_make_mut_bytes_mut_offset() { +fn test_bytesmut_from_bytes_bytes_mut_offset() { let bs = b"1b23exfcz3r"; // Test bytes_mut.SHARED_VTABLE.to_mut impl where offset != 0 @@ -1227,58 +1227,58 @@ fn test_bytes_make_mut_bytes_mut_offset() { let b1 = bytes_mut1.freeze(); let b2 = bytes_mut2.freeze(); - let b1m = b1.make_mut(); - let b2m = b2.make_mut(); + let b1m = BytesMut::from(b1); + let b2m = BytesMut::from(b2); assert_eq!(b2m, bs[9..]); assert_eq!(b1m, bs[..9]); } #[test] -fn test_bytes_make_mut_promotable_even_vec() { +fn test_bytesmut_from_bytes_promotable_even_vec() { let vec = vec![33u8; 1024]; // Test case where kind == KIND_VEC let b1 = Bytes::from(vec.clone()); - let b1m = b1.make_mut(); + let b1m = BytesMut::from(b1); assert_eq!(b1m, vec); } #[test] -fn test_bytes_make_mut_promotable_even_arc_1() { +fn test_bytesmut_from_bytes_promotable_even_arc_1() { let vec = vec![33u8; 1024]; // Test case where kind == KIND_ARC, ref_cnt == 1 let b1 = Bytes::from(vec.clone()); drop(b1.clone()); - let b1m = b1.make_mut(); + let b1m = BytesMut::from(b1); assert_eq!(b1m, vec); } #[test] -fn test_bytes_make_mut_promotable_even_arc_2() { +fn test_bytesmut_from_bytes_promotable_even_arc_2() { let vec = vec![33u8; 1024]; // Test case where kind == KIND_ARC, ref_cnt == 2 let b1 = Bytes::from(vec.clone()); let b2 = b1.clone(); - let b1m = b1.make_mut(); + let b1m = BytesMut::from(b1); assert_eq!(b1m, vec); // Test case where vtable = SHARED_VTABLE, kind == KIND_ARC, ref_cnt == 1 - let b2m = b2.make_mut(); + let b2m = BytesMut::from(b2); assert_eq!(b2m, vec); } #[test] -fn test_bytes_make_mut_promotable_even_arc_offset() { +fn test_bytesmut_from_bytes_promotable_even_arc_offset() { let vec = vec![33u8; 1024]; // Test case where offset != 0 let mut b1 = Bytes::from(vec.clone()); let b2 = b1.split_off(20); - let b1m = b1.make_mut(); - let b2m = b2.make_mut(); + let b1m = BytesMut::from(b1); + let b2m = BytesMut::from(b2); assert_eq!(b2m, vec[20..]); assert_eq!(b1m, vec[..20]); diff --git a/tests/test_bytes_odd_alloc.rs b/tests/test_bytes_odd_alloc.rs index 8008a0e47..4758dc2f9 100644 --- a/tests/test_bytes_odd_alloc.rs +++ b/tests/test_bytes_odd_alloc.rs @@ -6,7 +6,7 @@ use std::alloc::{GlobalAlloc, Layout, System}; use std::ptr; -use bytes::Bytes; +use bytes::{Bytes, BytesMut}; #[global_allocator] static ODD: Odd = Odd; @@ -97,50 +97,50 @@ fn test_bytes_into_vec() { } #[test] -fn test_bytes_make_mut_vec() { +fn test_bytesmut_from_bytes_vec() { let vec = vec![33u8; 1024]; // Test case where kind == KIND_VEC let b1 = Bytes::from(vec.clone()); - let b1m = b1.make_mut(); + let b1m = BytesMut::from(b1); assert_eq!(b1m, vec); } #[test] -fn test_bytes_make_mut_arc_1() { +fn test_bytesmut_from_bytes_arc_1() { let vec = vec![33u8; 1024]; // Test case where kind == KIND_ARC, ref_cnt == 1 let b1 = Bytes::from(vec.clone()); drop(b1.clone()); - let b1m = b1.make_mut(); + let b1m = BytesMut::from(b1); assert_eq!(b1m, vec); } #[test] -fn test_bytes_make_mut_arc_2() { +fn test_bytesmut_from_bytes_arc_2() { let vec = vec![33u8; 1024]; // Test case where kind == KIND_ARC, ref_cnt == 2 let b1 = Bytes::from(vec.clone()); let b2 = b1.clone(); - let b1m = b1.make_mut(); + let b1m = BytesMut::from(b1); assert_eq!(b1m, vec); // Test case where vtable = SHARED_VTABLE, kind == KIND_ARC, ref_cnt == 1 - let b2m = b2.make_mut(); + let b2m = BytesMut::from(b2); assert_eq!(b2m, vec); } #[test] -fn test_bytes_make_mut_arc_offset() { +fn test_bytesmut_from_bytes_arc_offset() { let vec = vec![33u8; 1024]; // Test case where offset != 0 let mut b1 = Bytes::from(vec.clone()); let b2 = b1.split_off(20); - let b1m = b1.make_mut(); - let b2m = b2.make_mut(); + let b1m = BytesMut::from(b1); + let b2m = BytesMut::from(b2); assert_eq!(b2m, vec[20..]); assert_eq!(b1m, vec[..20]);