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

Add new type DataPayloadOr #4463

Merged
merged 26 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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: 1 addition & 0 deletions provider/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub use crate::request::DataRequest;
pub use crate::request::DataRequestMetadata;
pub use crate::response::Cart;
pub use crate::response::DataPayload;
pub use crate::response::DataPayloadOr;
pub use crate::response::DataResponse;
pub use crate::response::DataResponseMetadata;
#[cfg(feature = "macros")]
Expand Down
2 changes: 1 addition & 1 deletion provider/core/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,8 +892,8 @@ impl AuxiliaryKeys {
/// # Examples
///
/// ```
/// use icu_provider::prelude::*;
/// use icu_locid::extensions::private::subtag;
/// use icu_provider::prelude::*;
///
/// // Single auxiliary key:
/// let a = AuxiliaryKeys::from_subtag(subtag!("abc"));
Expand Down
203 changes: 203 additions & 0 deletions provider/core/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,88 @@ pub struct DataResponseMetadata {
/// ```
pub struct DataPayload<M: DataMarker>(pub(crate) DataPayloadInner<M>);

/// A container for data payloads with storage for something else.
///
/// The type parameter `O` is stored as part of the interior enum, leading to
/// better stack size optimization. `O` can be as large as the [`DataPayload`]
/// minus two words without impacting stack size.
///
/// # Examples
///
/// Create and use DataPayloadOr:
///
/// ```
/// use icu_provider::hello_world::*;
/// use icu_provider::prelude::*;
/// use icu_provider::DataPayloadOr;
///
/// let payload: DataPayload<HelloWorldV1Marker> = HelloWorldProvider
/// .load(DataRequest {
/// locale: &"de".parse().unwrap(),
/// metadata: Default::default(),
/// })
/// .expect("Loading should succeed")
/// .take_payload()
/// .expect("Data should be present");
///
/// let payload_some =
/// DataPayloadOr::<HelloWorldV1Marker, ()>::from_payload(payload);
/// let payload_none = DataPayloadOr::<HelloWorldV1Marker, ()>::from_other(());
///
/// assert_eq!(
/// payload_some.get(),
/// Ok(&HelloWorldV1 {
/// message: "Hallo Welt".into()
/// })
/// );
/// assert_eq!(payload_none.get(), Err(&()));
/// ```
///
/// Stack size comparison:
///
/// ```
/// use core::mem::size_of;
/// use icu_provider::hello_world::*;
/// use icu_provider::prelude::*;
/// use icu_provider::DataPayloadOr;
///
/// const W: usize = size_of::<usize>();
///
/// // HelloWorldV1 is 3 words:
/// assert_eq!(W * 3, size_of::<HelloWorldV1>());
///
/// // DataPayload adds a word for a total of 4 words:
/// assert_eq!(W * 4, size_of::<DataPayload<HelloWorldV1Marker>>());
///
/// // Option<DataPayload> balloons to 5 words:
/// assert_eq!(W * 5, size_of::<Option<DataPayload<HelloWorldV1Marker>>>());
///
/// // But, using DataPayloadOr is the same size as DataPayload:
/// assert_eq!(W * 4, size_of::<DataPayloadOr<HelloWorldV1Marker, ()>>());
///
/// // The largest optimized Other type is two words smaller than the DataPayload:
/// assert_eq!(W * 4, size_of::<DataPayloadOr<HelloWorldV1Marker, [usize; 1]>>());
/// assert_eq!(W * 4, size_of::<DataPayloadOr<HelloWorldV1Marker, [usize; 2]>>());
/// assert_eq!(W * 5, size_of::<DataPayloadOr<HelloWorldV1Marker, [usize; 3]>>());
/// ```
#[doc(hidden)] // TODO(#4467): establish this as an internal API
pub struct DataPayloadOr<M: DataMarker, O>(pub(crate) DataPayloadOrInner<M, O>);
Copy link
Member

Choose a reason for hiding this comment

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

thought: perhaps this should be internal/unstable for now?

Copy link
Member

Choose a reason for hiding this comment

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

Not blocking


pub(crate) enum DataPayloadInner<M: DataMarker> {
Yoke(Yoke<M::Yokeable, CartableOptionPointer<CartInner>>),
StaticRef(&'static M::Yokeable),
}

pub(crate) enum DataPayloadOrInner<M: DataMarker, O> {
Yoke(Yoke<M::Yokeable, CartableOptionPointer<CartInner>>),
Inner(DataPayloadOrInnerInner<M, O>),
}

pub(crate) enum DataPayloadOrInnerInner<M: DataMarker, O> {
StaticRef(&'static M::Yokeable),
Other(O),
}

/// The type of the "cart" that is used by [`DataPayload`].
///
/// This type is public but the inner cart type is private. To create a
Expand Down Expand Up @@ -137,6 +214,19 @@ where
}
}

impl<M, O> Debug for DataPayloadOr<M, O>
where
M: DataMarker,
for<'a> &'a <M::Yokeable as Yokeable<'a>>::Output: Debug,
O: Debug,
{
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
self.get()
.map(|v| Debug::fmt(&v, f))
.unwrap_or_else(|v| Debug::fmt(v, f))
}
}

/// Cloning a DataPayload is generally a cheap operation.
/// See notes in the `Clone` impl for [`Yoke`].
///
Expand All @@ -162,6 +252,25 @@ where
}
}

impl<M, O> Clone for DataPayloadOr<M, O>
where
M: DataMarker,
for<'a> YokeTraitHack<<M::Yokeable as Yokeable<'a>>::Output>: Clone,
O: Clone,
{
fn clone(&self) -> Self {
Self(match &self.0 {
DataPayloadOrInner::Yoke(yoke) => DataPayloadOrInner::Yoke(yoke.clone()),
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::StaticRef(r)) => {
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::StaticRef(*r))
}
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::Other(o)) => {
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::Other(o.clone()))
}
})
}
}

impl<M> PartialEq for DataPayload<M>
where
M: DataMarker,
Expand All @@ -172,20 +281,64 @@ where
}
}

impl<M, O> PartialEq for DataPayloadOr<M, O>
where
M: DataMarker,
for<'a> YokeTraitHack<<M::Yokeable as Yokeable<'a>>::Output>: PartialEq,
O: Eq,
{
fn eq(&self, other: &Self) -> bool {
match (self.get(), other.get()) {
(Ok(x), Ok(y)) => YokeTraitHack(x).into_ref() == YokeTraitHack(y).into_ref(),
(Err(x), Err(y)) => x == y,
_ => false,
}
}
}

impl<M> Eq for DataPayload<M>
where
M: DataMarker,
for<'a> YokeTraitHack<<M::Yokeable as Yokeable<'a>>::Output>: Eq,
{
}

impl<M, O> Eq for DataPayloadOr<M, O>
where
M: DataMarker,
for<'a> YokeTraitHack<<M::Yokeable as Yokeable<'a>>::Output>: Eq,
O: Eq,
{
}

#[test]
fn test_clone_eq() {
use crate::hello_world::*;
let p1 = DataPayload::<HelloWorldV1Marker>::from_static_str("Demo");
#[allow(clippy::redundant_clone)]
let p2 = p1.clone();
assert_eq!(p1, p2);

let p1 = DataPayloadOr::<HelloWorldV1Marker, usize>::from_payload(p1);
#[allow(clippy::redundant_clone)]
let p2 = p1.clone();
assert_eq!(p1, p2);

let p3 = DataPayloadOr::<HelloWorldV1Marker, usize>::from_other(555);
#[allow(clippy::redundant_clone)]
let p4 = p3.clone();
assert_eq!(p3, p4);

let p5 = DataPayloadOr::<HelloWorldV1Marker, usize>::from_other(666);
assert_ne!(p3, p5);
assert_ne!(p4, p5);

assert_ne!(p1, p3);
assert_ne!(p1, p4);
assert_ne!(p1, p5);
assert_ne!(p2, p3);
assert_ne!(p2, p4);
assert_ne!(p2, p5);
}

impl<M> DataPayload<M>
Expand Down Expand Up @@ -678,6 +831,56 @@ where
}
}

impl<M, O> DataPayloadOr<M, O>
where
M: DataMarker,
{
/// Creates a [`DataPayloadOr`] from a [`DataPayload`].
#[inline]
pub fn from_payload(payload: DataPayload<M>) -> Self {
match payload.0 {
DataPayloadInner::Yoke(yoke) => Self(DataPayloadOrInner::Yoke(yoke)),
DataPayloadInner::StaticRef(r) => Self(DataPayloadOrInner::Inner(
DataPayloadOrInnerInner::StaticRef(r),
)),
}
}

/// Creates a [`DataPayloadOr`] from the other type `O`.
#[inline]
pub fn from_other(other: O) -> Self {
Self(DataPayloadOrInner::Inner(DataPayloadOrInnerInner::Other(
other,
)))
}

/// Gets the value from this [`DataPayload`] as `Ok` or the other type as `Err`.
#[allow(clippy::needless_lifetimes)]
#[inline]
pub fn get<'a>(&'a self) -> Result<&'a <M::Yokeable as Yokeable<'a>>::Output, &'a O> {
match &self.0 {
DataPayloadOrInner::Yoke(yoke) => Ok(yoke.get()),
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::StaticRef(r)) => {
Ok(Yokeable::transform(*r))
}
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::Other(o)) => Err(o),
}
}

/// Consumes this [`DataPayloadOr`], returning either the wrapped
/// [`DataPayload`] or the other type.
#[inline]
pub fn into_inner(self) -> Result<DataPayload<M>, O> {
match self.0 {
DataPayloadOrInner::Yoke(yoke) => Ok(DataPayload(DataPayloadInner::Yoke(yoke))),
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::StaticRef(r)) => {
Ok(DataPayload(DataPayloadInner::StaticRef(r)))
}
DataPayloadOrInner::Inner(DataPayloadOrInnerInner::Other(o)) => Err(o),
}
}
}

/// A response object containing an object as payload and metadata about it.
#[allow(clippy::exhaustive_structs)] // this type is stable
pub struct DataResponse<M>
Expand Down