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

Migrate some data structs to serde(borrow) #820

Merged
merged 1 commit into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Cargo.lock

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

1 change: 0 additions & 1 deletion components/decimal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ skip_optional_dependencies = true
all-features = true

[dependencies]
smallstr = { version = "0.2", features = ["serde"] }
icu_locid = { version = "0.2", path = "../locid" }
icu_provider = { version = "0.2", path = "../../provider/core" }
fixed_decimal = { version = "0.2", path = "../../utils/fixed_decimal" }
Expand Down
10 changes: 3 additions & 7 deletions components/decimal/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use writeable::Writeable;
pub struct FormattedFixedDecimal<'l> {
pub(crate) value: &'l FixedDecimal,
pub(crate) options: &'l FixedDecimalFormatOptions,
pub(crate) symbols: &'l DecimalSymbolsV1,
pub(crate) symbols: &'l DecimalSymbolsV1<'l>,
}

impl<'l> FormattedFixedDecimal<'l> {
Expand All @@ -38,9 +38,7 @@ impl<'l> Writeable for FormattedFixedDecimal<'l> {
{
let affixes = self.get_affixes();
if let Some(affixes) = affixes {
if let Some(prefix) = &affixes.prefix {
sink.write_str(prefix)?;
}
sink.write_str(&affixes.prefix)?;
}
let range = self.value.magnitude_range();
let upper_magnitude = *range.end();
Expand All @@ -60,9 +58,7 @@ impl<'l> Writeable for FormattedFixedDecimal<'l> {
}
}
if let Some(affixes) = affixes {
if let Some(suffix) = &affixes.suffix {
sink.write_str(suffix)?;
}
sink.write_str(&affixes.suffix)?;
}
Ok(())
}
Expand Down
40 changes: 23 additions & 17 deletions components/decimal/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//!
//! Read more about data providers: [`icu_provider`]

pub type SmallString8 = smallstr::SmallString<[u8; 8]>;
use std::borrow::Cow;

pub mod key {
//! Resource keys for [`icu_decimal`](crate).
Expand All @@ -22,12 +22,14 @@ pub mod key {
feature = "provider_serde",
derive(serde::Serialize, serde::Deserialize)
)]
pub struct AffixesV1 {
pub struct AffixesV1<'s> {
/// String to prepend before the decimal number.
pub prefix: Option<SmallString8>,
#[cfg_attr(feature = "provider_serde", serde(borrow))]
pub prefix: Cow<'s, str>,

/// String to append after the decimal number.
pub suffix: Option<SmallString8>,
#[cfg_attr(feature = "provider_serde", serde(borrow))]
pub suffix: Cow<'s, str>,
}

/// A collection of settings expressing where to put grouping separators in a decimal number.
Expand Down Expand Up @@ -55,18 +57,21 @@ pub struct GroupingSizesV1 {
feature = "provider_serde",
derive(serde::Serialize, serde::Deserialize)
)]
pub struct DecimalSymbolsV1 {
pub struct DecimalSymbolsV1<'s> {
/// Prefix and suffix to apply when a negative sign is needed.
pub minus_sign_affixes: AffixesV1,
#[cfg_attr(feature = "provider_serde", serde(borrow))]
pub minus_sign_affixes: AffixesV1<'s>,

/// Prefix and suffix to apply when a plus sign is needed.
pub plus_sign_affixes: AffixesV1,
#[cfg_attr(feature = "provider_serde", serde(borrow))]
pub plus_sign_affixes: AffixesV1<'s>,

/// Character used to separate the integer and fraction parts of the number.
pub decimal_separator: SmallString8,
#[cfg_attr(feature = "provider_serde", serde(borrow))]
pub decimal_separator: Cow<'s, str>,

/// Character used to separate groups in the integer part of the number.
pub grouping_separator: SmallString8,
pub grouping_separator: Cow<'s, str>,

/// Settings used to determine where to place groups in the integer part of the number.
pub grouping_sizes: GroupingSizesV1,
Expand All @@ -76,16 +81,16 @@ pub struct DecimalSymbolsV1 {
pub digits: [char; 10],
}

impl Default for DecimalSymbolsV1 {
impl Default for DecimalSymbolsV1<'static> {
fn default() -> Self {
Self {
minus_sign_affixes: AffixesV1 {
prefix: Some("-".into()),
suffix: None,
prefix: Cow::Borrowed("-"),
suffix: Cow::Borrowed(""),
},
plus_sign_affixes: AffixesV1 {
prefix: Some("+".into()),
suffix: None,
prefix: Cow::Borrowed("+"),
suffix: Cow::Borrowed(""),
},
decimal_separator: ".".into(),
grouping_separator: ",".into(),
Expand All @@ -99,8 +104,9 @@ impl Default for DecimalSymbolsV1 {
}
}

icu_provider::impl_data_marker_no_lifetime!(
DecimalSymbolsV1,
icu_provider::unsafe_impl_data_marker_with_lifetime!(
DecimalSymbolsV1<'s>,
/// Marker type for [`DecimalSymbolsV1`]
DecimalSymbolsV1Marker
DecimalSymbolsV1Marker,
TEMP_ZCF
);
7 changes: 7 additions & 0 deletions experimental/provider_static/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# icu_provider_static [![crates.io](http://meritbadge.herokuapp.com/icu_provider_static)](https://crates.io/crates/icu_provider_static)



## More Information

For more information on development, authorship, contributing etc. please visit [`ICU4X home page`](https://github.com/unicode-org/icu4x).
9 changes: 5 additions & 4 deletions experimental/provider_static/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ impl StaticDataProvider {
}
}

fn get_file(&self, req: &DataRequest) -> Result<&&str, DataError> {
fn get_file(&self, req: &DataRequest) -> Result<&'static str, DataError> {
let key_components = req.resource_path.key.get_components();
let opt_components = req.resource_path.options.get_components();
let key: Vec<&str> = key_components.iter().chain(opt_components.iter()).collect();
let key = "/".to_string() + &key.join("/");
self.json
.get(&*key)
.ok_or(DataError::UnsupportedResourceKey(req.resource_path.key))
.map(|v| *v)
}
}

Expand All @@ -46,13 +47,13 @@ impl Default for StaticDataProvider {
impl<'d, 's, M> DataProvider<'d, 's, M> for StaticDataProvider
where
M: DataMarker<'s>,
// TODO(#667): Use zero-copy Deserialize instead of DeserializeOwned
M::Yokeable: serde::de::DeserializeOwned,
// 'static is what we want here, because we are deserializing from a static buffer.
M::Yokeable: serde::de::Deserialize<'static>,
{
fn load_payload(&self, req: &DataRequest) -> Result<DataResponse<'d, 's, M>, DataError> {
let file = self.get_file(req)?;
let data: M::Yokeable =
M::Yokeable::deserialize(&mut serde_json::Deserializer::from_reader(file.as_bytes()))
M::Yokeable::deserialize(&mut serde_json::Deserializer::from_slice(file.as_bytes()))
.map_err(|e| DataError::Resource(Box::new(e)))?;
Ok(DataResponse {
metadata: DataResponseMetadata {
Expand Down
10 changes: 4 additions & 6 deletions provider/cldr/src/transform/numbers/cldr_serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ use serde_aux::prelude::*;
use std::collections::HashMap;
use tinystr::{TinyStr8, TinyStrAuto};

pub type SmallString8 = smallstr::SmallString<[u8; 8]>;

pub mod numbers_json {
//! Serde structs representing CLDR JSON numbers.json files.
//!
Expand All @@ -24,12 +22,12 @@ pub mod numbers_json {
#[derive(PartialEq, Debug, Deserialize)]
pub struct Symbols {
// This list is not comprehensive; add more fields when needed
pub decimal: SmallString8,
pub group: SmallString8,
pub decimal: String,
pub group: String,
#[serde(rename = "minusSign")]
pub minus_sign: SmallString8,
pub minus_sign: String,
#[serde(rename = "plusSign")]
pub plus_sign: SmallString8,
pub plus_sign: String,
}

#[derive(PartialEq, Debug, Deserialize)]
Expand Down
31 changes: 9 additions & 22 deletions provider/cldr/src/transform/numbers/decimal_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
//!
//! Spec reference: https://unicode.org/reports/tr35/tr35-numbers.html#Number_Format_Patterns

use std::str::FromStr;
type SmallString8 = smallstr::SmallString<[u8; 8]>;
use icu_decimal::provider::AffixesV1;
use itertools::Itertools;
use std::borrow::Cow;
use std::str::FromStr;
use thiserror::Error;

#[derive(Error, Debug, PartialEq)]
Expand All @@ -23,8 +23,8 @@ pub enum Error {
/// Representation of a UTS-35 number subpattern (part of a number pattern between ';'s).
#[derive(Debug, PartialEq)]
pub struct DecimalSubPattern {
pub prefix: SmallString8,
pub suffix: SmallString8,
pub prefix: String,
pub suffix: String,
pub primary_grouping: u8,
pub secondary_grouping: u8,
pub min_fraction_digits: u8,
Expand Down Expand Up @@ -98,29 +98,16 @@ impl FromStr for DecimalPattern {
}

impl DecimalPattern {
pub fn localize_sign(&self, sign_str: &str) -> AffixesV1 {
pub fn localize_sign(&self, sign_str: &str) -> AffixesV1<'static> {
// UTS 35: the absence of a negative pattern means a single prefixed sign
let signed_affixes = self
.negative
.as_ref()
.map(|subpattern| {
(
if subpattern.prefix.is_empty() {
None
} else {
Some(subpattern.prefix.clone())
},
if subpattern.suffix.is_empty() {
None
} else {
Some(subpattern.suffix.clone())
},
)
})
.unwrap_or_else(|| (Some("-".into()), None));
.map(|subpattern| (subpattern.prefix.as_str(), subpattern.suffix.as_str()))
.unwrap_or_else(|| ("-", ""));
AffixesV1 {
prefix: signed_affixes.0.map(|s| s.replace("-", sign_str).into()),
suffix: signed_affixes.1.map(|s| s.replace("-", sign_str).into()),
prefix: Cow::Owned(signed_affixes.0.replace("-", sign_str)),
suffix: Cow::Owned(signed_affixes.1.replace("-", sign_str)),
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions provider/cldr/src/transform/numbers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl<'d> IterableDataProviderCore for NumbersProvider {
}
}

impl TryFrom<&cldr_serde::numbers_json::Numbers> for DecimalSymbolsV1 {
impl TryFrom<&cldr_serde::numbers_json::Numbers> for DecimalSymbolsV1<'static> {
type Error = Cow<'static, str>;

fn try_from(other: &cldr_serde::numbers_json::Numbers) -> Result<Self, Self::Error> {
Expand All @@ -183,8 +183,8 @@ impl TryFrom<&cldr_serde::numbers_json::Numbers> for DecimalSymbolsV1 {
Ok(Self {
minus_sign_affixes: parsed_pattern.localize_sign(&symbols.minus_sign),
plus_sign_affixes: parsed_pattern.localize_sign(&symbols.plus_sign),
decimal_separator: symbols.decimal.clone(),
grouping_separator: symbols.group.clone(),
decimal_separator: Cow::Owned(symbols.decimal.clone()),
grouping_separator: Cow::Owned(symbols.group.clone()),
grouping_sizes: GroupingSizesV1 {
primary: parsed_pattern.positive.primary_grouping,
secondary: parsed_pattern.positive.secondary_grouping,
Expand Down
2 changes: 1 addition & 1 deletion provider/core/src/hello_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub mod key {
derive(serde::Serialize, serde::Deserialize)
)]
pub struct HelloWorldV1<'s> {
// TODO(#667): Use serde borrow.
#[cfg_attr(feature = "provider_serde", serde(borrow))]
pub message: Cow<'s, str>,
}

Expand Down
6 changes: 2 additions & 4 deletions provider/core/tests/data_receiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ fn test_deserializer_static() {
assert!(matches!(
payload.get(),
&HelloWorldV1 {
// TODO(#667): This should be Borrowed once HelloWorldV1 supports it
message: Cow::Owned(_)
message: Cow::Borrowed(_)
}
));
}
Expand All @@ -48,8 +47,7 @@ fn test_deserializer_owned() {
assert!(matches!(
payload.get(),
&HelloWorldV1 {
// TODO(#667): This should be Borrowed once HelloWorldV1 supports it
message: Cow::Owned(_)
message: Cow::Borrowed(_)
}
));
}
4 changes: 2 additions & 2 deletions provider/testdata/data/json/decimal/symbols@1/ar-EG.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"minus_sign_affixes": {
"prefix": "؜-",
"suffix": null
"suffix": ""
},
"plus_sign_affixes": {
"prefix": "؜+",
"suffix": null
"suffix": ""
},
"decimal_separator": "٫",
"grouping_separator": "٬",
Expand Down
4 changes: 2 additions & 2 deletions provider/testdata/data/json/decimal/symbols@1/ar.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"minus_sign_affixes": {
"prefix": "؜-",
"suffix": null
"suffix": ""
},
"plus_sign_affixes": {
"prefix": "؜+",
"suffix": null
"suffix": ""
},
"decimal_separator": "٫",
"grouping_separator": "٬",
Expand Down
4 changes: 2 additions & 2 deletions provider/testdata/data/json/decimal/symbols@1/bn.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"minus_sign_affixes": {
"prefix": "-",
"suffix": null
"suffix": ""
},
"plus_sign_affixes": {
"prefix": "+",
"suffix": null
"suffix": ""
},
"decimal_separator": ".",
"grouping_separator": ",",
Expand Down
4 changes: 2 additions & 2 deletions provider/testdata/data/json/decimal/symbols@1/ccp.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"minus_sign_affixes": {
"prefix": "-",
"suffix": null
"suffix": ""
},
"plus_sign_affixes": {
"prefix": "+",
"suffix": null
"suffix": ""
},
"decimal_separator": ".",
"grouping_separator": ",",
Expand Down
4 changes: 2 additions & 2 deletions provider/testdata/data/json/decimal/symbols@1/en-001.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"minus_sign_affixes": {
"prefix": "-",
"suffix": null
"suffix": ""
},
"plus_sign_affixes": {
"prefix": "+",
"suffix": null
"suffix": ""
},
"decimal_separator": ".",
"grouping_separator": ",",
Expand Down
4 changes: 2 additions & 2 deletions provider/testdata/data/json/decimal/symbols@1/en-ZA.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"minus_sign_affixes": {
"prefix": "-",
"suffix": null
"suffix": ""
},
"plus_sign_affixes": {
"prefix": "+",
"suffix": null
"suffix": ""
},
"decimal_separator": ",",
"grouping_separator": " ",
Expand Down
Loading