Skip to content

Commit

Permalink
better
Browse files Browse the repository at this point in the history
  • Loading branch information
robertbastian committed Aug 17, 2023
1 parent 622afe6 commit dcd27cd
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 15 deletions.
5 changes: 4 additions & 1 deletion utils/zerovec/src/varzerovec/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self, ZeroVecError> {
// The empty VZV is special-cased to the empty slice
if slice.is_empty() {
return Ok(VarZeroVecComponents {
len: 0,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -485,12 +487,13 @@ where
}

/// Collects the bytes for a VarZeroSlice into a Vec.
pub fn get_serializable_bytes<T, A, F>(elements: &[A]) -> Option<Vec<u8>>
pub fn get_serializable_bytes_non_empty<T, A, F>(elements: &[A]) -> Option<Vec<u8>>
where
T: VarULE + ?Sized,
A: EncodeAsVarULE<T>,
F: VarZeroVecFormat,
{
debug_assert!(!elements.is_empty());
let len = compute_serializable_len::<T, A, F>(elements)?;
debug_assert!(len >= LENGTH_WIDTH as u32);
let mut output: Vec<u8> = alloc::vec![0; len as usize];
Expand Down
17 changes: 11 additions & 6 deletions utils/zerovec/src/varzerovec/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,18 @@ impl<T: VarULE + ?Sized, F: VarZeroVecFormat> VarZeroVecOwned<T, F> {
where
A: EncodeAsVarULE<T>,
{
Ok(Self {
marker: PhantomData,
// TODO(#1410): Rethink length errors in VZV.
entire_slice: components::get_serializable_bytes::<T, A, F>(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::<T, A, F>(elements)
.ok_or(
"Attempted to build VarZeroVec out of elements that \
cumulatively are larger than a u32 in size",
)?,
)?,
}
})
}

Expand Down
4 changes: 2 additions & 2 deletions utils/zerovec/src/varzerovec/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ pub struct VarZeroSlice<T: ?Sized, F = Index16> {
impl<T: VarULE + ?Sized, F: VarZeroVecFormat> VarZeroSlice<T, F> {
/// 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
Expand Down
33 changes: 27 additions & 6 deletions utils/zerovec/src/varzerovec/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
}

Expand All @@ -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())
}
}
Expand Down Expand Up @@ -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::<str>::new().as_bytes(), VarZeroVec::<str>::from(&[] as &[&str]).as_bytes());
fn assert_single_empty_representation() {
assert_eq!(
VarZeroVec::<str>::new().as_bytes(),
VarZeroVec::<str>::from(&[] as &[&str]).as_bytes()
);
}

#[test]
fn weird_empty_representation_equality() {
assert_eq!(
VarZeroVec::<str>::parse_byte_slice(&[0, 0, 0, 0]).unwrap(),
VarZeroVec::<str>::parse_byte_slice(&[]).unwrap()
);
}

0 comments on commit dcd27cd

Please sign in to comment.