check struct invariants during deserialization

Author: robertbastian

components/collator/src/comparison.rs

@@ -21,7 +21,7 @@ use crate::elements::FALLBACK_CE32;
 use crate::elements::NON_ROUND_TRIP_MARKER;
 use crate::elements::{
     char_from_u32, CollationElement, CollationElements, NonPrimary, FFFD_CE32,
-    HANGUL_SYLLABLE_MARKER, HIGH_ZEROS_MASK, JAMO_COUNT, LOW_ZEROS_MASK, NO_CE, NO_CE_PRIMARY,
+    HANGUL_SYLLABLE_MARKER, HIGH_ZEROS_MASK, LOW_ZEROS_MASK, NO_CE, NO_CE_PRIMARY,
     NO_CE_QUATERNARY, NO_CE_SECONDARY, NO_CE_TERTIARY, OPTIMIZED_DIACRITICS_MAX_COUNT,
     QUATERNARY_MASK,
 };
@@ -43,7 +43,7 @@ use crate::provider::CollationSpecialPrimariesV1;
 use crate::provider::CollationSpecialPrimariesValidated;
 use crate::provider::CollationTailoringV1;
 use core::cmp::Ordering;
-use core::convert::{Infallible, TryFrom};
+use core::convert::Infallible;
 use icu_normalizer::provider::DecompositionData;
 use icu_normalizer::provider::DecompositionTables;
 use icu_normalizer::provider::NormalizerNfdDataV1;
@@ -55,7 +55,6 @@ use icu_provider::prelude::*;
 use smallvec::SmallVec;
 use utf16_iter::Utf16CharsEx;
 use utf8_iter::Utf8CharsEx;
-use zerovec::ule::AsULE;
 
 // Special sort key bytes for all levels.
 const LEVEL_SEPARATOR_BYTE: u8 = 1;
@@ -649,16 +648,6 @@ impl Collator {
         let locale_dependent =
             LocaleSpecificDataHolder::try_new_unstable_internal(provider, prefs, options)?;
 
-        // TODO: redesign Korean search collation handling
-        if jamo.get().ce32s.len() != JAMO_COUNT {
-            return Err(DataError::custom("invalid").with_marker(CollationJamoV1::INFO));
-        }
-
-        // `variant_count` isn't stable yet:
-        // https://github.com/rust-lang/rust/issues/73662
-        if special_primaries.get().last_primaries.len() <= (MaxVariable::Currency as usize) {
-            return Err(DataError::custom("invalid").with_marker(CollationSpecialPrimariesV1::INFO));
-        }
         let special_primaries = special_primaries.map_project(|csp, _| {
             let compressible_bytes = (csp.last_primaries.len()
                 == MaxVariable::Currency as usize + 16)
@@ -758,25 +747,6 @@ impl CollatorBorrowed<'static> {
         let locale_dependent =
             LocaleSpecificDataHolder::try_new_unstable_internal(provider, prefs, options)?;
 
-        // TODO: redesign Korean search collation handling
-        const _: () = assert!(
-            crate::provider::Baked::SINGLETON_COLLATION_JAMO_V1
-                .ce32s
-                .as_slice()
-                .len()
-                == JAMO_COUNT
-        );
-
-        // `variant_count` isn't stable yet:
-        // https://github.com/rust-lang/rust/issues/73662
-        const _: () = assert!(
-            crate::provider::Baked::SINGLETON_COLLATION_SPECIAL_PRIMARIES_V1
-                .last_primaries
-                .as_slice()
-                .len()
-                > (MaxVariable::Currency as usize)
-        );
-
         let special_primaries = const {
             &CollationSpecialPrimariesValidated {
                 last_primaries: zerovec::ZeroSlice::from_ule_slice(
@@ -791,7 +761,7 @@ impl CollatorBorrowed<'static> {
                 numeric_primary: crate::provider::Baked::SINGLETON_COLLATION_SPECIAL_PRIMARIES_V1
                     .numeric_primary,
                 compressible_bytes: {
-                    const C: &[<u16 as AsULE>::ULE] =
+                    const C: &[<u16 as zerovec::ule::AsULE>::ULE] =
                         crate::provider::Baked::SINGLETON_COLLATION_SPECIAL_PRIMARIES_V1
                             .last_primaries
                             .as_slice()
@@ -874,27 +844,25 @@ impl CollatorBorrowed<'static> {
     }
 }
 
-macro_rules! collation_elements {
-    ($self:expr, $chars:expr, $tailoring:expr, $numeric_primary:expr) => {{
-        let jamo = <&[<u32 as AsULE>::ULE; JAMO_COUNT]>::try_from($self.jamo.ce32s.as_ule_slice());
-
-        let jamo = jamo.unwrap();
-
+impl<'a> CollatorBorrowed<'a> {
+    fn collation_elements<C: Iterator<Item = char>>(
+        &self,
+        chars: C,
+        tailoring: &'a CollationData<'a>,
+        numeric_primary: Option<u8>,
+    ) -> CollationElements<'a, C> {
         CollationElements::new(
-            $chars,
-            $self.root,
-            $tailoring,
-            jamo,
-            &$self.diacritics.secondaries,
-            $self.decompositions,
-            $self.tables,
-            $numeric_primary,
-            $self.lithuanian_dot_above,
+            chars,
+            self.root,
+            tailoring,
+            self.jamo.as_array(),
+            &self.diacritics.secondaries,
+            self.decompositions,
+            self.tables,
+            numeric_primary,
+            self.lithuanian_dot_above,
         )
-    }};
-}
-
-impl CollatorBorrowed<'_> {
+    }
     /// The resolved options showing how the default options, the requested options,
     /// and the options from locale data were combined.
     pub fn resolved_options(&self) -> ResolvedCollatorOptions {
@@ -971,7 +939,7 @@ impl CollatorBorrowed<'_> {
     );
 
     #[inline(always)]
-    fn tailoring_or_root(&self) -> &CollationData<'_> {
+    fn tailoring_or_root(&self) -> &'a CollationData<'a> {
         if let Some(tailoring) = &self.tailoring {
             tailoring
         } else {
@@ -1052,8 +1020,8 @@ impl CollatorBorrowed<'_> {
 
         let tailoring = self.tailoring_or_root();
         let numeric_primary = self.numeric_primary();
-        let mut left = collation_elements!(self, left_chars, tailoring, numeric_primary);
-        let mut right = collation_elements!(self, right_chars, tailoring, numeric_primary);
+        let mut left = self.collation_elements(left_chars, tailoring, numeric_primary);
+        let mut right = self.collation_elements(right_chars, tailoring, numeric_primary);
 
         // Start identical prefix
 
@@ -1922,7 +1890,7 @@ impl CollatorBorrowed<'_> {
         let levels = self.sort_key_levels();
 
         let mut iter =
-            collation_elements!(self, iter, self.tailoring_or_root(), self.numeric_primary());
+            self.collation_elements(iter, self.tailoring_or_root(), self.numeric_primary());
         iter.init();
         let variable_top = self.variable_top();
 

components/collator/src/elements.rs

@@ -1502,16 +1502,7 @@ where
                 if (decomposition & !(BACKWARD_COMBINING_MARKER | NON_ROUND_TRIP_MARKER)) == 0 {
                     // The character is its own decomposition
                     let jamo_index = (c as usize).wrapping_sub(HANGUL_L_BASE as usize);
-                    // Attribute belongs on an inner expression, but
-                    // https://github.com/rust-lang/rust/issues/15701
-                    #[expect(clippy::indexing_slicing)]
-                    if jamo_index >= self.jamo.len() {
-                        ce32 = data.ce32_for_char(c);
-                        if ce32 == FALLBACK_CE32 {
-                            data = self.root;
-                            ce32 = data.ce32_for_char(c);
-                        }
-                    } else {
+                    if let Some(&jamo) = self.jamo.get(jamo_index) {
                         // The purpose of reading the CE32 from the jamo table instead
                         // of the trie even in this case is to make it unnecessary
                         // for all search collation tries to carry a copy of the Hangul
@@ -1531,7 +1522,13 @@ where
                         data = self.root;
                         // Index in range by construction above. Not using `get` with
                         // `if let` in order to put the likely branch first.
-                        ce32 = CollationElement32::new_from_ule(self.jamo[jamo_index]);
+                        ce32 = CollationElement32::new_from_ule(jamo);
+                    } else {
+                        ce32 = data.ce32_for_char(c);
+                        if ce32 == FALLBACK_CE32 {
+                            data = self.root;
+                            ce32 = data.ce32_for_char(c);
+                        }
                     }
                     if self.is_next_decomposition_starts_with_starter() {
                         if let Some(ce) = ce32.to_ce_simple_or_long_primary() {

components/collator/src/provider.rs

@@ -323,14 +323,50 @@ icu_provider::data_struct!(
 #[derive(Debug, PartialEq, Clone, yoke::Yokeable, zerofrom::ZeroFrom)]
 #[cfg_attr(feature = "datagen", derive(serde::Serialize, databake::Bake))]
 #[cfg_attr(feature = "datagen", databake(path = icu_collator::provider))]
-#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
 pub struct CollationJamo<'data> {
     /// `CollationElement32`s (as `u32`s) for the Hangul Jamo Unicode Block.
     /// The length must be equal to the size of the block (256).
-    #[cfg_attr(feature = "serde", serde(borrow))]
     pub ce32s: ZeroVec<'data, u32>,
 }
 
+impl<'data> CollationJamo<'data> {
+    pub(crate) fn as_array(
+        &'data self,
+    ) -> &'data [<u32 as AsULE>::ULE; crate::elements::JAMO_COUNT] {
+        #[allow(clippy::unwrap_used)] // by invariant
+        self.ce32s.as_ule_slice().try_into().unwrap()
+    }
+}
+
+// TODO: redesign Korean search collation handling
+
+#[cfg(feature = "compiled_data")]
+const _: () = assert!(
+    Baked::SINGLETON_COLLATION_JAMO_V1.ce32s.as_slice().len() == crate::elements::JAMO_COUNT
+);
+
+#[cfg(feature = "serde")]
+impl<'de> serde::Deserialize<'de> for CollationJamo<'de> {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        #[derive(serde::Deserialize)]
+        struct Raw<'data> {
+            #[cfg_attr(feature = "serde", serde(borrow))]
+            ce32s: ZeroVec<'data, u32>,
+        }
+
+        let Raw { ce32s } = Raw::deserialize(deserializer)?;
+
+        if ce32s.len() != crate::elements::JAMO_COUNT {
+            return Err(serde::de::Error::custom("invalid"));
+        }
+
+        Ok(Self { ce32s })
+    }
+}
+
 icu_provider::data_struct!(
     CollationJamo<'_>,
     #[cfg(feature = "datagen")]
@@ -554,7 +590,6 @@ impl CollationMetadata {
 #[derive(Debug, PartialEq, Clone, yoke::Yokeable, zerofrom::ZeroFrom)]
 #[cfg_attr(feature = "datagen", derive(serde::Serialize, databake::Bake))]
 #[cfg_attr(feature = "datagen", databake(path = icu_collator::provider))]
-#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
 pub struct CollationSpecialPrimaries<'data> {
     /// The primaries corresponding to `MaxVariable`
     /// character classes packed so that each fits in
@@ -564,12 +599,42 @@ pub struct CollationSpecialPrimaries<'data> {
     /// This is potentially followed by 256 bits
     /// (packed in 16 u16s) to classify every possible
     /// byte into compressible or non-compressible.
-    #[cfg_attr(feature = "serde", serde(borrow))]
     pub last_primaries: ZeroVec<'data, u16>,
     /// The high 8 bits of the numeric primary
     pub numeric_primary: u8,
 }
 
+#[cfg(feature = "serde")]
+impl<'de> serde::Deserialize<'de> for CollationSpecialPrimaries<'de> {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        #[derive(serde::Deserialize)]
+        struct Raw<'data> {
+            #[cfg_attr(feature = "serde", serde(borrow))]
+            last_primaries: ZeroVec<'data, u16>,
+            numeric_primary: u8,
+        }
+
+        let Raw {
+            last_primaries,
+            numeric_primary,
+        } = Raw::deserialize(deserializer)?;
+
+        // `variant_count` isn't stable yet:
+        // https://github.com/rust-lang/rust/issues/73662
+        if last_primaries.len() <= (MaxVariable::Currency as usize) {
+            return Err(serde::de::Error::custom("invalid"));
+        }
+
+        Ok(Self {
+            last_primaries,
+            numeric_primary,
+        })
+    }
+}
+
 #[derive(Debug, PartialEq, Clone, yoke::Yokeable, zerofrom::ZeroFrom)]
 pub(crate) struct CollationSpecialPrimariesValidated<'data> {
     /// The primaries corresponding to `MaxVariable`