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

Remove Yokeable::Output from ZeroCopyFrom trait #1499

Merged
merged 9 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
4 changes: 2 additions & 2 deletions experimental/segmenter/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ impl Deref for LineBreakPropertyTable<'_> {
}
}

impl<'a> ZeroCopyFrom<LineBreakPropertyTable<'a>> for LineBreakPropertyTable<'static> {
fn zero_copy_from<'b>(cart: &'b LineBreakPropertyTable<'a>) -> <Self as Yokeable<'b>>::Output {
impl<'zcf> ZeroCopyFrom<'zcf, LineBreakPropertyTable<'_>> for LineBreakPropertyTable<'zcf> {
Copy link
Member

Choose a reason for hiding this comment

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

praise: good call on 'zcf

fn zero_copy_from(cart: &'zcf LineBreakPropertyTable<'_>) -> Self {
LineBreakPropertyTable::Borrowed(&*cart)
}
}
Expand Down
5 changes: 2 additions & 3 deletions provider/core/src/erased.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ pub trait ErasedDataStruct: 'static {
fn as_any(&self) -> &dyn Any;
}

impl ZeroCopyFrom<dyn ErasedDataStruct> for &'static dyn ErasedDataStruct {
#[allow(clippy::needless_lifetimes)]
fn zero_copy_from<'b>(this: &'b (dyn ErasedDataStruct)) -> &'b dyn ErasedDataStruct {
impl<'zcf> ZeroCopyFrom<'zcf, dyn ErasedDataStruct> for &'zcf dyn ErasedDataStruct {
fn zero_copy_from(this: &'zcf (dyn ErasedDataStruct)) -> &'zcf dyn ErasedDataStruct {
this
}
}
Expand Down
6 changes: 3 additions & 3 deletions utils/yoke/derive/examples/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ pub struct HasTuples<'data> {
}

pub fn assert_zcf_tuples<'b, 'data>(x: &'b HasTuples<'data>) -> HasTuples<'b> {
HasTuples::<'static>::zero_copy_from(x)
HasTuples::<'b>::zero_copy_from(x)
Copy link
Member

Choose a reason for hiding this comment

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

question: is this explicit lt still necessary? one of the benefits of the simplification is that a bunch of these won't be needed anymore

}
pub fn assert_zcf_generics<'a, 'b>(
x: &'b ZeroVecExampleWithGenerics<'a, u8>,
) -> ZeroVecExampleWithGenerics<'b, u8> {
ZeroVecExampleWithGenerics::<'static, u8>::zero_copy_from(x)
ZeroVecExampleWithGenerics::<'b, u8>::zero_copy_from(x)
}

// Since ZeroMap has generic parameters, the Rust compiler cannot
Expand All @@ -78,7 +78,7 @@ pub struct ZeroMapGenericExample<'a, T: for<'b> ZeroMapKV<'b> + ?Sized> {
pub fn assert_zcf_map<'a, 'b>(
x: &'b ZeroMapGenericExample<'a, str>,
) -> ZeroMapGenericExample<'b, str> {
ZeroMapGenericExample::<'static, str>::zero_copy_from(x)
ZeroMapGenericExample::<'b, str>::zero_copy_from(x)
}

#[derive(Yokeable, Clone, ZeroCopyFrom)]
Expand Down
36 changes: 14 additions & 22 deletions utils/yoke/derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,8 @@ fn zcf_derive_impl(input: &DeriveInput) -> TokenStream2 {
.map(|ty| parse_quote!(#ty: #clone_trait + 'static))
.collect();
quote! {
impl<#(#tybounds),*> ZeroCopyFrom<#name<#(#typarams),*>> for #name<#(#typarams),*> where #(#bounds),* {
fn zero_copy_from(this: &Self) -> Self {
impl<'zcf, #(#tybounds),*> ZeroCopyFrom<'zcf, #name<#(#typarams),*>> for #name<#(#typarams),*> where #(#bounds),* {
fn zero_copy_from(this: &'zcf Self) -> Self {
#clone
}
}
Expand All @@ -269,8 +269,8 @@ fn zcf_derive_impl(input: &DeriveInput) -> TokenStream2 {
}
if has_clone {
return quote! {
impl<'data> ZeroCopyFrom<#name<'data>> for #name<'static> {
fn zero_copy_from<'b>(this: &'b #name<'data>) -> #name<'b> {
impl<'zcf> ZeroCopyFrom<'zcf, #name<'_>> for #name<'zcf> {
fn zero_copy_from(this: &'zcf #name<'_>) -> Self {
this.clone()
}
}
Expand All @@ -279,10 +279,6 @@ fn zcf_derive_impl(input: &DeriveInput) -> TokenStream2 {

let structure = Structure::new(input);
let generics_env = typarams.iter().cloned().collect();
let static_bounds: Vec<WherePredicate> = typarams
.iter()
.map(|ty| parse_quote!(#ty: 'static))
.collect();

let mut zcf_bounds: Vec<WherePredicate> = vec![];
let body = structure.each_variant(|vi| {
Expand All @@ -292,46 +288,42 @@ fn zcf_derive_impl(input: &DeriveInput) -> TokenStream2 {
let binding = format!("__binding_{}", i);
let field = Ident::new(&binding, Span::call_site());


if binding_cloning {
quote! {
#field.clone()
}
} else {
let fty = replace_lifetime(&f.ty, static_lt());
let lifetime_ty = replace_lifetime(&f.ty, custom_lt("'data"));
let fty = replace_lifetime(&f.ty, custom_lt("'zcf"));
let lifetime_ty = replace_lifetime(&f.ty, custom_lt("'zcf_inner"));

let (has_ty, has_lt) = visitor::check_type_for_parameters(&f.ty, &generics_env);
if has_ty {
// For types without type parameters, the compiler can figure out that the field implements
// ZeroCopyFrom on its own. However, if there are type parameters, there may be complex preconditions
// to `FieldTy: ZeroCopyFrom` that need to be satisfied. We get them to be satisfied by requiring
// `FieldTy<'static>: ZeroCopyFrom<FieldTy<'data>>` as well as
// `for<'data_hrtb> FieldTy<'static>: Yokeable<'data_hrtb, Output = FieldTy<'data_hrtb>>`
// `FieldTy<'zcf>: ZeroCopyFrom<'zcf, FieldTy<'zcf_inner>>`
if has_lt {
let hrtb_ty = replace_lifetime(&f.ty, custom_lt("'data_hrtb"));
zcf_bounds.push(parse_quote!(#fty: yoke::ZeroCopyFrom<#lifetime_ty>));
zcf_bounds.push(parse_quote!(for<'data_hrtb> #fty: yoke::Yokeable<'data_hrtb, Output = #hrtb_ty>));
zcf_bounds
.push(parse_quote!(#fty: yoke::ZeroCopyFrom<'zcf, #lifetime_ty>));
} else {
zcf_bounds.push(parse_quote!(#fty: yoke::ZeroCopyFrom<#fty>));
zcf_bounds.push(parse_quote!(for<'data_hrtb> #fty: yoke::Yokeable<'data_hrtb, Output = #fty>));
zcf_bounds.push(parse_quote!(#fty: yoke::ZeroCopyFrom<'zcf, #fty>));
}
}

// By doing this we essentially require ZCF to be implemented
// on all fields
quote! {
<#fty as yoke::ZeroCopyFrom<#lifetime_ty>>::zero_copy_from(#field)
<#fty as yoke::ZeroCopyFrom<'zcf, #lifetime_ty>>::zero_copy_from(#field)
}
}
})
});

quote! {
impl<'data, #(#tybounds),*> yoke::ZeroCopyFrom<#name<'data, #(#typarams),*>> for #name<'static, #(#typarams),*>
where #(#static_bounds,)*
impl<'zcf, 'zcf_inner, #(#tybounds),*> yoke::ZeroCopyFrom<'zcf, #name<'zcf_inner, #(#typarams),*>> for #name<'zcf, #(#typarams),*>
where
#(#zcf_bounds,)* {
fn zero_copy_from<'b>(this: &'b #name<'data, #(#typarams),*>) -> #name<'b, #(#typarams),*> {
fn zero_copy_from(this: &'zcf #name<'zcf_inner, #(#typarams),*>) -> Self {
match *this { #body }
}
}
Expand Down
4 changes: 2 additions & 2 deletions utils/yoke/src/is_covariant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ use alloc::{
/// }
/// }
///
/// impl<'a> ZeroCopyFrom<dyn ExampleTrait<'a> + 'a> for ExampleTraitDynRef<'static> {
/// fn zero_copy_from<'b>(this: &'b (dyn ExampleTrait<'a> + 'a)) -> ExampleTraitDynRef<'b> {
/// impl<'zcf, 'a> ZeroCopyFrom<'zcf, dyn ExampleTrait<'a> + 'a> for ExampleTraitDynRef<'zcf> {
/// fn zero_copy_from(this: &'zcf (dyn ExampleTrait<'a> + 'a)) -> ExampleTraitDynRef<'zcf> {
/// // This is safe because the trait object requires IsCovariant.
/// ExampleTraitDynRef(unsafe { core::mem::transmute(this) })
/// }
Expand Down
8 changes: 4 additions & 4 deletions utils/yoke/src/macro_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ macro_rules! impl_copy_type {
type Output = Self;
copy_yoke_impl!();
}
impl ZeroCopyFrom<$ty> for $ty {
impl<'a> ZeroCopyFrom<'a, $ty> for $ty {
#[inline]
fn zero_copy_from(this: &Self) -> Self {
fn zero_copy_from(this: &'a Self) -> Self {
// Essentially only works when the struct is fully Copy
*this
}
Expand Down Expand Up @@ -117,8 +117,8 @@ unsafe impl<'a, T: Yokeable<'a>, const N: usize> Yokeable<'a> for [T; N] {
// https://github.com/rust-lang/rust/issues/76118
macro_rules! array_zcf_impl {
($n:expr; $($i:expr),+) => {
impl<C, T: ZeroCopyFrom<C>> ZeroCopyFrom<[C; $n]> for [T; $n] {
fn zero_copy_from<'b>(this: &'b [C; $n]) -> [<T as Yokeable<'b>>::Output; $n] {
impl<'a, C, T: ZeroCopyFrom<'a, C>> ZeroCopyFrom<'a, [C; $n]> for [T; $n] {
fn zero_copy_from(this: &'a [C; $n]) -> Self {
[
$(
<T as ZeroCopyFrom<C>>::zero_copy_from(&this[$i])
Expand Down
75 changes: 44 additions & 31 deletions utils/yoke/src/zero_copy_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// called LICENSE at the top level of the ICU4X source tree
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

use crate::trait_hack::YokeTraitHack;
use crate::Yoke;
use crate::Yokeable;
#[cfg(feature = "alloc")]
Expand Down Expand Up @@ -61,31 +62,42 @@ use stable_deref_trait::StableDeref;
/// }
///
/// // Reference from a borrowed version of self
/// impl<'data> ZeroCopyFrom<MyStruct<'data>> for MyStruct<'static> {
/// fn zero_copy_from<'b>(cart: &'b MyStruct<'data>) -> MyStruct<'b> {
/// impl<'zcf> ZeroCopyFrom<'zcf, MyStruct<'_>> for MyStruct<'zcf> {
Copy link
Member

Choose a reason for hiding this comment

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

nit: the yokeable impl is no longer necessary in the docs above

/// fn zero_copy_from(cart: &'zcf MyStruct<'_>) -> Self {
/// MyStruct {
/// message: Cow::Borrowed(&cart.message)
/// }
/// }
/// }
///
/// // Reference from a string slice directly
/// impl ZeroCopyFrom<str> for MyStruct<'static> {
/// fn zero_copy_from<'b>(cart: &'b str) -> MyStruct<'b> {
/// impl<'zcf> ZeroCopyFrom<'zcf, str> for MyStruct<'zcf> {
/// fn zero_copy_from(cart: &'zcf str) -> Self {
/// MyStruct {
/// message: Cow::Borrowed(cart)
/// }
/// }
/// }
/// ```
pub trait ZeroCopyFrom<C: ?Sized>: for<'a> Yokeable<'a> {
/// Clone the cart `C` into a [`Yokeable`] struct, which may retain references into `C`.
fn zero_copy_from<'b>(cart: &'b C) -> <Self as Yokeable<'b>>::Output;
pub trait ZeroCopyFrom<'zcf, C: ?Sized>: 'zcf {
/// Clone the cart `C` into a struct that may retain references into `C`.
fn zero_copy_from(cart: &'zcf C) -> Self;
}

impl<'b, 's, Y, C> Yoke<Y, C>
impl<'zcf, C: ?Sized, T> ZeroCopyFrom<'zcf, C> for YokeTraitHack<T>
where
Y: for<'a> Yokeable<'a> + ZeroCopyFrom<<C as Deref>::Target>,
T: ZeroCopyFrom<'zcf, C>,
{
#[inline]
fn zero_copy_from(cart: &'zcf C) -> Self {
YokeTraitHack(T::zero_copy_from(cart))
}
}

impl<Y, C> Yoke<Y, C>
where
Y: for<'a> Yokeable<'a>,
for<'a> YokeTraitHack<<Y as Yokeable<'a>>::Output>: ZeroCopyFrom<'a, <C as Deref>::Target>,
C: StableDeref + Deref,
{
/// Construct a [`Yoke`]`<Y, C>` from a cart implementing `StableDeref` by zero-copy cloning
Expand All @@ -110,9 +122,10 @@ where
///
/// assert_eq!("demo", yoke.get());
/// ```
#[inline]
pub fn attach_to_zero_copy_cart(cart: C) -> Self {
Yoke::<Y, C>::attach_to_cart_badly(cart, Y::zero_copy_from)
Yoke::<Y, C>::attach_to_cart(cart, |c| {
YokeTraitHack::<<Y as Yokeable>::Output>::zero_copy_from(c).0
})
}
}

Expand All @@ -122,47 +135,47 @@ where
// specialization.

#[cfg(feature = "alloc")]
impl ZeroCopyFrom<str> for Cow<'static, str> {
impl<'zcf> ZeroCopyFrom<'zcf, str> for Cow<'zcf, str> {
#[inline]
fn zero_copy_from<'b>(cart: &'b str) -> Cow<'b, str> {
fn zero_copy_from(cart: &'zcf str) -> Self {
Cow::Borrowed(cart)
}
}

#[cfg(feature = "alloc")]
impl ZeroCopyFrom<String> for Cow<'static, str> {
impl<'zcf> ZeroCopyFrom<'zcf, String> for Cow<'zcf, str> {
#[inline]
fn zero_copy_from<'b>(cart: &'b String) -> Cow<'b, str> {
fn zero_copy_from(cart: &'zcf String) -> Self {
Cow::Borrowed(cart)
}
}

impl ZeroCopyFrom<str> for &'static str {
impl<'zcf> ZeroCopyFrom<'zcf, str> for &'zcf str {
#[inline]
fn zero_copy_from<'b>(cart: &'b str) -> &'b str {
fn zero_copy_from(cart: &'zcf str) -> Self {
cart
}
}

#[cfg(feature = "alloc")]
impl ZeroCopyFrom<String> for &'static str {
impl<'zcf> ZeroCopyFrom<'zcf, String> for &'zcf str {
#[inline]
fn zero_copy_from<'b>(cart: &'b String) -> &'b str {
fn zero_copy_from(cart: &'zcf String) -> Self {
cart
}
}

impl<C, T: ZeroCopyFrom<C>> ZeroCopyFrom<Option<C>> for Option<T> {
fn zero_copy_from<'b>(cart: &'b Option<C>) -> Option<<T as Yokeable<'b>>::Output> {
impl<'zcf, C, T: ZeroCopyFrom<'zcf, C>> ZeroCopyFrom<'zcf, Option<C>> for Option<T> {
fn zero_copy_from(cart: &'zcf Option<C>) -> Self {
cart.as_ref()
.map(|c| <T as ZeroCopyFrom<C>>::zero_copy_from(c))
}
}

impl<C1, T1: ZeroCopyFrom<C1>, C2, T2: ZeroCopyFrom<C2>> ZeroCopyFrom<(C1, C2)> for (T1, T2) {
fn zero_copy_from<'b>(
cart: &'b (C1, C2),
) -> (<T1 as Yokeable<'b>>::Output, <T2 as Yokeable<'b>>::Output) {
impl<'zcf, C1, T1: ZeroCopyFrom<'zcf, C1>, C2, T2: ZeroCopyFrom<'zcf, C2>>
ZeroCopyFrom<'zcf, (C1, C2)> for (T1, T2)
{
fn zero_copy_from(cart: &'zcf (C1, C2)) -> Self {
(
<T1 as ZeroCopyFrom<C1>>::zero_copy_from(&cart.0),
<T2 as ZeroCopyFrom<C2>>::zero_copy_from(&cart.1),
Expand All @@ -177,23 +190,23 @@ impl<C1, T1: ZeroCopyFrom<C1>, C2, T2: ZeroCopyFrom<C2>> ZeroCopyFrom<(C1, C2)>
// or inference are involved, and the proc macro does not necessarily have
// enough type information to figure this out on its own.
#[cfg(feature = "alloc")]
impl<B: ToOwned + ?Sized + 'static> ZeroCopyFrom<Cow<'_, B>> for Cow<'static, B> {
impl<'zcf, B: ToOwned + ?Sized> ZeroCopyFrom<'zcf, Cow<'_, B>> for Cow<'zcf, B> {
#[inline]
fn zero_copy_from<'b>(cart: &'b Cow<'_, B>) -> Cow<'b, B> {
fn zero_copy_from(cart: &'zcf Cow<'_, B>) -> Self {
Cow::Borrowed(cart)
}
}

impl ZeroCopyFrom<&'_ str> for &'static str {
impl<'zcf> ZeroCopyFrom<'zcf, &'_ str> for &'zcf str {
#[inline]
fn zero_copy_from<'b>(cart: &'b &'_ str) -> &'b str {
fn zero_copy_from(cart: &'zcf &'_ str) -> &'zcf str {
cart
}
}

impl<T: 'static> ZeroCopyFrom<[T]> for &'static [T] {
impl<'zcf, T> ZeroCopyFrom<'zcf, [T]> for &'zcf [T] {
#[inline]
fn zero_copy_from<'b>(cart: &'b [T]) -> &'b [T] {
fn zero_copy_from(cart: &'zcf [T]) -> &'zcf [T] {
cart
}
}
Loading