Reverted 'static constraint on T in Vec<T> and [T; N] (#663)

bincode-org · Sep 19, 2023 · a22afa3 · a22afa3
1 parent 2f08107
commit a22afa3
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 55 deletions.
diff --git a/src/de/impls.rs b/src/de/impls.rs
@@ -8,7 +8,6 @@ use crate::{
     impl_borrow_decode,
 };
 use core::{
-    any::TypeId,
     cell::{Cell, RefCell},
     num::{
         NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize, NonZeroU128,
@@ -442,63 +441,65 @@ impl<'a, 'de: 'a> BorrowDecode<'de> for &'a str {
 
 impl<T, const N: usize> Decode for [T; N]
 where
-    T: Decode + Sized + 'static,
+    T: Decode + Sized,
 {
     fn decode<D: Decoder>(decoder: &mut D) -> Result<Self, DecodeError> {
         decoder.claim_bytes_read(core::mem::size_of::<[T; N]>())?;
 
-        // Optimize for `[u8; N]`
-        if TypeId::of::<u8>() == TypeId::of::<T>() {
-            let mut buf = [0u8; N];
-            decoder.reader().read(&mut buf)?;
-            let ptr = &mut buf as *mut _ as *mut [T; N];
+        // TODO: we can't limit `T: 'static` because that would break other things
+        // but we want to have this optimization
+        // This will be another contendor for specialization implementation
+        // if TypeId::of::<u8>() == TypeId::of::<T>() {
+        //     let mut buf = [0u8; N];
+        //     decoder.reader().read(&mut buf)?;
+        //     let ptr = &mut buf as *mut _ as *mut [T; N];
 
-            // Safety: we know that T is a u8, so it is perfectly safe to
-            // translate an array of u8 into an array of T
-            let res = unsafe { ptr.read() };
-            Ok(res)
-        } else {
-            let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| {
-                // See the documentation on `unclaim_bytes_read` as to why we're doing this here
-                decoder.unclaim_bytes_read(core::mem::size_of::<T>());
-                T::decode(decoder)
-            }));
-
-            // result is only None if N does not match the values of `(0..N)`, which it always should
-            // So this unwrap should never occur
-            result.unwrap()
-        }
+        //     // Safety: we know that T is a u8, so it is perfectly safe to
+        //     // translate an array of u8 into an array of T
+        //     let res = unsafe { ptr.read() };
+        //     Ok(res)
+        // }
+        let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| {
+            // See the documentation on `unclaim_bytes_read` as to why we're doing this here
+            decoder.unclaim_bytes_read(core::mem::size_of::<T>());
+            T::decode(decoder)
+        }));
+
+        // result is only None if N does not match the values of `(0..N)`, which it always should
+        // So this unwrap should never occur
+        result.unwrap()
     }
 }
 
 impl<'de, T, const N: usize> BorrowDecode<'de> for [T; N]
 where
-    T: BorrowDecode<'de> + Sized + 'static,
+    T: BorrowDecode<'de> + Sized,
 {
     fn borrow_decode<D: BorrowDecoder<'de>>(decoder: &mut D) -> Result<Self, DecodeError> {
         decoder.claim_bytes_read(core::mem::size_of::<[T; N]>())?;
 
-        // Optimize for `[u8; N]`
-        if TypeId::of::<u8>() == TypeId::of::<T>() {
-            let mut buf = [0u8; N];
-            decoder.reader().read(&mut buf)?;
-            let ptr = &mut buf as *mut _ as *mut [T; N];
-
-            // Safety: we know that T is a u8, so it is perfectly safe to
-            // translate an array of u8 into an array of T
-            let res = unsafe { ptr.read() };
-            Ok(res)
-        } else {
-            let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| {
-                // See the documentation on `unclaim_bytes_read` as to why we're doing this here
-                decoder.unclaim_bytes_read(core::mem::size_of::<T>());
-                T::borrow_decode(decoder)
-            }));
-
-            // result is only None if N does not match the values of `(0..N)`, which it always should
-            // So this unwrap should never occur
-            result.unwrap()
-        }
+        // TODO: we can't limit `T: 'static` because that would break other things
+        // but we want to have this optimization
+        // This will be another contendor for specialization implementation
+        // if TypeId::of::<u8>() == TypeId::of::<T>() {
+        //     let mut buf = [0u8; N];
+        //     decoder.reader().read(&mut buf)?;
+        //     let ptr = &mut buf as *mut _ as *mut [T; N];
+
+        //     // Safety: we know that T is a u8, so it is perfectly safe to
+        //     // translate an array of u8 into an array of T
+        //     let res = unsafe { ptr.read() };
+        //     Ok(res)
+        // }
+        let result = super::impl_core::collect_into_array(&mut (0..N).map(|_| {
+            // See the documentation on `unclaim_bytes_read` as to why we're doing this here
+            decoder.unclaim_bytes_read(core::mem::size_of::<T>());
+            T::borrow_decode(decoder)
+        }));
+
+        // result is only None if N does not match the values of `(0..N)`, which it always should
+        // So this unwrap should never occur
+        result.unwrap()
     }
 }
 

diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs
@@ -1,5 +1,5 @@
 use crate::{
-    de::{read::Reader, BorrowDecoder, Decode, Decoder},
+    de::{BorrowDecoder, Decode, Decoder},
     enc::{
         self,
         write::{SizeWriter, Writer},
@@ -278,20 +278,23 @@ where
 
 impl<T> Decode for Vec<T>
 where
-    T: Decode + 'static,
+    T: Decode,
 {
     fn decode<D: Decoder>(decoder: &mut D) -> Result<Self, DecodeError> {
         let len = crate::de::decode_slice_len(decoder)?;
 
-        if core::any::TypeId::of::<T>() == core::any::TypeId::of::<u8>() {
-            decoder.claim_container_read::<T>(len)?;
-            // optimize for reading u8 vecs
-            let mut vec = Vec::new();
-            vec.resize(len, 0u8);
-            decoder.reader().read(&mut vec)?;
-            // Safety: Vec<T> is Vec<u8>
-            return Ok(unsafe { core::mem::transmute(vec) });
-        }
+        // TODO: we can't limit `T: 'static` because that would break other things
+        // but we want to have this optimization
+        // This will be another contendor for specialization implementation
+        // if core::any::TypeId::of::<T>() == core::any::TypeId::of::<u8>() {
+        //     decoder.claim_container_read::<T>(len)?;
+        //     // optimize for reading u8 vecs
+        //     let mut vec = Vec::new();
+        //     vec.resize(len, 0u8);
+        //     decoder.reader().read(&mut vec)?;
+        //     // Safety: Vec<T> is Vec<u8>
+        //     return Ok(unsafe { core::mem::transmute(vec) });
+        // }
         decoder.claim_container_read::<T>(len)?;
 
         let mut vec = Vec::with_capacity(len);

diff --git a/tests/std.rs b/tests/std.rs
@@ -179,3 +179,24 @@ impl std::hash::Hasher for ExampleCustomHasher {
         self.hash
     }
 }
+
+#[test]
+fn test_decode_borrow_str_in_array() {
+    let (strs, _): (Vec<&str>, usize) = bincode::borrow_decode_from_slice(
+        &[
+            3, 3, b'a', b'b', b'c', 3, b'd', b'e', b'f', 3, b'g', b'h', b'i',
+        ],
+        bincode::config::standard(),
+    )
+    .unwrap();
+    assert_eq!(strs, vec!["abc", "def", "ghi"]);
+
+    let (strs, _): ([&str; 3], usize) = bincode::borrow_decode_from_slice(
+        &[
+            3, b'a', b'b', b'c', 3, b'd', b'e', b'f', 3, b'g', b'h', b'i',
+        ],
+        bincode::config::standard(),
+    )
+    .unwrap();
+    assert_eq!(strs, ["abc", "def", "ghi"]);
+}