better

unicode-org · robertbastian · Aug 17, 2023 · Aug 17, 2023 · Aug 17, 2023 · Aug 17, 2023
commit dcd27cd62f5c33949a63495b4a44f10e162fff25
@@ -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,
@@ -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<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];

@@ -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",
-            )?,
+                    )?,
+            }
         })
     }
 

@@ -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

@@ -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::<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()
+    );
 }