Skip to content

Commit

Permalink
Remove compiled_data Default ctors from types which are prone to chan…
Browse files Browse the repository at this point in the history
…ge sufficiently often (#5958)

Also clarifies a `new_common()` vs `new_extended()` distinction for
locale data.


Fixes #5554
  • Loading branch information
Manishearth authored Jan 14, 2025
1 parent c9c9949 commit 8046758
Show file tree
Hide file tree
Showing 69 changed files with 601 additions and 1,229 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

- Components
- `icu_provider`
- `GeneralCategoryGroup::contains` now accepts `self` by value (unicode-org#5952)
- `GeneralCategoryGroup::contains` now accepts `self` by value (unicode-org#5952)
- `icu_locale`
- `LocaleExpander`, `LocaleDirectionality`, and `LocaleCanonicalizer` distinguish between `new_common()` and `new_extended()` constructors (unicode-org#5958)
- `icu_segmenter`
- Segmenters that can take a content locale now specify `_root()` on their default localeless constructors (unicode-org#5958)

## icu4x 2.0-beta1

Expand Down
2 changes: 1 addition & 1 deletion components/experimental/src/personnames/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ impl PersonNamesFormatterOptions {
usage: FormattingUsage,
formality: FormattingFormality,
) -> Self {
let lc = icu_locale::LocaleExpander::new();
let lc = icu_locale::LocaleExpander::new_extended();
let mut final_locale = target_locale.clone();
lc.maximize(&mut final_locale.id);
Self {
Expand Down
6 changes: 3 additions & 3 deletions components/locale/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions components/locale/benches/locale_canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use icu_locale::LocaleExpander;
use icu_locale_core::Locale;

fn canonicalize_bench(c: &mut Criterion) {
let lc = LocaleCanonicalizer::new();
let lc = LocaleCanonicalizer::new_common();

let mut group = c.benchmark_group("uncanonicalized");

Expand Down Expand Up @@ -38,7 +38,7 @@ fn canonicalize_bench(c: &mut Criterion) {
}

fn canonicalize_noop_bench(c: &mut Criterion) {
let lc = LocaleCanonicalizer::new();
let lc = LocaleCanonicalizer::new_common();

let mut group = c.benchmark_group("canonicalized");

Expand Down Expand Up @@ -69,7 +69,7 @@ fn canonicalize_noop_bench(c: &mut Criterion) {
}

fn maximize_bench(c: &mut Criterion) {
let lc = LocaleExpander::new();
let lc = LocaleExpander::new_common();

let mut group = c.benchmark_group("likelysubtags");

Expand Down
62 changes: 45 additions & 17 deletions components/locale/src/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use tinystr::TinyAsciiStr;
/// use icu::locale::Locale;
/// use icu::locale::{LocaleCanonicalizer, TransformResult};
///
/// let lc = LocaleCanonicalizer::new();
/// let lc = LocaleCanonicalizer::new_extended();
///
/// let mut locale: Locale = "ja-Latn-fonipa-hepburn-heploc".parse().unwrap();
/// assert_eq!(lc.canonicalize(&mut locale), TransformResult::Modified);
Expand Down Expand Up @@ -194,43 +194,71 @@ fn uts35_check_language_rules(
TransformResult::Unmodified
}

#[cfg(feature = "compiled_data")]
impl Default for LocaleCanonicalizer<LocaleExpander> {
fn default() -> Self {
Self::new()
impl LocaleCanonicalizer<LocaleExpander> {
/// A constructor which creates a [`LocaleCanonicalizer`] from compiled data,
/// using a [`LocaleExpander`] for common locales.
///
/// ✨ *Enabled with the `compiled_data` Cargo feature.*
///
/// [📚 Help choosing a constructor](icu_provider::constructors)
#[cfg(feature = "compiled_data")]
pub const fn new_common() -> Self {
Self::new_with_expander(LocaleExpander::new_common())
}
}

impl LocaleCanonicalizer<LocaleExpander> {
/// A constructor which creates a [`LocaleCanonicalizer`] from compiled data.
icu_provider::gen_any_buffer_data_constructors!(() -> error: DataError,
functions: [
new_common: skip,
try_new_common_with_any_provider,
try_new_common_with_buffer_provider,
try_new_common_unstable,
Self,
]
);

#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new_common)]
pub fn try_new_common_unstable<P>(provider: &P) -> Result<Self, DataError>
where
P: DataProvider<AliasesV2Marker>
+ DataProvider<LikelySubtagsForLanguageV1Marker>
+ DataProvider<LikelySubtagsForScriptRegionV1Marker>
+ ?Sized,
{
let expander = LocaleExpander::try_new_common_unstable(provider)?;
Self::try_new_with_expander_unstable(provider, expander)
}

/// A constructor which creates a [`LocaleCanonicalizer`] from compiled data,
/// using a [`LocaleExpander`] for all locales.
///
/// ✨ *Enabled with the `compiled_data` Cargo feature.*
///
/// [📚 Help choosing a constructor](icu_provider::constructors)
#[cfg(feature = "compiled_data")]
pub const fn new() -> Self {
pub const fn new_extended() -> Self {
Self::new_with_expander(LocaleExpander::new_extended())
}

icu_provider::gen_any_buffer_data_constructors!(() -> error: DataError,
functions: [
new: skip,
try_new_with_any_provider,
try_new_with_buffer_provider,
try_new_unstable,
new_extended: skip,
try_new_extended_with_any_provider,
try_new_extended_with_buffer_provider,
try_new_extended_unstable,
Self,
]
);

#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new)]
pub fn try_new_unstable<P>(provider: &P) -> Result<Self, DataError>
#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new_extended)]
pub fn try_new_extended_unstable<P>(provider: &P) -> Result<Self, DataError>
where
P: DataProvider<AliasesV2Marker>
+ DataProvider<LikelySubtagsForLanguageV1Marker>
+ DataProvider<LikelySubtagsForScriptRegionV1Marker>
+ DataProvider<LikelySubtagsExtendedV1Marker>
+ ?Sized,
{
let expander = LocaleExpander::try_new_unstable(provider)?;
let expander = LocaleExpander::try_new_extended_unstable(provider)?;
Self::try_new_with_expander_unstable(provider, expander)
}
}
Expand Down Expand Up @@ -289,7 +317,7 @@ impl<Expander: AsRef<LocaleExpander>> LocaleCanonicalizer<Expander> {
/// ```
/// use icu::locale::{Locale, LocaleCanonicalizer, TransformResult};
///
/// let lc = LocaleCanonicalizer::new();
/// let lc = LocaleCanonicalizer::new_extended();
///
/// let mut locale: Locale = "ja-Latn-fonipa-hepburn-heploc".parse().unwrap();
/// assert_eq!(lc.canonicalize(&mut locale), TransformResult::Modified);
Expand Down
81 changes: 58 additions & 23 deletions components/locale/src/directionality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub enum Direction {
/// ```
/// use icu::locale::{langid, Direction, LocaleDirectionality};
///
/// let ld = LocaleDirectionality::new();
/// let ld = LocaleDirectionality::new_common();
///
/// assert_eq!(ld.get(&langid!("en")), Some(Direction::LeftToRight));
/// ```
Expand All @@ -39,48 +39,83 @@ pub struct LocaleDirectionality<Expander = LocaleExpander> {
expander: Expander,
}

#[cfg(feature = "compiled_data")]
impl Default for LocaleDirectionality<LocaleExpander> {
fn default() -> Self {
Self::new()
impl LocaleDirectionality<LocaleExpander> {
/// Creates a [`LocaleDirectionality`] from compiled data, using [`LocaleExpander`]
/// data for common locales.
///
/// This includes limited likely subtags data, see [`LocaleExpander::new_common()`].
#[cfg(feature = "compiled_data")]
pub const fn new_common() -> Self {
Self::new_with_expander(LocaleExpander::new_common())
}
}

impl LocaleDirectionality<LocaleExpander> {
/// Creates a [`LocaleDirectionality`] from compiled data.
// Note: This is a custom impl because the bounds on `try_new_unstable` don't suffice
#[doc = icu_provider::gen_any_buffer_unstable_docs!(ANY, Self::new_common)]
pub fn try_new_common_with_any_provider(
provider: &(impl AnyProvider + ?Sized),
) -> Result<Self, DataError> {
let expander = LocaleExpander::try_new_common_with_any_provider(provider)?;
Self::try_new_with_expander_unstable(&provider.as_downcasting(), expander)
}

// Note: This is a custom impl because the bounds on `try_new_unstable` don't suffice
#[doc = icu_provider::gen_any_buffer_unstable_docs!(BUFFER, Self::new_common)]
#[cfg(feature = "serde")]
pub fn try_new_common_with_buffer_provider(
provider: &(impl BufferProvider + ?Sized),
) -> Result<Self, DataError> {
let expander = LocaleExpander::try_new_common_with_buffer_provider(provider)?;
Self::try_new_with_expander_unstable(&provider.as_deserializing(), expander)
}
#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new_common)]
pub fn try_new_common_unstable<P>(provider: &P) -> Result<LocaleDirectionality, DataError>
where
P: DataProvider<ScriptDirectionV1Marker>
+ DataProvider<LikelySubtagsForLanguageV1Marker>
+ DataProvider<LikelySubtagsForScriptRegionV1Marker>
+ ?Sized,
{
let expander = LocaleExpander::try_new_common_unstable(provider)?;
Self::try_new_with_expander_unstable(provider, expander)
}

/// Creates a [`LocaleDirectionality`] from compiled data, using [`LocaleExpander`]
/// data for all locales.
///
/// This includes limited likely subtags data, see [`LocaleExpander::new()`].
/// This includes all likely subtags data, see [`LocaleExpander::new_extended()`].
#[cfg(feature = "compiled_data")]
pub const fn new() -> Self {
Self::new_with_expander(LocaleExpander::new())
pub const fn new_extended() -> Self {
Self::new_with_expander(LocaleExpander::new_extended())
}

// Note: This is a custom impl because the bounds on `try_new_unstable` don't suffice
#[doc = icu_provider::gen_any_buffer_unstable_docs!(ANY, Self::new)]
pub fn try_new_with_any_provider(
#[doc = icu_provider::gen_any_buffer_unstable_docs!(ANY, Self::new_extended)]
pub fn try_new_extended_with_any_provider(
provider: &(impl AnyProvider + ?Sized),
) -> Result<Self, DataError> {
let expander = LocaleExpander::try_new_with_any_provider(provider)?;
let expander = LocaleExpander::try_new_extended_with_any_provider(provider)?;
Self::try_new_with_expander_unstable(&provider.as_downcasting(), expander)
}

// Note: This is a custom impl because the bounds on `try_new_unstable` don't suffice
#[doc = icu_provider::gen_any_buffer_unstable_docs!(BUFFER, Self::new)]
#[doc = icu_provider::gen_any_buffer_unstable_docs!(BUFFER, Self::new_extended)]
#[cfg(feature = "serde")]
pub fn try_new_with_buffer_provider(
pub fn try_new_extended_with_buffer_provider(
provider: &(impl BufferProvider + ?Sized),
) -> Result<Self, DataError> {
let expander = LocaleExpander::try_new_with_buffer_provider(provider)?;
let expander = LocaleExpander::try_new_extended_with_buffer_provider(provider)?;
Self::try_new_with_expander_unstable(&provider.as_deserializing(), expander)
}
#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new)]
pub fn try_new_unstable<P>(provider: &P) -> Result<LocaleDirectionality, DataError>
#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new_extended)]
pub fn try_new_extended_unstable<P>(provider: &P) -> Result<LocaleDirectionality, DataError>
where
P: DataProvider<ScriptDirectionV1Marker>
+ DataProvider<LikelySubtagsForLanguageV1Marker>
+ DataProvider<LikelySubtagsForScriptRegionV1Marker>
+ DataProvider<LikelySubtagsExtendedV1Marker>
+ ?Sized,
{
let expander = LocaleExpander::try_new_unstable(provider)?;
let expander = LocaleExpander::try_new_extended_unstable(provider)?;
Self::try_new_with_expander_unstable(provider, expander)
}
}
Expand All @@ -97,7 +132,7 @@ impl<Expander: AsRef<LocaleExpander>> LocaleDirectionality<Expander> {
/// langid, Direction, LocaleDirectionality, LocaleExpander,
/// };
///
/// let ld_default = LocaleDirectionality::new();
/// let ld_default = LocaleDirectionality::new_common();
///
/// assert_eq!(ld_default.get(&langid!("jbn")), None);
///
Expand Down Expand Up @@ -154,7 +189,7 @@ impl<Expander: AsRef<LocaleExpander>> LocaleDirectionality<Expander> {
/// ```
/// use icu::locale::{langid, Direction, LocaleDirectionality};
///
/// let ld = LocaleDirectionality::new();
/// let ld = LocaleDirectionality::new_common();
///
/// assert_eq!(ld.get(&langid!("en-US")), Some(Direction::LeftToRight));
///
Expand All @@ -171,7 +206,7 @@ impl<Expander: AsRef<LocaleExpander>> LocaleDirectionality<Expander> {
/// use icu::locale::subtags::script;
/// use icu::locale::{Direction, LanguageIdentifier, LocaleDirectionality};
///
/// let ld = LocaleDirectionality::new();
/// let ld = LocaleDirectionality::new_common();
///
/// assert_eq!(
/// ld.get(&LanguageIdentifier::from(Some(script!("Latn")))),
Expand Down
Loading

0 comments on commit 8046758

Please sign in to comment.