Skip to content

Commit

Permalink
Migrate DateSkeletonPatternsV1Marker to marker attributes (#5289)
Browse files Browse the repository at this point in the history
  • Loading branch information
robertbastian authored Jul 24, 2024
1 parent 633bc79 commit 1ac5262
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 142 deletions.
59 changes: 1 addition & 58 deletions components/datetime/src/provider/calendar/skeletons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,6 @@ use litemap::LiteMap;

size_test!(DateSkeletonPatternsV1, date_skeleton_patterns_v1_size, 24);

// Manually implement DataMarker so that we can keep it in the proper experimental feature
// #[icu_provider::data_struct(marker(
// DateSkeletonPatternsV1Marker,
// "datetime/skeletons@1",
// extension_key = "ca"
// ))]
//
/// Skeleton data for dates and times, along with the corresponding plural pattern
/// information.
#[doc = date_skeleton_patterns_v1_size!()]
Expand All @@ -28,14 +21,8 @@ size_test!(DateSkeletonPatternsV1, date_skeleton_patterns_v1_size, 24);
/// 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>
#[icu_provider::data_struct(marker(
DateSkeletonPatternsV1Marker,
"datetime/skeletons@1",
fallback_by = "language",
extension_key = "ca",
))]
#[icu_provider::data_struct(DateSkeletonPatternsV1Marker = "datetime/skeletons@1")]
#[derive(Debug, PartialEq, Clone, Default)]
#[cfg_attr(feature = "datagen", derive(serde::Serialize))]
#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
pub struct DateSkeletonPatternsV1<'data>(
#[cfg_attr(feature = "serde", serde(borrow))]
Expand All @@ -56,7 +43,6 @@ pub struct DateSkeletonPatternsV1<'data>(
/// to be stable, their Rust representation might not be. Use with caution.
/// </div>
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
#[cfg_attr(feature = "datagen", derive(serde::Serialize))]
#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
pub struct SkeletonV1(pub Skeleton);

Expand All @@ -70,46 +56,3 @@ impl TryFrom<&str> for SkeletonV1 {
}
}
}

// DateSkeletonPatternsV1 uses heap-allocations, so it cannot be const-constructed
// (and it also isn't zero-copy). For baking, we work around this by using an equivalent
// const type (BakedDateSkeletonPatternsV1) and then zero-froming that into the final
// struct. This operation contains the required allocation (which also happens in the
// serde codepath, ZeroFrom<Self> uses #[zerofrom(clone)]).
// See https://github.com/unicode-org/icu4x/issues/1678.

#[cfg(feature = "datagen")]
impl databake::Bake for DateSkeletonPatternsV1<'_> {
fn bake(&self, env: &databake::CrateEnv) -> databake::TokenStream {
use zerofrom::ZeroFrom;
env.insert("icu_datetime");
databake::Bake::bake(
&self
.0
.iter()
.map(|(skeleton, pattern)| {
(skeleton.0 .0.as_slice(), PatternPlurals::zero_from(pattern))
})
.collect::<Vec<_>>()
.as_slice(),
env,
)
}
}

#[cfg(feature = "datagen")]
impl Default for DateSkeletonPatternsV1Marker {
fn default() -> Self {
Self
}
}

#[cfg(feature = "datagen")]
impl databake::Bake for DateSkeletonPatternsV1Marker {
fn bake(&self, env: &databake::CrateEnv) -> databake::TokenStream {
env.insert("icu_datetime");
databake::quote! {
icu_datetime::provider::calendar::DateSkeletonPatternsV1Marker
}
}
}
21 changes: 10 additions & 11 deletions components/datetime/src/provider/date_time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,28 +345,27 @@ where
fn skeleton_data_payload(
&self,
) -> Result<DataPayload<DateSkeletonPatternsV1Marker>, DataError> {
use icu_locale_core::{
extensions::unicode::{key, value},
subtags::subtag,
};
let mut locale = self.locale.clone();
#[allow(clippy::expect_used)] // experimental
#![allow(clippy::expect_used)] // experimental
use icu_locale_core::{extensions::unicode::value, subtags::subtag};
let cal_val = self.cal_val.expect("should be present for components bag");
// Skeleton data for ethioaa is stored under ethiopic
if cal_val == &value!("ethioaa") {
locale.set_unicode_ext(key!("ca"), value!("ethiopic"));
let cal = if cal_val == &value!("ethioaa") {
"ethiopic"
} else if cal_val == &value!("islamicc")
|| cal_val.get_subtag(0) == Some(&subtag!("islamic"))
{
// All islamic calendars store skeleton data under islamic, not their individual extension keys
locale.set_unicode_ext(key!("ca"), value!("islamic"));
"islamic"
} else {
locale.set_unicode_ext(key!("ca"), cal_val.clone());
cal_val.as_single_subtag().expect("single").as_str()
};

self.data_provider
.load(DataRequest {
id: DataIdentifierBorrowed::for_locale(&locale),
id: DataIdentifierBorrowed::for_marker_attributes_and_locale(
DataMarkerAttributes::from_str_or_panic(cal),
self.locale,
),
..Default::default()
})
.map(|r| r.payload)
Expand Down
21 changes: 14 additions & 7 deletions components/datetime/src/skeleton/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,24 @@ mod test {
DataPayload<GregorianDateLengthsV1Marker>,
DataPayload<DateSkeletonPatternsV1Marker>,
) {
let locale = "en-u-ca-gregory".parse().unwrap();
let req = DataRequest {
id: DataIdentifierBorrowed::for_locale(&locale),
..Default::default()
};
let locale = icu_locale_core::locale!("en").into();
let marker_attributes = DataMarkerAttributes::from_str_or_panic("gregory");

let patterns = crate::provider::Baked
.load(req)
.load(DataRequest {
id: DataIdentifierBorrowed::for_locale(&locale),
..Default::default()
})
.expect("Failed to load payload")
.payload;
let skeletons = crate::provider::Baked
.load(req)
.load(DataRequest {
id: DataIdentifierBorrowed::for_marker_attributes_and_locale(
marker_attributes,
&locale,
),
..Default::default()
})
.expect("Failed to load payload")
.payload;
(patterns, skeletons)
Expand Down

Large diffs are not rendered by default.

42 changes: 22 additions & 20 deletions components/datetime/tests/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ use icu_datetime::{
TypedDateTimeFormatter, TypedZonedDateTimeFormatter,
};
use icu_decimal::provider::DecimalSymbolsV1Marker;
use icu_locale_core::{
extensions::unicode::{key, value},
locale, LanguageIdentifier, Locale,
};
use icu_locale_core::{locale, LanguageIdentifier, Locale};
use icu_provider::prelude::*;
use icu_provider_adapters::any_payload::AnyPayloadProvider;
use icu_provider_adapters::fork::MultiForkByMarkerProvider;
Expand Down Expand Up @@ -387,13 +384,8 @@ fn test_dayperiod_patterns() {
.unwrap()
.0
{
let mut locale: Locale = test.locale.parse().unwrap();
locale
.extensions
.unicode
.keywords
.set(key!("ca"), value!("gregory"));
let mut data_locale = DataLocale::from(&locale);
let locale: Locale = test.locale.parse().unwrap();
let data_locale = DataLocale::from(&locale);
let req = DataRequest {
id: DataIdentifierBorrowed::for_locale(&data_locale),
..Default::default()
Expand All @@ -414,10 +406,17 @@ fn test_dayperiod_patterns() {
icu_datetime::provider::Baked.load(req).unwrap();
#[cfg(feature = "experimental")]
let skeleton_data: DataResponse<DateSkeletonPatternsV1Marker> =
icu_datetime::provider::Baked.load(req).unwrap();
icu_datetime::provider::Baked
.load(DataRequest {
id: DataIdentifierBorrowed::for_marker_attributes_and_locale(
DataMarkerAttributes::from_str_or_panic("gregory"),
&data_locale,
),
..Default::default()
})
.unwrap();
let week_data: DataResponse<WeekDataV1Marker> =
icu_calendar::provider::Baked.load(req).unwrap();
data_locale.retain_unicode_ext(|_| false);
let decimal_data: DataResponse<DecimalSymbolsV1Marker> = icu_decimal::provider::Baked
.load(DataRequest {
id: DataIdentifierBorrowed::for_locale(&data_locale),
Expand Down Expand Up @@ -562,12 +561,7 @@ fn test_time_zone_patterns() {
.unwrap()
.0
{
let mut locale: Locale = test.locale.parse().unwrap();
locale
.extensions
.unicode
.keywords
.set(key!("ca"), value!("gregory"));
let locale: Locale = test.locale.parse().unwrap();
let data_locale = DataLocale::from(&locale);
let req = DataRequest {
id: DataIdentifierBorrowed::for_locale(&data_locale),
Expand All @@ -585,7 +579,15 @@ fn test_time_zone_patterns() {
icu_datetime::provider::Baked.load(req).unwrap();
#[cfg(feature = "experimental")]
let skeleton_data: DataResponse<DateSkeletonPatternsV1Marker> =
icu_datetime::provider::Baked.load(req).unwrap();
icu_datetime::provider::Baked
.load(DataRequest {
id: DataIdentifierBorrowed::for_marker_attributes_and_locale(
DataMarkerAttributes::from_str_or_panic("gregory"),
&data_locale,
),
..Default::default()
})
.unwrap();
let symbols_data: DataResponse<GregorianDateSymbolsV1Marker> =
icu_datetime::provider::Baked.load(req).unwrap();
let week_data: DataResponse<WeekDataV1Marker> =
Expand Down
63 changes: 18 additions & 45 deletions provider/source/src/datetime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::IterableDataProviderCached;
use crate::SourceDataProvider;
use either::Either;
use icu::datetime::provider::calendar::*;
use icu::locale::extensions::unicode::value;
use icu::locale::extensions::unicode::Value;
use icu::locale::extensions::unicode::{key, value};
use icu::locale::LanguageIdentifier;
use icu_provider::prelude::*;
use std::collections::HashMap;
Expand Down Expand Up @@ -206,47 +206,30 @@ macro_rules! impl_data_provider {

let langid = req.id.locale.get_langid();

let calendar = if DateSkeletonPatternsV1Marker::INFO == $marker::INFO {
req.id
.locale
.get_unicode_ext(&key!("ca"))
.ok_or_else(|| DataErrorKind::IdentifierNotFound.into_error())?
} else {
value!($calendar)
};

let data = self.get_datetime_resources(&langid, Either::Left(&calendar))?;
let data =
self.get_datetime_resources(&langid, Either::Left(&value!($calendar)))?;

#[allow(clippy::redundant_closure_call)]
Ok(DataResponse {
metadata: Default::default(),
payload: DataPayload::from_owned(($expr)(&data, &calendar.to_string())),
payload: DataPayload::from_owned(($expr)(&data, $calendar)),
})
}
}

impl IterableDataProviderCached<$marker> for SourceDataProvider {
fn iter_ids_cached(&self) -> Result<HashSet<DataIdentifierCow<'static>>, DataError> {
let mut r = HashSet::new();
if DateSkeletonPatternsV1Marker::INFO == $marker::INFO {
for (cal_value, cldr_cal) in supported_cals() {
r.extend(self.cldr()?.dates(cldr_cal).list_langs()?.map(|lid| {
let mut locale = DataLocale::from(lid);
locale.set_unicode_ext(key!("ca"), cal_value.clone());
DataIdentifierCow::from_locale(locale)
}));
}
} else {
let cldr_cal = supported_cals()
.get(&value!($calendar))
.ok_or_else(|| DataErrorKind::IdentifierNotFound.into_error())?;
r.extend(
self.cldr()?
.dates(cldr_cal)
.list_langs()?
.map(|l| DataIdentifierCow::from_locale(DataLocale::from(l))),
);
}

let cldr_cal = supported_cals()
.get(&value!($calendar))
.ok_or_else(|| DataErrorKind::IdentifierNotFound.into_error())?;
r.extend(
self.cldr()?
.dates(cldr_cal)
.list_langs()?
.map(|l| DataIdentifierCow::from_locale(DataLocale::from(l))),
);

// TODO(#3212): Remove
if $marker::INFO == TimeLengthsV1Marker::INFO {
Expand Down Expand Up @@ -384,12 +367,6 @@ impl_data_provider!(
"gregory"
);

impl_data_provider!(
DateSkeletonPatternsV1Marker,
|dates, _| { DateSkeletonPatternsV1::from(dates) },
"unused"
);

#[cfg(test)]
mod test {
use super::*;
Expand Down Expand Up @@ -431,15 +408,11 @@ mod test {
use icu::plurals::PluralCategory;
use std::convert::TryFrom;

let provider = SourceDataProvider::new_testing();
let data = SourceDataProvider::new_testing()
.get_datetime_resources(&langid!("fil"), Either::Left(&value!("gregory")))
.unwrap();

let skeletons: DataResponse<DateSkeletonPatternsV1Marker> = provider
.load(DataRequest {
id: DataIdentifierBorrowed::for_locale(&"fil-u-ca-gregory".parse().unwrap()),
..Default::default()
})
.expect("Failed to load payload");
let skeletons = &skeletons.payload.get().0;
let skeletons = DateSkeletonPatternsV1::from(&data).0;

assert_eq!(
Some(
Expand Down

0 comments on commit 1ac5262

Please sign in to comment.