Improve handling of trait bounds when deriving fmap-like traits

components/style/values/computed/image.rs

@@ -203,33 +203,3 @@ impl ToComputedValue for SpecifiedGradient {
         }
     }
 }
-
-impl ToComputedValue for SpecifiedGradientKind {
-    type ComputedValue = GradientKind;
-
-    fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
-        match self {
-            &GenericGradientKind::Linear(ref line_direction) => {
-                GenericGradientKind::Linear(line_direction.to_computed_value(context))
-            },
-            &GenericGradientKind::Radial(ref ending_shape, ref position, ref angle) => {
-                GenericGradientKind::Radial(ending_shape.to_computed_value(context),
-                                            position.to_computed_value(context),
-                                            angle.map(|angle| angle.to_computed_value(context)))
-            }
-        }
-    }
-
-    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
-        match *computed {
-            GenericGradientKind::Linear(line_direction) => {
-                GenericGradientKind::Linear(SpecifiedLineDirection::from_computed_value(&line_direction))
-            },
-            GenericGradientKind::Radial(ending_shape, position, angle) => {
-                GenericGradientKind::Radial(ToComputedValue::from_computed_value(&ending_shape),
-                                            ToComputedValue::from_computed_value(&position),
-                                            angle.map(|angle| ToComputedValue::from_computed_value(&angle)))
-            }
-        }
-    }
-}

components/style/values/generics/grid.rs

@@ -204,7 +204,7 @@ impl<L: ToComputedValue> ToComputedValue for TrackBreadth<L> {
 ///
 /// https://drafts.csswg.org/css-grid/#typedef-track-size
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-#[derive(Clone, Debug, HasViewportPercentage, PartialEq)]
+#[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToComputedValue)]
 pub enum TrackSize<L> {
     /// A flexible `<track-breadth>`
     Breadth(TrackBreadth<L>),
@@ -286,39 +286,6 @@ impl<L: ToCss> ToCss for TrackSize<L> {
     }
 }
 
-impl<L: ToComputedValue> ToComputedValue for TrackSize<L> {
-    type ComputedValue = TrackSize<L::ComputedValue>;
-
-    #[inline]
-    fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
-        match *self {
-            TrackSize::Breadth(ref b) => match *b {
-                // <flex> outside `minmax()` expands to `mimmax(auto, <flex>)`
-                // https://drafts.csswg.org/css-grid/#valdef-grid-template-columns-flex
-                TrackBreadth::Flex(f) =>
-                    TrackSize::Minmax(TrackBreadth::Keyword(TrackKeyword::Auto), TrackBreadth::Flex(f)),
-                _ => TrackSize::Breadth(b.to_computed_value(context)),
-            },
-            TrackSize::Minmax(ref b_1, ref b_2) =>
-                TrackSize::Minmax(b_1.to_computed_value(context), b_2.to_computed_value(context)),
-            TrackSize::FitContent(ref lop) => TrackSize::FitContent(lop.to_computed_value(context)),
-        }
-    }
-
-    #[inline]
-    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
-        match *computed {
-            TrackSize::Breadth(ref b) =>
-                TrackSize::Breadth(ToComputedValue::from_computed_value(b)),
-            TrackSize::Minmax(ref b_1, ref b_2) =>
-                TrackSize::Minmax(ToComputedValue::from_computed_value(b_1),
-                                  ToComputedValue::from_computed_value(b_2)),
-            TrackSize::FitContent(ref lop) =>
-                TrackSize::FitContent(ToComputedValue::from_computed_value(lop)),
-        }
-    }
-}
-
 /// Helper function for serializing identifiers with a prefix and suffix, used
 /// for serializing <line-names> (in grid).
 pub fn concat_serialize_idents<W>(prefix: &str, suffix: &str,
@@ -698,8 +665,8 @@ no_viewport_percentage!(LineNameList);
 /// Variants for `<grid-template-rows> | <grid-template-columns>`
 /// Subgrid deferred to Level 2 spec due to lack of implementation.
 /// But it's implemented in gecko, so we have to as well.
-#[derive(Clone, Debug, PartialEq, ToCss)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, Debug, PartialEq, ToComputedValue, ToCss)]
 pub enum GridTemplateComponent<L> {
     /// `none` value.
     None,

components/style/values/generics/image.rs

@@ -61,8 +61,8 @@ pub enum CompatMode {
 }
 
 /// A gradient kind.
-#[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq, ToComputedValue)]
 pub enum GradientKind<LineDirection, Length, LengthOrPercentage, Position, Angle> {
     /// A linear gradient.
     Linear(LineDirection),

components/style/values/specified/grid.rs

@@ -354,27 +354,3 @@ impl HasViewportPercentage for GridTemplateComponent<LengthOrPercentage> {
         }
     }
 }
-
-impl ToComputedValue for GridTemplateComponent<LengthOrPercentage> {
-    type ComputedValue = GridTemplateComponent<computed::LengthOrPercentage>;
-
-    #[inline]
-    fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
-        match *self {
-            GridTemplateComponent::None => GridTemplateComponent::None,
-            GridTemplateComponent::TrackList(ref l) => GridTemplateComponent::TrackList(l.to_computed_value(context)),
-            GridTemplateComponent::Subgrid(ref n) => GridTemplateComponent::Subgrid(n.to_computed_value(context)),
-        }
-    }
-
-    #[inline]
-    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
-        match *computed {
-            GridTemplateComponent::None => GridTemplateComponent::None,
-            GridTemplateComponent::TrackList(ref l) =>
-                GridTemplateComponent::TrackList(ToComputedValue::from_computed_value(l)),
-            GridTemplateComponent::Subgrid(ref n) =>
-                GridTemplateComponent::Subgrid(ToComputedValue::from_computed_value(n)),
-        }
-    }
-}

components/style_derive/animate.rs

@@ -30,14 +30,16 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
         computations.append_all(iter.map(|(result, (this, other))| {
             let field_attrs = cg::parse_field_attrs::<AnimateFieldAttrs>(&result.field);
             if field_attrs.constant {
-                if cg::is_parameterized(&result.field.ty, where_clause.params) {
+                if cg::is_parameterized(&result.field.ty, where_clause.params, None) {
                     where_clause.inner.predicates.push(cg::where_predicate(
                         result.field.ty.clone(),
                         &["std", "cmp", "PartialEq"],
+                        None,
                     ));
                     where_clause.inner.predicates.push(cg::where_predicate(
                         result.field.ty.clone(),
                         &["std", "clone", "Clone"],
+                        None,
                     ));
                 }
                 quote! {
@@ -47,7 +49,7 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
                     let #result = ::std::clone::Clone::clone(#this);
                 }
             } else {
-                where_clause.add_trait_bound(result.field.ty.clone());
+                where_clause.add_trait_bound(&result.field.ty);
                 quote! {
                     let #result =
                         ::values::animated::Animate::animate(#this, #other, procedure)?;

components/style_derive/cg.rs

@@ -10,7 +10,7 @@ use std::iter;
 use syn::{self, AngleBracketedParameterData, Body, DeriveInput, Field, Ident};
 use syn::{ImplGenerics, Path, PathParameters, PathSegment, PolyTraitRef};
 use syn::{QSelf, TraitBoundModifier, Ty, TyGenerics, TyParam, TyParamBound};
-use syn::{Variant, WhereBoundPredicate, WherePredicate};
+use syn::{TypeBinding, Variant, WhereBoundPredicate, WherePredicate};
 use syn::visit::{self, Visitor};
 use synstructure::{self, BindOpts, BindStyle, BindingInfo};
 
@@ -18,6 +18,7 @@ pub struct WhereClause<'input, 'path> {
     pub inner: syn::WhereClause,
     pub params: &'input [TyParam],
     trait_path: &'path [&'path str],
+    trait_output: Option<&'path str>,
     bounded_types: HashSet<Ty>,
 }
 
@@ -28,10 +29,55 @@ impl<'input, 'path> ToTokens for WhereClause<'input, 'path> {
 }
 
 impl<'input, 'path> WhereClause<'input, 'path> {
-    pub fn add_trait_bound(&mut self, ty: Ty) {
+    pub fn add_trait_bound(&mut self, ty: &Ty) {
-        if is_parameterized(&ty, self.params) && !self.bounded_types.contains(&ty) {
+        let trait_path = self.trait_path;
-            self.bounded_types.insert(ty.clone());
+        let params = self.params;
-            self.inner.predicates.push(where_predicate(ty, self.trait_path));
+        let mut found = self.trait_output.map(|_| HashSet::new());
+        if self.bounded_types.contains(&ty) {
+            return;
+        }
+        if !is_parameterized(&ty, params, found.as_mut()) {
+            return;
+        }
+        self.bounded_types.insert(ty.clone());
+
+        let output = if let Some(output) = self.trait_output {
+            output
+        } else {
+            self.inner.predicates.push(where_predicate(ty.clone(), trait_path, None));
+            return;
+        };
+
+        if let Ty::Path(None, ref path) = *ty {
+            if path_to_ident(path).is_some() {
+                self.inner.predicates.push(where_predicate(ty.clone(), trait_path, None));
+                return;
+            }
+        }
+
+        let output_type = map_type_params(ty, params, &mut |ident| {
+            let ty = Ty::Path(None, ident.clone().into());
+            fmap_output_type(ty, trait_path, output)
+        });
+
+        let pred = where_predicate(
+            ty.clone(),
+            trait_path,
+            Some((output, output_type)),
+        );
+
+        self.inner.predicates.push(pred);
+
+        if let Some(found) = found {
+            for ident in found {
+                let ty = Ty::Path(None, ident.into());
+                if !self.bounded_types.contains(&ty) {
+                    self.bounded_types.insert(ty.clone());
+                    self.inner.predicates.push(
+                        where_predicate(ty, trait_path, None),
+                    );
+                };
+            }
         }
     }
 }
@@ -58,23 +104,36 @@ where
     })
 }
 
+fn fmap_output_type(
+    ty: Ty,
+    trait_path: &[&str],
+    trait_output: &str,
+) -> Ty {
+    Ty::Path(
+        Some(QSelf {
+            ty: Box::new(ty),
+            position: trait_path.len(),
+        }),
+        path(trait_path.iter().chain(iter::once(&trait_output))),
+    )
+}
+
 pub fn fmap_trait_parts<'input, 'path>(
     input: &'input DeriveInput,
     trait_path: &'path [&'path str],
-    trait_output: &str,
+    trait_output: &'path str,
 ) -> (ImplGenerics<'input>, TyGenerics<'input>, WhereClause<'input, 'path>, Path) {
-    let (impl_generics, ty_generics, where_clause) = trait_parts(input, trait_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 {
         ident: input.ident.clone(),
         parameters: PathParameters::AngleBracketed(AngleBracketedParameterData {
             lifetimes: input.generics.lifetimes.iter().map(|l| l.lifetime.clone()).collect(),
             types: input.generics.ty_params.iter().map(|ty| {
-                Ty::Path(
+                fmap_output_type(
-                    Some(QSelf {
+                    Ty::Path(None, ty.ident.clone().into()),
-                        ty: Box::new(Ty::Path(None, ty.ident.clone().into())),
+                    trait_path,
-                        position: trait_path.len(),
+                    trait_output,
-                    }),
-                    path(trait_path.iter().chain(iter::once(&trait_output))),
                 )
             }).collect(),
             .. Default::default()
@@ -83,28 +142,106 @@ pub fn fmap_trait_parts<'input, 'path>(
     (impl_generics, ty_generics, where_clause, output_ty)
 }
 
-pub fn is_parameterized(ty: &Ty, params: &[TyParam]) -> bool {
+pub fn is_parameterized(
-    struct IsParameterized<'a> {
+    ty: &Ty,
+    params: &[TyParam],
+    found: Option<&mut HashSet<Ident>>,
+) -> bool {
+    struct IsParameterized<'a, 'b> {
         params: &'a [TyParam],
         has_free: bool,
+        found: Option<&'b mut HashSet<Ident>>,
     }
 
-    impl<'a> Visitor for IsParameterized<'a> {
+    impl<'a, 'b> Visitor for IsParameterized<'a, 'b> {
         fn visit_path(&mut self, path: &Path) {
-            if !path.global && path.segments.len() == 1 {
+            if let Some(ident) = path_to_ident(path) {
-                if self.params.iter().any(|param| param.ident == path.segments[0].ident) {
+                if self.params.iter().any(|param| param.ident == ident) {
                     self.has_free = true;
+                    if let Some(ref mut found) = self.found {
+                        found.insert(ident.clone());
+                    }
                 }
             }
             visit::walk_path(self, path);
         }
     }
 
-    let mut visitor = IsParameterized { params: params, has_free: false };
+    let mut visitor = IsParameterized { params, has_free: false, found };
     visitor.visit_ty(ty);
     visitor.has_free
 }
 
+pub fn map_type_params<F>(ty: &Ty, params: &[TyParam], f: &mut F) -> Ty
+where
+    F: FnMut(&Ident) -> Ty,
+{
+    match *ty {
+        Ty::Slice(ref ty) => Ty::Slice(Box::new(map_type_params(ty, params, f))),
+        Ty::Array(ref ty, ref expr) => {
+            Ty::Array(Box::new(map_type_params(ty, params, f)), expr.clone())
+        },
+        Ty::Never => Ty::Never,
+        Ty::Tup(ref items) => {
+            Ty::Tup(items.iter().map(|ty| map_type_params(ty, params, f)).collect())
+        },
+        Ty::Path(None, ref path) => {
+            if let Some(ident) = path_to_ident(path) {
+                if params.iter().any(|param| param.ident == ident) {
+                    return f(ident);
+                }
+            }
+            Ty::Path(None, map_type_params_in_path(path, params, f))
+        }
+        Ty::Path(ref qself, ref path) => {
+            Ty::Path(
+                qself.as_ref().map(|qself| {
+                    QSelf {
+                        ty: Box::new(map_type_params(&qself.ty, params, f)),
+                        position: qself.position,
+                    }
+                }),
+                map_type_params_in_path(path, params, f),
+            )
+        },
+        Ty::Paren(ref ty) => Ty::Paren(Box::new(map_type_params(ty, params, f))),
+        ref ty => panic!("type {:?} cannot be mapped yet", ty),
+    }
+}
+
+fn map_type_params_in_path<F>(path: &Path, params: &[TyParam], f: &mut F) -> Path
+where
+    F: FnMut(&Ident) -> Ty,
+{
+    Path {
+        global: path.global,
+        segments: path.segments.iter().map(|segment| {
+            PathSegment {
+                ident: segment.ident.clone(),
+                parameters: match segment.parameters {
+                    PathParameters::AngleBracketed(ref data) => {
+                        PathParameters::AngleBracketed(AngleBracketedParameterData {
+                            lifetimes: data.lifetimes.clone(),
+                            types: data.types.iter().map(|ty| {
+                                map_type_params(ty, params, f)
+                            }).collect(),
+                            bindings: data.bindings.iter().map(|binding| {
+                                TypeBinding {
+                                    ident: binding.ident.clone(),
+                                    ty: map_type_params(&binding.ty, params, f),
+                                }
+                            }).collect(),
+                        })
+                    },
+                    ref parameters => {
+                        panic!("parameters {:?} cannot be mapped yet", parameters)
+                    },
+                },
+            }
+        }).collect(),
+    }
+}
+
 pub fn path<S>(segments: S) -> Path
 where
     S: IntoIterator,
@@ -116,6 +253,19 @@ where
     }
 }
 
+fn path_to_ident(path: &Path) -> Option<&Ident> {
+    match *path {
+        Path { global: false, ref segments } if segments.len() == 1 => {
+            if segments[0].parameters.is_empty() {
+                Some(&segments[0].ident)
+            } else {
+                None
+            }
+        },
+        _ => None,
+    }
+}
+
 pub fn parse_field_attrs<A>(field: &Field) -> A
 where
     A: FromField,
@@ -157,11 +307,38 @@ pub fn trait_parts<'input, 'path>(
         inner: where_clause.clone(),
         params: &input.generics.ty_params,
         trait_path,
+        trait_output: None,
         bounded_types: HashSet::new()
     };
     (impl_generics, ty_generics, where_clause)
 }
 
+fn trait_ref(path: &[&str], output: Option<(&str, Ty)>) -> Path {
+    let (name, parent) = path.split_last().unwrap();
+    let last_segment = PathSegment {
+        ident: (*name).into(),
+        parameters: PathParameters::AngleBracketed(
+            AngleBracketedParameterData {
+                bindings: output.into_iter().map(|(param, ty)| {
+                    TypeBinding { ident: param.into(), ty }
+                }).collect(),
+                .. Default::default()
+            }
+        )
+    };
+    Path {
+        global: true,
+        segments: {
+            parent
+                .iter()
+                .cloned()
+                .map(Into::into)
+                .chain(iter::once(last_segment))
+                .collect()
+        },
+    }
+}
+
 pub fn value<'a>(
     name: &Ident,
     variant: &'a Variant,
@@ -202,16 +379,20 @@ pub fn variants(input: &DeriveInput) -> Cow<[Variant]> {
     }
 }
 
-pub fn where_predicate(ty: Ty, segments: &[&str]) -> WherePredicate {
+pub fn where_predicate(
+    bounded_ty: Ty,
+    trait_path: &[&str],
+    trait_output: Option<(&str, Ty)>,
+) -> WherePredicate {
     WherePredicate::BoundPredicate(WhereBoundPredicate {
         bound_lifetimes: vec![],
-        bounded_ty: ty,
+        bounded_ty,
         bounds: vec![TyParamBound::Trait(
             PolyTraitRef {
                 bound_lifetimes: vec![],
-                trait_ref: path(segments),
+                trait_ref: trait_ref(trait_path, trait_output),
             },
-            TraitBoundModifier::None,
+            TraitBoundModifier::None
         )],
     })
 }

components/style_derive/compute_squared_distance.rs

@@ -30,7 +30,7 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
         } else {
             let mut sum = quote!();
             sum.append_separated(this_info.iter().zip(&other_info).map(|(this, other)| {
-                where_clause.add_trait_bound(this.field.ty.clone());
+                where_clause.add_trait_bound(&this.field.ty);
                 quote! {
                     ::values::distance::ComputeSquaredDistance::compute_squared_distance(#this, #other)?
                 }

components/style_derive/has_viewport_percentage.rs

@@ -19,10 +19,10 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
             None => return Some(quote!(false)),
             Some(pair) => pair,
         };
-        where_clause.add_trait_bound(first.field.ty.clone());
+        where_clause.add_trait_bound(&first.field.ty);
         let mut expr = quote!(::style_traits::HasViewportPercentage::has_viewport_percentage(#first));
         for binding in rest {
-            where_clause.add_trait_bound(binding.field.ty.clone());
+            where_clause.add_trait_bound(&binding.field.ty);
             expr = quote!(#expr || ::style_traits::HasViewportPercentage::has_viewport_percentage(#binding));
         }
         Some(expr)

components/style_derive/to_animated_value.rs

@@ -12,13 +12,9 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
     let trait_path = &["values", "animated", "ToAnimatedValue"];
     let (impl_generics, ty_generics, mut where_clause, animated_value_type) =
         cg::fmap_trait_parts(&input, trait_path, "AnimatedValue");
-    for param in &input.generics.ty_params {
-        where_clause.add_trait_bound(
-            syn::Ty::Path(None, param.ident.clone().into()),
-        );
-    }
 
     let to_body = cg::fmap_match(&input, BindStyle::Move, |binding| {
+        where_clause.add_trait_bound(&binding.field.ty);
         quote!(::values::animated::ToAnimatedValue::to_animated_value(#binding))
     });
     let from_body = cg::fmap_match(&input, BindStyle::Move, |binding| {

components/style_derive/to_animated_zero.rs

@@ -27,17 +27,18 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
         computations.append_all(bindings_pairs.map(|(binding, mapped_binding)| {
             let field_attrs = cg::parse_field_attrs::<AnimateFieldAttrs>(&binding.field);
             if field_attrs.constant {
-                if cg::is_parameterized(&binding.field.ty, where_clause.params) {
+                if cg::is_parameterized(&binding.field.ty, where_clause.params, None) {
                     where_clause.inner.predicates.push(cg::where_predicate(
                         binding.field.ty.clone(),
                         &["std", "clone", "Clone"],
+                        None,
                     ));
                 }
                 quote! {
                     let #mapped_binding = ::std::clone::Clone::clone(#binding);
                 }
             } else {
-                where_clause.add_trait_bound(binding.field.ty.clone());
+                where_clause.add_trait_bound(&binding.field.ty);
                 quote! {
                     let #mapped_binding =
                         ::values::animated::ToAnimatedZero::to_animated_zero(#binding)?;

components/style_derive/to_computed_value.rs

@@ -3,26 +3,26 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cg;
-use quote;
+use quote::Tokens;
-use syn;
+use syn::DeriveInput;
 use synstructure::BindStyle;
 
-pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
+pub fn derive(input: DeriveInput) -> Tokens {
     let name = &input.ident;
     let trait_path = &["values", "computed", "ToComputedValue"];
     let (impl_generics, ty_generics, mut where_clause, computed_value_type) =
         cg::fmap_trait_parts(&input, trait_path, "ComputedValue");
-    for param in &input.generics.ty_params {
-        where_clause.add_trait_bound(
-            syn::Ty::Path(None, param.ident.clone().into()),
-        );
-    }
 
     let to_body = cg::fmap_match(&input, BindStyle::Ref, |binding| {
-        quote!(::values::computed::ToComputedValue::to_computed_value(#binding, context))
+        where_clause.add_trait_bound(&binding.field.ty);
+        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))
+        quote! {
+            ::values::computed::ToComputedValue::from_computed_value(#binding)
+        }
     });
 
     quote! {

components/style_derive/to_css.rs

@@ -21,7 +21,7 @@ pub fn derive(input: syn::DeriveInput) -> quote::Tokens {
         let mut expr = if !bindings.is_empty() {
             let mut expr = quote! {};
             for binding in bindings {
-                where_clause.add_trait_bound(binding.field.ty.clone());
+                where_clause.add_trait_bound(&binding.field.ty);
                 expr = quote! {
                     #expr
                     writer.item(#binding)?;