Bump bitvec, byte slice cast, and refactor a bit (#236)

Cargo.toml

@@ -12,9 +12,9 @@ edition = "2018"
 arrayvec = { version = "0.5.1", default-features = false, features = ["array-sizes-33-128", "array-sizes-129-255"] }
 serde = { version = "1.0.102", optional = true }
 parity-scale-codec-derive = { path = "derive", version = "1.2.0", default-features = false, optional = true }
-bitvec = { version = "0.17.4", default-features = false, features = ["alloc"], optional = true }
-byte-slice-cast = { version = "0.3.4", default-features = false, features = ["alloc"] }
-generic-array = { version = "0.13.2", optional = true }
+bitvec = { version = "0.20.1", default-features = false, features = ["alloc"], optional = true }
+byte-slice-cast = { version = "1.0.0", default-features = false }
+generic-array = { version = "0.14.4", optional = true }
 arbitrary = { version = "0.4.1", features = ["derive"], optional = true }
 
 [dev-dependencies]

benches/benches.rs

@@ -15,7 +15,7 @@
 use std::{time::Duration, any::type_name, convert::{TryFrom, TryInto}};
 
 #[cfg(feature = "bit-vec")]
-use bitvec::vec::BitVec;
+use bitvec::{vec::BitVec, order::Lsb0};
 use criterion::{Criterion, black_box, Bencher, criterion_group, criterion_main};
 use parity_scale_codec::*;
 use parity_scale_codec_derive::{Encode, Decode};
@@ -203,7 +203,7 @@ fn encode_decode_bitvec_u8(c: &mut Criterion) {
 
 	#[cfg(feature = "bit-vec")]
 	c.bench_function_over_inputs("bitvec_u8_encode - BitVec<u8>", |b, &size| {
-		let vec: BitVec = [true, false]
+		let vec: BitVec<Lsb0, u8> = [true, false]
 			.iter()
 			.cloned()
 			.cycle()
@@ -216,7 +216,7 @@ fn encode_decode_bitvec_u8(c: &mut Criterion) {
 
 	#[cfg(feature = "bit-vec")]
 	c.bench_function_over_inputs("bitvec_u8_decode - BitVec<u8>", |b, &size| {
-		let vec: BitVec = [true, false]
+		let vec: BitVec<Lsb0, u8> = [true, false]
 			.iter()
 			.cloned()
 			.cycle()
@@ -227,7 +227,7 @@ fn encode_decode_bitvec_u8(c: &mut Criterion) {
 
 		let vec = black_box(vec);
 		b.iter(|| {
-			let _: BitVec = Decode::decode(&mut &vec[..]).unwrap();
+			let _: BitVec<Lsb0, u8> = Decode::decode(&mut &vec[..]).unwrap();
 		})
 	}, vec![1, 2, 5, 32, 1024]);
 }

fuzzer/Cargo.toml

@@ -9,4 +9,4 @@ publish = false
 parity-scale-codec = { path = "../", features = [ "derive", "bit-vec", "fuzz" ] }
 honggfuzz = "0.5.47"
 arbitrary = { version = "0.4.1", features = ["derive"] }
-bitvec = { version = "0.17.4", features = ["alloc"] }
+bitvec = { version = "0.20.1", features = ["alloc"] }

src/bit_vec.rs

@@ -14,52 +14,14 @@
 
 //! `BitVec` specific serialization.
 
-use core::mem;
-use crate::alloc::vec::Vec;
-
-use bitvec::{vec::BitVec, store::BitStore, order::BitOrder, slice::BitSlice, boxed::BitBox};
-use byte_slice_cast::{AsByteSlice, ToByteSlice, FromByteSlice, Error as FromByteSliceError};
-
-use crate::codec::{Encode, Decode, Input, Output, Error, read_vec_from_u8s};
+use bitvec::{
+	vec::BitVec, store::BitStore, order::BitOrder, slice::BitSlice, boxed::BitBox, mem::BitMemory
+};
+use crate::codec::{Encode, Decode, Input, Output, Error, decode_vec_with_len, encode_slice_no_len};
 use crate::compact::Compact;
 use crate::EncodeLike;
 
-impl From<FromByteSliceError> for Error {
-	fn from(e: FromByteSliceError) -> Error {
-		match e {
-			FromByteSliceError::AlignmentMismatch {..} =>
-				"failed to cast from byte slice: alignment mismatch".into(),
-			FromByteSliceError::LengthMismatch {..} =>
-				"failed to cast from byte slice: length mismatch".into(),
-			FromByteSliceError::CapacityMismatch {..} =>
-				"failed to cast from byte slice: capacity mismatch".into(),
-		}
-	}
-}
-
-impl<O: BitOrder, T: BitStore + ToByteSlice> Encode for BitSlice<O, T> {
-	fn encode_to<W: Output>(&self, dest: &mut W) {
-		self.to_vec().encode_to(dest)
-	}
-}
-
-/// Reverse bytes of element for element of size `size_of_t`.
-///
-/// E.g. if size is 2 `[1, 2, 3, 4]` is changed to `[2, 1, 4, 3]`.
-fn reverse_endian(vec_u8: &mut [u8], size_of_t: usize) {
-	for i in 0..vec_u8.len() / size_of_t {
-		for j in 0..size_of_t / 2 {
-			vec_u8.swap(i * size_of_t + j, i * size_of_t + (size_of_t - 1) - j);
-		}
-	}
-}
-
-/// # WARNING
-///
-/// In bitvec v0.17.4 the only implementations of BitStore are u8, u16, u32, u64, and usize.
-/// This implementation actually only support u8, u16, u32 and u64, as encoding of uszie
-/// is inconsistent between platforms.
-impl<O: BitOrder, T: BitStore + ToByteSlice> Encode for BitVec<O, T> {
+impl<O: BitOrder, T: BitStore + Encode> Encode for BitSlice<O, T> {
 	fn encode_to<W: Output>(&self, dest: &mut W) {
 		let len = self.len();
 		assert!(
@@ -68,72 +30,75 @@ impl<O: BitOrder, T: BitStore + ToByteSlice> Encode for BitVec<O, T> {
 		);
 		Compact(len as u32).encode_to(dest);
 
-		let byte_slice: &[u8] = self.as_slice().as_byte_slice();
+		// NOTE: doc of `BitSlice::as_slice`:
+		// > The returned slice handle views all elements touched by self
+		//
+		// Thus we are sure the slice doesn't contain unused elements at the end.
+		let slice = self.as_slice();
 
-		if cfg!(target_endian = "big") && mem::size_of::<T>() > 1 {
-			let mut vec_u8: Vec<u8> = byte_slice.into();
-			reverse_endian(&mut vec_u8[..], mem::size_of::<T>());
-			dest.write(&vec_u8);
-		} else {
-			dest.write(byte_slice);
-		}
+		encode_slice_no_len(slice, dest)
 	}
 }
 
-impl<O: BitOrder, T: BitStore + ToByteSlice> EncodeLike for BitVec<O, T> {}
-
-/// # WARNING
-///
-/// In bitvec v0.17.4 the only implementations of BitStore are u8, u16, u32, u64, and usize.
-/// This implementation actually only support u8, u16, u32 and u64, as encoding of usize
-/// is inconsistent between platforms.
-impl<O: BitOrder, T: BitStore + FromByteSlice> Decode for BitVec<O, T> {
-	fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
-		<Compact<u32>>::decode(input).and_then(move |Compact(bits)| {
-			let bits = bits as usize;
-			let required_bytes = required_bytes::<T>(bits);
-
-			let mut vec_u8 = read_vec_from_u8s::<I, u8>(input, required_bytes)?;
+impl<O: BitOrder, T: BitStore + Encode> Encode for BitVec<O, T> {
+	fn encode_to<W: Output>(&self, dest: &mut W) {
+		self.as_bitslice().encode_to(dest)
+	}
+}
 
-			if cfg!(target_endian = "big") && mem::size_of::<T>() > 1 {
-				reverse_endian(&mut vec_u8[..], mem::size_of::<T>());
-			}
+impl<O: BitOrder, T: BitStore + Encode> EncodeLike for BitVec<O, T> {}
 
-			let mut aligned_vec: Vec<T> = vec![0u8.into(); required_bytes / mem::size_of::<T>()];
+/// Equivalent of `BitStore::MAX_BITS` on 32bit machine.
+const ARCH32BIT_BITSLICE_MAX_BITS: usize = 0x1fff_ffff;
 
-			unsafe {
-				let aligned_u8_ptr = aligned_vec.as_mut_ptr() as *mut u8;
-				for (i, v) in vec_u8.iter().enumerate() {
-					*aligned_u8_ptr.add(i) = *v;
-				}
+impl<O: BitOrder, T: BitStore + Decode> Decode for BitVec<O, T> {
+	fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
+		<Compact<u32>>::decode(input).and_then(move |Compact(bits)| {
+			// Otherwise it is impossible to store it on 32bit machine.
+			if bits as usize > ARCH32BIT_BITSLICE_MAX_BITS {
+				return Err("Attempt to decode a bitvec with too many bits".into());
 			}
-
-			let mut result = Self::from_vec(aligned_vec);
-			assert!(bits <= result.len());
-			unsafe { result.set_len(bits); }
+			let required_elements = required_elements::<T>(bits)? as usize;
+			let vec = decode_vec_with_len(input, required_elements)?;
+
+			let mut result = Self::try_from_vec(vec)
+				.map_err(|_| {
+					Error::from("UNEXPECTED ERROR: `bits` is less or equal to
+					`ARCH32BIT_BITSLICE_MAX_BITS`; So BitVec must be able to handle the number of
+					segment needed for `bits` to be represented; qed")
+				})?;
+
+			assert!(bits as usize <= result.len());
+			result.truncate(bits as usize);
 			Ok(result)
 		})
 	}
 }
 
-impl<O: BitOrder, T: BitStore + ToByteSlice> Encode for BitBox<O, T> {
+impl<O: BitOrder, T: BitStore + Encode> Encode for BitBox<O, T> {
 	fn encode_to<W: Output>(&self, dest: &mut W) {
 		self.as_bitslice().encode_to(dest)
 	}
 }
 
-impl<O: BitOrder, T: BitStore + ToByteSlice> EncodeLike for BitBox<O, T> {}
+impl<O: BitOrder, T: BitStore + Encode> EncodeLike for BitBox<O, T> {}
 
-impl<O: BitOrder, T: BitStore + FromByteSlice> Decode for BitBox<O, T> {
+impl<O: BitOrder, T: BitStore + Decode> Decode for BitBox<O, T> {
 	fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
 		Ok(Self::from_bitslice(BitVec::<O, T>::decode(input)?.as_bitslice()))
 	}
 }
 
-// Calculates bytes required to store given amount of `bits` as if they were stored in the array of `T`.
-fn required_bytes<T>(bits: usize) -> usize {
-	let element_bits = mem::size_of::<T>() * 8;
-	(bits + element_bits - 1) / element_bits * mem::size_of::<T>()
+/// Calculates the number of element `T` required to store given amount of `bits` as if they were
+/// stored in `BitVec<_, T>`
+///
+/// Returns an error if the number of bits + number of bits in element overflow u32 capacity.
+/// NOTE: this should never happen if `bits` is already checked to be less than
+/// `BitStore::MAX_BITS`.
+fn required_elements<T: BitStore>(bits: u32) -> Result<u32, Error> {
+	let element_bits = T::Mem::BITS as u32;
+	let error = Error::from("Attempt to decode bitvec with too many bits");
+	Ok((bits.checked_add(element_bits).ok_or_else(|| error)?  - 1) / element_bits)
 }
 
 #[cfg(test)]
@@ -177,34 +142,33 @@ mod tests {
 	}
 
 	#[test]
-	fn required_bytes_test() {
-		assert_eq!(0, required_bytes::<u8>(0));
-		assert_eq!(1, required_bytes::<u8>(1));
-		assert_eq!(1, required_bytes::<u8>(7));
-		assert_eq!(1, required_bytes::<u8>(8));
-		assert_eq!(2, required_bytes::<u8>(9));
-
-		assert_eq!(0, required_bytes::<u16>(0));
-		assert_eq!(2, required_bytes::<u16>(1));
-		assert_eq!(2, required_bytes::<u16>(15));
-		assert_eq!(2, required_bytes::<u16>(16));
-		assert_eq!(4, required_bytes::<u16>(17));
-
-		assert_eq!(0, required_bytes::<u32>(0));
-		assert_eq!(4, required_bytes::<u32>(1));
-		assert_eq!(4, required_bytes::<u32>(31));
-		assert_eq!(4, required_bytes::<u32>(32));
-		assert_eq!(8, required_bytes::<u32>(33));
-
-		assert_eq!(0, required_bytes::<u64>(0));
-		assert_eq!(8, required_bytes::<u64>(1));
-		assert_eq!(8, required_bytes::<u64>(63));
-		assert_eq!(8, required_bytes::<u64>(64));
-		assert_eq!(16, required_bytes::<u64>(65));
+	fn required_items_test() {
+		assert_eq!(Ok(0), required_elements::<u8>(0));
+		assert_eq!(Ok(1), required_elements::<u8>(1));
+		assert_eq!(Ok(1), required_elements::<u8>(7));
+		assert_eq!(Ok(1), required_elements::<u8>(8));
+		assert_eq!(Ok(2), required_elements::<u8>(9));
+
+		assert_eq!(Ok(0), required_elements::<u16>(0));
+		assert_eq!(Ok(1), required_elements::<u16>(1));
+		assert_eq!(Ok(1), required_elements::<u16>(15));
+		assert_eq!(Ok(1), required_elements::<u16>(16));
+		assert_eq!(Ok(2), required_elements::<u16>(17));
+
+		assert_eq!(Ok(0), required_elements::<u32>(0));
+		assert_eq!(Ok(1), required_elements::<u32>(1));
+		assert_eq!(Ok(1), required_elements::<u32>(31));
+		assert_eq!(Ok(1), required_elements::<u32>(32));
+		assert_eq!(Ok(2), required_elements::<u32>(33));
+
+		assert_eq!(Ok(0), required_elements::<u64>(0));
+		assert_eq!(Ok(1), required_elements::<u64>(1));
+		assert_eq!(Ok(1), required_elements::<u64>(63));
+		assert_eq!(Ok(1), required_elements::<u64>(64));
+		assert_eq!(Ok(2), required_elements::<u64>(65));
 	}
 
 	#[test]
-	#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
 	fn bitvec_u8() {
 		for v in &test_data!(u8) {
 			let encoded = v.encode();
@@ -213,7 +177,6 @@ mod tests {
 	}
 
 	#[test]
-	#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
 	fn bitvec_u16() {
 		for v in &test_data!(u16) {
 			let encoded = v.encode();
@@ -222,7 +185,6 @@ mod tests {
 	}
 
 	#[test]
-	#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
 	fn bitvec_u32() {
 		for v in &test_data!(u32) {
 			let encoded = v.encode();
@@ -231,7 +193,6 @@ mod tests {
 	}
 
 	#[test]
-	#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
 	fn bitvec_u64() {
 		for v in &test_data!(u64) {
 			let encoded = dbg!(v.encode());
@@ -240,10 +201,9 @@ mod tests {
 	}
 
 	#[test]
-	#[cfg_attr(miri, ignore)] // BitVec error due to outdated version of bitvec
 	fn bitslice() {
 		let data: &[u8] = &[0x69];
-		let slice = BitSlice::<Msb0, u8>::from_slice(data);
+		let slice = BitSlice::<Msb0, u8>::from_slice(data).unwrap();
 		let encoded = slice.encode();
 		let decoded = BitVec::<Msb0, u8>::decode(&mut &encoded[..]).unwrap();
 		assert_eq!(slice, decoded.as_bitslice());
@@ -252,30 +212,10 @@ mod tests {
 	#[test]
 	fn bitbox() {
 		let data: &[u8] = &[5, 10];
-		let bb = BitBox::<Msb0, u8>::from_slice(data);
+		let slice = BitSlice::<Msb0, u8>::from_slice(data).unwrap();
+		let bb = BitBox::<Msb0, u8>::from_bitslice(slice);
 		let encoded = bb.encode();
 		let decoded = BitBox::<Msb0, u8>::decode(&mut &encoded[..]).unwrap();
 		assert_eq!(bb, decoded);
 	}
-
-	#[test]
-	fn reverse_endian_works() {
-		let data = vec![1, 2, 3, 4, 5, 6, 7, 8];
-
-		let mut data_to_u8 = data.clone();
-		reverse_endian(&mut data_to_u8[..], mem::size_of::<u8>());
-		assert_eq!(data_to_u8, data);
-
-		let mut data_to_u16 = data.clone();
-		reverse_endian(&mut data_to_u16[..], mem::size_of::<u16>());
-		assert_eq!(data_to_u16, vec![2, 1, 4, 3, 6, 5, 8, 7]);
-
-		let mut data_to_u32 = data.clone();
-		reverse_endian(&mut data_to_u32[..], mem::size_of::<u32>());
-		assert_eq!(data_to_u32, vec![4, 3, 2, 1, 8, 7, 6, 5]);
-
-		let mut data_to_u64 = data.clone();
-		reverse_endian(&mut data_to_u64[..], mem::size_of::<u64>());
-		assert_eq!(data_to_u64, vec![8, 7, 6, 5, 4, 3, 2, 1]);
-	}
 }

src/codec.rs

@@ -601,6 +601,90 @@ macro_rules! impl_for_non_zero {
 		)*
 	}
 }
+
+/// Encode the slice without prepending the len.
+///
+/// This is equivalent to encoding all the element one by one, but it is optimized for some types.
+pub(crate) fn encode_slice_no_len<T: Encode, W: Output>(slice: &[T], dest: &mut W) {
+	macro_rules! encode_to {
+		( u8, $slice:ident, $dest:ident ) => {{
+			let typed = unsafe { mem::transmute::<&[T], &[u8]>(&$slice[..]) };
+			$dest.write(&typed)
+		}};
+		( i8, $slice:ident, $dest:ident ) => {{
+			// `i8` has the same size as `u8`. We can just convert it here and write to the
+			// dest buffer directly.
+			let typed = unsafe { mem::transmute::<&[T], &[u8]>(&$slice[..]) };
+			$dest.write(&typed)
+		}};
+		( $ty:ty, $slice:ident, $dest:ident ) => {{
+			if cfg!(target_endian = "little") {
+				let typed = unsafe { mem::transmute::<&[T], &[$ty]>(&$slice[..]) };
+				$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed))
+			} else {
+				for item in $slice.iter() {
+					item.encode_to(dest);
+				}
+			}
+		}};
+	}
+
+	with_type_info! {
+		<T as Encode>::TYPE_INFO,
+		encode_to(slice, dest),
+		{
+			for item in slice.iter() {
+				item.encode_to(dest);
+			}
+		},
+	}
+}
+
+/// Decode the slice (without prepended the len).
+///
+/// This is equivalent to decode all elements one by one, but it is optimized in some
+/// situation.
+pub(crate) fn decode_vec_with_len<T: Decode, I: Input>(
+	input: &mut I,
+	len: usize,
+) -> Result<Vec<T>, Error> {
+	fn decode_unoptimized<I: Input, T: Decode>(
+		input: &mut I,
+		items_len: usize,
+	) -> Result<Vec<T>, Error> {
+		let input_capacity = input.remaining_len()?
+			.unwrap_or(MAX_PREALLOCATION)
+			.checked_div(mem::size_of::<T>())
+			.unwrap_or(0);
+		let mut r = Vec::with_capacity(input_capacity.min(items_len));
+		input.descend_ref()?;
+		for _ in 0..items_len {
+			r.push(T::decode(input)?);
+		}
+		input.ascend_ref();
+		Ok(r)
+	}
+
+	macro_rules! decode {
+		( $ty:ty, $input:ident, $len:ident ) => {{
+			if cfg!(target_endian = "little") || mem::size_of::<T>() == 1 {
+				let vec = read_vec_from_u8s::<_, $ty>($input, $len)?;
+				Ok(unsafe { mem::transmute::<Vec<$ty>, Vec<T>>(vec) })
+			} else {
+				decode_unoptimized($input, $len)
+			}
+		}};
+	}
+
+	with_type_info! {
+		<T as Decode>::TYPE_INFO,
+		decode(input, len),
+		{
+			decode_unoptimized(input, len)
+		},
+	}
+}
+
 impl_for_non_zero! {
 	NonZeroI8,
 	NonZeroI16,
@@ -623,38 +707,7 @@ macro_rules! impl_array {
 				}
 
 				fn encode_to<W: Output>(&self, dest: &mut W) {
-					macro_rules! encode_to {
-						( u8, $self:ident, $dest:ident ) => {{
-							let typed = unsafe { mem::transmute::<&[T], &[u8]>(&$self[..]) };
-							$dest.write(&typed)
-						}};
-						( i8, $self:ident, $dest:ident ) => {{
-							// `i8` has the same size as `u8`. We can just convert it here and write to the
-							// dest buffer directly.
-							let typed = unsafe { mem::transmute::<&[T], &[u8]>(&$self[..]) };
-							$dest.write(&typed)
-						}};
-						( $ty:ty, $self:ident, $dest:ident ) => {{
-							if cfg!(target_endian = "little") {
-								let typed = unsafe { mem::transmute::<&[T], &[$ty]>(&$self[..]) };
-								$dest.write(<[$ty] as AsByteSlice<$ty>>::as_byte_slice(typed))
-							} else {
-								for item in $self.iter() {
-									item.encode_to($dest);
-								}
-							}
-						}};
-					}
-
-					with_type_info! {
-						<T as Encode>::TYPE_INFO,
-						encode_to(self, dest),
-						{
-							for item in self.iter() {
-								item.encode_to(dest);
-							}
-						},
-					}
+					encode_slice_no_len(&self[..], dest)
 				}
 			}
 
@@ -742,6 +795,7 @@ impl Decode for String {
 	}
 }
 
+/// Writes the compact encoding of `len` do `dest`.
 pub(crate) fn compact_encode_len_to<W: Output>(dest: &mut W, len: usize) -> Result<(), Error> {
 	if len > u32::max_value() as usize {
 		return Err("Attempted to serialize a collection with too many elements.".into());
@@ -759,38 +813,7 @@ impl<T: Encode> Encode for [T] {
 	fn encode_to<W: Output>(&self, dest: &mut W) {
 		compact_encode_len_to(dest, self.len()).expect("Compact encodes length");
 
-		macro_rules! encode_to {
-			( u8, $self:ident, $dest:ident ) => {{
-				let typed = unsafe { mem::transmute::<&[T], &[u8]>($self) };
+		encode_slice_no_len(self, dest)