From 73d5b82f9f448d167a75eed2c4b32ef57c24211e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 14 Feb 2019 20:45:10 +0100 Subject: [PATCH] style: Improve #[derive(Parse)]. I want to do this so that I can get rid of Either<>. The reasons for getting rid of either are multiple: * It doesn't generate as nice C++ code using cbindgen. * It isn't that nice to use either from Rust. * cbindgen has bugs with zero-sized types. I started using this for ColorOrAuto and a few others, for now. Differential Revision: https://phabricator.services.mozilla.com/D19844 --- .../sugar/style_complex_color.rs | 14 +- .../properties/longhands/inherited_ui.mako.rs | 2 +- components/style/values/computed/color.rs | 5 +- components/style/values/computed/mod.rs | 4 +- components/style/values/computed/ui.rs | 4 - components/style/values/generics/color.rs | 23 +++ components/style/values/generics/column.rs | 1 + components/style/values/generics/flex.rs | 1 + components/style/values/generics/length.rs | 23 +-- components/style/values/generics/mod.rs | 40 +++-- components/style/values/generics/svg.rs | 36 ++-- components/style/values/generics/url.rs | 21 +-- components/style/values/mod.rs | 14 +- components/style/values/specified/color.rs | 5 +- components/style/values/specified/column.rs | 17 -- components/style/values/specified/flex.rs | 17 -- components/style/values/specified/font.rs | 66 ++------ components/style/values/specified/image.rs | 18 +- components/style/values/specified/mod.rs | 4 +- components/style/values/specified/svg.rs | 78 +-------- components/style/values/specified/text.rs | 23 +-- .../style/values/specified/transform.rs | 21 +-- components/style/values/specified/ui.rs | 4 - components/style_derive/parse.rs | 159 ++++++++++++++---- 24 files changed, 238 insertions(+), 362 deletions(-) diff --git a/components/style/gecko_bindings/sugar/style_complex_color.rs b/components/style/gecko_bindings/sugar/style_complex_color.rs index 4d31459a628..69565950b3e 100644 --- a/components/style/gecko_bindings/sugar/style_complex_color.rs +++ b/components/style/gecko_bindings/sugar/style_complex_color.rs @@ -7,10 +7,8 @@ use crate::gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor}; use crate::gecko_bindings::structs::StyleComplexColor; use crate::gecko_bindings::structs::StyleComplexColor_Tag as Tag; -use crate::values::computed::ui::ColorOrAuto; -use crate::values::computed::{Color as ComputedColor, RGBAColor as ComputedRGBA}; -use crate::values::generics::color::{Color as GenericColor, ComplexColorRatios}; -use crate::values::{Auto, Either}; +use crate::values::computed::{Color as ComputedColor, ColorOrAuto, RGBAColor as ComputedRGBA}; +use crate::values::generics::color::{Color as GenericColor, ColorOrAuto as GenericColorOrAuto, ComplexColorRatios}; impl StyleComplexColor { /// Create a `StyleComplexColor` value that represents `currentColor`. @@ -94,8 +92,8 @@ impl From for ComputedColor { impl From for StyleComplexColor { fn from(other: ColorOrAuto) -> Self { match other { - Either::First(color) => color.into(), - Either::Second(_) => StyleComplexColor::auto(), + GenericColorOrAuto::Color(color) => color.into(), + GenericColorOrAuto::Auto => StyleComplexColor::auto(), } } } @@ -103,9 +101,9 @@ impl From for StyleComplexColor { impl From for ColorOrAuto { fn from(other: StyleComplexColor) -> Self { if other.mTag != Tag::eAuto { - Either::First(other.into()) + GenericColorOrAuto::Color(other.into()) } else { - Either::Second(Auto) + GenericColorOrAuto::Auto } } } diff --git a/components/style/properties/longhands/inherited_ui.mako.rs b/components/style/properties/longhands/inherited_ui.mako.rs index 87c7cd9ac4b..94709e1a681 100644 --- a/components/style/properties/longhands/inherited_ui.mako.rs +++ b/components/style/properties/longhands/inherited_ui.mako.rs @@ -60,7 +60,7 @@ ${helpers.single_keyword( ${helpers.predefined_type( "caret-color", "ColorOrAuto", - "Either::Second(Auto)", + "generics::color::ColorOrAuto::Auto", spec="https://drafts.csswg.org/css-ui/#caret-color", animation_value_type="AnimatedCaretColor", ignored_when_colors_disabled=True, diff --git a/components/style/values/computed/color.rs b/components/style/values/computed/color.rs index 758d86be1b8..6795b399888 100644 --- a/components/style/values/computed/color.rs +++ b/components/style/values/computed/color.rs @@ -6,7 +6,7 @@ use crate::values::animated::color::RGBA as AnimatedRGBA; use crate::values::animated::ToAnimatedValue; -use crate::values::generics::color::Color as GenericColor; +use crate::values::generics::color::{Color as GenericColor, ColorOrAuto as GenericColorOrAuto}; use cssparser::{Color as CSSParserColor, RGBA}; use std::fmt; use style_traits::{CssWriter, ToCss}; @@ -101,3 +101,6 @@ impl ToAnimatedValue for RGBA { RGBA::from_floats(animated.red, animated.green, animated.blue, animated.alpha) } } + +/// auto | +pub type ColorOrAuto = GenericColorOrAuto; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index bc8dd8ab109..6fe36d684f2 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -45,7 +45,7 @@ pub use self::box_::{Appearance, BreakBetween, BreakWithin, Clear, Float}; pub use self::box_::{Display, Overflow, OverflowAnchor, TransitionProperty}; pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize}; pub use self::box_::{ScrollSnapAlign, ScrollSnapType, TouchAction, VerticalAlign, WillChange}; -pub use self::color::{Color, ColorPropertyValue, RGBAColor}; +pub use self::color::{Color, ColorOrAuto, ColorPropertyValue, RGBAColor}; pub use self::column::ColumnCount; pub use self::counters::{Content, ContentItem, CounterIncrement, CounterReset}; pub use self::easing::TimingFunction; @@ -85,7 +85,7 @@ pub use self::transform::{Rotate, Scale, Transform, TransformOperation}; pub use self::transform::{TransformOrigin, TransformStyle, Translate}; #[cfg(feature = "gecko")] pub use self::ui::CursorImage; -pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon, UserSelect}; +pub use self::ui::{Cursor, MozForceBrokenImageIcon, UserSelect}; pub use super::specified::{BorderStyle, TextDecorationLine}; pub use super::{Auto, Either, None_}; pub use app_units::Au; diff --git a/components/style/values/computed/ui.rs b/components/style/values/computed/ui.rs index ef19b975494..0c37ea4f518 100644 --- a/components/style/values/computed/ui.rs +++ b/components/style/values/computed/ui.rs @@ -8,14 +8,10 @@ use crate::values::computed::color::Color; use crate::values::computed::url::ComputedImageUrl; use crate::values::computed::Number; use crate::values::generics::ui as generics; -use crate::values::{Auto, Either}; pub use crate::values::specified::ui::CursorKind; pub use crate::values::specified::ui::{MozForceBrokenImageIcon, UserSelect}; -/// auto | -pub type ColorOrAuto = Either; - /// A computed value for the `cursor` property. pub type Cursor = generics::Cursor; diff --git a/components/style/values/generics/color.rs b/components/style/values/generics/color.rs index 594df927ca9..58e4e9fa97c 100644 --- a/components/style/values/generics/color.rs +++ b/components/style/values/generics/color.rs @@ -74,3 +74,26 @@ impl From for Color { Self::rgba(color) } } + +/// Either `` or `auto`. +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + PartialEq, + Parse, + SpecifiedValueInfo, + ToAnimatedValue, + ToAnimatedZero, + ToComputedValue, + ToCss, +)] +pub enum ColorOrAuto { + /// A ` + Color(C), + /// `auto` + Auto, +} diff --git a/components/style/values/generics/column.rs b/components/style/values/generics/column.rs index d27e47ed4c7..6f8c9daef5b 100644 --- a/components/style/values/generics/column.rs +++ b/components/style/values/generics/column.rs @@ -12,6 +12,7 @@ Copy, Debug, MallocSizeOf, + Parse, PartialEq, SpecifiedValueInfo, ToAnimatedValue, diff --git a/components/style/values/generics/flex.rs b/components/style/values/generics/flex.rs index 258247ec2a8..de67b56e35b 100644 --- a/components/style/values/generics/flex.rs +++ b/components/style/values/generics/flex.rs @@ -12,6 +12,7 @@ ComputeSquaredDistance, Copy, Debug, + Parse, PartialEq, SpecifiedValueInfo, ToAnimatedValue, diff --git a/components/style/values/generics/length.rs b/components/style/values/generics/length.rs index da7fe431dfe..1a83f58054c 100644 --- a/components/style/values/generics/length.rs +++ b/components/style/values/generics/length.rs @@ -165,6 +165,7 @@ impl MaxSize { Copy, Debug, MallocSizeOf, + Parse, PartialEq, SpecifiedValueInfo, ToAnimatedValue, @@ -174,29 +175,17 @@ impl MaxSize { )] #[repr(C, u8)] pub enum GenericLengthOrNumber { + /// A number. + /// + /// NOTE: Numbers need to be before lengths, in order to parse them + /// first, since `0` should be a number, not the `0px` length. + Number(N), /// A length. Length(L), - /// A number. - Number(N), } pub use self::GenericLengthOrNumber as LengthOrNumber; -impl Parse for LengthOrNumber { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(number) = input.try(|i| N::parse(context, i)) { - // Numbers need to be parsed first because `0` must be recognised - // as the number `0` and not the length `0px`. - return Ok(LengthOrNumber::Number(number)); - } - - Ok(LengthOrNumber::Length(L::parse(context, input)?)) - } -} - impl LengthOrNumber { /// Returns `0`. pub fn zero() -> Self diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index f23d04c99c2..5de31317935 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -119,27 +119,25 @@ impl Parse for CounterStyleOrNone { if input.try(|i| i.expect_ident_matching("none")).is_ok() { return Ok(CounterStyleOrNone::None); } - if input.try(|i| i.expect_function_matching("symbols")).is_ok() { - return input.parse_nested_block(|input| { - let symbols_type = input - .try(|i| SymbolsType::parse(i)) - .unwrap_or(SymbolsType::Symbolic); - let symbols = Symbols::parse(context, input)?; - // There must be at least two symbols for alphabetic or - // numeric system. - if (symbols_type == SymbolsType::Alphabetic || - symbols_type == SymbolsType::Numeric) && symbols.0.len() < 2 - { - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); - } - // Identifier is not allowed in symbols() function. - if symbols.0.iter().any(|sym| !sym.is_allowed_in_symbols()) { - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); - } - Ok(CounterStyleOrNone::Symbols(symbols_type, symbols)) - }); - } - Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + input.expect_function_matching("symbols")?; + input.parse_nested_block(|input| { + let symbols_type = input + .try(SymbolsType::parse) + .unwrap_or(SymbolsType::Symbolic); + let symbols = Symbols::parse(context, input)?; + // There must be at least two symbols for alphabetic or + // numeric system. + if (symbols_type == SymbolsType::Alphabetic || + symbols_type == SymbolsType::Numeric) && symbols.0.len() < 2 + { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + // Identifier is not allowed in symbols() function. + if symbols.0.iter().any(|sym| !sym.is_allowed_in_symbols()) { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + Ok(CounterStyleOrNone::Symbols(symbols_type, symbols)) + }) } } diff --git a/components/style/values/generics/svg.rs b/components/style/values/generics/svg.rs index 914de88999c..7f52126a748 100644 --- a/components/style/values/generics/svg.rs +++ b/components/style/values/generics/svg.rs @@ -136,6 +136,7 @@ impl Parse for SVGPaint Parse for SVGPaint { + /// + /// + /// Note that this needs to be before, so it gets parsed before the length, + /// to handle `0` correctly as a number instead of a ``. + Number(Number), /// | LengthPercentage(LengthPercentage), - /// - Number(Number), } -/// Parsing the SvgLengthPercentageOrNumber. At first, we need to parse number -/// since prevent converting to the length. -impl Parse - for SvgLengthPercentageOrNumber -{ - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(num) = input.try(|i| NumberType::parse(context, i)) { - return Ok(SvgLengthPercentageOrNumber::Number(num)); - } +/// Whether the `context-value` value is enabled. +#[cfg(feature = "gecko")] +pub fn is_context_value_enabled(_: &ParserContext) -> bool { + use crate::gecko_bindings::structs::mozilla; + unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled } +} - let lp = LengthPercentageType::parse(context, input)?; - Ok(SvgLengthPercentageOrNumber::LengthPercentage(lp)) - } +/// Whether the `context-value` value is enabled. +#[cfg(not(feature = "gecko"))] +pub fn is_context_value_enabled(_: &ParserContext) -> bool { + false } /// An SVG length value supports `context-value` in addition to length. @@ -175,6 +174,7 @@ impl Parse Debug, MallocSizeOf, PartialEq, + Parse, SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, @@ -185,6 +185,7 @@ pub enum SVGLength { /// ` | | ` Length(LengthType), /// `context-value` + #[parse(condition = "is_context_value_enabled")] ContextValue, } @@ -216,6 +217,7 @@ pub enum SVGStrokeDashArray { Debug, MallocSizeOf, PartialEq, + Parse, SpecifiedValueInfo, ToAnimatedZero, ToComputedValue, diff --git a/components/style/values/generics/url.rs b/components/style/values/generics/url.rs index a1f1aeb1884..80fa48ba882 100644 --- a/components/style/values/generics/url.rs +++ b/components/style/values/generics/url.rs @@ -4,10 +4,6 @@ //! Generic types for url properties. -use crate::parser::{Parse, ParserContext}; -use cssparser::Parser; -use style_traits::ParseError; - /// An image url or none, used for example in list-style-image #[derive( Animate, @@ -16,6 +12,7 @@ use style_traits::ParseError; Debug, MallocSizeOf, PartialEq, + Parse, SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, @@ -35,19 +32,3 @@ impl UrlOrNone { UrlOrNone::None } } - -impl Parse for UrlOrNone -where - Url: Parse, -{ - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result, ParseError<'i>> { - if let Ok(url) = input.try(|input| Url::parse(context, input)) { - return Ok(UrlOrNone::Url(url)); - } - input.expect_ident_matching("none")?; - Ok(UrlOrNone::None) - } -} diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index 659ff95b1a7..ccda0f290c1 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -125,6 +125,7 @@ impl Parse for Impossible { Copy, MallocSizeOf, PartialEq, + Parse, SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, @@ -147,19 +148,6 @@ impl Debug for Either { } } -impl Parse for Either { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result, ParseError<'i>> { - if let Ok(v) = input.try(|i| A::parse(context, i)) { - Ok(Either::First(v)) - } else { - B::parse(context, input).map(Either::Second) - } - } -} - /// #[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue)] pub struct CustomIdent(pub Atom); diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index 97b450362b3..2ee159987f8 100644 --- a/components/style/values/specified/color.rs +++ b/components/style/values/specified/color.rs @@ -11,7 +11,7 @@ use crate::parser::{Parse, ParserContext}; #[cfg(feature = "gecko")] use crate::properties::longhands::system_colors::SystemColor; use crate::values::computed::{Color as ComputedColor, Context, ToComputedValue}; -use crate::values::generics::color::Color as GenericColor; +use crate::values::generics::color::{Color as GenericColor, ColorOrAuto as GenericColorOrAuto}; use crate::values::specified::calc::CalcNode; use cssparser::{AngleOrNumber, Color as CSSParserColor, Parser, Token, RGBA}; use cssparser::{BasicParseErrorKind, NumberOrPercentage, ParseErrorKind}; @@ -469,3 +469,6 @@ impl Parse for ColorPropertyValue { Color::parse_quirky(context, input, AllowQuirks::Yes).map(ColorPropertyValue) } } + +/// auto | +pub type ColorOrAuto = GenericColorOrAuto; diff --git a/components/style/values/specified/column.rs b/components/style/values/specified/column.rs index 5d612de437a..2dd7bb0144d 100644 --- a/components/style/values/specified/column.rs +++ b/components/style/values/specified/column.rs @@ -4,25 +4,8 @@ //! Specified types for the column properties. -use crate::parser::{Parse, ParserContext}; use crate::values::generics::column::ColumnCount as GenericColumnCount; use crate::values::specified::PositiveInteger; -use cssparser::Parser; -use style_traits::ParseError; /// A specified type for `column-count` values. pub type ColumnCount = GenericColumnCount; - -impl Parse for ColumnCount { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if input.try(|i| i.expect_ident_matching("auto")).is_ok() { - return Ok(GenericColumnCount::Auto); - } - Ok(GenericColumnCount::Integer(PositiveInteger::parse( - context, input, - )?)) - } -} diff --git a/components/style/values/specified/flex.rs b/components/style/values/specified/flex.rs index 4c5bf0efb4b..7c767cdf34b 100644 --- a/components/style/values/specified/flex.rs +++ b/components/style/values/specified/flex.rs @@ -4,29 +4,12 @@ //! Specified types for CSS values related to flexbox. -use crate::parser::{Parse, ParserContext}; use crate::values::generics::flex::FlexBasis as GenericFlexBasis; use crate::values::specified::Size; -use cssparser::Parser; -use style_traits::ParseError; /// A specified value for the `flex-basis` property. pub type FlexBasis = GenericFlexBasis; -impl Parse for FlexBasis { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(size) = input.try(|i| Size::parse(context, i)) { - return Ok(GenericFlexBasis::Size(size)); - } - try_match_ident_ignore_ascii_case! { input, - "content" => Ok(GenericFlexBasis::Content), - } - } -} - impl FlexBasis { /// `auto` #[inline] diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index 5d428656d40..57725294808 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -17,7 +17,7 @@ use crate::values::generics::font::{KeywordSize, VariationValue}; use crate::values::generics::NonNegative; use crate::values::specified::length::{FontBaseSize, AU_PER_PT, AU_PER_PX}; use crate::values::specified::{AllowQuirks, Angle, Integer, LengthPercentage}; -use crate::values::specified::{NoCalcLength, Number, Percentage}; +use crate::values::specified::{NoCalcLength, Number, NonNegativeNumber, Percentage}; use crate::values::CustomIdent; use crate::Atom; use app_units::Au; @@ -81,7 +81,7 @@ pub const MAX_FONT_WEIGHT: f32 = 1000.; /// A specified font-weight value. /// /// https://drafts.csswg.org/css-fonts-4/#propdef-font-weight -#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)] +#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss)] pub enum FontWeight { /// `` Absolute(AbsoluteFontWeight), @@ -111,22 +111,6 @@ impl FontWeight { } } -impl Parse for FontWeight { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(absolute) = input.try(|input| AbsoluteFontWeight::parse(context, input)) { - return Ok(FontWeight::Absolute(absolute)); - } - - Ok(try_match_ident_ignore_ascii_case! { input, - "bolder" => FontWeight::Bolder, - "lighter" => FontWeight::Lighter, - }) - } -} - impl ToComputedValue for FontWeight { type ComputedValue = computed::FontWeight; @@ -335,7 +319,7 @@ impl SpecifiedFontStyle { } /// The specified value of the `font-style` property. -#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)] +#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss)] #[allow(missing_docs)] pub enum FontStyle { Specified(SpecifiedFontStyle), @@ -368,20 +352,11 @@ impl ToComputedValue for FontStyle { } } -impl Parse for FontStyle { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - Ok(FontStyle::Specified(SpecifiedFontStyle::parse( - context, input, - )?)) - } -} - /// A value for the `font-stretch` property. /// /// https://drafts.csswg.org/css-fonts-4/#font-stretch-prop +/// +/// TODO(emilio): We could derive Parse if we had NonNegativePercentage. #[allow(missing_docs)] #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)] #[repr(u8)] @@ -628,13 +603,13 @@ impl Parse for FamilyName { } } -#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)] +#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss)] /// Preserve the readability of text when font fallback occurs pub enum FontSizeAdjust { /// None variant None, /// Number variant - Number(Number), + Number(NonNegativeNumber), /// system font #[css(skip)] System(SystemFont), @@ -657,7 +632,9 @@ impl ToComputedValue for FontSizeAdjust { match *self { FontSizeAdjust::None => computed::FontSizeAdjust::None, FontSizeAdjust::Number(ref n) => { - computed::FontSizeAdjust::Number(n.to_computed_value(context)) + // The computed version handles clamping of animated values + // itself. + computed::FontSizeAdjust::Number(n.to_computed_value(context).0) }, FontSizeAdjust::System(_) => self.compute_system(context), } @@ -666,32 +643,13 @@ impl ToComputedValue for FontSizeAdjust { fn from_computed_value(computed: &computed::FontSizeAdjust) -> Self { match *computed { computed::FontSizeAdjust::None => FontSizeAdjust::None, - computed::FontSizeAdjust::Number(ref v) => { - FontSizeAdjust::Number(Number::from_computed_value(v)) + computed::FontSizeAdjust::Number(v) => { + FontSizeAdjust::Number(NonNegativeNumber::from_computed_value(&v.into())) }, } } } -impl Parse for FontSizeAdjust { - /// none | - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if input - .try(|input| input.expect_ident_matching("none")) - .is_ok() - { - return Ok(FontSizeAdjust::None); - } - - Ok(FontSizeAdjust::Number(Number::parse_non_negative( - context, input, - )?)) - } -} - /// Additional information for specified keyword-derived font sizes. pub type KeywordInfo = generics::KeywordInfo; diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 2dc56f8ad3f..c9d0f10215d 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -305,7 +305,7 @@ impl Gradient { ) -> Result> { type Point = GenericPosition, Component>; - #[derive(Clone, Copy)] + #[derive(Clone, Copy, Parse)] enum Component { Center, Number(NumberOrPercentage), @@ -408,22 +408,6 @@ impl Gradient { } } - impl Parse for Component { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(side) = input.try(|i| S::parse(context, i)) { - return Ok(Component::Side(side)); - } - if let Ok(number) = input.try(|i| NumberOrPercentage::parse(context, i)) { - return Ok(Component::Number(number)); - } - input.try(|i| i.expect_ident_matching("center"))?; - Ok(Component::Center) - } - } - let ident = input.expect_ident_cloned()?; input.expect_comma()?; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 16c9e268330..1896235bf61 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -42,7 +42,7 @@ pub use self::box_::{Clear, Float, Overflow, OverflowAnchor}; pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize}; pub use self::box_::{ScrollSnapAlign, ScrollSnapType}; pub use self::box_::{TouchAction, TransitionProperty, VerticalAlign, WillChange}; -pub use self::color::{Color, ColorPropertyValue, RGBAColor}; +pub use self::color::{Color, ColorOrAuto, ColorPropertyValue, RGBAColor}; pub use self::column::ColumnCount; pub use self::counters::{Content, ContentItem, CounterIncrement, CounterReset}; pub use self::easing::TimingFunction; @@ -87,7 +87,7 @@ pub use self::transform::{Rotate, Scale, Transform}; pub use self::transform::{TransformOrigin, TransformStyle, Translate}; #[cfg(feature = "gecko")] pub use self::ui::CursorImage; -pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon, UserSelect}; +pub use self::ui::{Cursor, MozForceBrokenImageIcon, UserSelect}; pub use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent; #[cfg(feature = "gecko")] diff --git a/components/style/values/specified/svg.rs b/components/style/values/specified/svg.rs index 9e9c435ab9d..d2d8dd13798 100644 --- a/components/style/values/specified/svg.rs +++ b/components/style/values/specified/svg.rs @@ -23,31 +23,6 @@ pub type SVGPaint = generic::SVGPaint; /// Specified SVG Paint Kind value pub type SVGPaintKind = generic::SVGPaintKind; -#[cfg(feature = "gecko")] -fn is_context_value_enabled() -> bool { - // The prefs can only be mutated on the main thread, so it is safe - // to read whenever we are on the main thread or the main thread is - // blocked. - use crate::gecko_bindings::structs::mozilla; - unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled } -} -#[cfg(not(feature = "gecko"))] -fn is_context_value_enabled() -> bool { - false -} - -fn parse_context_value<'i, 't, T>( - input: &mut Parser<'i, 't>, - value: T, -) -> Result> { - if !is_context_value_enabled() { - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); - } - - input.expect_ident_matching("context-value")?; - Ok(value) -} - /// A value of | | for stroke-dashoffset. /// pub type SvgLengthPercentageOrNumber = @@ -56,18 +31,6 @@ pub type SvgLengthPercentageOrNumber = /// | | | context-value pub type SVGLength = generic::SVGLength; -impl Parse for SVGLength { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - input - .try(|i| SvgLengthPercentageOrNumber::parse(context, i)) - .map(Into::into) - .or_else(|_| parse_context_value(input, generic::SVGLength::ContextValue)) - } -} - impl From for SVGLength { fn from(length: SvgLengthPercentageOrNumber) -> Self { generic::SVGLength::Length(length) @@ -82,18 +45,6 @@ pub type NonNegativeSvgLengthPercentageOrNumber = /// A non-negative version of SVGLength. pub type SVGWidth = generic::SVGLength; -impl Parse for SVGWidth { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - input - .try(|i| NonNegativeSvgLengthPercentageOrNumber::parse(context, i)) - .map(Into::into) - .or_else(|_| parse_context_value(input, generic::SVGLength::ContextValue)) - } -} - impl From for SVGWidth { fn from(length: NonNegativeSvgLengthPercentageOrNumber) -> Self { generic::SVGLength::Length(length) @@ -113,11 +64,14 @@ impl Parse for SVGStrokeDashArray { NonNegativeSvgLengthPercentageOrNumber::parse(context, i) }) }) { - Ok(generic::SVGStrokeDashArray::Values(values)) - } else if let Ok(_) = input.try(|i| i.expect_ident_matching("none")) { - Ok(generic::SVGStrokeDashArray::Values(vec![])) - } else { - parse_context_value(input, generic::SVGStrokeDashArray::ContextValue) + return Ok(generic::SVGStrokeDashArray::Values(values)) + } + + try_match_ident_ignore_ascii_case! { input, + "context-value" if generic::is_context_value_enabled(context) => { + Ok(generic::SVGStrokeDashArray::ContextValue) + }, + "none" => Ok(generic::SVGStrokeDashArray::Values(vec![])), } } } @@ -125,22 +79,6 @@ impl Parse for SVGStrokeDashArray { /// | context-fill-opacity | context-stroke-opacity pub type SVGOpacity = generic::SVGOpacity; -impl Parse for SVGOpacity { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(opacity) = input.try(|i| Opacity::parse(context, i)) { - return Ok(generic::SVGOpacity::Opacity(opacity)); - } - - try_match_ident_ignore_ascii_case! { input, - "context-fill-opacity" => Ok(generic::SVGOpacity::ContextFillOpacity), - "context-stroke-opacity" => Ok(generic::SVGOpacity::ContextStrokeOpacity), - } - } -} - /// The specified value for a single CSS paint-order property. #[repr(u8)] #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, ToCss)] diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 2094630a5a7..60f70537878 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -419,7 +419,7 @@ pub enum TextAlignKeyword { /// Specified value of text-align property. #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, SpecifiedValueInfo, ToCss)] +#[derive(Clone, Copy, Debug, Eq, Hash, Parse, PartialEq, SpecifiedValueInfo, ToCss)] pub enum TextAlign { /// Keyword value of text-align property. Keyword(TextAlignKeyword), @@ -435,27 +435,6 @@ pub enum TextAlign { MozCenterOrInherit, } -impl Parse for TextAlign { - fn parse<'i, 't>( - _context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - // MozCenterOrInherit cannot be parsed, only set directly on the elements - if let Ok(key) = input.try(TextAlignKeyword::parse) { - return Ok(TextAlign::Keyword(key)); - } - #[cfg(feature = "gecko")] - { - input.expect_ident_matching("match-parent")?; - return Ok(TextAlign::MatchParent); - } - #[cfg(feature = "servo")] - { - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); - } - } -} - impl TextAlign { /// Convert an enumerated value coming from Gecko to a `TextAlign`. #[cfg(feature = "gecko")] diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index 859cbd80e2a..77c92c36654 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -231,7 +231,7 @@ impl Parse for Transform { } /// The specified value of a component of a CSS ``. -#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)] +#[derive(Clone, Debug, MallocSizeOf, PartialEq, Parse, SpecifiedValueInfo, ToCss)] pub enum OriginComponent { /// `center` Center, @@ -295,25 +295,6 @@ impl Parse for TransformOrigin { } } -impl Parse for OriginComponent -where - S: Parse, -{ - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if input.try(|i| i.expect_ident_matching("center")).is_ok() { - return Ok(OriginComponent::Center); - } - if let Ok(lp) = input.try(|i| LengthPercentage::parse(context, i)) { - return Ok(OriginComponent::Length(lp)); - } - let keyword = S::parse(context, input)?; - Ok(OriginComponent::Side(keyword)) - } -} - impl ToComputedValue for OriginComponent where S: Side, diff --git a/components/style/values/specified/ui.rs b/components/style/values/specified/ui.rs index 2290ac06d6d..0ccca5d359e 100644 --- a/components/style/values/specified/ui.rs +++ b/components/style/values/specified/ui.rs @@ -9,14 +9,10 @@ use crate::values::generics::ui as generics; use crate::values::specified::color::Color; use crate::values::specified::url::SpecifiedImageUrl; use crate::values::specified::Number; -use crate::values::{Auto, Either}; use cssparser::Parser; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; -/// auto | -pub type ColorOrAuto = Either; - /// A specified value for the `cursor` property. pub type Cursor = generics::Cursor; diff --git a/components/style_derive/parse.rs b/components/style_derive/parse.rs index 0f68f1df5ff..6dc7de21890 100644 --- a/components/style_derive/parse.rs +++ b/components/style_derive/parse.rs @@ -5,8 +5,8 @@ use crate::cg; use crate::to_css::CssVariantAttrs; use proc_macro2::TokenStream; -use syn::{DeriveInput, Path}; -use synstructure; +use syn::{self, DeriveInput, Path}; +use synstructure::{Structure, VariantInfo}; #[darling(attributes(parse), default)] #[derive(Default, FromVariant)] @@ -15,22 +15,88 @@ pub struct ParseVariantAttrs { pub condition: Option, } -pub fn derive(input: DeriveInput) -> TokenStream { +fn parse_non_keyword_variant( + name: &syn::Ident, + variant: &VariantInfo, + variant_attrs: &CssVariantAttrs, + parse_attrs: &ParseVariantAttrs, + skip_try: bool, +) -> TokenStream { + let bindings = variant.bindings(); + assert!(parse_attrs.aliases.is_none()); + assert!(variant_attrs.function.is_none()); + assert!(variant_attrs.keyword.is_none()); + assert_eq!( + bindings.len(), + 1, + "We only support deriving parse for simple variants" + ); + let variant_name = &variant.ast().ident; + let ty = &bindings[0].ast().ty; + let mut parse = if skip_try { + quote! { + let v = <#ty as crate::parser::Parse>::parse(context, input)?; + return Ok(#name::#variant_name(v)); + } + } else { + quote! { + if let Ok(v) = input.try(|i| <#ty as crate::parser::Parse>::parse(context, i)) { + return Ok(#name::#variant_name(v)); + } + } + }; + + if let Some(ref condition) = parse_attrs.condition { + parse = quote! { + if #condition(context) { + #parse + } + }; + + if skip_try { + // We're the last variant and we can fail to parse due to the + // condition clause. If that happens, we need to return an error. + parse = quote! { + #parse + Err(input.new_custom_error(style_traits::StyleParseErrorKind::UnspecifiedError)) + }; + } + } + + parse +} + +pub fn derive(mut input: DeriveInput) -> TokenStream { + { + 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: crate::parser::Parse)); + } + input.generics.where_clause = where_clause; + } + + let name = &input.ident; - let s = synstructure::Structure::new(&input); + let s = Structure::new(&input); let mut saw_condition = false; - let match_body = s.variants().iter().fold(quote!(), |match_body, variant| { - let bindings = variant.bindings(); - assert!( - bindings.is_empty(), - "Parse is only supported for single-variant enums for now" - ); + let mut match_keywords = quote! {}; + let mut non_keywords = vec![]; + let mut effective_variants = 0; + for variant in s.variants().iter() { let css_variant_attrs = cg::parse_variant_attrs_from_ast::(&variant.ast()); let parse_attrs = cg::parse_variant_attrs_from_ast::(&variant.ast()); if css_variant_attrs.skip { - return match_body; + continue; + } + effective_variants += 1; + + saw_condition |= parse_attrs.condition.is_some(); + + if !variant.bindings().is_empty() { + non_keywords.push((variant, css_variant_attrs, parse_attrs)); + continue; } let identifier = cg::to_css_identifier( @@ -40,55 +106,78 @@ pub fn derive(input: DeriveInput) -> TokenStream { ); let ident = &variant.ast().ident; - saw_condition |= parse_attrs.condition.is_some(); let condition = match parse_attrs.condition { Some(ref p) => quote! { if #p(context) }, None => quote! {}, }; - let mut body = quote! { - #match_body + match_keywords.extend(quote! { #identifier #condition => Ok(#name::#ident), - }; + }); let aliases = match parse_attrs.aliases { Some(aliases) => aliases, - None => return body, + None => continue, }; for alias in aliases.split(',') { - body = quote! { - #body + match_keywords.extend(quote! { #alias #condition => Ok(#name::#ident), - }; + }); } + } - body - }); + let needs_context = saw_condition || !non_keywords.is_empty(); - let context_ident = if saw_condition { + let context_ident = if needs_context { quote! { context } } else { quote! { _ } }; - let parse_body = if saw_condition { - quote! { - let location = input.current_source_location(); - let ident = input.expect_ident()?; - match_ignore_ascii_case! { &ident, - #match_body - _ => Err(location.new_unexpected_token_error( - cssparser::Token::Ident(ident.clone()) - )) + let has_keywords = non_keywords.len() != effective_variants; + + let mut parse_non_keywords = quote! {}; + for (i, (variant, css_attrs, parse_attrs)) in non_keywords.iter().enumerate() { + let skip_try = !has_keywords && i == non_keywords.len() - 1; + let parse_variant = parse_non_keyword_variant( + name, + variant, + css_attrs, + parse_attrs, + skip_try, + ); + parse_non_keywords.extend(parse_variant); + } + + let parse_body = if needs_context { + let parse_keywords = if has_keywords { + quote! { + let location = input.current_source_location(); + let ident = input.expect_ident()?; + match_ignore_ascii_case! { &ident, + #match_keywords + _ => Err(location.new_unexpected_token_error( + cssparser::Token::Ident(ident.clone()) + )) + } } + } else { + quote! {} + }; + + quote! { + #parse_non_keywords + #parse_keywords } } else { quote! { Self::parse(input) } }; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + let parse_trait_impl = quote! { - impl crate::parser::Parse for #name { + impl #impl_generics crate::parser::Parse for #name #ty_generics #where_clause { #[inline] fn parse<'i, 't>( #context_ident: &crate::parser::ParserContext, @@ -99,10 +188,12 @@ pub fn derive(input: DeriveInput) -> TokenStream { } }; - if saw_condition { + if needs_context { return parse_trait_impl; } + assert!(non_keywords.is_empty()); + // TODO(emilio): It'd be nice to get rid of these, but that makes the // conversion harder... let methods_impl = quote! { @@ -125,7 +216,7 @@ pub fn derive(input: DeriveInput) -> TokenStream { #[inline] pub fn from_ident(ident: &str) -> Result { match_ignore_ascii_case! { ident, - #match_body + #match_keywords _ => Err(()), } }