Bug 1624298 - Ensure that derived types are right for optimized-away implementations. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 26 Mar 2020 13:04:20 +0000
changeset 520548 73d3541cb9700c40059e2e92e72cf151fecc3e7e
parent 520547 c774a4b050e163551e0706a7f10e3134aa588c93
child 520549 3b4a8229aacfe0b89aaeb0543b29494c6bd765c2
push id37253
push usernerli@mozilla.com
push dateThu, 26 Mar 2020 21:36:52 +0000
treeherdermozilla-central@c644dd16e2cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1624298
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1624298 - Ensure that derived types are right for optimized-away implementations. r=heycam 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
servo/components/style/counter_style/mod.rs
servo/components/style/gecko_string_cache/namespace.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/values/computed/font.rs
servo/components/style/values/computed/length.rs
servo/components/style/values/computed/mod.rs
servo/components/style/values/generics/image.rs
servo/components/style/values/resolved/mod.rs
servo/components/style/values/specified/align.rs
servo/components/style/values/specified/border.rs
servo/components/style/values/specified/font.rs
servo/components/style/values/specified/position.rs
servo/components/style/values/specified/svg_path.rs
servo/components/style/values/specified/text.rs
servo/components/style_derive/to_animated_value.rs
servo/components/style_derive/to_computed_value.rs
servo/components/style_derive/to_resolved_value.rs
servo/components/style_traits/arc_slice.rs
--- a/servo/components/style/counter_style/mod.rs
+++ b/servo/components/style/counter_style/mod.rs
@@ -403,17 +403,17 @@ impl ToCss for System {
                 dest.write_str("extends ")?;
                 other.to_css(dest)
             },
         }
     }
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#typedef-symbol>
-#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToShmem)]
+#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToCss, ToShmem)]
 #[repr(u8)]
 pub enum Symbol {
     /// <string>
     String(crate::OwnedStr),
     /// <custom-ident>
     Ident(CustomIdent),
     // Not implemented:
     // /// <image>
@@ -549,17 +549,17 @@ impl Parse for Fallback {
         _context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         Ok(Fallback(parse_counter_style_name(input)?))
     }
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-symbols>
-#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToShmem)]
+#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToCss, ToShmem)]
 #[repr(C)]
 pub struct Symbols(#[css(iterable)] pub crate::OwnedSlice<Symbol>);
 
 impl Parse for Symbols {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
--- a/servo/components/style/gecko_string_cache/namespace.rs
+++ b/servo/components/style/gecko_string_cache/namespace.rs
@@ -19,17 +19,17 @@ macro_rules! ns {
         $crate::string_cache::Namespace(atom!(""))
     };
     ($s:tt) => {
         $crate::string_cache::Namespace(atom!($s))
     };
 }
 
 /// A Gecko namespace is just a wrapped atom.
-#[derive(Clone, Debug, Default, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)]
+#[derive(Clone, Debug, Default, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 #[repr(transparent)]
 pub struct Namespace(pub Atom);
 
 impl PrecomputedHash for Namespace {
     #[inline]
     fn precomputed_hash(&self) -> u32 {
         self.0.precomputed_hash()
     }
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1039,17 +1039,17 @@ bitflags! {
 pub enum LogicalGroup {
     % for group in logical_groups.iterkeys():
     /// ${group}
     ${to_camel_case(group)},
     % endfor
 }
 
 /// An identifier for a given longhand property.
-#[derive(Clone, Copy, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)]
+#[derive(Clone, Copy, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 #[repr(u16)]
 pub enum LonghandId {
     % for i, property in enumerate(data.longhands):
         /// ${property.name}
         ${property.camel_case} = ${i},
     % endfor
 }
 
@@ -1348,17 +1348,17 @@ where
             if !self.filter || id.into().enabled_for_all_content() {
                 return Some(id)
             }
         }
     }
 }
 
 /// An identifier for a given shorthand property.
-#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)]
+#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 #[repr(u16)]
 pub enum ShorthandId {
     % for i, property in enumerate(data.shorthands):
         /// ${property.name}
         ${property.camel_case} = ${i},
     % endfor
 }
 
--- a/servo/components/style/values/computed/font.rs
+++ b/servo/components/style/values/computed/font.rs
@@ -174,17 +174,17 @@ impl ToAnimatedValue for FontSize {
     fn from_animated_value(animated: Self::AnimatedValue) -> Self {
         FontSize {
             size: NonNegative(animated.clamp_to_non_negative()),
             keyword_info: None,
         }
     }
 }
 
-#[derive(Clone, Debug, Eq, PartialEq, ToResolvedValue)]
+#[derive(Clone, Debug, Eq, PartialEq, ToComputedValue, ToResolvedValue)]
 #[cfg_attr(feature = "servo", derive(Hash, MallocSizeOf))]
 /// Specifies a prioritized list of font family names or generic family names.
 pub struct FontFamily {
     /// The actual list of family names.
     pub families: FontFamilyList,
     /// Whether this font-family came from a specified system-font.
     pub is_system_font: bool,
 }
@@ -223,17 +223,17 @@ impl ToCss for FontFamily {
         for family in iter {
             dest.write_str(", ")?;
             family.to_css(dest)?;
         }
         Ok(())
     }
 }
 
-#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToResolvedValue, ToShmem)]
+#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 /// The name of a font family of choice
 pub struct FamilyName {
     /// Name of the font family
     pub name: Atom,
     /// Syntax of the font family
     pub syntax: FontFamilyNameSyntax,
 }
@@ -266,48 +266,48 @@ impl ToCss for FamilyName {
                     serialize_identifier(ident, dest)?;
                 }
                 Ok(())
             },
         }
     }
 }
 
-#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToResolvedValue, ToShmem)]
+#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 /// Font family names must either be given quoted as strings,
 /// or unquoted as a sequence of one or more identifiers.
 #[repr(u8)]
 pub enum FontFamilyNameSyntax {
     /// The family name was specified in a quoted form, e.g. "Font Name"
     /// or 'Font Name'.
     Quoted,
 
     /// The family name was specified in an unquoted form as a sequence of
     /// identifiers.
     Identifiers,
 }
 
-#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToCss, ToResolvedValue, ToShmem)]
+#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToCss, ToComputedValue, ToResolvedValue, ToShmem)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize, Hash))]
 /// A set of faces that vary in weight, width or slope.
 pub enum SingleFontFamily {
     /// The name of a font family of choice.
     FamilyName(FamilyName),
     /// Generic family name.
     Generic(GenericFontFamily),
 }
 
 /// A generic font-family name.
 ///
 /// The order here is important, if you change it make sure that
 /// `gfxPlatformFontList.h`s ranged array and `gfxFontFamilyList`'s
 /// sSingleGenerics are updated as well.
 #[derive(
-    Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, Parse, ToCss, ToResolvedValue, ToShmem,
+    Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, Parse, ToCss, ToComputedValue, ToResolvedValue, ToShmem,
 )]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[repr(u8)]
 #[allow(missing_docs)]
 pub enum GenericFontFamily {
     /// No generic family specified, only for internal usage.
     ///
     /// NOTE(emilio): Gecko code relies on this variant being zero.
@@ -423,26 +423,30 @@ impl SingleFontFamily {
         SingleFontFamily::FamilyName(FamilyName {
             name,
             syntax: family.mSyntax,
         })
     }
 }
 
 #[cfg(feature = "servo")]
-#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToResolvedValue, ToShmem)]
+#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 /// A list of SingleFontFamily
 pub struct FontFamilyList(Box<[SingleFontFamily]>);
 
 #[cfg(feature = "gecko")]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, ToComputedValue, ToResolvedValue)]
 /// A list of SingleFontFamily
 pub enum FontFamilyList {
     /// A strong reference to a Gecko SharedFontList object.
-    SharedFontList(RefPtr<structs::SharedFontList>),
+    SharedFontList(
+        #[compute(no_field_bound)]
+        #[resolve(no_field_bound)]
+        RefPtr<structs::SharedFontList>,
+    ),
     /// A font-family generic ID.
     Generic(GenericFontFamily),
 }
 
 #[cfg(feature = "gecko")]
 impl ToShmem for FontFamilyList {
     fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
         // In practice, the only SharedFontList objects we create from shared
--- a/servo/components/style/values/computed/length.rs
+++ b/servo/components/style/values/computed/length.rs
@@ -187,16 +187,17 @@ impl Size {
     Copy,
     Deserialize,
     MallocSizeOf,
     PartialEq,
     PartialOrd,
     Serialize,
     ToAnimatedValue,
     ToAnimatedZero,
+    ToComputedValue,
     ToResolvedValue,
     ToShmem,
 )]
 #[repr(C)]
 pub struct CSSPixelLength(CSSFloat);
 
 impl fmt::Debug for CSSPixelLength {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -16,20 +16,21 @@ use super::specified;
 use super::{CSSFloat, CSSInteger};
 use crate::context::QuirksMode;
 use crate::font_metrics::{get_metrics_provider_for_product, FontMetricsProvider};
 use crate::media_queries::Device;
 #[cfg(feature = "gecko")]
 use crate::properties;
 use crate::properties::{ComputedValues, LonghandId, StyleBuilder};
 use crate::rule_cache::RuleCacheConditions;
-use crate::Atom;
+use crate::{ArcSlice, Atom};
 #[cfg(feature = "servo")]
 use crate::Prefix;
 use euclid::default::Size2D;
+use servo_arc::Arc;
 use std::cell::RefCell;
 use std::cmp;
 use std::f32;
 
 #[cfg(feature = "gecko")]
 pub use self::align::{AlignContent, AlignItems, JustifyContent, JustifyItems, SelfAlignment};
 #[cfg(feature = "gecko")]
 pub use self::align::{AlignSelf, JustifySelf};
@@ -445,30 +446,71 @@ where
     }
 
     #[inline]
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {
         computed.iter().map(T::from_computed_value).collect()
     }
 }
 
+// NOTE(emilio): This is implementable more generically, but it's unlikely
+// what you want there, as it forces you to have an extra allocation.
+//
+// We could do that if needed, ideally with specialization for the case where
+// ComputedValue = T. But we don't need it for now.
+impl<T> ToComputedValue for Arc<T>
+where
+    T: ToComputedValue<ComputedValue = T>,
+{
+    type ComputedValue = Self;
+
+    #[inline]
+    fn to_computed_value(&self, _: &Context) -> Self {
+        self.clone()
+    }
+
+    #[inline]
+    fn from_computed_value(computed: &Self) -> Self {
+        computed.clone()
+    }
+}
+
+// Same caveat as above applies.
+impl<T> ToComputedValue for ArcSlice<T>
+where
+    T: ToComputedValue<ComputedValue = T>,
+{
+    type ComputedValue = Self;
+
+    #[inline]
+    fn to_computed_value(&self, _: &Context) -> Self {
+        self.clone()
+    }
+
+    #[inline]
+    fn from_computed_value(computed: &Self) -> Self {
+        computed.clone()
+    }
+}
+
 trivial_to_computed_value!(());
 trivial_to_computed_value!(bool);
 trivial_to_computed_value!(f32);
 trivial_to_computed_value!(i32);
 trivial_to_computed_value!(u8);
 trivial_to_computed_value!(u16);
 trivial_to_computed_value!(u32);
 trivial_to_computed_value!(usize);
 trivial_to_computed_value!(Atom);
 #[cfg(feature = "servo")]
 trivial_to_computed_value!(Prefix);
 trivial_to_computed_value!(String);
 trivial_to_computed_value!(Box<str>);
 trivial_to_computed_value!(crate::OwnedStr);
+trivial_to_computed_value!(style_traits::values::specified::AllowedNumericType);
 
 #[allow(missing_docs)]
 #[derive(
     Animate,
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
--- a/servo/components/style/values/generics/image.rs
+++ b/servo/components/style/values/generics/image.rs
@@ -232,16 +232,18 @@ impl<Color, T> ColorStop<Color, T> {
 #[cfg_attr(feature = "servo", derive(MallocSizeOf))]
 #[derive(Clone, Debug, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
 pub struct PaintWorklet {
     /// The name the worklet was registered with.
     pub name: Atom,
     /// The arguments for the worklet.
     /// TODO: store a parsed representation of the arguments.
     #[cfg_attr(feature = "servo", ignore_malloc_size_of = "Arc")]
+    #[compute(no_field_bound)]
+    #[resolve(no_field_bound)]
     pub arguments: Vec<Arc<custom_properties::SpecifiedValue>>,
 }
 
 impl ::style_traits::SpecifiedValueInfo for PaintWorklet {}
 
 impl ToCss for PaintWorklet {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
--- a/servo/components/style/values/resolved/mod.rs
+++ b/servo/components/style/values/resolved/mod.rs
@@ -1,17 +1,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 //! Resolved values. These are almost always computed values, but in some cases
 //! there are used values.
 
 use crate::properties::ComputedValues;
+use crate::ArcSlice;
 use cssparser;
+use servo_arc::Arc;
 use smallvec::SmallVec;
 
 mod color;
 mod counters;
 
 use crate::values::computed;
 
 /// Information needed to resolve a given value.
@@ -74,16 +76,17 @@ trivial_to_resolved_value!(cssparser::RG
 trivial_to_resolved_value!(crate::Atom);
 trivial_to_resolved_value!(app_units::Au);
 trivial_to_resolved_value!(computed::url::ComputedUrl);
 #[cfg(feature = "gecko")]
 trivial_to_resolved_value!(computed::url::ComputedImageUrl);
 #[cfg(feature = "servo")]
 trivial_to_resolved_value!(html5ever::Prefix);
 trivial_to_resolved_value!(computed::LengthPercentage);
+trivial_to_resolved_value!(style_traits::values::specified::AllowedNumericType);
 
 impl<A, B> ToResolvedValue for (A, B)
 where
     A: ToResolvedValue,
     B: ToResolvedValue,
 {
     type ResolvedValue = (
         <A as ToResolvedValue>::ResolvedValue,
@@ -209,8 +212,48 @@ where
         self.into_box().to_resolved_value(context).into()
     }
 
     #[inline]
     fn from_resolved_value(resolved: Self::ResolvedValue) -> Self {
         Self::from(Box::from_resolved_value(resolved.into_box()))
     }
 }
+
+// NOTE(emilio): This is implementable more generically, but it's unlikely what
+// you want there, as it forces you to have an extra allocation.
+//
+// We could do that if needed, ideally with specialization for the case where
+// ResolvedValue = T. But we don't need it for now.
+impl<T> ToResolvedValue for Arc<T>
+where
+    T: ToResolvedValue<ResolvedValue = T>,
+{
+    type ResolvedValue = Self;
+
+    #[inline]
+    fn to_resolved_value(self, _: &Context) -> Self {
+        self
+    }
+
+    #[inline]
+    fn from_resolved_value(resolved: Self) -> Self {
+        resolved
+    }
+}
+
+// Same caveat as above applies.
+impl<T> ToResolvedValue for ArcSlice<T>
+where
+    T: ToResolvedValue<ResolvedValue = T>,
+{
+    type ResolvedValue = Self;
+
+    #[inline]
+    fn to_resolved_value(self, _: &Context) -> Self {
+        self
+    }
+
+    #[inline]
+    fn from_resolved_value(resolved: Self) -> Self {
+        resolved
+    }
+}
--- a/servo/components/style/values/specified/align.rs
+++ b/servo/components/style/values/specified/align.rs
@@ -551,17 +551,17 @@ impl SpecifiedValueInfo for AlignItems {
         list_overflow_position_keywords(f);
         list_self_position_keywords(f, AxisDirection::Block);
     }
 }
 
 /// Value of the `justify-items` property
 ///
 /// <https://drafts.csswg.org/css-align/#justify-items-property>
-#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToCss, ToShmem)]
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToCss, ToResolvedValue, ToShmem)]
 #[repr(C)]
 pub struct JustifyItems(pub AlignFlags);
 
 impl JustifyItems {
     /// The initial value 'legacy'
     #[inline]
     pub fn legacy() -> Self {
         JustifyItems(AlignFlags::LEGACY)
--- a/servo/components/style/values/specified/border.rs
+++ b/servo/components/style/values/specified/border.rs
@@ -229,17 +229,28 @@ impl Parse for BorderSpacing {
         .map(GenericBorderSpacing)
     }
 }
 
 /// A single border-image-repeat keyword.
 #[allow(missing_docs)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[derive(
-    Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem,
+    Clone,
+    Copy,
+    Debug,
+    Eq,
+    MallocSizeOf,
+    Parse,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToCss,
+    ToResolvedValue,
+    ToShmem,
 )]
 pub enum BorderImageRepeatKeyword {
     Stretch,
     Repeat,
     Round,
     Space,
 }
 
--- a/servo/components/style/values/specified/font.rs
+++ b/servo/components/style/values/specified/font.rs
@@ -488,17 +488,19 @@ impl ToComputedValue for FontStretch {
     Copy,
     Debug,
     MallocSizeOf,
     Parse,
     PartialEq,
     SpecifiedValueInfo,
     ToAnimatedValue,
     ToAnimatedZero,
+    ToComputedValue,
     ToCss,
+    ToResolvedValue,
     ToShmem,
 )]
 #[allow(missing_docs)]
 pub enum KeywordSize {
     #[css(keyword = "xx-small")]
     XXSmall,
     XSmall,
     Small,
@@ -530,17 +532,19 @@ impl Default for KeywordSize {
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
     MallocSizeOf,
     PartialEq,
     ToAnimatedValue,
     ToAnimatedZero,
+    ToComputedValue,
     ToCss,
+    ToResolvedValue,
     ToShmem,
 )]
 /// Additional information for keyword-derived font sizes.
 pub struct KeywordInfo {
     /// The keyword used
     pub kw: KeywordSize,
     /// A factor to be multiplied by the computed size of the keyword
     #[css(skip)]
@@ -563,17 +567,17 @@ impl KeywordInfo {
             factor: 1.,
             offset: CSSPixelLength::new(0.),
         }
     }
 
     /// Computes the final size for this font-size keyword, accounting for
     /// text-zoom.
     fn to_computed_value(&self, context: &Context) -> CSSPixelLength {
-        let base = context.maybe_zoom_text(self.kw.to_computed_value(context).0);
+        let base = context.maybe_zoom_text(self.kw.to_length(context).0);
         base * self.factor + context.maybe_zoom_text(self.offset)
     }
 
     /// Given a parent keyword info (self), apply an additional factor/offset to
     /// it.
     fn compose(self, factor: f32) -> Self {
         KeywordInfo {
             kw: self.kw,
@@ -756,46 +760,37 @@ impl ToComputedValue for FontSizeAdjust 
 
 /// This is the ratio applied for font-size: larger
 /// and smaller by both Firefox and Chrome
 const LARGER_FONT_SIZE_RATIO: f32 = 1.2;
 
 /// The default font size.
 pub const FONT_MEDIUM_PX: i32 = 16;
 
-#[cfg(feature = "servo")]
-impl ToComputedValue for KeywordSize {
-    type ComputedValue = NonNegativeLength;
+impl KeywordSize {
     #[inline]
-    fn to_computed_value(&self, _: &Context) -> NonNegativeLength {
+    #[cfg(feature = "servo")]
+    fn to_length(&self, _: &Context) -> NonNegativeLength {
         let medium = Length::new(FONT_MEDIUM_PX as f32);
         // https://drafts.csswg.org/css-fonts-3/#font-size-prop
         NonNegative(match *self {
             KeywordSize::XXSmall => medium * 3.0 / 5.0,
             KeywordSize::XSmall => medium * 3.0 / 4.0,
             KeywordSize::Small => medium * 8.0 / 9.0,
             KeywordSize::Medium => medium,
             KeywordSize::Large => medium * 6.0 / 5.0,
             KeywordSize::XLarge => medium * 3.0 / 2.0,
             KeywordSize::XXLarge => medium * 2.0,
             KeywordSize::XXXLarge => medium * 3.0,
         })
     }
 
+    #[cfg(feature = "gecko")]
     #[inline]
-    fn from_computed_value(_: &NonNegativeLength) -> Self {
-        unreachable!()
-    }
-}
-
-#[cfg(feature = "gecko")]
-impl ToComputedValue for KeywordSize {
-    type ComputedValue = NonNegativeLength;
-    #[inline]
-    fn to_computed_value(&self, cx: &Context) -> NonNegativeLength {
+    fn to_length(&self, cx: &Context) -> NonNegativeLength {
         use crate::context::QuirksMode;
 
         // The tables in this function are originally from
         // nsRuleNode::CalcFontPointSize in Gecko:
         //
         // https://searchfox.org/mozilla-central/rev/c05d9d61188d32b8/layout/style/nsRuleNode.cpp#3150
         //
         // Mapping from base size and HTML size to pixels
@@ -853,21 +848,16 @@ impl ToComputedValue for KeywordSize {
             } else {
                 FONT_SIZE_MAPPING
             };
             Length::new(mapping[(base_size_px - 9) as usize][html_size] as f32)
         } else {
             base_size * FONT_SIZE_FACTORS[html_size] as f32 / 100.0
         })
     }
-
-    #[inline]
-    fn from_computed_value(_: &NonNegativeLength) -> Self {
-        unreachable!()
-    }
 }
 
 impl FontSize {
     /// <https://html.spec.whatwg.org/multipage/#rules-for-parsing-a-legacy-font-size>
     pub fn from_html_size(size: u8) -> Self {
         FontSize::Keyword(KeywordInfo::new(match size {
             // If value is less than 1, let it be 1.
             0 | 1 => KeywordSize::XSmall,
--- a/servo/components/style/values/specified/position.rs
+++ b/servo/components/style/values/specified/position.rs
@@ -534,17 +534,17 @@ impl TemplateAreas {
                 } else if width != column {
                     return Err(());
                 }
             }
         }
         Ok(TemplateAreas {
             areas: areas.into(),
             strings: strings.into(),
-            width: width,
+            width,
         })
     }
 }
 
 impl Parse for TemplateAreas {
     fn parse<'i, 't>(
         _context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -584,25 +584,43 @@ impl Parse for TemplateAreasArc {
         let parsed = TemplateAreas::parse(context, input)?;
         Ok(TemplateAreasArc(Arc::new(parsed)))
     }
 }
 
 /// A range of rows or columns. Using this instead of std::ops::Range for FFI
 /// purposes.
 #[repr(C)]
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToShmem)]
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToResolvedValue,
+    ToShmem,
+)]
 pub struct UnsignedRange {
     /// The start of the range.
     pub start: u32,
     /// The end of the range.
     pub end: u32,
 }
 
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToShmem)]
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToResolvedValue,
+    ToShmem,
+)]
 #[repr(C)]
 /// Not associated with any particular grid item, but can be referenced from the
 /// grid-placement properties.
 pub struct NamedArea {
     /// Name of the `named area`
     pub name: Atom,
     /// Rows of the `named area`
     pub rows: UnsignedRange,
--- a/servo/components/style/values/specified/svg_path.rs
+++ b/servo/components/style/values/specified/svg_path.rs
@@ -30,17 +30,18 @@ use style_traits::{CssWriter, ParseError
     ToComputedValue,
     ToResolvedValue,
     ToShmem,
 )]
 #[repr(C)]
 pub struct SVGPathData(
     // TODO(emilio): Should probably measure this somehow only from the
     // specified values.
-    #[ignore_malloc_size_of = "Arc"] pub crate::ArcSlice<PathCommand>,
+    #[ignore_malloc_size_of = "Arc"]
+    pub crate::ArcSlice<PathCommand>,
 );
 
 impl SVGPathData {
     /// Get the array of PathCommand.
     #[inline]
     pub fn commands(&self) -> &[PathCommand] {
         &self.0
     }
@@ -154,16 +155,18 @@ impl ComputeSquaredDistance for SVGPathD
     Copy,
     Debug,
     Deserialize,
     MallocSizeOf,
     PartialEq,
     Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
+    ToComputedValue,
+    ToResolvedValue,
     ToShmem,
 )]
 #[allow(missing_docs)]
 #[repr(C, u8)]
 pub enum PathCommand {
     /// The unknown type.
     /// https://www.w3.org/TR/SVG/paths.html#__svg__SVGPathSeg__PATHSEG_UNKNOWN
     Unknown,
@@ -483,16 +486,18 @@ impl ToCss for PathCommand {
     Copy,
     Debug,
     Deserialize,
     MallocSizeOf,
     PartialEq,
     Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
+    ToComputedValue,
+    ToResolvedValue,
     ToShmem,
 )]
 #[repr(u8)]
 pub enum IsAbsolute {
     Yes,
     No,
 }
 
@@ -513,33 +518,35 @@ impl IsAbsolute {
     Copy,
     Debug,
     Deserialize,
     MallocSizeOf,
     PartialEq,
     Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
+    ToComputedValue,
     ToCss,
+    ToResolvedValue,
     ToShmem,
 )]
 #[repr(C)]
 pub struct CoordPair(CSSFloat, CSSFloat);
 
 impl CoordPair {
     /// Create a CoordPair.
     #[inline]
     pub fn new(x: CSSFloat, y: CSSFloat) -> Self {
         CoordPair(x, y)
     }
 }
 
 /// The EllipticalArc flag type.
 #[derive(
-    Clone, Copy, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize, SpecifiedValueInfo, ToShmem,
+    Clone, Copy, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem,
 )]
 #[repr(C)]
 pub struct ArcFlag(bool);
 
 impl ToCss for ArcFlag {
     #[inline]
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
--- a/servo/components/style/values/specified/text.rs
+++ b/servo/components/style/values/specified/text.rs
@@ -117,17 +117,17 @@ impl ToComputedValue for LineHeight {
             GenericLineHeight::Length(ref length) => {
                 GenericLineHeight::Length(NoCalcLength::from_computed_value(&length.0).into())
             },
         }
     }
 }
 
 /// A generic value for the `text-overflow` property.
-#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
+#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, ToResolvedValue, ToShmem)]
 #[repr(C, u8)]
 pub enum TextOverflowSide {
     /// Clip inline content.
     Clip,
     /// Render ellipsis to represent clipped inline content.
     Ellipsis,
     /// Render a given string to represent clipped inline content.
     String(crate::OwnedStr),
@@ -676,17 +676,17 @@ pub enum TextEmphasisStyle {
     },
     /// `none`
     None,
     /// `<string>` (of which only the first grapheme cluster will be used).
     String(crate::OwnedStr),
 }
 
 /// Fill mode for the text-emphasis-style property
-#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
+#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToComputedValue, ToResolvedValue, ToShmem)]
 #[repr(u8)]
 pub enum TextEmphasisFillMode {
     /// `filled`
     Filled,
     /// `open`
     Open,
 }
 
@@ -695,17 +695,28 @@ impl TextEmphasisFillMode {
     #[inline]
     pub fn is_filled(&self) -> bool {
         matches!(*self, TextEmphasisFillMode::Filled)
     }
 }
 
 /// Shape keyword for the text-emphasis-style property
 #[derive(
-    Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem,
+    Clone,
+    Copy,
+    Debug,
+    Eq,
+    MallocSizeOf,
+    Parse,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToCss,
+    ToComputedValue,
+    ToResolvedValue,
+    ToShmem,
 )]
 #[repr(u8)]
 pub enum TextEmphasisShapeKeyword {
     /// `dot`
     Dot,
     /// `circle`
     Circle,
     /// `double-circle`
--- a/servo/components/style_derive/to_animated_value.rs
+++ b/servo/components/style_derive/to_animated_value.rs
@@ -6,38 +6,30 @@ use proc_macro2::TokenStream;
 use syn::DeriveInput;
 use synstructure::BindStyle;
 use to_computed_value;
 
 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,
     )
 }
--- a/servo/components/style_derive/to_computed_value.rs
+++ b/servo/components/style_derive/to_computed_value.rs
@@ -8,82 +8,129 @@ use syn::{DeriveInput, Ident, Path};
 use synstructure::{BindStyle, BindingInfo};
 
 pub fn derive_to_value(
     mut input: DeriveInput,
     trait_path: Path,
     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,
     // Returns a tokenstream of the form:
     // fn from_function_syntax(foobar) -> Baz {
     //     #first_arg
     // }
     //
     // fn to_function_syntax(foobar) -> Baz {
     //     #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,
         &input.generics,
         &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 = cg::fmap_match(&input, bind_style, |binding| {
-            if field_bound(&binding) {
-                let ty = &binding.ast().ty;
+        let to_body = if moves {
+            quote! { self }
+        } else {
+            quote! { std::clone::Clone::clone(self) }
+        };
 
-                let output_type = cg::map_type_params(
-                    ty,
-                    &params,
-                    &mut |ident| parse_quote!(<#ident as #trait_path>::#output_type_name),
-                );
+        let from_body = if moves {
+            quote! { from }
+        } else {
+            quote! { std::clone::Clone::clone(from) }
+        };
 
-                cg::add_predicate(
-                    &mut where_clause,
-                    parse_quote!(
-                        #ty: #trait_path<#output_type_name = #output_type>
-                    ),
-                );
+        (to_body, from_body)
+    } else {
+        let to_body = cg::fmap_match(&input, bind_style, |binding| {
+            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)
     };
 
     input.generics.where_clause = where_clause;
     let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
     let computed_value_type = cg::fmap_trait_output(&input, &trait_path, &output_type_name);
 
     let impl_ = trait_impl(from_body, to_body);
@@ -96,58 +143,50 @@ pub fn derive_to_value(
         }
     }
 }
 
 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,
 }
--- a/servo/components/style_derive/to_resolved_value.rs
+++ b/servo/components/style_derive/to_resolved_value.rs
@@ -7,63 +7,46 @@ use proc_macro2::TokenStream;
 use syn::DeriveInput;
 use synstructure::BindStyle;
 use to_computed_value;
 
 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]
              fn to_resolved_value(
                  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,
     )
 }
 
 #[darling(attributes(resolve), default)]
 #[derive(Default, FromField)]
 struct ResolvedValueAttrs {
     field_bound: bool,
+    no_field_bound: bool,
 }
--- a/servo/components/style_traits/arc_slice.rs
+++ b/servo/components/style_traits/arc_slice.rs
@@ -21,29 +21,35 @@ use std::{iter, mem};
 /// empty slice, even if the types they hold are aligned differently.
 const ARC_SLICE_CANARY: u64 = 0xf3f3f3f3f3f3f3f3;
 
 /// A wrapper type for a refcounted slice using ThinArc.
 ///
 /// cbindgen:derive-eq=false
 /// cbindgen:derive-neq=false
 #[repr(C)]
-#[derive(Clone, Debug, Eq, PartialEq, ToShmem)]
+#[derive(Debug, Eq, PartialEq, ToShmem)]
 pub struct ArcSlice<T>(#[shmem(field_bound)] ThinArc<u64, T>);
 
 impl<T> Deref for ArcSlice<T> {
     type Target = [T];
 
     #[inline]
     fn deref(&self) -> &Self::Target {
         debug_assert_eq!(self.0.header.header, ARC_SLICE_CANARY);
         &self.0.slice
     }
 }
 
+impl<T> Clone for ArcSlice<T> {
+    fn clone(&self) -> Self {
+        ArcSlice(self.0.clone())
+    }
+}
+
 lazy_static! {
     // ThinArc doesn't support alignments greater than align_of::<u64>.
     static ref EMPTY_ARC_SLICE: ArcSlice<u64> = {
         ArcSlice::from_iter_leaked(iter::empty())
     };
 }
 
 impl<T> Default for ArcSlice<T> {