Fixing Option borrows (follow-up #1550)

components/datetime/Cargo.toml

@@ -67,7 +67,7 @@ bench = false  # This option is required for Benchmark CI
 std = ["icu_provider/std", "icu_locid/std", "icu_calendar/std"]
 default = ["provider_serde"]
 bench = []
-provider_serde = ["serde", "litemap/serde_serialize", "smallvec/serde", "litemap/serde", "zerovec/serde", "tinystr/serde"]
+provider_serde = ["serde", "litemap/serde_serialize", "smallvec/serde", "litemap/serde", "zerovec/serde", "tinystr/serde", "icu_provider/serde"]
 provider_transform_internals = ["std"]
 
 [[bench]]

components/datetime/src/provider/calendar/symbols.rs

@@ -42,61 +42,14 @@ pub struct Eras<'data> {
 }
 
 macro_rules! symbols {
-        ($name: ident, $expr: ty) => {
-            pub mod $name {
-                use super::*;
+    ($name: ident, $symbols: item) => {
+        pub mod $name {
+            use super::*;
 
-                #[derive(Debug, PartialEq, Clone, Default, ZeroCopyFrom, Yokeable)]
-                #[cfg_attr(feature="provider_serde", derive(serde::Serialize, serde::Deserialize))]
-                pub struct SymbolsV1<'data>(#[cfg_attr(feature="provider_serde", serde(borrow))] pub $expr);
+            #[derive(Debug, PartialEq, Clone, Default, ZeroCopyFrom, Yokeable)]
+            #[cfg_attr(feature="provider_serde", derive(serde::Serialize, serde::Deserialize))]
+            $symbols
 
-                symbols!();
-            }
-        };
-        ($name: ident { $($tokens: tt)* }) => {
-            symbols!($name { $($tokens)* } -> ());
-        };
-        ($name: ident { $element: ident: Option<$ty: ty>, $($tokens: tt)+ } -> ($($members:tt)*)) => {
-            symbols!($name { $($tokens)* } -> (
-                $($members)*
-                #[cfg_attr(feature = "provider_serde", serde(borrow))]
-                pub $element: Option<$ty>,
-            ));
-        };
-        ($name: ident { $element: ident: $ty: ty, $($tokens: tt)+ } -> ($($members:tt)*)) => {
-            symbols!($name { $($tokens)* } -> (
-                $($members)*
-                #[cfg_attr(feature = "provider_serde", serde(borrow))]
-                pub $element: $ty,
-            ));
-        };
-        ($name: ident { $element: ident: Option<$ty: ty> $(,)? } -> ($($members:tt)*)) => {
-            symbols!($name { } -> (
-                $($members)*
-                #[cfg_attr(feature = "provider_serde", serde(borrow))]
-                pub $element: Option<$ty>,
-            ));
-        };
-        ($name: ident { $element: ident: $ty: ty $(,)? } -> ($($members:tt)*)) => {
-            symbols!($name { } -> (
-                $($members)*
-                #[cfg_attr(feature = "provider_serde", serde(borrow))]
-                pub $element: $ty,
-            ));
-        };
-        ($name: ident { } -> ($($members: tt)*)) => {
-            pub mod $name {
-                use super::*;
-
-                #[derive(Debug, PartialEq, Clone, Default, Yokeable, ZeroCopyFrom)]
-                #[cfg_attr(feature="provider_serde", derive(serde::Serialize, serde::Deserialize))]
-                pub struct SymbolsV1<'data> {
-                    $($members)*
-                }
-                symbols!();
-            }
-        };
-        () => {
             // UTS 35 specifies that `format` widths are mandatory
             // except of `short`.
             #[derive(Debug, PartialEq, Clone, Default, Yokeable, ZeroCopyFrom)]
@@ -134,18 +87,195 @@ macro_rules! symbols {
                 #[cfg_attr(feature = "provider_serde", serde(borrow))]
                 pub stand_alone: Option<StandAloneWidthsV1<'data>>,
             }
-        };
-    }
+        }
+    };
+}
 
-symbols!(months, [Cow<'data, str>; 12]);
+symbols!(
+    months,
+    pub struct SymbolsV1<'data>(
+        #[cfg_attr(
+            feature = "provider_serde",
+            serde(
+                borrow,
+                deserialize_with = "icu_provider::serde::borrow_de_utils::array_of_cow"
+            )
+        )]
+        pub [Cow<'data, str>; 12],
+    );
+);
 
-symbols!(weekdays, [Cow<'data, str>; 7]);
+symbols!(
+    weekdays,
+    pub struct SymbolsV1<'data>(
+        #[cfg_attr(
+            feature = "provider_serde",
+            serde(
+                borrow,
+                deserialize_with = "icu_provider::serde::borrow_de_utils::array_of_cow"
+            )
+        )]
+        pub [Cow<'data, str>; 7],
+    );
+);
 
 symbols!(
-    day_periods {
-        am: Cow<'data, str>,
-        pm: Cow<'data, str>,
-        noon: Option<Cow<'data, str>>,
-        midnight: Option<Cow<'data, str>>,
+    day_periods,
+    pub struct SymbolsV1<'data> {
+        #[cfg_attr(feature = "provider_serde", serde(borrow))]
+        pub am: Cow<'data, str>,
+        #[cfg_attr(feature = "provider_serde", serde(borrow))]
+        pub pm: Cow<'data, str>,
+        #[cfg_attr(
+            feature = "provider_serde",
+            serde(
+                borrow,
+                deserialize_with = "icu_provider::serde::borrow_de_utils::option_of_cow"
+            )
+        )]
+        pub noon: Option<Cow<'data, str>>,
+        #[cfg_attr(
+            feature = "provider_serde",
+            serde(
+                borrow,
+                deserialize_with = "icu_provider::serde::borrow_de_utils::option_of_cow"
+            )
+        )]
+        pub midnight: Option<Cow<'data, str>>,
     }
 );
+
+#[cfg(all(test, feature = "provider_serde"))]
+mod test {
+    use super::*;
+
+    fn serialize() -> Vec<u8> {
+        let months = months::SymbolsV1([
+            Cow::Owned("January".to_string()),
+            Cow::Owned("February".to_string()),
+            Cow::Owned("March".to_string()),
+            Cow::Owned("April".to_string()),
+            Cow::Owned("May".to_string()),
+            Cow::Owned("June".to_string()),
+            Cow::Owned("July".to_string()),
+            Cow::Owned("August".to_string()),
+            Cow::Owned("September".to_string()),
+            Cow::Owned("October".to_string()),
+            Cow::Owned("November".to_string()),
+            Cow::Owned("December".to_string()),
+        ]);
+
+        let weekdays = weekdays::SymbolsV1([
+            Cow::Owned("Monday".to_string()),
+            Cow::Owned("Tuesday".to_string()),
+            Cow::Owned("Wednesday".to_string()),
+            Cow::Owned("Thursday".to_string()),
+            Cow::Owned("Friday".to_string()),
+            Cow::Owned("Saturday".to_string()),
+            Cow::Owned("Sunday".to_string()),
+        ]);
+
+        let day_periods = day_periods::SymbolsV1 {
+            am: Cow::Owned("am".to_string()),
+            pm: Cow::Owned("pm".to_string()),
+            noon: Some(Cow::Owned("noon".to_string())),
+            midnight: None,
+        };
+
+        bincode::serialize(&DateSymbolsV1 {
+            months: months::ContextsV1 {
+                format: months::FormatWidthsV1 {
+                    abbreviated: months.clone(),
+                    narrow: months.clone(),
+                    short: Some(months.clone()),
+                    wide: months.clone(),
+                },
+                stand_alone: Some(months::StandAloneWidthsV1 {
+                    abbreviated: Some(months.clone()),
+                    narrow: Some(months.clone()),
+                    short: Some(months.clone()),
+                    wide: Some(months.clone()),
+                }),
+            },
+            weekdays: weekdays::ContextsV1 {
+                format: weekdays::FormatWidthsV1 {
+                    abbreviated: weekdays.clone(),
+                    narrow: weekdays.clone(),
+                    short: Some(weekdays.clone()),
+                    wide: weekdays.clone(),
+                },
+                stand_alone: Some(weekdays::StandAloneWidthsV1 {
+                    abbreviated: Some(weekdays.clone()),
+                    narrow: Some(weekdays.clone()),
+                    short: Some(weekdays.clone()),
+                    wide: Some(weekdays.clone()),
+                }),
+            },
+            day_periods: day_periods::ContextsV1 {
+                format: day_periods::FormatWidthsV1 {
+                    abbreviated: day_periods.clone(),
+                    narrow: day_periods.clone(),
+                    short: Some(day_periods.clone()),
+                    wide: day_periods.clone(),
+                },
+                stand_alone: Some(day_periods::StandAloneWidthsV1 {
+                    abbreviated: Some(day_periods.clone()),
+                    narrow: Some(day_periods.clone()),
+                    short: Some(day_periods.clone()),
+                    wide: Some(day_periods.clone()),
+                }),
+            },
+            eras: Eras {
+                names: ZeroMap::new(),
+                abbr: ZeroMap::new(),
+                narrow: ZeroMap::new(),
+            },
+        })
+        .unwrap()
+    }
+
+    #[test]
+    fn months_borrows() {
+        let bytes = serialize();
+        let de = bincode::deserialize::<DateSymbolsV1>(&bytes).unwrap();
+
+        assert!(matches!(de.months.format.narrow.0[2], Cow::Borrowed(_)));
+        assert!(matches!(
+            de.months.format.short.as_ref().unwrap().0[11],
+            Cow::Borrowed(_)
+        ));
+    }
+
+    #[test]
+    fn weekdays_borrows() {
+        let bytes = serialize();
+        let de = bincode::deserialize::<DateSymbolsV1>(&bytes).unwrap();
+
+        assert!(matches!(de.weekdays.format.narrow.0[2], Cow::Borrowed(_)));
+        assert!(matches!(
+            de.weekdays.format.short.as_ref().unwrap().0[4],
+            Cow::Borrowed(_)
+        ));
+    }
+
+    #[test]
+    fn day_periods_borrows() {
+        let bytes = serialize();
+        let de = bincode::deserialize::<DateSymbolsV1>(&bytes).unwrap();
+
+        assert!(matches!(
+            de.day_periods.format.narrow.noon,
+            Some(Cow::Borrowed(_))
+        ));
+        assert!(matches!(
+            de.day_periods.format.short.as_ref().unwrap().noon,
+            Some(Cow::Borrowed(_))
+        ));
+
+        assert!(matches!(de.day_periods.format.narrow.am, Cow::Borrowed(_)));
+        assert!(matches!(
+            de.day_periods.format.short.as_ref().unwrap().am,
+            Cow::Borrowed(_)
+        ));
+    }
+}

provider/core/src/serde/borrow_de_utils.rs

@@ -0,0 +1,51 @@
+// This file is part of ICU4X. For terms of use, please see the file
+// called LICENSE at the top level of the ICU4X source tree
+// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
+
+use alloc::borrow::Cow;
+use serde::de::Deserializer;
+use serde::Deserialize;
+
+#[derive(Deserialize)]
+#[serde(transparent)]
+// Cows fail to borrow in some situations (array, option), but structs of Cows don't.
+pub struct CowWrap<'data>(#[serde(borrow)] Cow<'data, str>);
+
+pub fn array_of_cow<'de, D, const N: usize>(deserializer: D) -> Result<[Cow<'de, str>; N], D::Error>
+where
+    D: Deserializer<'de>,
+    [CowWrap<'de>; N]: Deserialize<'de>,
+{
+    <[CowWrap<'de>; N]>::deserialize(deserializer).map(|array| array.map(|wrap| wrap.0))
+}
+
+pub fn option_of_cow<'de, D>(deserializer: D) -> Result<Option<Cow<'de, str>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    <Option<CowWrap<'de>>>::deserialize(deserializer).map(|opt| opt.map(|wrap| wrap.0))
+}
+
+#[test]
+fn test_option() {
+    #[derive(Debug, PartialEq, serde::Serialize, serde::Deserialize)]
+    struct Demo<'s>(#[serde(borrow, deserialize_with = "option_of_cow")] Option<Cow<'s, str>>);
+
+    let data_orig = Demo(Some("Hello world".into()));
+    let json = serde_json::to_string(&data_orig).expect("serialize");
+    let data_new = serde_json::from_str::<Demo>(&json).expect("deserialize");
+    assert_eq!(data_orig, data_new);
+    assert!(matches!(data_new.0, Some(Cow::Borrowed(_))));
+}
+
+#[test]
+fn test_array() {
+    #[derive(Debug, PartialEq, serde::Serialize, serde::Deserialize)]
+    struct Demo<'s>(#[serde(borrow, deserialize_with = "array_of_cow")] [Cow<'s, str>; 1]);
+
+    let data_orig = Demo(["Hello world".into()]);
+    let json = serde_json::to_string(&data_orig).expect("serialize");
+    let data_new = serde_json::from_str::<Demo>(&json).expect("deserialize");
+    assert_eq!(data_orig, data_new);
+    assert!(matches!(data_new.0, [Cow::Borrowed(_)]));
+}

provider/core/src/serde/mod.rs

@@ -22,6 +22,7 @@
 //! [`DataProvider`]: crate::data_provider::DataProvider
 //! [`BufferProvider`]: crate::buf::BufferProvider
 
+pub mod borrow_de_utils;
 mod de;
 #[cfg(feature = "serialize")]
 mod ser;