Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Yoke work on types with borrowing generics #4119

Open
Tracked by #3771
sffc opened this issue Oct 5, 2023 · 2 comments · May be fixed by #4120
Open
Tracked by #3771

Make Yoke work on types with borrowing generics #4119

sffc opened this issue Oct 5, 2023 · 2 comments · May be fixed by #4120
Assignees
Labels
C-zerovec Component: Yoke, ZeroVec, DataBake S-medium Size: Less than a week (larger bug fix or enhancement)
Milestone

Comments

@sffc
Copy link
Member

sffc commented Oct 5, 2023

Same kind of change as #3770. It's a bit more involved due to Yoke's associated types

@Manishearth Manishearth added this to the Utilities 1.0 milestone Oct 5, 2023
@Manishearth Manishearth added S-medium Size: Less than a week (larger bug fix or enhancement) C-zerovec Component: Yoke, ZeroVec, DataBake labels Oct 5, 2023
@Manishearth
Copy link
Member

The WIP #3771 change that this needs to work with is below. #4120 has a WIP patch, we should write a test similar to #3770

commit c61033391354f51df4d226cecd3b986cd3b1d304
Author: Manish Goregaokar <manishsmail@gmail.com>
Date:   Wed Aug 2 17:08:26 2023 -0700

    wip

diff --git a/components/datetime/src/provider/calendar/symbols.rs b/components/datetime/src/provider/calendar/symbols.rs
index 563d9d229..4c85bf4e6 100644
--- a/components/datetime/src/provider/calendar/symbols.rs
+++ b/components/datetime/src/provider/calendar/symbols.rs
@@ -126,6 +126,97 @@ pub struct Eras<'data> {
     pub narrow: ZeroMap<'data, UnvalidatedStr, str>,
 }
 
+// UTS 35 specifies that `format` widths are mandatory,
+// except for `short`.
+#[derive(Debug, PartialEq, Clone, Default, yoke::Yokeable, zerofrom::ZeroFrom)]
+#[cfg_attr(
+    feature = "datagen",
+    derive(serde::Serialize, databake::Bake),
+    databake(path = icu_datetime::provider::calendar::$name),
+)]
+#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
+#[yoke(prove_covariance_manually)]
+#[zerofrom(may_borrow(SymbolsV1))]
+#[yoke(may_borrow(SymbolsV1))]
+/// Symbol data for the “format” style formatting of Month.
+///
+/// <div class="stab unstable">
+/// 🚧 This code is considered unstable; it may change at any time, in breaking or non-breaking ways,
+/// including in SemVer minor releases. While the serde representation of data structs is guaranteed
+/// to be stable, their Rust representation might not be. Use with caution.
+/// </div>
+pub struct FormatWidthsV1<SymbolsV1> {
+    /// Abbreviated length symbol for “format” style symbol.
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub abbreviated: SymbolsV1,
+    /// Narrow length symbol for “format” style symbol.
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub narrow: SymbolsV1,
+    /// Short length symbol for “format” style symbol.
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub short: Option<SymbolsV1>,
+    /// Wide length symbol for “format” style symbol.
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub wide: SymbolsV1,
+}
+
+// // UTS 35 specifies that `stand_alone` widths are optional
+// #[derive(Debug, PartialEq, Clone, Default, yoke::Yokeable, zerofrom::ZeroFrom)]
+// #[cfg_attr(
+//     feature = "datagen",
+//     derive(serde::Serialize, databake::Bake),
+//     databake(path = icu_datetime::provider::calendar::$name),
+// )]
+// #[cfg_attr(feature = "serde", derive(serde::Deserialize))]
+// #[yoke(prove_covariance_manually)]
+// #[doc = concat!("Symbol data for the \"stand-alone\" style formatting of ", stringify!($field_id),
+//     ".\n\nThe stand-alone style is used in contexts where the field is displayed by itself.")]
+// ///
+// /// <div class="stab unstable">
+// /// 🚧 This code is considered unstable; it may change at any time, in breaking or non-breaking ways,
+// /// including in SemVer minor releases. While the serde representation of data structs is guaranteed
+// /// to be stable, their Rust representation might not be. Use with caution.
+// /// </div>
+// pub struct StandAloneWidthsV1<'data> {
+//     #[doc = concat!("Abbreviated length symbol for \"stand-alone\" style symbol for ", stringify!($name), ".")]
+//     #[cfg_attr(feature = "serde", serde(borrow))]
+//     pub abbreviated: Option<SymbolsV1<'data>>,
+//     #[doc = concat!("Narrow length symbol for \"stand-alone\" style symbol for ", stringify!($name), ".")]
+//     #[cfg_attr(feature = "serde", serde(borrow))]
+//     pub narrow: Option<SymbolsV1<'data>>,
+//     #[doc = concat!("Short length symbol for \"stand-alone\" style symbol for ", stringify!($name), ".")]
+//     #[cfg_attr(feature = "serde", serde(borrow))]
+//     pub short: Option<SymbolsV1<'data>>,
+//     #[doc = concat!("Wide length symbol for \"stand-alone\" style symbol for ", stringify!($name), ".")]
+//     #[cfg_attr(feature = "serde", serde(borrow))]
+//     pub wide: Option<SymbolsV1<'data>>,
+// }
+
+// #[derive(Debug, PartialEq, Clone, Default, yoke::Yokeable, zerofrom::ZeroFrom)]
+// #[cfg_attr(
+//     feature = "datagen",
+//     derive(serde::Serialize, databake::Bake),
+//     databake(path = icu_datetime::provider::calendar::$name),
+// )]
+// #[cfg_attr(feature = "serde", derive(serde::Deserialize))]
+// #[yoke(prove_covariance_manually)]
+// #[doc = concat!("The struct containing the symbol data for ", stringify!($field_id),
+//     " that contains the \"format\" style symbol data ([`FormatWidthsV1`]) and \"stand-alone\" style symbol data ([`StandAloneWidthsV1`]).")]
+// ///
+// /// <div class="stab unstable">
+// /// 🚧 This code is considered unstable; it may change at any time, in breaking or non-breaking ways,
+// /// including in SemVer minor releases. While the serde representation of data structs is guaranteed
+// /// to be stable, their Rust representation might not be. Use with caution.
+// /// </div>
+// pub struct ContextsV1<'data> {
+//     /// The symbol data for "format" style symbols.
+//     #[cfg_attr(feature = "serde", serde(borrow))]
+//     pub format: FormatWidthsV1<'data>,
+//     /// The symbol data for "stand-alone" style symbols.
+//     #[cfg_attr(feature = "serde", serde(borrow))]
+//     pub stand_alone: Option<StandAloneWidthsV1<'data>>,
+// }
+
 // Note: the SymbolsV* struct doc strings metadata are attached to `$name` in the macro invocation to
 // avoid macro parsing ambiguity caused by other metadata already attached to `$symbols`.
 macro_rules! symbols {
@@ -158,39 +249,7 @@ macro_rules! symbols {
             /// </div>
             $symbols
 
-            // UTS 35 specifies that `format` widths are mandatory,
-            // except for `short`.
-            #[derive(Debug, PartialEq, Clone, Default, yoke::Yokeable, zerofrom::ZeroFrom)]
-            #[cfg_attr(
-                feature = "datagen",
-                derive(serde::Serialize, databake::Bake),
-                databake(path = icu_datetime::provider::calendar::$name),
-            )]
-            #[cfg_attr(feature = "serde", derive(serde::Deserialize))]
-            #[yoke(prove_covariance_manually)]
-            #[doc = concat!("Symbol data for the \"format\" style formatting of ", stringify!($field_id),
-                ".\n\nThe format style is used in contexts where it is different from the stand-alone form, ex: ",
-                "a case inflected form where the stand-alone form is the nominative case.")]
-            ///
-            /// <div class="stab unstable">
-            /// 🚧 This code is considered unstable; it may change at any time, in breaking or non-breaking ways,
-            /// including in SemVer minor releases. While the serde representation of data structs is guaranteed
-            /// to be stable, their Rust representation might not be. Use with caution.
-            /// </div>
-            pub struct FormatWidthsV1<'data> {
-                #[doc = concat!("Abbreviated length symbol for \"format\" style symbol for ", stringify!($name), ".")]
-                #[cfg_attr(feature = "serde", serde(borrow))]
-                pub abbreviated: SymbolsV1<'data>,
-                #[doc = concat!("Narrow length symbol for \"format\" style symbol for ", stringify!($name), ".")]
-                #[cfg_attr(feature = "serde", serde(borrow))]
-                pub narrow: SymbolsV1<'data>,
-                #[doc = concat!("Short length symbol for \"format\" style symbol for ", stringify!($name), ", if present.")]
-                #[cfg_attr(feature = "serde", serde(borrow))]
-                pub short: Option<SymbolsV1<'data>>,
-                #[doc = concat!("Wide length symbol for \"format\" style symbol for ", stringify!($name), ".")]
-                #[cfg_attr(feature = "serde", serde(borrow))]
-                pub wide: SymbolsV1<'data>,
-            }
+            pub type FormatWidthsV1<'data>  = super::FormatWidthsV1<SymbolsV1<'data>>;
 
             // UTS 35 specifies that `stand_alone` widths are optional
             #[derive(Debug, PartialEq, Clone, Default, yoke::Yokeable, zerofrom::ZeroFrom)]
@Manishearth
Copy link
Member

When we handle this we should also correctly handle ?Sized on generics in Yoke. #4659 is implementing a stopgap solution.

Longterm solutions are:

  • Distinguish between may_borrow and non-may_borrow parameters, and use Sized on the former ones.
  • (easier) Implement yoke(additional_bounds(T: Sized)), which we may want anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-zerovec Component: Yoke, ZeroVec, DataBake S-medium Size: Less than a week (larger bug fix or enhancement)
2 participants