Change ToCss to take a CssWriter<W>

This more concrete wrapper type can write a prefix the very first time something
is written to it. This allows removing plenty of useless monomorphisations caused
by the former W/SequenceWriter<W> pair of types.

1 files changed, 99 insertions, 102 deletions

diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs
index 3e951548e6c..56574ac161d 100644
--- a/components/style_traits/values.rs
+++ b/components/style_traits/values.rs
@@ -34,7 +34,7 @@ use std::fmt::{self, Write};
 ///   implement `Debug` by a single call to `ToCss::to_css`.
 pub trait ToCss {
     /// Serialize `self` in CSS syntax, writing to `dest`.
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write;
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write;
 
     /// Serialize `self` in CSS syntax and return a string.
     ///
@@ -42,27 +42,27 @@ pub trait ToCss {
     #[inline]
     fn to_css_string(&self) -> String {
         let mut s = String::new();
-        self.to_css(&mut s).unwrap();
+        self.to_css(&mut CssWriter::new(&mut s)).unwrap();
         s
     }
 }
 
 impl<'a, T> ToCss for &'a T where T: ToCss + ?Sized {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write {
         (*self).to_css(dest)
     }
 }
 
 impl ToCss for str {
     #[inline]
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write {
         serialize_string(self, dest)
     }
 }
 
 impl ToCss for String {
     #[inline]
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write {
         serialize_string(self, dest)
     }
 }
@@ -72,11 +72,62 @@ where
     T: ToCss,
 {
     #[inline]
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write {
         self.as_ref().map_or(Ok(()), |value| value.to_css(dest))
     }
 }
 
+/// A writer tailored for serialising CSS.
+///
+/// Coupled with SequenceWriter, this allows callers to transparently handle
+/// things like comma-separated values etc.
+pub struct CssWriter<'w, W: 'w> {
+    inner: &'w mut W,
+    prefix: Option<&'static str>,
+}
+
+impl<'w, W> CssWriter<'w, W>
+where
+    W: Write,
+{
+    /// Creates a new `CssWriter`.
+    #[inline]
+    pub fn new(inner: &'w mut W) -> Self {
+        Self { inner, prefix: Some("") }
+    }
+}
+
+impl<'w, W> Write for CssWriter<'w, W>
+where
+    W: Write,
+{
+    #[inline]
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        if s.is_empty() {
+            return Ok(());
+        }
+        if let Some(prefix) = self.prefix.take() {
+            // We are going to write things, but first we need to write
+            // the prefix that was set by `SequenceWriter::item`.
+            if !prefix.is_empty() {
+                self.inner.write_str(prefix)?;
+            }
+        }
+        self.inner.write_str(s)
+    }
+
+    #[inline]
+    fn write_char(&mut self, c: char) -> fmt::Result {
+        if let Some(prefix) = self.prefix.take() {
+            // See comment in `write_str`.
+            if !prefix.is_empty() {
+                self.inner.write_str(prefix)?;
+            }
+        }
+        self.inner.write_char(c)
+    }
+}
+
 #[macro_export]
 macro_rules! serialize_function {
     ($dest: expr, $name: ident($( $arg: expr, )+)) => {
@@ -96,22 +147,23 @@ macro_rules! serialize_function {
 }
 
 /// Convenience wrapper to serialise CSS values separated by a given string.
-pub struct SequenceWriter<'a, W> {
-    writer: TrackedWriter<W>,
-    separator: &'a str,
+pub struct SequenceWriter<'a, 'b: 'a, W: 'b> {
+    inner: &'a mut CssWriter<'b, W>,
+    separator: &'static str,
 }
 
-impl<'a, W> SequenceWriter<'a, W>
+impl<'a, 'b, W> SequenceWriter<'a, 'b, W>
 where
-    W: Write,
+    W: Write + 'b,
 {
     /// Create a new sequence writer.
     #[inline]
-    pub fn new(writer: W, separator: &'a str) -> Self {
-        SequenceWriter {
-            writer: TrackedWriter::new(writer),
-            separator: separator,
+    pub fn new(inner: &'a mut CssWriter<'b, W>, separator: &'static str) -> Self {
+        if inner.prefix.is_none() {
+            // See comment in `item`.
+            inner.prefix = Some("");
         }
+        Self { inner, separator }
     }
 
     /// Serialises a CSS value, writing any separator as necessary.
@@ -125,89 +177,34 @@ where
     where
         T: ToCss,
     {
-        if self.writer.has_written {
-            item.to_css(&mut PrefixedWriter::new(&mut self.writer, self.separator))
-        } else {
-            item.to_css(&mut self.writer)
-        }
-    }
-}
-
-struct TrackedWriter<W> {
-    writer: W,
-    has_written: bool,
-}
-
-impl<W> TrackedWriter<W>
-where
-    W: Write,
-{
-    #[inline]
-    fn new(writer: W) -> Self {
-        TrackedWriter {
-            writer: writer,
-            has_written: false,
+        let old_prefix = self.inner.prefix;
+        if old_prefix.is_none() {
+            // If there is no prefix in the inner writer, a previous
+            // call to this method produced output, which means we need
+            // to write the separator next time we produce output again.
+            self.inner.prefix = Some(self.separator);
         }
-    }
-}
-
-impl<W> Write for TrackedWriter<W>
-where
-    W: Write,
-{
-    #[inline]
-    fn write_str(&mut self, s: &str) -> fmt::Result {
-        if !s.is_empty() {
-            self.has_written = true;
-        }
-        self.writer.write_str(s)
-    }
-
-    #[inline]
-    fn write_char(&mut self, c: char) -> fmt::Result {
-        self.has_written = true;
-        self.writer.write_char(c)
-    }
-}
-
-struct PrefixedWriter<'a, W> {
-    writer: W,
-    prefix: Option<&'a str>,
-}
-
-impl<'a, W> PrefixedWriter<'a, W>
-where
-    W: Write,
-{
-    #[inline]
-    fn new(writer: W, prefix: &'a str) -> Self {
-        PrefixedWriter {
-            writer: writer,
-            prefix: Some(prefix),
-        }
-    }
-}
-
-impl<'a, W> Write for PrefixedWriter<'a, W>
-where
-    W: Write,
-{
-    #[inline]
-    fn write_str(&mut self, s: &str) -> fmt::Result {
-        if !s.is_empty() {
-            if let Some(prefix) = self.prefix.take() {
-                self.writer.write_str(prefix)?;
+        item.to_css(&mut self.inner)?;
+        match (old_prefix, self.inner.prefix) {
+            (_, None) => {
+                // This call produced output and cleaned up after itself.
+            }
+            (None, Some(p)) => {
+                // Some previous call to `item` produced output,
+                // but this one did not, prefix should be the same as
+                // the one we set.
+                debug_assert_eq!(self.separator, p);
+                // We clean up here even though it's not necessary just
+                // to be able to do all these assertion checks.
+                self.inner.prefix = None;
+            }
+            (Some(old), Some(new)) => {
+                // No previous call to `item` produced output, and this one
+                // either.
+                debug_assert_eq!(old, new);
             }
         }
-        self.writer.write_str(s)
-    }
-
-    #[inline]
-    fn write_char(&mut self, c: char) -> fmt::Result {
-        if let Some(prefix) = self.prefix.take() {
-            self.writer.write_str(prefix)?;
-        }
-        self.writer.write_char(c)
+        Ok(())
     }
 }
 
@@ -332,7 +329,7 @@ impl OneOrMoreSeparated for UnicodeRange {
 }
 
 impl<T> ToCss for Vec<T> where T: ToCss + OneOrMoreSeparated {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write {
         let mut iter = self.iter();
         iter.next().unwrap().to_css(dest)?;
         for item in iter {
@@ -344,7 +341,7 @@ impl<T> ToCss for Vec<T> where T: ToCss + OneOrMoreSeparated {
 }
 
 impl<T> ToCss for Box<T> where T: ?Sized + ToCss {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
         where W: Write,
     {
         (**self).to_css(dest)
@@ -352,7 +349,7 @@ impl<T> ToCss for Box<T> where T: ?Sized + ToCss {
 }
 
 impl<T> ToCss for Arc<T> where T: ?Sized + ToCss {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
         where W: Write,
     {
         (**self).to_css(dest)
@@ -360,7 +357,7 @@ impl<T> ToCss for Arc<T> where T: ?Sized + ToCss {
 }
 
 impl ToCss for Au {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write {
         self.to_f64_px().to_css(dest)?;
         dest.write_str("px")
     }
@@ -369,7 +366,7 @@ impl ToCss for Au {
 macro_rules! impl_to_css_for_predefined_type {
     ($name: ty) => {
         impl<'a> ToCss for $name {
-            fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: Write {
+            fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: Write {
                 ::cssparser::ToCss::to_css(self, dest)
             }
         }
@@ -479,11 +476,11 @@ macro_rules! __define_css_keyword_enum__actual {
         }
 
         impl $crate::ToCss for $name {
-            fn to_css<W>(&self, dest: &mut W) -> ::std::fmt::Result
+            fn to_css<W>(&self, dest: &mut $crate::CssWriter<W>) -> ::std::fmt::Result
                 where W: ::std::fmt::Write
             {
                 match *self {
-                    $( $name::$variant => dest.write_str($css) ),+
+                    $( $name::$variant => ::std::fmt::Write::write_str(dest, $css) ),+
                 }
             }
         }