servo: Merge #19139 - style: Get rid of parse_specified (from emilio:bye-parse-specified); r=jdm
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 07 Nov 2017 23:44:17 -0600
changeset 443951 20d4b128753bf57a03d56e4b73e7c96049e875b5
parent 443950 9903b9dbd27f2b5183c2a4c3bf6b9ed7b4d34fbd
child 443952 3e95c596ad5bc0c0b99608ed26380994973e7665
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
milestone58.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
servo: Merge #19139 - style: Get rid of parse_specified (from emilio:bye-parse-specified); r=jdm It has a single use, and I don't think we should use it in the future. Source-Repo: https://github.com/servo/servo Source-Revision: 4a2103b9e600a3b372d2770669b3f9f597d1e4b3
servo/components/style/properties/helpers.mako.rs
servo/components/style/properties/longhand/position.mako.rs
servo/components/style/properties/shorthand/position.mako.rs
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -397,39 +397,28 @@
                 % if property.custom_cascade:
                     cascade_property_custom(declaration, context);
                 % endif
             % else:
                 // Do not allow stylesheets to set derived properties.
             % endif
         }
         % if not property.derived_from:
-            pub fn parse_specified<'i, 't>(
+            pub fn parse_declared<'i, 't>(
                 context: &ParserContext,
                 input: &mut Parser<'i, 't>,
-            % if property.boxed:
-            ) -> Result<Box<SpecifiedValue>, ParseError<'i>> {
-            % else:
-            ) -> Result<SpecifiedValue, ParseError<'i>> {
-            % endif
+            ) -> Result<PropertyDeclaration, ParseError<'i>> {
                 % if property.allow_quirks:
                     parse_quirky(context, input, specified::AllowQuirks::Yes)
                 % else:
                     parse(context, input)
                 % endif
                 % if property.boxed:
                     .map(Box::new)
                 % endif
-            }
-
-            pub fn parse_declared<'i, 't>(
-                context: &ParserContext,
-                input: &mut Parser<'i, 't>,
-            ) -> Result<PropertyDeclaration, ParseError<'i>> {
-                parse_specified(context, input)
                     .map(PropertyDeclaration::${property.camel_case})
             }
         % endif
     }
 </%def>
 
 <%def name="single_keyword_system(name, values, **kwargs)">
     <%
@@ -933,16 +922,18 @@
         if ident == "float":
             ident = "float_"
         return "nsCSSPropertyID::eCSSProperty_%s" % ident
     %>
 </%def>
 
 // Define property that supports prefixed intrinsic size keyword values for gecko.
 // E.g. -moz-max-content, -moz-min-content, etc.
+//
+// FIXME(emilio): This feels a lot like a huge hack, get rid of this.
 <%def name="gecko_size_type(name, length_type, initial_value, logical, **kwargs)">
     <%call expr="longhand(name,
                           predefined_type=length_type,
                           logical=logical,
                           **kwargs)">
         % if not logical:
             use values::specified::AllowQuirks;
         % endif
@@ -977,30 +968,42 @@
         }
         % endif
 
         #[inline]
         pub fn get_initial_value() -> computed_value::T {
             use values::computed::${length_type};
             ${length_type}::${initial_value}
         }
-        fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
-                         -> Result<SpecifiedValue, ParseError<'i>> {
-            % if logical:
-            let ret = ${length_type}::parse(context, input);
-            % else:
-            let ret = ${length_type}::parse_quirky(context, input, AllowQuirks::Yes);
-            % endif
-            // Keyword values don't make sense in the block direction; don't parse them
-            % if "block" in name:
-                if let Ok(${length_type}::ExtremumLength(..)) = ret {
-                    return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
-                }
-            % endif
-            ret.map(SpecifiedValue)
+
+        impl Parse for SpecifiedValue {
+            fn parse<'i, 't>(
+                context: &ParserContext,
+                input: &mut Parser<'i, 't>,
+            ) -> Result<SpecifiedValue, ParseError<'i>> {
+                % if logical:
+                let ret = ${length_type}::parse(context, input);
+                % else:
+                let ret = ${length_type}::parse_quirky(context, input, AllowQuirks::Yes);
+                % endif
+                // Keyword values don't make sense in the block direction; don't parse them
+                % if "block" in name:
+                    if let Ok(${length_type}::ExtremumLength(..)) = ret {
+                        return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
+                    }
+                % endif
+                ret.map(SpecifiedValue)
+            }
+        }
+
+        fn parse<'i, 't>(
+            context: &ParserContext,
+            input: &mut Parser<'i, 't>,
+        ) -> Result<SpecifiedValue, ParseError<'i>> {
+            SpecifiedValue::parse(context, input)
         }
 
         impl ToComputedValue for SpecifiedValue {
             type ComputedValue = computed_value::T;
             #[inline]
             fn to_computed_value(&self, context: &Context) -> computed_value::T {
                 % if not logical or "block" in name:
                     use values::computed::${length_type};
--- a/servo/components/style/properties/longhand/position.mako.rs
+++ b/servo/components/style/properties/longhand/position.mako.rs
@@ -155,16 +155,21 @@ macro_rules! impl_align_conversions {
 % endif
 
 // https://drafts.csswg.org/css-flexbox/#propdef-order
 ${helpers.predefined_type("order", "Integer", "0",
                           extra_prefixes="webkit",
                           animation_value_type="ComputedValue",
                           spec="https://drafts.csswg.org/css-flexbox/#order-property")}
 
+// FIXME(emilio): All the sizes stuff, and the MozLength values should be
+// unified with Servo, or at least be less hacky.
+//
+// The block direction ones don't even accept extremum lengths during parsing,
+// and should be converted to just LengthOrPercentage.
 % if product == "gecko":
     // FIXME: Gecko doesn't support content value yet.
     ${helpers.gecko_size_type("flex-basis", "MozLength", "auto()",
                               logical=False,
                               spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property",
                               extra_prefixes="webkit",
                               animation_value_type="MozLength")}
 % else:
--- a/servo/components/style/properties/shorthand/position.mako.rs
+++ b/servo/components/style/properties/shorthand/position.mako.rs
@@ -43,16 +43,17 @@
 
 <%helpers:shorthand name="flex"
                     sub_properties="flex-grow flex-shrink flex-basis"
                     extra_prefixes="webkit"
                     derive_serialize="True"
                     spec="https://drafts.csswg.org/css-flexbox/#flex-property">
     use parser::Parse;
     use values::specified::NonNegativeNumber;
+    use properties::longhands::flex_basis::SpecifiedValue as FlexBasis;
 
     fn parse_flexibility<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                  -> Result<(NonNegativeNumber, Option<NonNegativeNumber>),ParseError<'i>> {
         let grow = NonNegativeNumber::parse(context, input)?;
         let shrink = input.try(|i| NonNegativeNumber::parse(context, i)).ok();
         Ok((grow, shrink))
     }
 
@@ -61,29 +62,29 @@
         let mut grow = None;
         let mut shrink = None;
         let mut basis = None;
 
         if input.try(|input| input.expect_ident_matching("none")).is_ok() {
             return Ok(expanded! {
                 flex_grow: NonNegativeNumber::new(0.0),
                 flex_shrink: NonNegativeNumber::new(0.0),
-                flex_basis: longhands::flex_basis::SpecifiedValue::auto(),
+                flex_basis: FlexBasis::auto(),
             })
         }
         loop {
             if grow.is_none() {
                 if let Ok((flex_grow, flex_shrink)) = input.try(|i| parse_flexibility(context, i)) {
                     grow = Some(flex_grow);
                     shrink = flex_shrink;
                     continue
                 }
             }
             if basis.is_none() {
-                if let Ok(value) = input.try(|input| longhands::flex_basis::parse_specified(context, input)) {
+                if let Ok(value) = input.try(|input| FlexBasis::parse(context, input)) {
                     basis = Some(value);
                     continue
                 }
             }
             break
         }
 
         if grow.is_none() && basis.is_none() {
@@ -91,17 +92,17 @@
         }
         Ok(expanded! {
             flex_grow: grow.unwrap_or(NonNegativeNumber::new(1.0)),
             flex_shrink: shrink.unwrap_or(NonNegativeNumber::new(1.0)),
             // Per spec, this should be SpecifiedValue::zero(), but all
             // browsers currently agree on using `0%`. This is a spec
             // change which hasn't been adopted by browsers:
             // https://github.com/w3c/csswg-drafts/commit/2c446befdf0f686217905bdd7c92409f6bd3921b
-            flex_basis: basis.unwrap_or(longhands::flex_basis::SpecifiedValue::zero_percent()),
+            flex_basis: basis.unwrap_or(FlexBasis::zero_percent()),
         })
     }
 </%helpers:shorthand>
 
 <%helpers:shorthand name="grid-gap" sub_properties="grid-row-gap grid-column-gap"
                     spec="https://drafts.csswg.org/css-grid/#propdef-grid-gap"
                     products="gecko">
   use properties::longhands::{grid_row_gap, grid_column_gap};