Skip to content

Clean up FormattableAnyCalendar #7318

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.

Sign up for GitHub

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

Open
robertbastian wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Clean up FormattableAnyCalendar #7318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07ba1fd
remove UntaggedFormattableAnyCalendar
robertbastian Dec 15, 2025
38a9bfe
use preferences in calendar constructors
robertbastian Dec 15, 2025
212a92c
reduce FormattableAnyCalendarKind usage
robertbastian Dec 15, 2025
0b52a8d
single constructor
robertbastian Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions components/datetime/src/external_loaders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@

//! Internal traits and structs for loading data from other crates.

use icu_calendar::preferences::CalendarPreferences;
use icu_decimal::options::DecimalFormatterOptions;
use icu_decimal::{DecimalFormatter, DecimalFormatterPreferences};
use icu_provider::prelude::*;

use crate::scaffold::{FormattableAnyCalendar, FormattableAnyCalendarKind};
use crate::scaffold::FormattableAnyCalendar;

/// Trait for loading a DecimalFormatter.
///
Expand All @@ -25,7 +26,7 @@ pub(crate) trait DecimalFormatterLoader {
///
/// Implemented on the provider-specific loader types in this module.
pub(crate) trait FormattableAnyCalendarLoader {
fn load(&self, kind: FormattableAnyCalendarKind) -> Result<FormattableAnyCalendar, DataError>;
fn load(&self, prefs: CalendarPreferences) -> Result<FormattableAnyCalendar, DataError>;
}

/// Loader for types from other crates using compiled data.
Expand All @@ -47,8 +48,8 @@ impl DecimalFormatterLoader for ExternalLoaderCompiledData {
#[cfg(feature = "compiled_data")]
impl FormattableAnyCalendarLoader for ExternalLoaderCompiledData {
#[inline]
fn load(&self, kind: FormattableAnyCalendarKind) -> Result<FormattableAnyCalendar, DataError> {
FormattableAnyCalendar::try_new(kind)
fn load(&self, prefs: CalendarPreferences) -> Result<FormattableAnyCalendar, DataError> {
FormattableAnyCalendar::try_new(prefs)
}
}

Expand Down Expand Up @@ -77,8 +78,8 @@ where
P: ?Sized + BufferProvider,
{
#[inline]
fn load(&self, kind: FormattableAnyCalendarKind) -> Result<FormattableAnyCalendar, DataError> {
FormattableAnyCalendar::try_new_with_buffer_provider(self.0, kind)
fn load(&self, prefs: CalendarPreferences) -> Result<FormattableAnyCalendar, DataError> {
FormattableAnyCalendar::try_new_with_buffer_provider(self.0, prefs)
}
}

Expand Down Expand Up @@ -106,7 +107,7 @@ where
P: DataProvider<icu_calendar::provider::CalendarJapaneseModernV1> + ?Sized,
{
#[inline]
fn load(&self, kind: FormattableAnyCalendarKind) -> Result<FormattableAnyCalendar, DataError> {
FormattableAnyCalendar::try_new_unstable(self.0, kind)
fn load(&self, prefs: CalendarPreferences) -> Result<FormattableAnyCalendar, DataError> {
FormattableAnyCalendar::try_new_unstable(self.0, prefs)
}
}
18 changes: 7 additions & 11 deletions components/datetime/src/neo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ size_test!(
pub struct DateTimeFormatter<FSet: DateTimeNamesMarker> {
pub(crate) selection: DateTimeZonePatternSelectionData,
pub(crate) names: RawDateTimeNames<FSet>,
pub(crate) calendar: UntaggedFormattableAnyCalendar,
pub(crate) calendar: FormattableAnyCalendar,
Copy link
Member

@sffc sffc Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observation: I think I had introduced UntaggedFormattableAnyCalendar because it reduced the stack size of DateTimeFormatter. But, the tests that enforce stack size are still passing, so maybe there's a new niche where the extra field can fit now.

Copy link
Member Author

@robertbastian robertbastian Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FormattableAnyCalendar is now just untagged

}

impl<FSet: DateTimeMarkers> DateTimeFormatter<FSet>
Expand Down Expand Up @@ -503,8 +503,7 @@ where
P: ?Sized + AllAnyCalendarFormattingDataMarkers<FSet>,
L: DecimalFormatterLoader + FormattableAnyCalendarLoader,
{
let kind = FormattableAnyCalendarKind::from_preferences(prefs);
let calendar = FormattableAnyCalendarLoader::load(loader, kind)?;
let calendar = FormattableAnyCalendarLoader::load(loader, (&prefs).into())?;
let names = RawDateTimeNames::new_without_number_formatting();
Self::try_new_internal_with_calendar_and_names(
provider,
Expand Down Expand Up @@ -549,8 +548,7 @@ where
{
let selection = DateTimeZonePatternSelectionData::try_new_with_skeleton(
&FormattableAnyCalendarNamesLoader::<<FSet::D as DateDataMarkers>::Skel, _>::new(
provider_p,
calendar.kind(),
provider_p, &calendar,
),
&<FSet::T as TimeMarkers>::TimeSkeletonPatternsV1::bind(provider_p),
&FSet::GluePatternV1::bind(provider_p),
Expand All @@ -568,12 +566,10 @@ where
};
let result = names.load_for_pattern(
&FormattableAnyCalendarNamesLoader::<<FSet::D as DateDataMarkers>::Year, _>::new(
provider,
calendar.kind(),
provider, &calendar,
),
&FormattableAnyCalendarNamesLoader::<<FSet::D as DateDataMarkers>::Month, _>::new(
provider,
calendar.kind(),
provider, &calendar,
),
&<FSet::D as DateDataMarkers>::WeekdayNamesV1::bind(provider),
&<FSet::T as TimeMarkers>::DayPeriodNamesV1::bind(provider),
Expand Down Expand Up @@ -605,7 +601,7 @@ where
Ok(Self {
selection,
names,
calendar: calendar.into_untagged(),
calendar,
})
}
}
Expand Down Expand Up @@ -781,7 +777,7 @@ impl<C: CldrCalendar, FSet: DateTimeMarkers> FixedCalendarDateTimeFormatter<C, F
DateTimeFormatter {
selection: self.selection,
names: self.names,
calendar: FormattableAnyCalendar::from_calendar(calendar).into_untagged(),
calendar: FormattableAnyCalendar::from_calendar(calendar),
}
}

Expand Down
8 changes: 2 additions & 6 deletions components/datetime/src/pattern/names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,9 +1079,8 @@ impl<FSet: DateTimeNamesMarker> DateTimeNames<FSet> {
prefs: DateTimeFormatterPreferences,
calendar: AnyCalendar,
) -> Result<Self, UnsupportedCalendarError> {
let kind = calendar.kind();
let calendar = FormattableAnyCalendar::try_from_any_calendar(calendar)
.ok_or(UnsupportedCalendarError { kind })?;
.map_err(|c| UnsupportedCalendarError { kind: c.kind() })?;
Ok(Self {
inner: FixedCalendarDateTimeNames::new_without_number_formatting(prefs),
calendar,
Expand Down Expand Up @@ -1137,10 +1136,7 @@ impl<FSet: DateTimeNamesMarker> DateTimeNames<FSet> {
formatter: DateTimeFormatter<FSet>,
) -> Self {
let metadata = DateTimeNamesMetadata::new_from_previous(&formatter.names);
Self::from_parts(
prefs,
(formatter.calendar.into_tagged(), formatter.names, metadata),
)
Self::from_parts(prefs, (formatter.calendar, formatter.names, metadata))
}

fn from_parts(
Expand Down
Loading