From 92068ca5409413ce8ecbca984b6b1d9a119423d1 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 9 Mar 2018 15:11:17 +0100 Subject: [PATCH 1/4] Simplify the derived bounds for ToAnimatedValue --- components/style_derive/cg.rs | 13 ++++++++-- components/style_derive/to_animated_value.rs | 25 ++++++++++++++------ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/components/style_derive/cg.rs b/components/style_derive/cg.rs index 364fc63af13..5289eaf3f6b 100644 --- a/components/style_derive/cg.rs +++ b/components/style_derive/cg.rs @@ -126,7 +126,16 @@ pub fn fmap_trait_parts<'input, 'path>( ) -> (ImplGenerics<'input>, TypeGenerics<'input>, WhereClause<'input, 'path>, Path) { let (impl_generics, ty_generics, mut where_clause) = trait_parts(input, trait_path); where_clause.trait_output = Some(trait_output); - let output_ty = PathSegment { + let output_ty = fmap_trait_output(input, trait_path, trait_output); + (impl_generics, ty_generics, where_clause, output_ty) +} + +pub fn fmap_trait_output( + input: &DeriveInput, + trait_path: &Path, + trait_output: Ident, +) -> Path { + let segment = PathSegment { ident: input.ident.clone(), arguments: PathArguments::AngleBracketed(AngleBracketedGenericArguments { args: input.generics.params.iter().map(|arg| { @@ -151,7 +160,7 @@ pub fn fmap_trait_parts<'input, 'path>( }) }; - (impl_generics, ty_generics, where_clause, output_ty.into()) + segment.into() } pub fn is_parameterized( diff --git a/components/style_derive/to_animated_value.rs b/components/style_derive/to_animated_value.rs index 5d9cdbc0138..c2e8e4ef984 100644 --- a/components/style_derive/to_animated_value.rs +++ b/components/style_derive/to_animated_value.rs @@ -4,23 +4,34 @@ use cg; use quote; -use syn::{self, Ident}; +use syn::DeriveInput; use synstructure::BindStyle; -pub fn derive(input: syn::DeriveInput) -> quote::Tokens { - let name = &input.ident; - let trait_path = parse_quote!(values::animated::ToAnimatedValue); - let (impl_generics, ty_generics, mut where_clause, animated_value_type) = - cg::fmap_trait_parts(&input, &trait_path, Ident::from("AnimatedValue")); +pub fn derive(mut input: DeriveInput) -> quote::Tokens { + let mut where_clause = input.generics.where_clause.take(); + for param in input.generics.type_params() { + cg::add_predicate( + &mut where_clause, + parse_quote!(#param: ::values::animated::ToAnimatedValue), + ); + } + input.generics.where_clause = where_clause; let to_body = cg::fmap_match(&input, BindStyle::Move, |binding| { - where_clause.add_trait_bound(&binding.ast().ty); quote!(::values::animated::ToAnimatedValue::to_animated_value(#binding)) }); let from_body = cg::fmap_match(&input, BindStyle::Move, |binding| { quote!(::values::animated::ToAnimatedValue::from_animated_value(#binding)) }); + let name = &input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + let animated_value_type = cg::fmap_trait_output( + &input, + &parse_quote!(values::animated::ToAnimatedValue), + "AnimatedValue".into(), + ); + quote! { impl #impl_generics ::values::animated::ToAnimatedValue for #name #ty_generics #where_clause { type AnimatedValue = #animated_value_type; From 2efd06c12dc30dfa35ebbc13934fef78445601a5 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 9 Mar 2018 15:45:34 +0100 Subject: [PATCH 2/4] Remove #[compute(clone)] --- components/style/values/computed/mod.rs | 3 +-- .../style/values/computed/percentage.rs | 2 +- components/style/values/generics/grid.rs | 1 - components/style/values/generics/image.rs | 2 -- components/style/values/generics/transform.rs | 1 - components/style_derive/to_computed_value.rs | 25 ++++--------------- 6 files changed, 7 insertions(+), 27 deletions(-) diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index ff16e328826..ab962c055c1 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -293,8 +293,7 @@ impl<'a, 'cx, 'cx_a: 'cx, S: ToComputedValue + 'a> Iterator for ComputedVecIter< /// /// This trait is derivable with `#[derive(ToComputedValue)]`. The derived /// implementation just calls `ToComputedValue::to_computed_value` on each field -/// of the passed value, or `Clone::clone` if the field is annotated with -/// `#[compute(clone)]`. The deriving code assumes that if the type isn't +/// of the passed value. The deriving code assumes that if the type isn't /// generic, then the trait can be implemented as simple `Clone::clone` calls, /// this means that a manual implementation with `ComputedValue = Self` is bogus /// if it returns anything else than a clone. diff --git a/components/style/values/computed/percentage.rs b/components/style/values/computed/percentage.rs index 842019f03c8..4992151a1b8 100644 --- a/components/style/values/computed/percentage.rs +++ b/components/style/values/computed/percentage.rs @@ -11,7 +11,7 @@ use values::{CSSFloat, serialize_percentage}; /// A computed percentage. #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] #[derive(Animate, Clone, ComputeSquaredDistance, Copy, Debug, Default, MallocSizeOf)] -#[derive(PartialEq, PartialOrd, ToAnimatedZero)] +#[derive(PartialEq, PartialOrd, ToAnimatedZero, ToComputedValue)] pub struct Percentage(pub CSSFloat); impl Percentage { diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index cc3256df7ab..3d8005fc62d 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -388,7 +388,6 @@ pub struct TrackRepeat { /// If there's no ``, then it's represented by an empty vector. /// For N `` values, there will be N+1 ``, and so this vector's /// length is always one value more than that of the ``. - #[compute(clone)] pub line_names: Box<[Box<[CustomIdent]>]>, /// `` values. pub track_sizes: Vec>, diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index 7f02fdec4f4..ca041d5ed9d 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -42,10 +42,8 @@ pub struct Gradient>, /// True if this is a repeating gradient. - #[compute(clone)] pub repeating: bool, /// Compatibility mode. - #[compute(clone)] pub compat_mode: CompatMode, } diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index 1d96c9e9d84..be863d1e005 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -267,7 +267,6 @@ pub enum TransformOperation LengthOrPercentage, >, >, - #[compute(clone)] progress: computed::Percentage, }, /// A intermediate type for accumulation of mismatched transform lists. diff --git a/components/style_derive/to_computed_value.rs b/components/style_derive/to_computed_value.rs index 1a64faa480d..8ef05f8ecff 100644 --- a/components/style_derive/to_computed_value.rs +++ b/components/style_derive/to_computed_value.rs @@ -38,25 +38,11 @@ pub fn derive(input: DeriveInput) -> Tokens { let to_body = cg::fmap_match(&input, BindStyle::Ref, |binding| { let attrs = cg::parse_field_attrs::(&binding.ast()); - if attrs.clone { - if cg::is_parameterized(&binding.ast().ty, &where_clause.params, None) { - cg::add_predicate( - &mut where_clause.inner, - cg::where_predicate( - binding.ast().ty.clone(), - &parse_quote!(std::clone::Clone), - None, - ), - ); - } - quote! { ::std::clone::Clone::clone(#binding) } - } else { - if !attrs.ignore_bound { - where_clause.add_trait_bound(&binding.ast().ty); - } - quote! { - ::values::computed::ToComputedValue::to_computed_value(#binding, context) - } + if !attrs.ignore_bound { + where_clause.add_trait_bound(&binding.ast().ty); + } + quote! { + ::values::computed::ToComputedValue::to_computed_value(#binding, context) } }); let from_body = cg::fmap_match(&input, BindStyle::Ref, |binding| { @@ -95,6 +81,5 @@ pub fn derive(input: DeriveInput) -> Tokens { #[darling(attributes(compute), default)] #[derive(Default, FromField)] struct ComputedValueAttrs { - clone: bool, ignore_bound: bool, } From 4d0bf2ddf9f4c9515e89b6457837097071f676b5 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 9 Mar 2018 17:56:16 +0100 Subject: [PATCH 3/4] Opt into field bounds for #[derive(ToComputedValue)] --- components/style/values/generics/grid.rs | 2 +- components/style/values/generics/transform.rs | 4 - components/style_derive/cg.rs | 13 +-- components/style_derive/to_animated_value.rs | 2 +- components/style_derive/to_computed_value.rs | 80 ++++++++++++------- 5 files changed, 55 insertions(+), 46 deletions(-) diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 3d8005fc62d..cafc4672a81 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -666,7 +666,7 @@ pub enum GridTemplateComponent { /// `none` value. None, /// The grid `` - TrackList(TrackList), + TrackList(#[compute(field_bound)] TrackList), /// A `subgrid ?` Subgrid(LineNameList), } diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index be863d1e005..b80bc8b14f4 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -247,7 +247,6 @@ pub enum TransformOperation #[allow(missing_docs)] #[css(comma, function = "interpolatematrix")] InterpolateMatrix { - #[compute(ignore_bound)] from_list: Transform< TransformOperation< Angle, @@ -257,7 +256,6 @@ pub enum TransformOperation LengthOrPercentage, >, >, - #[compute(ignore_bound)] to_list: Transform< TransformOperation< Angle, @@ -273,7 +271,6 @@ pub enum TransformOperation #[allow(missing_docs)] #[css(comma, function = "accumulatematrix")] AccumulateMatrix { - #[compute(ignore_bound)] from_list: Transform< TransformOperation< Angle, @@ -283,7 +280,6 @@ pub enum TransformOperation LengthOrPercentage, >, >, - #[compute(ignore_bound)] to_list: Transform< TransformOperation< Angle, diff --git a/components/style_derive/cg.rs b/components/style_derive/cg.rs index 5289eaf3f6b..ffe30b89d0b 100644 --- a/components/style_derive/cg.rs +++ b/components/style_derive/cg.rs @@ -111,7 +111,7 @@ where }) } -fn fmap_output_type( +pub fn fmap_output_type( ty: Type, trait_path: &Path, trait_output: Ident, @@ -119,17 +119,6 @@ fn fmap_output_type( parse_quote!(<#ty as ::#trait_path>::#trait_output) } -pub fn fmap_trait_parts<'input, 'path>( - input: &'input DeriveInput, - trait_path: &'path Path, - trait_output: Ident, -) -> (ImplGenerics<'input>, TypeGenerics<'input>, WhereClause<'input, 'path>, Path) { - let (impl_generics, ty_generics, mut where_clause) = trait_parts(input, trait_path); - where_clause.trait_output = Some(trait_output); - let output_ty = fmap_trait_output(input, trait_path, trait_output); - (impl_generics, ty_generics, where_clause, output_ty) -} - pub fn fmap_trait_output( input: &DeriveInput, trait_path: &Path, diff --git a/components/style_derive/to_animated_value.rs b/components/style_derive/to_animated_value.rs index c2e8e4ef984..a7378b70145 100644 --- a/components/style_derive/to_animated_value.rs +++ b/components/style_derive/to_animated_value.rs @@ -15,7 +15,6 @@ pub fn derive(mut input: DeriveInput) -> quote::Tokens { parse_quote!(#param: ::values::animated::ToAnimatedValue), ); } - input.generics.where_clause = where_clause; let to_body = cg::fmap_match(&input, BindStyle::Move, |binding| { quote!(::values::animated::ToAnimatedValue::to_animated_value(#binding)) @@ -24,6 +23,7 @@ pub fn derive(mut input: DeriveInput) -> quote::Tokens { quote!(::values::animated::ToAnimatedValue::from_animated_value(#binding)) }); + input.generics.where_clause = where_clause; let name = &input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let animated_value_type = cg::fmap_trait_output( diff --git a/components/style_derive/to_computed_value.rs b/components/style_derive/to_computed_value.rs index 8ef05f8ecff..9b7544ffbb8 100644 --- a/components/style_derive/to_computed_value.rs +++ b/components/style_derive/to_computed_value.rs @@ -4,21 +4,59 @@ use cg; use quote::Tokens; -use syn::{Ident, DeriveInput}; +use syn::DeriveInput; use synstructure::BindStyle; -pub fn derive(input: DeriveInput) -> Tokens { - let name = &input.ident; - let trait_path = parse_quote!(values::computed::ToComputedValue); - let (impl_generics, ty_generics, mut where_clause, computed_value_type) = - cg::fmap_trait_parts(&input, &trait_path, Ident::from("ComputedValue")); +pub fn derive(mut input: DeriveInput) -> Tokens { + let mut where_clause = input.generics.where_clause.take(); + let (to_body, from_body) = { + let params = input.generics.type_params().collect::>(); + for param in ¶ms { + cg::add_predicate( + &mut where_clause, + parse_quote!(#param: ::values::computed::ToComputedValue), + ); + } - if input.generics.params.is_empty() { + let to_body = cg::fmap_match(&input, BindStyle::Ref, |binding| { + let attrs = cg::parse_field_attrs::(&binding.ast()); + if attrs.field_bound { + let ty = &binding.ast().ty; + + let output_type = cg::map_type_params(ty, ¶ms, &mut |ident| { + parse_quote!(<#ident as ::values::computed::ToComputedValue>::ComputedValue) + }); + + cg::add_predicate( + &mut where_clause, + parse_quote!( + #ty: ::values::computed::ToComputedValue + ), + ); + } + quote! { + ::values::computed::ToComputedValue::to_computed_value(#binding, context) + } + }); + let from_body = cg::fmap_match(&input, BindStyle::Ref, |binding| { + quote! { + ::values::computed::ToComputedValue::from_computed_value(#binding) + } + }); + + (to_body, from_body) + }; + + input.generics.where_clause = where_clause; + let name = &input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + + if input.generics.type_params().next().is_none() { return quote! { impl #impl_generics ::values::computed::ToComputedValue for #name #ty_generics #where_clause { - type ComputedValue = #computed_value_type; + type ComputedValue = Self; #[inline] fn to_computed_value( @@ -36,25 +74,11 @@ pub fn derive(input: DeriveInput) -> Tokens { } } - let to_body = cg::fmap_match(&input, BindStyle::Ref, |binding| { - let attrs = cg::parse_field_attrs::(&binding.ast()); - if !attrs.ignore_bound { - where_clause.add_trait_bound(&binding.ast().ty); - } - quote! { - ::values::computed::ToComputedValue::to_computed_value(#binding, context) - } - }); - let from_body = cg::fmap_match(&input, BindStyle::Ref, |binding| { - let attrs = cg::parse_field_attrs::(&binding.ast()); - if attrs.clone { - quote! { ::std::clone::Clone::clone(#binding) } - } else { - quote! { - ::values::computed::ToComputedValue::from_computed_value(#binding) - } - } - }); + let computed_value_type = cg::fmap_trait_output( + &input, + &parse_quote!(values::computed::ToComputedValue), + "ComputedValue".into(), + ); quote! { impl #impl_generics ::values::computed::ToComputedValue for #name #ty_generics #where_clause { @@ -81,5 +105,5 @@ pub fn derive(input: DeriveInput) -> Tokens { #[darling(attributes(compute), default)] #[derive(Default, FromField)] struct ComputedValueAttrs { - ignore_bound: bool, + field_bound: bool, } From 68be1aae8cee2715abe89fd87d1755e1b3f90a52 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 9 Mar 2018 18:35:02 +0100 Subject: [PATCH 4/4] Kill fmap_output_type --- components/style_derive/cg.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/components/style_derive/cg.rs b/components/style_derive/cg.rs index ffe30b89d0b..694973103ce 100644 --- a/components/style_derive/cg.rs +++ b/components/style_derive/cg.rs @@ -54,8 +54,7 @@ impl<'input, 'path> WhereClause<'input, 'path> { } let output_type = map_type_params(ty, &self.params, &mut |ident| { - let ty = Type::Path(syn::TypePath { qself: None, path: ident.clone().into() }); - fmap_output_type(ty, trait_path, output) + parse_quote!(<#ident as ::#trait_path>::#output) }); let pred = where_predicate( @@ -111,14 +110,6 @@ where }) } -pub fn fmap_output_type( - ty: Type, - trait_path: &Path, - trait_output: Ident, -) -> Type { - parse_quote!(<#ty as ::#trait_path>::#trait_output) -} - pub fn fmap_trait_output( input: &DeriveInput, trait_path: &Path, @@ -133,11 +124,7 @@ pub fn fmap_trait_output( &GenericParam::Type(ref data) => { let ident = data.ident; GenericArgument::Type( - fmap_output_type( - parse_quote!(#ident), - trait_path, - trait_output - ) + parse_quote!(<#ident as ::#trait_path>::#trait_output), ) }, ref arg => panic!("arguments {:?} cannot be mapped yet", arg)