Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-07 14:35:33 +01:00
Code Issues Projects Releases Packages Wiki Activity

style: Ensure that derived types are right for optimized-away implementations.

Browse source 
We have this optimization where, for non-generic structs, we generate just a
clone / move as the ToComputedValue / ToResolvedValue implementation.

This moves the optimization a bit further down, and refines it so that we still
generate all the relevant where clauses that make it sound, that is, that all
the ToComputedValue implementations of the fields return the same type.

Otherwise this wouldn't be sound and the type would need to become generic.

We add an escape hatch (no_field_bound) for fields that need to be cloned but
which don't implement the trait. This is right now only for the RefPtr<> in the
shared font-family list, and a piece of code in PaintWorklet which looks kinda
fishy, and probably should be fixed (but we don't ship it in Firefox and there's
a pre-existing FIXME for servo, so I punted on it for now).

The other thing this patch does is adding a bunch of ToComputedValue /
ToResolvedValue implementations that are trivial and were missing.

Differential Revision: https://phabricator.services.mozilla.com/D67913
This commit is contained in:
Emilio Cobos Álvarez 2020-03-26 13:04:20 +00:00
parent 0514059618
commit 7d438cd816
18 changed files with 295 additions and 146 deletions
Download patch file Download diff file Expand all files Collapse all files

16
components/style_derive/to_animated_value.rs
View file

@ -11,33 +11,25 @@ pub fn derive(input: DeriveInput) -> TokenStream {
let trait_impl = |from_body, to_body| {
quote! {
#[inline]
fn from_animated_value(animated: Self::AnimatedValue) -> Self {
match animated {
#from_body
}
fn from_animated_value(from: Self::AnimatedValue) -> Self {
#from_body
}
#[inline]
fn to_animated_value(self) -> Self::AnimatedValue {
match self {
#to_body
}
#to_body
}
}
};
// TODO(emilio): Consider optimizing away non-generic cases as well?
let non_generic_implementation = || None;
to_computed_value::derive_to_value(
input,
parse_quote!(crate::values::animated::ToAnimatedValue),
parse_quote!(AnimatedValue),
BindStyle::Move,
|_| false,
|_| Default::default(),
|binding| quote!(crate::values::animated::ToAnimatedValue::from_animated_value(#binding)),
|binding| quote!(crate::values::animated::ToAnimatedValue::to_animated_value(#binding)),
trait_impl,
non_generic_implementation,
)
}

161
components/style_derive/to_computed_value.rs
View file

@ -13,7 +13,7 @@ pub fn derive_to_value(
output_type_name: Ident,
bind_style: BindStyle,
// Returns whether to apply the field bound for a given item.
mut field_bound: impl FnMut(&BindingInfo) -> bool,
mut binding_attrs: impl FnMut(&BindingInfo) -> ToValueAttrs,
// Returns a token stream of the form: trait_path::from_foo(#binding)
mut call_from: impl FnMut(&BindingInfo) -> TokenStream,
mut call_to: impl FnMut(&BindingInfo) -> TokenStream,
@ -26,25 +26,9 @@ pub fn derive_to_value(
// #second_arg
// }
mut trait_impl: impl FnMut(TokenStream, TokenStream) -> TokenStream,
// if this is provided, the derive for non-generic types will be simplified
// to this token stream, which should be the body of the impl block.
non_generic_implementation: impl FnOnce() -> Option<TokenStream>,
) -> TokenStream {
let name = &input.ident;
if input.generics.type_params().next().is_none() {
if let Some(non_generic_implementation) = non_generic_implementation() {
let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
return quote! {
impl #impl_generics #trait_path for #name #ty_generics
#where_clause
{
#non_generic_implementation
}
};
}
}
let mut where_clause = input.generics.where_clause.take();
cg::propagate_clauses_to_output_type(
&mut where_clause,
@ -52,33 +36,96 @@ pub fn derive_to_value(
&trait_path,
&output_type_name,
);
let (to_body, from_body) = {
let params = input.generics.type_params().collect::<Vec<_>>();
for param in &params {
cg::add_predicate(&mut where_clause, parse_quote!(#param: #trait_path));
let moves = match bind_style {
BindStyle::Move | BindStyle::MoveMut => true,
BindStyle::Ref | BindStyle::RefMut => false,
};
let params = input.generics.type_params().collect::<Vec<_>>();
for param in &params {
cg::add_predicate(&mut where_clause, parse_quote!(#param: #trait_path));
}
let mut add_field_bound = |binding: &BindingInfo| {
let ty = &binding.ast().ty;
let output_type = cg::map_type_params(
ty,
&params,
&mut |ident| parse_quote!(<#ident as #trait_path>::#output_type_name),
);
cg::add_predicate(
&mut where_clause,
parse_quote!(
#ty: #trait_path<#output_type_name = #output_type>
),
);
};
let (to_body, from_body) = if params.is_empty() {
let mut s = synstructure::Structure::new(&input);
s.variants_mut().iter_mut().for_each(|v| {
v.bind_with(|_| bind_style);
});
for variant in s.variants() {
for binding in variant.bindings() {
let attrs = binding_attrs(&binding);
assert!(
!attrs.field_bound,
"It is default on a non-generic implementation",
);
if !attrs.no_field_bound {
// Add field bounds to all bindings except the manually
// excluded. This ensures the correctness of the clone() /
// move based implementation.
add_field_bound(binding);
}
}
}
let to_body = if moves {
quote! { self }
} else {
quote! { std::clone::Clone::clone(self) }
};
let from_body = if moves {
quote! { from }
} else {
quote! { std::clone::Clone::clone(from) }
};
(to_body, from_body)
} else {
let to_body = cg::fmap_match(&input, bind_style, |binding| {
if field_bound(&binding) {
let ty = &binding.ast().ty;
let output_type = cg::map_type_params(
ty,
&params,
&mut |ident| parse_quote!(<#ident as #trait_path>::#output_type_name),
);
cg::add_predicate(
&mut where_clause,
parse_quote!(
#ty: #trait_path<#output_type_name = #output_type>
),
);
let attrs = binding_attrs(&binding);
assert!(!attrs.no_field_bound, "It doesn't make sense on a generic implementation");
if attrs.field_bound {
add_field_bound(&binding);
}
call_to(&binding)
});
let from_body = cg::fmap_match(&input, bind_style, |binding| call_from(&binding));
let self_ = if moves { quote! { self } } else { quote! { *self } };
let from_ = if moves { quote! { from } } else { quote! { *from } };
let to_body = quote! {
match #self_ {
#to_body
}
};
let from_body = quote! {
match #from_ {
#from_body
}
};
(to_body, from_body)
};
@ -101,53 +148,45 @@ pub fn derive(input: DeriveInput) -> TokenStream {
let trait_impl = |from_body, to_body| {
quote! {
#[inline]
fn from_computed_value(computed: &Self::ComputedValue) -> Self {
match *computed {
#from_body
}
fn from_computed_value(from: &Self::ComputedValue) -> Self {
#from_body
}
#[allow(unused_variables)]
#[inline]
fn to_computed_value(&self, context: &crate::values::computed::Context) -> Self::ComputedValue {
match *self {
#to_body
}
#to_body
}
}
};
let non_generic_implementation = || {
Some(quote! {
type ComputedValue = Self;
#[inline]
fn to_computed_value(&self, _: &crate::values::computed::Context) -> Self::ComputedValue {
std::clone::Clone::clone(self)
}
#[inline]
fn from_computed_value(computed: &Self::ComputedValue) -> Self {
std::clone::Clone::clone(computed)
}
})
};
derive_to_value(
input,
parse_quote!(crate::values::computed::ToComputedValue),
parse_quote!(ComputedValue),
BindStyle::Ref,
|binding| cg::parse_field_attrs::<ComputedValueAttrs>(&binding.ast()).field_bound,
|binding| {
let attrs = cg::parse_field_attrs::<ComputedValueAttrs>(&binding.ast());
ToValueAttrs {
field_bound: attrs.field_bound,
no_field_bound: attrs.no_field_bound,
}
},
|binding| quote!(crate::values::computed::ToComputedValue::from_computed_value(#binding)),
|binding| quote!(crate::values::computed::ToComputedValue::to_computed_value(#binding, context)),
trait_impl,
non_generic_implementation,
)
}
#[derive(Default)]
pub struct ToValueAttrs {
pub field_bound: bool,
pub no_field_bound: bool,
}
#[darling(attributes(compute), default)]
#[derive(Default, FromField)]
struct ComputedValueAttrs {
field_bound: bool,
no_field_bound: bool,
}

39
components/style_derive/to_resolved_value.rs
View file

@ -12,10 +12,8 @@ pub fn derive(input: DeriveInput) -> TokenStream {
let trait_impl = |from_body, to_body| {
quote! {
#[inline]
fn from_resolved_value(resolved: Self::ResolvedValue) -> Self {
match resolved {
#from_body
}
fn from_resolved_value(from: Self::ResolvedValue) -> Self {
#from_body
}
#[inline]
@ -23,42 +21,26 @@ pub fn derive(input: DeriveInput) -> TokenStream {
self,
context: &crate::values::resolved::Context,
) -> Self::ResolvedValue {
match self {
#to_body
}
#to_body
}
}
};
let non_generic_implementation = || {
Some(quote! {
type ResolvedValue = Self;
#[inline]
fn from_resolved_value(resolved: Self::ResolvedValue) -> Self {
resolved
}
#[inline]
fn to_resolved_value(
self,
context: &crate::values::resolved::Context,
) -> Self {
self
}
})
};
to_computed_value::derive_to_value(
input,
parse_quote!(crate::values::resolved::ToResolvedValue),
parse_quote!(ResolvedValue),
BindStyle::Move,
|binding| cg::parse_field_attrs::<ResolvedValueAttrs>(&binding.ast()).field_bound,
|binding| {
let attrs = cg::parse_field_attrs::<ResolvedValueAttrs>(&binding.ast());
to_computed_value::ToValueAttrs {
field_bound: attrs.field_bound,
no_field_bound: attrs.no_field_bound,
}
},
|binding| quote!(crate::values::resolved::ToResolvedValue::from_resolved_value(#binding)),
|binding| quote!(crate::values::resolved::ToResolvedValue::to_resolved_value(#binding, context)),
trait_impl,
non_generic_implementation,
)
}
@ -66,4 +48,5 @@ pub fn derive(input: DeriveInput) -> TokenStream {
#[derive(Default, FromField)]
struct ResolvedValueAttrs {
field_bound: bool,
no_field_bound: bool,
}