From dcd27cd62f5c33949a63495b4a44f10e162fff25 Mon Sep 17 00:00:00 2001 From: Robert Bastian Date: Thu, 17 Aug 2023 17:21:02 +0200 Subject: [PATCH] better --- utils/zerovec/src/varzerovec/components.rs | 5 +++- utils/zerovec/src/varzerovec/owned.rs | 17 +++++++---- utils/zerovec/src/varzerovec/slice.rs | 4 +-- utils/zerovec/src/varzerovec/vec.rs | 33 ++++++++++++++++++---- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/utils/zerovec/src/varzerovec/components.rs b/utils/zerovec/src/varzerovec/components.rs index 5168fb7143b..795b67d5467 100644 --- a/utils/zerovec/src/varzerovec/components.rs +++ b/utils/zerovec/src/varzerovec/components.rs @@ -168,6 +168,7 @@ impl<'a, T: VarULE + ?Sized, F: VarZeroVecFormat> VarZeroVecComponents<'a, T, F> /// `things`, such that it parses to a `T::VarULE` #[inline] pub fn parse_byte_slice(slice: &'a [u8]) -> Result { + // The empty VZV is special-cased to the empty slice if slice.is_empty() { return Ok(VarZeroVecComponents { len: 0, @@ -219,6 +220,7 @@ impl<'a, T: VarULE + ?Sized, F: VarZeroVecFormat> VarZeroVecComponents<'a, T, F> /// The bytes must have previously successfully run through /// [`VarZeroVecComponents::parse_byte_slice()`] pub unsafe fn from_bytes_unchecked(slice: &'a [u8]) -> Self { + // The empty VZV is special-cased to the empty slice if slice.is_empty() { return VarZeroVecComponents { len: 0, @@ -485,12 +487,13 @@ where } /// Collects the bytes for a VarZeroSlice into a Vec. -pub fn get_serializable_bytes(elements: &[A]) -> Option> +pub fn get_serializable_bytes_non_empty(elements: &[A]) -> Option> where T: VarULE + ?Sized, A: EncodeAsVarULE, F: VarZeroVecFormat, { + debug_assert!(!elements.is_empty()); let len = compute_serializable_len::(elements)?; debug_assert!(len >= LENGTH_WIDTH as u32); let mut output: Vec = alloc::vec![0; len as usize]; diff --git a/utils/zerovec/src/varzerovec/owned.rs b/utils/zerovec/src/varzerovec/owned.rs index 0990a5d91d5..c5556315fbd 100644 --- a/utils/zerovec/src/varzerovec/owned.rs +++ b/utils/zerovec/src/varzerovec/owned.rs @@ -84,13 +84,18 @@ impl VarZeroVecOwned { where A: EncodeAsVarULE, { - Ok(Self { - marker: PhantomData, - // TODO(#1410): Rethink length errors in VZV. - entire_slice: components::get_serializable_bytes::(elements).ok_or( - "Attempted to build VarZeroVec out of elements that \ + Ok(if elements.is_empty() { + Self::from_slice(VarZeroSlice::new_empty()) + } else { + Self { + marker: PhantomData, + // TODO(#1410): Rethink length errors in VZV. + entire_slice: components::get_serializable_bytes_non_empty::(elements) + .ok_or( + "Attempted to build VarZeroVec out of elements that \ cumulatively are larger than a u32 in size", - )?, + )?, + } }) } diff --git a/utils/zerovec/src/varzerovec/slice.rs b/utils/zerovec/src/varzerovec/slice.rs index 65858ed6c8c..190153fe017 100644 --- a/utils/zerovec/src/varzerovec/slice.rs +++ b/utils/zerovec/src/varzerovec/slice.rs @@ -108,8 +108,8 @@ pub struct VarZeroSlice { impl VarZeroSlice { /// Construct a new empty VarZeroSlice pub const fn new_empty() -> &'static Self { - let arr: &[u8] = &[0; components::LENGTH_WIDTH]; - unsafe { mem::transmute(arr) } + // The empty VZV is special-cased to the empty slice + unsafe { mem::transmute(&[] as &[u8]) } } /// Obtain a [`VarZeroVecComponents`] borrowing from the internal buffer diff --git a/utils/zerovec/src/varzerovec/vec.rs b/utils/zerovec/src/varzerovec/vec.rs index 4617f86e962..64928509f8e 100644 --- a/utils/zerovec/src/varzerovec/vec.rs +++ b/utils/zerovec/src/varzerovec/vec.rs @@ -425,8 +425,12 @@ where { #[inline] fn from(elements: &[A]) -> Self { - #[allow(clippy::unwrap_used)] // TODO(#1410) Better story for fallibility - VarZeroVecOwned::try_from_elements(elements).unwrap().into() + if elements.is_empty() { + VarZeroSlice::new_empty().into() + } else { + #[allow(clippy::unwrap_used)] // TODO(#1410) Better story for fallibility + VarZeroVecOwned::try_from_elements(elements).unwrap().into() + } } } @@ -451,8 +455,14 @@ where { #[inline] fn eq(&self, other: &VarZeroVec<'b, T, F>) -> bool { - // VarULE has an API guarantee that this is equivalent - // to `T::VarULE::eq()` + // VZV::from_elements used to produce a non-canonical representation of the + // empty VZV, so we cannot use byte equality for empty vecs. + if self.is_empty() || other.is_empty() { + return self.is_empty() && other.is_empty(); + } + // VarULE has an API guarantee that byte equality is semantic equality. + // For non-empty VZVs, there's only a single metadata representation, + // so this guarantee extends to the whole VZV representation. self.as_bytes().eq(other.as_bytes()) } } @@ -505,6 +515,17 @@ impl<'a, T: VarULE + ?Sized + Ord, F: VarZeroVecFormat> Ord for VarZeroVec<'a, T } #[test] -fn single_empty_representation() { - assert_eq!(VarZeroVec::::new().as_bytes(), VarZeroVec::::from(&[] as &[&str]).as_bytes()); +fn assert_single_empty_representation() { + assert_eq!( + VarZeroVec::::new().as_bytes(), + VarZeroVec::::from(&[] as &[&str]).as_bytes() + ); +} + +#[test] +fn weird_empty_representation_equality() { + assert_eq!( + VarZeroVec::::parse_byte_slice(&[0, 0, 0, 0]).unwrap(), + VarZeroVec::::parse_byte_slice(&[]).unwrap() + ); }