Auto merge of #18355 - servo:no-fmt, r=emilio

Reduce usage of fmt in serialization and error reporting

`format!` and `write!` create a somewhat-heavyweight `Formatting` struct and use dynamic dispatch to call into impls of `Dispaly` and related traits. The former also allocates an intermediate string that is sometimes unnecessary.

I started looking into this from https://bugzilla.mozilla.org/show_bug.cgi?id=1355599, but I expect the impact there will be small to insignificant. It might be a slightly less so on parsing (error reporting).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18355)
<!-- Reviewable:end -->

components/script_layout_interface/reporter.rs

@@ -31,7 +31,7 @@ impl ParseErrorReporter for CSSErrorReporter {
                   url.as_str(),
                   location.line,
                   location.column,
-                  error.to_string())
+                  error)
         }
 
         //TODO: report a real filename

components/style/counter_style/mod.rs

@@ -327,7 +327,8 @@ impl ToCss for System {
             System::Additive => dest.write_str("additive"),
             System::Fixed { first_symbol_value } => {
                 if let Some(value) = first_symbol_value {
-                    write!(dest, "fixed {}", value)
+                    dest.write_str("fixed ")?;
+                    value.to_css(dest)
                 } else {
                     dest.write_str("fixed")
                 }
@@ -455,7 +456,7 @@ where W: fmt::Write {
 
 fn bound_to_css<W>(range: Option<i32>, dest: &mut W) -> fmt::Result where W: fmt::Write {
     if let Some(finite) = range {
-        write!(dest, "{}", finite)
+        finite.to_css(dest)
     } else {
         dest.write_str("infinite")
     }

components/style/error_reporting.rs

@@ -9,6 +9,7 @@
 use cssparser::{BasicParseError, Token, SourceLocation};
 use cssparser::ParseError as CssParseError;
 use log;
+use std::fmt;
 use style_traits::ParseError;
 use stylesheets::UrlExtraData;
 
@@ -46,104 +47,127 @@ pub enum ContextualParseError<'a> {
     InvalidCounterStyleExtendsWithAdditiveSymbols
 }
 
-impl<'a> ContextualParseError<'a> {
+impl<'a> fmt::Display for ContextualParseError<'a> {
-    /// Turn a parse error into a string representation.
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    pub fn to_string(&self) -> String {
+        fn token_to_str(t: &Token, f: &mut fmt::Formatter) -> fmt::Result {
-        fn token_to_str(t: &Token) -> String {
             match *t {
-                Token::Ident(ref i) => format!("identifier {}", i),
+                Token::Ident(ref i) => write!(f, "identifier {}", i),
-                Token::AtKeyword(ref kw) => format!("keyword @{}", kw),
+                Token::AtKeyword(ref kw) => write!(f, "keyword @{}", kw),
-                Token::Hash(ref h) => format!("hash #{}", h),
+                Token::Hash(ref h) => write!(f, "hash #{}", h),
-                Token::IDHash(ref h) => format!("id selector #{}", h),
+                Token::IDHash(ref h) => write!(f, "id selector #{}", h),
-                Token::QuotedString(ref s) => format!("quoted string \"{}\"", s),
+                Token::QuotedString(ref s) => write!(f, "quoted string \"{}\"", s),
-                Token::UnquotedUrl(ref u) => format!("url {}", u),
+                Token::UnquotedUrl(ref u) => write!(f, "url {}", u),
-                Token::Delim(ref d) => format!("delimiter {}", d),
+                Token::Delim(ref d) => write!(f, "delimiter {}", d),
-                Token::Number { int_value: Some(i), .. } => format!("number {}", i),
+                Token::Number { int_value: Some(i), .. } => write!(f, "number {}", i),
-                Token::Number { value, .. } => format!("number {}", value),
+                Token::Number { value, .. } => write!(f, "number {}", value),
-                Token::Percentage { int_value: Some(i), .. } => format!("percentage {}", i),
+                Token::Percentage { int_value: Some(i), .. } => write!(f, "percentage {}", i),
-                Token::Percentage { unit_value, .. } => format!("percentage {}", unit_value * 100.),
+                Token::Percentage { unit_value, .. } => write!(f, "percentage {}", unit_value * 100.),
-                Token::Dimension { value, ref unit, .. } => format!("dimension {}{}", value, unit),
+                Token::Dimension { value, ref unit, .. } => write!(f, "dimension {}{}", value, unit),
-                Token::WhiteSpace(_) => format!("whitespace"),
+                Token::WhiteSpace(_) => write!(f, "whitespace"),
-                Token::Comment(_) => format!("comment"),
+                Token::Comment(_) => write!(f, "comment"),
-                Token::Colon => format!("colon (:)"),
+                Token::Colon => write!(f, "colon (:)"),
-                Token::Semicolon => format!("semicolon (;)"),
+                Token::Semicolon => write!(f, "semicolon (;)"),
-                Token::Comma => format!("comma (,)"),
+                Token::Comma => write!(f, "comma (,)"),
-                Token::IncludeMatch => format!("include match (~=)"),
+                Token::IncludeMatch => write!(f, "include match (~=)"),
-                Token::DashMatch => format!("dash match (|=)"),
+                Token::DashMatch => write!(f, "dash match (|=)"),
-                Token::PrefixMatch => format!("prefix match (^=)"),
+                Token::PrefixMatch => write!(f, "prefix match (^=)"),
-                Token::SuffixMatch => format!("suffix match ($=)"),
+                Token::SuffixMatch => write!(f, "suffix match ($=)"),
-                Token::SubstringMatch => format!("substring match (*=)"),
+                Token::SubstringMatch => write!(f, "substring match (*=)"),
-                Token::Column => format!("column (||)"),
+                Token::Column => write!(f, "column (||)"),
-                Token::CDO => format!("CDO (<!--)"),
+                Token::CDO => write!(f, "CDO (<!--)"),
-                Token::CDC => format!("CDC (-->)"),
+                Token::CDC => write!(f, "CDC (-->)"),
-                Token::Function(ref f) => format!("function {}", f),
+                Token::Function(ref name) => write!(f, "function {}", name),
-                Token::ParenthesisBlock => format!("parenthesis ("),
+                Token::ParenthesisBlock => write!(f, "parenthesis ("),
-                Token::SquareBracketBlock => format!("square bracket ["),
+                Token::SquareBracketBlock => write!(f, "square bracket ["),
-                Token::CurlyBracketBlock => format!("curly bracket {{"),
+                Token::CurlyBracketBlock => write!(f, "curly bracket {{"),
-                Token::BadUrl(ref _u) => format!("bad url parse error"),
+                Token::BadUrl(ref _u) => write!(f, "bad url parse error"),
-                Token::BadString(ref _s) => format!("bad string parse error"),
+                Token::BadString(ref _s) => write!(f, "bad string parse error"),
-                Token::CloseParenthesis => format!("unmatched close parenthesis"),
+                Token::CloseParenthesis => write!(f, "unmatched close parenthesis"),
-                Token::CloseSquareBracket => format!("unmatched close square bracket"),
+                Token::CloseSquareBracket => write!(f, "unmatched close square bracket"),
-                Token::CloseCurlyBracket => format!("unmatched close curly bracket"),
+                Token::CloseCurlyBracket => write!(f, "unmatched close curly bracket"),
             }
         }
 
-        fn parse_error_to_str(err: &ParseError) -> String {
+        fn parse_error_to_str(err: &ParseError, f: &mut fmt::Formatter) -> fmt::Result {
             match *err {
-                CssParseError::Basic(BasicParseError::UnexpectedToken(ref t)) =>
+                CssParseError::Basic(BasicParseError::UnexpectedToken(ref t)) => {
-                    format!("found unexpected {}", token_to_str(t)),
+                    write!(f, "found unexpected ")?;
-                CssParseError::Basic(BasicParseError::EndOfInput) =>
+                    token_to_str(t, f)
-                    format!("unexpected end of input"),
+                }
-                CssParseError::Basic(BasicParseError::AtRuleInvalid(ref i)) =>
+                CssParseError::Basic(BasicParseError::EndOfInput) => {
-                    format!("@ rule invalid: {}", i),
+                    write!(f, "unexpected end of input")
-                CssParseError::Basic(BasicParseError::AtRuleBodyInvalid) =>
+                }
-                    format!("@ rule invalid"),
+                CssParseError::Basic(BasicParseError::AtRuleInvalid(ref i)) => {
-                CssParseError::Basic(BasicParseError::QualifiedRuleInvalid) =>
+                    write!(f, "@ rule invalid: {}", i)
-                    format!("qualified rule invalid"),
+                }
-                CssParseError::Custom(ref err) =>
+                CssParseError::Basic(BasicParseError::AtRuleBodyInvalid) => {
-                    format!("{:?}", err)
+                    write!(f, "@ rule invalid")
+                }
+                CssParseError::Basic(BasicParseError::QualifiedRuleInvalid) => {
+                    write!(f, "qualified rule invalid")
+                }
+                CssParseError::Custom(ref err) => {
+                    write!(f, "{:?}", err)
+                }
             }
         }
 
         match *self {
-            ContextualParseError::UnsupportedPropertyDeclaration(decl, ref err) =>
+            ContextualParseError::UnsupportedPropertyDeclaration(decl, ref err) => {
-                format!("Unsupported property declaration: '{}', {}", decl,
+                write!(f, "Unsupported property declaration: '{}', ", decl)?;
-                        parse_error_to_str(err)),
+                parse_error_to_str(err, f)
-            ContextualParseError::UnsupportedFontFaceDescriptor(decl, ref err) =>
+            }
-                format!("Unsupported @font-face descriptor declaration: '{}', {}", decl,
+            ContextualParseError::UnsupportedFontFaceDescriptor(decl, ref err) => {
-                        parse_error_to_str(err)),
+                write!(f, "Unsupported @font-face descriptor declaration: '{}', ", decl)?;
-            ContextualParseError::UnsupportedFontFeatureValuesDescriptor(decl, ref err) =>
+                parse_error_to_str(err, f)
-            format!("Unsupported @font-feature-values descriptor declaration: '{}', {}", decl,
+            }
-                    parse_error_to_str(err)),
+            ContextualParseError::UnsupportedFontFeatureValuesDescriptor(decl, ref err) => {
-            ContextualParseError::InvalidKeyframeRule(rule, ref err) =>
+                write!(f, "Unsupported @font-feature-values descriptor declaration: '{}', ", decl)?;
-                format!("Invalid keyframe rule: '{}', {}", rule,
+                parse_error_to_str(err, f)
-                        parse_error_to_str(err)),
+            }
-            ContextualParseError::InvalidFontFeatureValuesRule(rule, ref err) =>
+            ContextualParseError::InvalidKeyframeRule(rule, ref err) => {
-            format!("Invalid font feature value rule: '{}', {}", rule,
+                write!(f, "Invalid keyframe rule: '{}', ", rule)?;
-                    parse_error_to_str(err)),
+                parse_error_to_str(err, f)
-            ContextualParseError::UnsupportedKeyframePropertyDeclaration(decl, ref err) =>
+            }
-                format!("Unsupported keyframe property declaration: '{}', {}", decl,
+            ContextualParseError::InvalidFontFeatureValuesRule(rule, ref err) => {
-                        parse_error_to_str(err)),
+                write!(f, "Invalid font feature value rule: '{}', ", rule)?;
-            ContextualParseError::InvalidRule(rule, ref err) =>
+                parse_error_to_str(err, f)
-                format!("Invalid rule: '{}', {}", rule, parse_error_to_str(err)),
+            }
-            ContextualParseError::UnsupportedRule(rule, ref err) =>
+            ContextualParseError::UnsupportedKeyframePropertyDeclaration(decl, ref err) => {
-                format!("Unsupported rule: '{}', {}", rule, parse_error_to_str(err)),
+                write!(f, "Unsupported keyframe property declaration: '{}', ", decl)?;
-            ContextualParseError::UnsupportedViewportDescriptorDeclaration(decl, ref err) =>
+                parse_error_to_str(err, f)
-                format!("Unsupported @viewport descriptor declaration: '{}', {}", decl,
+            }
-                        parse_error_to_str(err)),
+            ContextualParseError::InvalidRule(rule, ref err) => {
-            ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(decl, ref err) =>
+                write!(f, "Invalid rule: '{}', ", rule)?;
-                format!("Unsupported @counter-style descriptor declaration: '{}', {}", decl,
+                parse_error_to_str(err, f)
-                        parse_error_to_str(err)),
+            }
-            ContextualParseError::InvalidCounterStyleWithoutSymbols(ref system) =>
+            ContextualParseError::UnsupportedRule(rule, ref err) => {
-                format!("Invalid @counter-style rule: 'system: {}' without 'symbols'", system),
+                write!(f, "Unsupported rule: '{}', ", rule)?;
-            ContextualParseError::InvalidCounterStyleNotEnoughSymbols(ref system) =>
+                parse_error_to_str(err, f)
-                format!("Invalid @counter-style rule: 'system: {}' less than two 'symbols'", system),
+            }
-            ContextualParseError::InvalidCounterStyleWithoutAdditiveSymbols =>
+            ContextualParseError::UnsupportedViewportDescriptorDeclaration(decl, ref err) => {
-                "Invalid @counter-style rule: 'system: additive' without 'additive-symbols'".into(),
+                write!(f, "Unsupported @viewport descriptor declaration: '{}', ", decl)?;
-            ContextualParseError::InvalidCounterStyleExtendsWithSymbols =>
+                parse_error_to_str(err, f)
-                "Invalid @counter-style rule: 'system: extends …' with 'symbols'".into(),
+            }
-            ContextualParseError::InvalidCounterStyleExtendsWithAdditiveSymbols =>
+            ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(decl, ref err) => {
-                "Invalid @counter-style rule: 'system: extends …' with 'additive-symbols'".into(),
+                write!(f, "Unsupported @counter-style descriptor declaration: '{}', ", decl)?;
+                parse_error_to_str(err, f)
+            }
+            ContextualParseError::InvalidCounterStyleWithoutSymbols(ref system) => {
+                write!(f, "Invalid @counter-style rule: 'system: {}' without 'symbols'", system)
+            }
+            ContextualParseError::InvalidCounterStyleNotEnoughSymbols(ref system) => {
+                write!(f, "Invalid @counter-style rule: 'system: {}' less than two 'symbols'", system)
+            }
+            ContextualParseError::InvalidCounterStyleWithoutAdditiveSymbols => {
+                write!(f, "Invalid @counter-style rule: 'system: additive' without 'additive-symbols'")
+            }
+            ContextualParseError::InvalidCounterStyleExtendsWithSymbols => {
+                write!(f, "Invalid @counter-style rule: 'system: extends …' with 'symbols'")
+            }
+            ContextualParseError::InvalidCounterStyleExtendsWithAdditiveSymbols => {
+                write!(f, "Invalid @counter-style rule: 'system: extends …' with 'additive-symbols'")
+            }
         }
     }
 }
@@ -174,7 +198,7 @@ impl ParseErrorReporter for RustLogReporter {
                     location: SourceLocation,
                     error: ContextualParseError) {
         if log_enabled!(log::LogLevel::Info) {
-            info!("Url:\t{}\n{}:{} {}", url.as_str(), location.line, location.column, error.to_string())
+            info!("Url:\t{}\n{}:{} {}", url.as_str(), location.line, location.column, error)
         }
     }
 }

components/style/gecko/media_queries.rs

@@ -399,7 +399,9 @@ impl MediaExpressionValue {
                 dest.write_str(if v { "1" } else { "0" })
             },
             MediaExpressionValue::IntRatio(a, b) => {
-                write!(dest, "{}/{}", a, b)
+                a.to_css(dest)?;
+                dest.write_char('/')?;
+                b.to_css(dest)
             },
             MediaExpressionValue::Resolution(ref r) => r.to_css(dest),
             MediaExpressionValue::Ident(ref ident) => {

components/style/gecko/selector_parser.rs

@@ -70,7 +70,7 @@ impl ToCss for NonTSPseudoClass {
                 match *self {
                     $(NonTSPseudoClass::$name => concat!(":", $css),)*
                     $(NonTSPseudoClass::$s_name(ref s) => {
-                        write!(dest, ":{}(", $s_css)?;
+                        dest.write_str(concat!(":", $s_css, "("))?;
                         {
                             // FIXME(emilio): Avoid the extra allocation!
                             let mut css = CssStringWriter::new(dest);
@@ -84,7 +84,9 @@ impl ToCss for NonTSPseudoClass {
                     $(NonTSPseudoClass::$k_name(ref s) => {
                         // Don't include the terminating nul.
                         let value = String::from_utf16(&s[..s.len() - 1]).unwrap();
-                        return write!(dest, ":{}({})", $k_css, value)
+                        dest.write_str(concat!(":", $k_css, "("))?;
+                        dest.write_str(&value)?;
+                        return dest.write_char(')')
                     }, )*
                     NonTSPseudoClass::MozAny(ref selectors) => {
                         dest.write_str(":-moz-any(")?;

components/style/macros.rs

@@ -88,7 +88,7 @@ macro_rules! define_keyword_type {
 
         impl fmt::Debug for $name {
             fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-                write!(f, $css)
+                f.write_str($css)
             }
         }
 

components/style/media_queries.rs

@@ -96,7 +96,7 @@ impl ToCss for MediaQuery {
     {
         if let Some(qual) = self.qualifier {
             qual.to_css(dest)?;
-            write!(dest, " ")?;
+            dest.write_char(' ')?;
         }
 
         match self.media_type {
@@ -107,7 +107,7 @@ impl ToCss for MediaQuery {
                 // Otherwise, we'd serialize media queries like "(min-width:
                 // 40px)" in "all (min-width: 40px)", which is unexpected.
                 if self.qualifier.is_some() || self.expressions.is_empty() {
-                    write!(dest, "all")?;
+                    dest.write_str("all")?;
                 }
             },
             MediaQueryType::Concrete(MediaType(ref desc)) => desc.to_css(dest)?,
@@ -118,13 +118,13 @@ impl ToCss for MediaQuery {
         }
 
         if self.media_type != MediaQueryType::All || self.qualifier.is_some() {
-            write!(dest, " and ")?;
+            dest.write_str(" and ")?;
         }
 
         self.expressions[0].to_css(dest)?;
 
         for expr in self.expressions.iter().skip(1) {
-            write!(dest, " and ")?;
+            dest.write_str(" and ")?;
             expr.to_css(dest)?;
         }
         Ok(())

components/style/properties/longhand/box.mako.rs

@@ -631,7 +631,6 @@ ${helpers.predefined_type(
     use values::specified::{Angle, Integer, Length, LengthOrPercentage};
     use values::specified::{LengthOrNumber, LengthOrPercentageOrNumber as LoPoNumber, Number};
     use style_traits::ToCss;
-    use style_traits::values::Css;
 
     use std::fmt;
 
@@ -819,58 +818,99 @@ ${helpers.predefined_type(
                     m11, m12, m13, m14,
                     m21, m22, m23, m24,
                     m31, m32, m33, m34,
-                    m41, m42, m43, m44 } => write!(
+                    m41, m42, m43, m44,
-                        dest, "matrix3d({}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {})",
+                } => {
-                        Css(m11), Css(m12), Css(m13), Css(m14),
+                    serialize_function!(dest, matrix3d(
-                        Css(m21), Css(m22), Css(m23), Css(m24),
+                        m11, m12, m13, m14,
-                        Css(m31), Css(m32), Css(m33), Css(m34),
+                        m21, m22, m23, m24,
-                        Css(m41), Css(m42), Css(m43), Css(m44)),
+                        m31, m32, m33, m34,
+                        m41, m42, m43, m44,
+                    ))
+                }
                 SpecifiedOperation::PrefixedMatrix3D {
                     m11, m12, m13, m14,
                     m21, m22, m23, m24,
                     m31, m32, m33, m34,
-                    ref m41, ref m42, ref m43, m44 } => write!(
+                    ref m41, ref m42, ref m43, m44,
-                        dest, "matrix3d({}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {})",
+                } => {
-                        Css(m11), Css(m12), Css(m13), Css(m14),
+                    serialize_function!(dest, matrix3d(
-                        Css(m21), Css(m22), Css(m23), Css(m24),
+                        m11, m12, m13, m14,
-                        Css(m31), Css(m32), Css(m33), Css(m34),
+                        m21, m22, m23, m24,
-                        Css(m41), Css(m42), Css(m43), Css(m44)),
+                        m31, m32, m33, m34,
-                SpecifiedOperation::Skew(ax, None) => write!(dest, "skew({})", Css(ax)),
+                        m41, m42, m43, m44,
-                SpecifiedOperation::Skew(ax, Some(ay)) => write!(dest, "skew({}, {})", Css(ax), Css(ay)),
+                    ))
-                SpecifiedOperation::SkewX(angle) => write!(dest, "skewX({})", Css(angle)),
+                }
-                SpecifiedOperation::SkewY(angle) => write!(dest, "skewY({})", Css(angle)),
+                SpecifiedOperation::Skew(ax, None) => {
-                SpecifiedOperation::Translate(ref tx, None) => write!(dest, "translate({})", Css(tx)),
+                    serialize_function!(dest, skew(ax))
+                }
+                SpecifiedOperation::Skew(ax, Some(ay)) => {
+                    serialize_function!(dest, skew(ax, ay))
+                }
+                SpecifiedOperation::SkewX(angle) => {
+                    serialize_function!(dest, skewX(angle))
+                }
+                SpecifiedOperation::SkewY(angle) => {
+                    serialize_function!(dest, skewY(angle))
+                }
+                SpecifiedOperation::Translate(ref tx, None) => {
+                    serialize_function!(dest, translate(tx))
+                }
                 SpecifiedOperation::Translate(ref tx, Some(ref ty)) => {
-                    write!(dest, "translate({}, {})", Css(tx), Css(ty))
+                    serialize_function!(dest, translate(tx, ty))
-                },
+                }
-                SpecifiedOperation::TranslateX(ref tx) => write!(dest, "translateX({})", Css(tx)),
+                SpecifiedOperation::TranslateX(ref tx) => {
-                SpecifiedOperation::TranslateY(ref ty) => write!(dest, "translateY({})", Css(ty)),
+                    serialize_function!(dest, translateX(tx))
-                SpecifiedOperation::TranslateZ(ref tz) => write!(dest, "translateZ({})", Css(tz)),
+                }
-                SpecifiedOperation::Translate3D(ref tx, ref ty, ref tz) => write!(
+                SpecifiedOperation::TranslateY(ref ty) => {
-                    dest, "translate3d({}, {}, {})", Css(tx), Css(ty), Css(tz)),
+                    serialize_function!(dest, translateY(ty))
-                SpecifiedOperation::Scale(factor, None) => write!(dest, "scale({})", Css(factor)),
+                }
-                SpecifiedOperation::Scale(sx, Some(sy)) => write!(dest, "scale({}, {})", Css(sx), Css(sy)),
+                SpecifiedOperation::TranslateZ(ref tz) => {
-                SpecifiedOperation::ScaleX(sx) => write!(dest, "scaleX({})", Css(sx)),
+                    serialize_function!(dest, translateZ(tz))
-                SpecifiedOperation::ScaleY(sy) => write!(dest, "scaleY({})", Css(sy)),
+                }
-                SpecifiedOperation::ScaleZ(sz) => write!(dest, "scaleZ({})", Css(sz)),
+                SpecifiedOperation::Translate3D(ref tx, ref ty, ref tz) => {
+                    serialize_function!(dest, translate3d(tx, ty, tz))
+                }
+                SpecifiedOperation::Scale(factor, None) => {
+                    serialize_function!(dest, scale(factor))
+                }
+                SpecifiedOperation::Scale(sx, Some(sy)) => {
+                    serialize_function!(dest, scale(sx, sy))
+                }
+                SpecifiedOperation::ScaleX(sx) => {
+                    serialize_function!(dest, scaleX(sx))
+                }
+                SpecifiedOperation::ScaleY(sy) => {
+                    serialize_function!(dest, scaleY(sy))
+                }
+                SpecifiedOperation::ScaleZ(sz) => {
+                    serialize_function!(dest, scaleZ(sz))
+                }
                 SpecifiedOperation::Scale3D(sx, sy, sz) => {
-                    write!(dest, "scale3d({}, {}, {})", Css(sx), Css(sy), Css(sz))
+                    serialize_function!(dest, scale3d(sx, sy, sz))
-                },
+                }
-                SpecifiedOperation::Rotate(theta) => write!(dest, "rotate({})", Css(theta)),
+                SpecifiedOperation::Rotate(theta) => {
-                SpecifiedOperation::RotateX(theta) => write!(dest, "rotateX({})", Css(theta)),
+                    serialize_function!(dest, rotate(theta))
-                SpecifiedOperation::RotateY(theta) => write!(dest, "rotateY({})", Css(theta)),
+                }
-                SpecifiedOperation::RotateZ(theta) => write!(dest, "rotateZ({})", Css(theta)),
+                SpecifiedOperation::RotateX(theta) => {
-                SpecifiedOperation::Rotate3D(x, y, z, theta) => write!(
+                    serialize_function!(dest, rotateX(theta))
-                    dest, "rotate3d({}, {}, {}, {})",
+                }
-                    Css(x), Css(y), Css(z), Css(theta)),
+                SpecifiedOperation::RotateY(theta) => {
-                SpecifiedOperation::Perspective(ref length) => write!(dest, "perspective({})", Css(length)),
+                    serialize_function!(dest, rotateY(theta))
+                }
+                SpecifiedOperation::RotateZ(theta) => {
+                    serialize_function!(dest, rotateZ(theta))
+                }
+                SpecifiedOperation::Rotate3D(x, y, z, theta) => {
+                    serialize_function!(dest, rotate3d(x, y, z, theta))
+                }
+                SpecifiedOperation::Perspective(ref length) => {
+                    serialize_function!(dest, perspective(length))
+                }
                 SpecifiedOperation::InterpolateMatrix { ref from_list, ref to_list, progress } => {
-                    write!(dest, "interpolatematrix({}, {}, {})",
+                    serialize_function!(dest, interpolatematrix(from_list, to_list, progress))
-                           Css(from_list), Css(to_list), Css(progress))
+                }
-                },
                 SpecifiedOperation::AccumulateMatrix { ref from_list, ref to_list, count } => {
-                    write!(dest, "accumulatematrix({}, {}, {})",
+                    serialize_function!(dest, accumulatematrix(from_list, to_list, count))
-                           Css(from_list), Css(to_list), Css(count))
                 }
             }
         }

components/style/properties/longhand/font.mako.rs

@@ -273,7 +273,7 @@ macro_rules! impl_gecko_keyword_conversions {
                         % if product == "gecko":
                             // We should treat -moz-fixed as monospace
                             if name == &atom!("-moz-fixed") {
-                                return write!(dest, "monospace");
+                                return dest.write_str("monospace");
                             }
                         % endif
 

components/style/properties/shorthand/box.mako.rs

@@ -176,7 +176,7 @@ macro_rules! try_parse_one {
 
             for i in 0..len {
                 if i != 0 {
-                    write!(dest, ", ")?;
+                    dest.write_str(", ")?;
                 }
                 self.transition_property.0[i].to_css(dest)?;
                 % for name in "duration timing_function delay".split():
@@ -289,7 +289,7 @@ macro_rules! try_parse_one {
 
             for i in 0..len {
                 if i != 0 {
-                    write!(dest, ", ")?;
+                    dest.write_str(", ")?;
                 }
 
                 % for name in props[1:]:

components/style/servo/media_queries.rs

@@ -215,15 +215,14 @@ impl ToCss for Expression {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
         where W: fmt::Write,
     {
-        write!(dest, "(")?;
+        let (s, l) = match self.0 {
-        let (mm, l) = match self.0 {
+            ExpressionKind::Width(Range::Min(ref l)) => ("(min-width: ", l),
-            ExpressionKind::Width(Range::Min(ref l)) => ("min-", l),
+            ExpressionKind::Width(Range::Max(ref l)) => ("(max-width: ", l),
-            ExpressionKind::Width(Range::Max(ref l)) => ("max-", l),
+            ExpressionKind::Width(Range::Eq(ref l)) => ("(width: ", l),
-            ExpressionKind::Width(Range::Eq(ref l)) => ("", l),
         };
-        write!(dest, "{}width: ", mm)?;
+        dest.write_str(s)?;
         l.to_css(dest)?;
-        write!(dest, ")")
+        dest.write_char(')')
     }
 }
 

components/style/stylesheets/font_feature_values_rule.rs

@@ -68,7 +68,7 @@ impl Parse for SingleValue {
 
 impl ToCss for SingleValue {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-        write!(dest, "{}", self.0)
+        self.0.to_css(dest)
     }
 }
 
@@ -105,9 +105,10 @@ impl Parse for PairValues {
 
 impl ToCss for PairValues {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-        write!(dest, "{}", self.0)?;
+        self.0.to_css(dest)?;
         if let Some(second) = self.1 {
-            write!(dest, " {}", second)?;
+            dest.write_char(' ')?;
+            second.to_css(dest)?;
         }
         Ok(())
     }
@@ -158,10 +159,10 @@ impl ToCss for VectorValues {
         let mut iter = self.0.iter();
         let first = iter.next();
         if let Some(first) = first {
-            write!(dest, "{}", first)?;
+            first.to_css(dest)?;
             for value in iter {
-                dest.write_str(" ")?;
+                dest.write_char(' ')?;
-                write!(dest, "{}", value)?;
+                value.to_css(dest)?;
             }
         }
         Ok(())

components/style/stylesheets/keyframes_rule.rs

@@ -155,7 +155,7 @@ impl ToCss for KeyframeSelector {
         let mut iter = self.0.iter();
         iter.next().unwrap().to_css(dest)?;
         for percentage in iter {
-            write!(dest, ", ")?;
+            dest.write_str(", ")?;
             percentage.to_css(dest)?;
         }
         Ok(())

components/style/stylesheets/viewport_rule.rs

@@ -151,7 +151,7 @@ impl ToCss for ViewportLength {
     {
         match *self {
             ViewportLength::Specified(ref length) => length.to_css(dest),
-            ViewportLength::ExtendToZoom => write!(dest, "extend-to-zoom"),
+            ViewportLength::ExtendToZoom => dest.write_str("extend-to-zoom"),
         }
     }
 }

components/style/values/generics/mod.rs

@@ -226,7 +226,10 @@ impl ToCss for FontSettingTagInt {
         match self.0 {
             1 => Ok(()),
             0 => dest.write_str(" off"),
-            x => write!(dest, " {}", x)
+            x => {
+                dest.write_char(' ')?;
+                x.to_css(dest)
+            }
         }
     }
 }

components/style/values/specified/length.rs

@@ -424,7 +424,11 @@ impl ToCss for NoCalcLength {
             NoCalcLength::FontRelative(length) => length.to_css(dest),
             NoCalcLength::ViewportPercentage(length) => length.to_css(dest),
             /* This should only be reached from style dumping code */
-            NoCalcLength::ServoCharacterWidth(CharacterWidth(i)) => write!(dest, "CharWidth({})", i),
+            NoCalcLength::ServoCharacterWidth(CharacterWidth(i)) => {
+                dest.write_str("CharWidth(")?;
+                i.to_css(dest)?;
+                dest.write_char(')')
+            }
             #[cfg(feature = "gecko")]
             NoCalcLength::Physical(length) => length.to_css(dest),
         }

components/style_traits/values.rs

@@ -6,6 +6,7 @@
 
 use app_units::Au;
 use cssparser::{BasicParseError, ParseError, Parser, Token, UnicodeRange, serialize_string};
+use cssparser::ToCss as CssparserToCss;
 use std::fmt::{self, Write};
 
 /// Serialises a value according to its CSS representation.
@@ -69,6 +70,24 @@ where
     }
 }
 
+#[macro_export]
+macro_rules! serialize_function {
+    ($dest: expr, $name: ident($( $arg: expr, )+)) => {
+        serialize_function!($dest, $name($($arg),+))
+    };
+    ($dest: expr, $name: ident($first_arg: expr $( , $arg: expr )*)) => {
+        {
+            $dest.write_str(concat!(stringify!($name), "("))?;
+            $first_arg.to_css($dest)?;
+            $(
+                $dest.write_str(", ")?;
+                $arg.to_css($dest)?;
+            )*
+            $dest.write_char(')')
+        }
+    }
+}
+
 /// Convenience wrapper to serialise CSS values separated by a given string.
 pub struct SequenceWriter<'a, W> {
     writer: TrackedWriter<W>,
@@ -326,7 +345,8 @@ impl<T> ToCss for Box<T> where T: ?Sized + ToCss {
 
 impl ToCss for Au {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
-        write!(dest, "{}px", self.to_f64_px())
+        self.to_f64_px().to_css(dest)?;
+        dest.write_str("px")
     }
 }
 
@@ -531,14 +551,3 @@ pub mod specified {
         }
     }
 }
-
-
-/// Wrap CSS types for serialization with `write!` or `format!` macros.
-/// Used by ToCss of SpecifiedOperation.
-pub struct Css<T>(pub T);
-
-impl<T: ToCss> fmt::Display for Css<T> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.0.to_css(f)
-    }
-}

components/style_traits/viewport.rs

@@ -45,19 +45,31 @@ impl ToCss for ViewportConstraints {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
         where W: fmt::Write
     {
-        write!(dest, "@viewport {{")?;
+        dest.write_str("@viewport { width: ")?;
-        write!(dest, " width: {}px;", self.size.width)?;
+        self.size.width.to_css(dest)?;
-        write!(dest, " height: {}px;", self.size.height)?;
+
-        write!(dest, " zoom: {};", self.initial_zoom.get())?;
+        dest.write_str("px; height: ")?;
+        self.size.height.to_css(dest)?;
+
+        dest.write_str("px; zoom: ")?;
+        self.initial_zoom.get().to_css(dest)?;
+
         if let Some(min_zoom) = self.min_zoom {
-            write!(dest, " min-zoom: {};", min_zoom.get())?;
+            dest.write_str("; min-zoom: ")?;
+            min_zoom.get().to_css(dest)?;
         }
+
         if let Some(max_zoom) = self.max_zoom {
-            write!(dest, " max-zoom: {};", max_zoom.get())?;
+            dest.write_str("; max-zoom: ")?;
+            max_zoom.get().to_css(dest)?;
         }
-        write!(dest, " user-zoom: ")?; self.user_zoom.to_css(dest)?;
+
-        write!(dest, "; orientation: ")?; self.orientation.to_css(dest)?;
+        dest.write_str("; user-zoom: ")?;
-        write!(dest, "; }}")
+        self.user_zoom.to_css(dest)?;
+
+        dest.write_str("; orientation: ")?;
+        self.orientation.to_css(dest)?;
+        dest.write_str("; }")
     }
 }
 
@@ -78,9 +90,12 @@ impl ToCss for Zoom {
         where W: fmt::Write,
     {
         match *self {
-            Zoom::Number(number) => write!(dest, "{}", number),
+            Zoom::Number(number) => number.to_css(dest),
-            Zoom::Percentage(percentage) => write!(dest, "{}%", percentage * 100.),
+            Zoom::Auto => dest.write_str("auto"),
-            Zoom::Auto => write!(dest, "auto")
+            Zoom::Percentage(percentage) => {
+                (percentage * 100.).to_css(dest)?;
+                dest.write_char('%')
+            }
         }
     }
 }

ports/geckolib/error_reporter.rs

@@ -6,8 +6,8 @@
 
 #![allow(unsafe_code)]
 
+use cssparser::{CowRcStr, serialize_identifier, ToCss};
 use cssparser::{SourceLocation, ParseError as CssParseError, Token, BasicParseError};
-use cssparser::CowRcStr;
 use selectors::parser::SelectorParseError;
 use std::ptr;
 use style::error_reporting::{ParseErrorReporter, ContextualParseError};
@@ -53,136 +53,13 @@ impl<'a> ErrorString<'a> {
     fn into_str(self) -> CowRcStr<'a> {
         match self {
             ErrorString::Snippet(s) => s,
-            ErrorString::Ident(i) => escape_css_ident(&i).into(),
+            ErrorString::UnexpectedToken(t) => t.to_css_string().into(),
-            ErrorString::UnexpectedToken(t) => token_to_str(t).into(),
+            ErrorString::Ident(i) => {
+                let mut s = String::new();
+                serialize_identifier(&i, &mut s).unwrap();
+                s.into()
             }
         }
-}
-
-// This is identical to the behaviour of cssparser::serialize_identifier, except that
-// it uses numerical escapes for a larger set of characters.
-fn escape_css_ident(ident: &str) -> String {
-    // The relevant parts of the CSS grammar are:
-    //   ident    ([-]?{nmstart}|[-][-]){nmchar}*
-    //   nmstart  [_a-z]|{nonascii}|{escape}
-    //   nmchar   [_a-z0-9-]|{nonascii}|{escape}
-    //   nonascii [^\0-\177]
-    //   escape   {unicode}|\\[^\n\r\f0-9a-f]
-    //   unicode  \\[0-9a-f]{1,6}(\r\n|[ \n\r\t\f])?
-    // from http://www.w3.org/TR/CSS21/syndata.html#tokenization but
-    // modified for idents by
-    // http://dev.w3.org/csswg/cssom/#serialize-an-identifier and
-    // http://dev.w3.org/csswg/css-syntax/#would-start-an-identifier
-    if ident.is_empty() {
-        return ident.into()
-    }
-
-    let mut escaped = String::with_capacity(ident.len());
-
-    // A leading dash does not need to be escaped as long as it is not the
-    // *only* character in the identifier.
-    let mut iter = ident.chars().peekable();
-    if iter.peek() == Some(&'-') {
-        if ident.len() == 1 {
-            return "\\-".into();
-        }
-
-        escaped.push('-');
-        // Skip the first character.
-        let _ = iter.next();
-    }
-
-    // Escape a digit at the start (including after a dash),
-    // numerically.  If we didn't escape it numerically, it would get
-    // interpreted as a numeric escape for the wrong character.
-    if iter.peek().map_or(false, |&c| '0' <= c && c <= '9') {
-        let ch = iter.next().unwrap();
-        escaped.push_str(&format!("\\{:x} ", ch as u32));
-    }
-
-    while let Some(ch) = iter.next() {
-        if ch == '\0' {
-            escaped.push_str("\u{FFFD}");
-        } else if ch < (0x20 as char) || (0x7f as char <= ch && ch < (0xA0 as char)) {
-            // Escape U+0000 through U+001F and U+007F through U+009F numerically.
-            escaped.push_str(&format!("\\{:x} ", ch as u32));
-        } else {
-            // Escape ASCII non-identifier printables as a backslash plus
-            // the character.
-            if (ch < (0x7F as char)) &&
-                ch != '_' && ch != '-' &&
-                (ch < '0' || '9' < ch) &&
-                (ch < 'A' || 'Z' < ch) &&
-                (ch < 'a' || 'z' < ch)
-            {
-                escaped.push('\\');
-            }
-            escaped.push(ch);
-        }
-    }
-
-    escaped
-}
-
-// This is identical to the behaviour of cssparser::CssStringWriter, except that
-// the characters between 0x7F and 0xA0 as numerically escaped as well.
-fn escape_css_string(s: &str) -> String {
-    let mut escaped = String::new();
-    for ch in s.chars() {
-        if ch < ' ' || (ch >= (0x7F as char) && ch < (0xA0 as char)) {
-            escaped.push_str(&format!("\\{:x} ", ch as u32));
-        } else {
-            if ch == '"' || ch == '\'' || ch == '\\' {
-                // Escape backslash and quote characters symbolically.
-                // It's not technically necessary to escape the quote
-                // character that isn't being used to delimit the string,
-                // but we do it anyway because that makes testing simpler.
-                escaped.push('\\');
-            }
-            escaped.push(ch);
-        }
-    }
-    escaped
-}
-
-fn token_to_str<'a>(t: Token<'a>) -> String {
-    match t {
-        Token::Ident(i) => escape_css_ident(&i),
-        Token::AtKeyword(kw) => format!("@{}", escape_css_ident(&kw)),
-        Token::Hash(h) | Token::IDHash(h) => format!("#{}", escape_css_ident(&h)),
-        Token::QuotedString(s) => format!("'{}'", escape_css_string(&s)),
-        Token::UnquotedUrl(u) => format!("'{}'", escape_css_string(&u)),
-        Token::Delim(d) => d.to_string(),
-        Token::Number { int_value: Some(i), .. } => i.to_string(),
-        Token::Number { value, .. } => value.to_string(),
-        Token::Percentage { int_value: Some(i), .. } => i.to_string(),
-        Token::Percentage { unit_value, .. } => unit_value.to_string(),
-        Token::Dimension { int_value: Some(i), ref unit, .. } =>
-            format!("{}{}", i, escape_css_ident(&*unit)),
-        Token::Dimension { value, ref unit, .. } =>
-            format!("{}{}", value, escape_css_ident(&*unit)),
-        Token::WhiteSpace(s) => s.into(),
-        Token::Comment(_) => "comment".into(),
-        Token::Colon => ":".into(),
-        Token::Semicolon => ";".into(),
-        Token::Comma => ",".into(),
-        Token::IncludeMatch => "~=".into(),
-        Token::DashMatch => "|=".into(),
-        Token::PrefixMatch => "^=".into(),
-        Token::SuffixMatch => "$=".into(),
-        Token::SubstringMatch => "*=".into(),
-        Token::Column => "||".into(),
-        Token::CDO => "<!--".into(),
-        Token::CDC => "-->".into(),
-        Token::Function(f) => format!("{}(", escape_css_ident(&f)),
-        Token::ParenthesisBlock => "(".into(),
-        Token::SquareBracketBlock => "[".into(),
-        Token::CurlyBracketBlock => "{".into(),
-        Token::BadUrl(url) => format!("url('{}", escape_css_string(&url)).into(),
-        Token::BadString(s) => format!("'{}", escape_css_string(&s)).into(),
-        Token::CloseParenthesis => "unmatched close parenthesis".into(),
-        Token::CloseSquareBracket => "unmatched close square bracket".into(),
-        Token::CloseCurlyBracket => "unmatched close curly bracket".into(),
     }
 }
 
@@ -200,30 +77,44 @@ trait ErrorHelpers<'a> {
 
 fn extract_error_param<'a>(err: ParseError<'a>) -> Option<ErrorString<'a>> {
     Some(match err {
-        CssParseError::Basic(BasicParseError::UnexpectedToken(t)) =>
+        CssParseError::Basic(BasicParseError::UnexpectedToken(t)) => {
-            ErrorString::UnexpectedToken(t),
+            ErrorString::UnexpectedToken(t)
+        }
 
         CssParseError::Basic(BasicParseError::AtRuleInvalid(i)) |
         CssParseError::Custom(SelectorParseError::Custom(
-            StyleParseError::UnsupportedAtRule(i))) =>
+            StyleParseError::UnsupportedAtRule(i)
-            ErrorString::Snippet(format!("@{}", escape_css_ident(&i)).into()),
+        )) => {
+            let mut s = String::from("@");
+            serialize_identifier(&i, &mut s).unwrap();
+            ErrorString::Snippet(s.into())
+        }
 
         CssParseError::Custom(SelectorParseError::Custom(
             StyleParseError::PropertyDeclaration(
-                PropertyDeclarationParseError::InvalidValue(property, None)))) =>
+                PropertyDeclarationParseError::InvalidValue(property, None)
-            ErrorString::Snippet(property),
+            )
+        )) => {
+            ErrorString::Snippet(property)
+        }
 
-        CssParseError::Custom(SelectorParseError::UnexpectedIdent(ident)) =>
+        CssParseError::Custom(SelectorParseError::UnexpectedIdent(ident)) => {
-            ErrorString::Ident(ident),
+            ErrorString::Ident(ident)
+        }
 
         CssParseError::Custom(SelectorParseError::Custom(
             StyleParseError::PropertyDeclaration(
-                PropertyDeclarationParseError::UnknownProperty(property)))) =>
+                PropertyDeclarationParseError::UnknownProperty(property)
-            ErrorString::Ident(property),
+            )
+        )) => {
+            ErrorString::Ident(property)
+        }
 
         CssParseError::Custom(SelectorParseError::Custom(
-            StyleParseError::UnexpectedTokenWithinNamespace(token))) =>
+            StyleParseError::UnexpectedTokenWithinNamespace(token)
-            ErrorString::UnexpectedToken(token),
+        )) => {
+            ErrorString::UnexpectedToken(token)
+        }
 
         _ => return None,
     })

tests/unit/style/rule_tree/bench.rs

@@ -22,7 +22,7 @@ impl ParseErrorReporter for ErrorringErrorReporter {
                     url: &ServoUrl,
                     location: SourceLocation,
                     error: ContextualParseError) {
-        panic!("CSS error: {}\t\n{}:{} {}", url.as_str(), location.line, location.column, error.to_string());
+        panic!("CSS error: {}\t\n{}:{} {}", url.as_str(), location.line, location.column, error);
     }
 }
 

tests/unit/style/stylesheets.rs

@@ -280,7 +280,7 @@ impl ParseErrorReporter for CSSInvalidErrorReporterTest {
                 url: url.clone(),
                 line: location.line,
                 column: location.column,
-                message: error.to_string()
+                message: error.to_string(),
             }
         );
     }

tests/unit/style/viewport.rs

@@ -11,7 +11,7 @@ use servo_config::prefs::{PREFS, PrefValue};
 use servo_url::ServoUrl;
 use style::context::QuirksMode;
 use style::media_queries::{Device, MediaList, MediaType};
-use style::parser::{Parse, ParserContext, ParserErrorContext};
+use style::parser::{ParserContext, ParserErrorContext};
 use style::shared_lock::SharedRwLock;
 use style::stylesheets::{CssRuleType, Stylesheet, StylesheetInDocument, Origin};
 use style::stylesheets::viewport_rule::*;

tests/unit/stylo/lib.rs

@@ -12,7 +12,6 @@ extern crate selectors;
 #[macro_use] extern crate style;
 extern crate style_traits;
 
-mod sanity_checks;
 #[cfg(target_pointer_width = "64")]
 mod size_of;
 mod specified_values;

tests/unit/stylo/sanity_checks.rs

@@ -1,28 +0,0 @@
-/* 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 http://mozilla.org/MPL/2.0/. */
-
-//! Different static asserts that ensure the build does what it's expected to.
-//!
-//! TODO: maybe cfg(test) this?
-
-#![allow(unused_imports)]
-
-use std::mem;
-
-macro_rules! check_enum_value {
-    ($a:expr, $b:expr) => {
-        unsafe {
-            mem::transmute::<[u32; $a as usize],
-                             [u32; $b as usize]>([0; $a as usize]);
-        }
-    }
-}
-
-// NB: It's a shame we can't do this statically with bitflags, but no
-// const-fn and no other way to access the numerical value :-(
-macro_rules! check_enum_value_non_static {
-    ($a:expr, $b:expr) => {
-        assert_eq!($a.0 as usize, $b as usize);
-    }
-}

tests/unit/stylo/specified_values.rs

@@ -5,6 +5,7 @@
 use style;
 
 #[cfg(all(test, target_pointer_width = "64"))]
+#[test]
 fn size_of_specified_values() {
     use std::mem::size_of;
     let threshold = 24;

tests/wpt/mozilla/meta/MANIFEST.json

@@ -26530,7 +26530,7 @@
    "testharness"
   ],
   "mozilla/calc.html": [
-   "5b3ea33205a9f2404bd6976a2c7f6fc451be8e96",
+   "4f65929cacf623da2d3e310a6663d6165a1b0cdc",
    "testharness"
   ],
   "mozilla/canvas.initial.reset.2dstate.html": [

tests/wpt/mozilla/tests/mozilla/calc.html

@@ -42,7 +42,7 @@ var widthTests = [
     ['calc(10px + 10em - 10px)', 'calc(10em + 0px)', '160px'],
 
     // Fold absolute units
-    ['calc(1px + 1pt + 1pc + 1in + 1cm + 1mm)', 'calc(155.91666666666666px)', '155.91666666666666px'],
+    ['calc(96px + 72pt + 6pc + 1in + 2.54cm + 25.4mm)', 'calc(576px)', '576px'],
 
     // Alphabetical order
     ['calc(0ch + 0px + 0pt + 0pc + 0in + 0cm + 0mm + 0rem + 0em + 0ex + 0% + 0vw + 0vh + 0vmin + 0vmax)',