feat(codegen): Optimize deserializing fields from byte strings

serde_codegen/src/attr.rs

@@ -5,7 +5,7 @@ use syntax::codemap::Span;
 use syntax::ext::base::ExtCtxt;
 use syntax::fold::Folder;
 use syntax::parse::parser::PathParsingMode;
-use syntax::parse::token;
+use syntax::parse::token::{self, InternedString};
 use syntax::parse;
 use syntax::print::pprust::{lit_to_string, meta_item_to_string};
 use syntax::ptr::P;
@@ -14,22 +14,66 @@ use aster::AstBuilder;
 
 use error::Error;
 
+#[derive(Debug)]
+pub struct Name {
+    ident: ast::Ident,
+    serialize_name: Option<InternedString>,
+    deserialize_name: Option<InternedString>,
+}
+
+impl Name {
+    fn new(ident: ast::Ident) -> Self {
+        Name {
+            ident: ident,
+            serialize_name: None,
+            deserialize_name: None,
+        }
+    }
+
+    /// Return the string expression of the field ident.
+    pub fn ident_expr(&self) -> P<ast::Expr> {
+        AstBuilder::new().expr().str(self.ident)
+    }
+
+    /// Return the container name for the container when serializing.
+    pub fn serialize_name(&self) -> InternedString {
+        match self.serialize_name {
+            Some(ref name) => name.clone(),
+            None => self.ident.name.as_str(),
+        }
+    }
+
+    /// Return the container name expression for the container when deserializing.
+    pub fn serialize_name_expr(&self) -> P<ast::Expr> {
+        AstBuilder::new().expr().str(self.serialize_name())
+    }
+
+    /// Return the container name for the container when deserializing.
+    pub fn deserialize_name(&self) -> InternedString {
+        match self.deserialize_name {
+            Some(ref name) => name.clone(),
+            None => self.ident.name.as_str(),
+        }
+    }
+
+    /// Return the container name expression for the container when deserializing.
+    pub fn deserialize_name_expr(&self) -> P<ast::Expr> {
+        AstBuilder::new().expr().str(self.deserialize_name())
+    }
+}
+
 /// Represents container (e.g. struct) attribute information
 #[derive(Debug)]
 pub struct ContainerAttrs {
-    ident: ast::Ident,
+    name: Name,
-    serialize_name: Option<ast::Lit>,
-    deserialize_name: Option<ast::Lit>,
     deny_unknown_fields: bool,
 }
 
 impl ContainerAttrs {
     /// Extract out the `#[serde(...)]` attributes from an item.
-    pub fn from_item(cx: &ExtCtxt, item: &ast::Item) -> Result<ContainerAttrs, Error> {
+    pub fn from_item(cx: &ExtCtxt, item: &ast::Item) -> Result<Self, Error> {
         let mut container_attrs = ContainerAttrs {
-            ident: item.ident,
+            name: Name::new(item.ident),
-            serialize_name: None,
-            deserialize_name: None,
             deny_unknown_fields: false,
         };
 
@@ -38,15 +82,18 @@ impl ContainerAttrs {
                 match meta_item.node {
                     // Parse `#[serde(rename="foo")]`
                     ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"rename" => {
-                        container_attrs.serialize_name = Some(lit.clone());
+                        let s = try!(get_str_from_lit(cx, name, lit));
-                        container_attrs.deserialize_name = Some(lit.clone());
+
+                        container_attrs.name.serialize_name = Some(s.clone());
+                        container_attrs.name.deserialize_name = Some(s);
                     }
 
                     // Parse `#[serde(rename(serialize="foo", deserialize="bar"))]`
                     ast::MetaItemKind::List(ref name, ref meta_items) if name == &"rename" => {
                         let (ser_name, de_name) = try!(get_renames(cx, meta_items));
-                        container_attrs.serialize_name = ser_name;
+
-                        container_attrs.deserialize_name = de_name;
+                        container_attrs.name.serialize_name = ser_name;
+                        container_attrs.name.deserialize_name = de_name;
                     }
 
                     // Parse `#[serde(deny_unknown_fields)]`
@@ -69,25 +116,8 @@ impl ContainerAttrs {
         Ok(container_attrs)
     }
 
-    /// Return the string expression of the field ident.
+    pub fn name(&self) -> &Name {
-    pub fn ident_expr(&self) -> P<ast::Expr> {
+        &self.name
-        AstBuilder::new().expr().str(self.ident)
-    }
-
-    /// Return the field name for the field when serializing.
-    pub fn serialize_name_expr(&self) -> P<ast::Expr> {
-        match self.serialize_name {
-            Some(ref name) => AstBuilder::new().expr().build_lit(P(name.clone())),
-            None => self.ident_expr(),
-        }
-    }
-
-    /// Return the field name for the field when serializing.
-    pub fn deserialize_name_expr(&self) -> P<ast::Expr> {
-        match self.deserialize_name {
-            Some(ref name) => AstBuilder::new().expr().build_lit(P(name.clone())),
-            None => self.ident_expr(),
-        }
     }
 
     pub fn deny_unknown_fields(&self) -> bool {
@@ -98,17 +128,13 @@ impl ContainerAttrs {
 /// Represents variant attribute information
 #[derive(Debug)]
 pub struct VariantAttrs {
-    ident: ast::Ident,
+    name: Name,
-    serialize_name: Option<ast::Lit>,
-    deserialize_name: Option<ast::Lit>,
 }
 
 impl VariantAttrs {
     pub fn from_variant(cx: &ExtCtxt, variant: &ast::Variant) -> Result<Self, Error> {
         let mut variant_attrs = VariantAttrs {
-            ident: variant.node.name,
+            name: Name::new(variant.node.name),
-            serialize_name: None,
-            deserialize_name: None,
         };
 
         for meta_items in variant.node.attrs.iter().filter_map(get_serde_meta_items) {
@@ -116,15 +142,18 @@ impl VariantAttrs {
                 match meta_item.node {
                     // Parse `#[serde(rename="foo")]`
                     ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"rename" => {
-                        variant_attrs.serialize_name = Some(lit.clone());
+                        let s = try!(get_str_from_lit(cx, name, lit));
-                        variant_attrs.deserialize_name = Some(lit.clone());
+
+                        variant_attrs.name.serialize_name = Some(s.clone());
+                        variant_attrs.name.deserialize_name = Some(s);
                     }
 
                     // Parse `#[serde(rename(serialize="foo", deserialize="bar"))]`
                     ast::MetaItemKind::List(ref name, ref meta_items) if name == &"rename" => {
                         let (ser_name, de_name) = try!(get_renames(cx, meta_items));
-                        variant_attrs.serialize_name = ser_name;
+
-                        variant_attrs.deserialize_name = de_name;
+                        variant_attrs.name.serialize_name = ser_name;
+                        variant_attrs.name.deserialize_name = de_name;
                     }
 
                     _ => {
@@ -142,34 +171,15 @@ impl VariantAttrs {
         Ok(variant_attrs)
     }
 
-    /// Return the string expression of the field ident.
+    pub fn name(&self) -> &Name {
-    pub fn ident_expr(&self) -> P<ast::Expr> {
+        &self.name
-        AstBuilder::new().expr().str(self.ident)
-    }
-
-    /// Return the field name for the field when serializing.
-    pub fn serialize_name_expr(&self) -> P<ast::Expr> {
-        match self.serialize_name {
-            Some(ref name) => AstBuilder::new().expr().build_lit(P(name.clone())),
-            None => self.ident_expr(),
-        }
-    }
-
-    /// Return the field name for the field when serializing.
-    pub fn deserialize_name_expr(&self) -> P<ast::Expr> {
-        match self.deserialize_name {
-            Some(ref name) => AstBuilder::new().expr().build_lit(P(name.clone())),
-            None => self.ident_expr(),
-        }
     }
 }
 
 /// Represents field attribute information
 #[derive(Debug)]
 pub struct FieldAttrs {
-    ident: ast::Ident,
+    name: Name,
-    serialize_name: Option<ast::Lit>,
-    deserialize_name: Option<ast::Lit>,
     skip_serializing_field: bool,
     skip_serializing_field_if: Option<P<ast::Expr>>,
     default_expr_if_missing: Option<P<ast::Expr>>,
@@ -192,9 +202,7 @@ impl FieldAttrs {
         };
 
         let mut field_attrs = FieldAttrs {
-            ident: field_ident,
+            name: Name::new(field_ident),
-            serialize_name: None,
-            deserialize_name: None,
             skip_serializing_field: false,
             skip_serializing_field_if: None,
             default_expr_if_missing: None,
@@ -207,15 +215,18 @@ impl FieldAttrs {
                 match meta_item.node {
                     // Parse `#[serde(rename="foo")]`
                     ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"rename" => {
-                        field_attrs.serialize_name = Some(lit.clone());
+                        let s = try!(get_str_from_lit(cx, name, lit));
-                        field_attrs.deserialize_name = Some(lit.clone());
+
+                        field_attrs.name.serialize_name = Some(s.clone());
+                        field_attrs.name.deserialize_name = Some(s);
                     }
 
                     // Parse `#[serde(rename(serialize="foo", deserialize="bar"))]`
                     ast::MetaItemKind::List(ref name, ref meta_items) if name == &"rename" => {
                         let (ser_name, de_name) = try!(get_renames(cx, meta_items));
-                        field_attrs.serialize_name = ser_name;
+
-                        field_attrs.deserialize_name = de_name;
+                        field_attrs.name.serialize_name = ser_name;
+                        field_attrs.name.deserialize_name = de_name;
                     }
 
                     // Parse `#[serde(default)]`
@@ -290,25 +301,8 @@ impl FieldAttrs {
         Ok(field_attrs)
     }
 
-    /// Return the string expression of the field ident.
+    pub fn name(&self) -> &Name {
-    pub fn ident_expr(&self) -> P<ast::Expr> {
+        &self.name
-        AstBuilder::new().expr().str(self.ident)
-    }
-
-    /// Return the field name for the field when serializing.
-    pub fn serialize_name_expr(&self) -> P<ast::Expr> {
-        match self.serialize_name {
-            Some(ref name) => AstBuilder::new().expr().build_lit(P(name.clone())),
-            None => self.ident_expr(),
-        }
-    }
-
-    /// Return the field name for the field when deserializing.
-    pub fn deserialize_name_expr(&self) -> P<ast::Expr> {
-        match self.deserialize_name {
-            Some(ref name) => AstBuilder::new().expr().build_lit(P(name.clone())),
-            None => self.ident_expr(),
-        }
     }
 
     /// Predicate for using a field's default value
@@ -316,7 +310,7 @@ impl FieldAttrs {
         match self.default_expr_if_missing {
             Some(ref expr) => expr.clone(),
             None => {
-                let name = self.ident_expr();
+                let name = self.name.ident_expr();
                 AstBuilder::new().expr()
                     .try()
                     .method_call("missing_field").id("visitor")
@@ -357,18 +351,21 @@ pub fn get_struct_field_attrs(cx: &ExtCtxt,
 }
 
 fn get_renames(cx: &ExtCtxt,
-               items: &[P<ast::MetaItem>]) -> Result<(Option<ast::Lit>, Option<ast::Lit>), Error> {
+               items: &[P<ast::MetaItem>],
+              )-> Result<(Option<InternedString>, Option<InternedString>), Error> {
     let mut ser_name = None;
     let mut de_name = None;
 
     for item in items {
         match item.node {
             ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"serialize" => {
-                ser_name = Some(lit.clone());
+                let s = try!(get_str_from_lit(cx, name, lit));
+                ser_name = Some(s);
             }
 
             ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"deserialize" => {
-                de_name = Some(lit.clone());
+                let s = try!(get_str_from_lit(cx, name, lit));
+                de_name = Some(s);
             }
 
             _ => {
@@ -442,9 +439,9 @@ impl<'a, 'b> Folder for Respanner<'a, 'b> {
     }
 }
 
-fn parse_lit_into_path(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<ast::Path, Error> {
+fn get_str_from_lit(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<InternedString, Error> {
-    let source: &str = match lit.node {
+    match lit.node {
-        ast::LitKind::Str(ref source, _) => &source,
+        ast::LitKind::Str(ref s, _) => Ok(s.clone()),
         _ => {
             cx.span_err(
                 lit.span,
@@ -454,7 +451,11 @@ fn parse_lit_into_path(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<ast::
 
             return Err(Error);
         }
-    };
+    }
+}
+
+fn parse_lit_into_path(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<ast::Path, Error> {
+    let source = try!(get_str_from_lit(cx, name, lit));
 
     // If we just parse the string into an expression, any syntax errors in the source will only
     // have spans that point inside the string, and not back to the attribute. So to have better
@@ -463,7 +464,7 @@ fn parse_lit_into_path(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<ast::
     // and then finally parse them into an expression.
     let tts = parse::parse_tts_from_source_str(
         format!("<serde {} expansion>", name),
-        source.to_owned(),
+        (*source).to_owned(),
         cx.cfg(),
         cx.parse_sess());
 

serde_codegen/src/de.rs

@@ -10,6 +10,7 @@ use syntax::ast::{
 use syntax::codemap::Span;
 use syntax::ext::base::{Annotatable, ExtCtxt};
 use syntax::ext::build::AstBuilder;
+use syntax::parse::token::InternedString;
 use syntax::ptr::P;
 
 use attr;
@@ -266,7 +267,7 @@ fn deserialize_unit_struct(
     type_ident: Ident,
     container_attrs: &attr::ContainerAttrs,
 ) -> Result<P<ast::Expr>, Error> {
-    let type_name = container_attrs.deserialize_name_expr();
+    let type_name = container_attrs.name().deserialize_name_expr();
 
     Ok(quote_expr!(cx, {
         struct __Visitor;
@@ -318,7 +319,7 @@ fn deserialize_newtype_struct(
         1,
     );
 
-    let type_name = container_attrs.deserialize_name_expr();
+    let type_name = container_attrs.name().deserialize_name_expr();
 
     Ok(quote_expr!(cx, {
         $visitor_item
@@ -371,7 +372,7 @@ fn deserialize_tuple_struct(
         fields,
     );
 
-    let type_name = container_attrs.deserialize_name_expr();
+    let type_name = container_attrs.name().deserialize_name_expr();
 
     Ok(quote_expr!(cx, {
         $visitor_item
@@ -510,7 +511,7 @@ fn deserialize_struct(
         false,
     ));
 
-    let type_name = container_attrs.deserialize_name_expr();
+    let type_name = container_attrs.name().deserialize_name_expr();
 
     Ok(quote_expr!(cx, {
         $field_visitor
@@ -552,7 +553,7 @@ fn deserialize_item_enum(
 ) -> Result<P<ast::Expr>, Error> {
     let where_clause = &impl_generics.where_clause;
 
-    let type_name = container_attrs.deserialize_name_expr();
+    let type_name = container_attrs.name().deserialize_name_expr();
 
     let variant_visitor = deserialize_field_visitor(
         cx,
@@ -561,7 +562,7 @@ fn deserialize_item_enum(
             enum_def.variants.iter()
                 .map(|variant| {
                     let attrs = try!(attr::VariantAttrs::from_variant(cx, variant));
-                    Ok(attrs.deserialize_name_expr())
+                    Ok(attrs.name().deserialize_name())
                 })
                 .collect()
         ),
@@ -806,12 +807,12 @@ fn deserialize_struct_variant(
 fn deserialize_field_visitor(
     cx: &ExtCtxt,
     builder: &aster::AstBuilder,
-    field_names: Vec<P<ast::Expr>>,
+    field_names: Vec<InternedString>,
     container_attrs: &attr::ContainerAttrs,
     is_variant: bool,
 ) -> Vec<P<ast::Item>> {
     // Create the field names for the fields.
-    let field_idents: Vec<ast::Ident> = (0 .. field_names.len())
+    let field_idents: Vec<_> = (0 .. field_names.len())
         .map(|i| builder.id(format!("__field{}", i)))
         .collect();
 
@@ -846,22 +847,34 @@ fn deserialize_field_visitor(
         (builder.expr().str("expected a field"), builder.id("unknown_field"))
     };
 
+    let fallthrough_index_arm_expr = if !is_variant && !container_attrs.deny_unknown_fields() {
+        quote_expr!(cx, Ok(__Field::__ignore))
+    } else {
+        quote_expr!(cx, {
+            Err(::serde::de::Error::invalid_value($index_error_msg))
+        })
+    };
+
     let index_body = quote_expr!(cx,
         match value {
             $index_field_arms
-            _ => { Err(::serde::de::Error::syntax($index_error_msg)) }
+            _ => $fallthrough_index_arm_expr
         }
     );
 
+    // Convert the field names into byte strings.
+    let str_field_names: Vec<_> = field_names.iter()
+        .map(|name| builder.expr().lit().str(&name))
+        .collect();
+
     // Match arms to extract a field from a string
-    let default_field_arms: Vec<_> = field_idents.iter()
+    let str_field_arms: Vec<_> = field_idents.iter().zip(str_field_names.iter())
-        .zip(field_names.iter())
         .map(|(field_ident, field_name)| {
             quote_arm!(cx, $field_name => { Ok(__Field::$field_ident) })
         })
         .collect();
 
-    let fallthrough_arm_expr = if !is_variant && !container_attrs.deny_unknown_fields() {
+    let fallthrough_str_arm_expr = if !is_variant && !container_attrs.deny_unknown_fields() {
         quote_expr!(cx, Ok(__Field::__ignore))
     } else {
         quote_expr!(cx, Err(::serde::de::Error::$unknown_ident(value)))
@@ -869,8 +882,39 @@ fn deserialize_field_visitor(
 
     let str_body = quote_expr!(cx,
         match value {
-            $default_field_arms
+            $str_field_arms
-            _ => $fallthrough_arm_expr
+            _ => $fallthrough_str_arm_expr
+        }
+    );
+
+    // Convert the field names into byte strings.
+    let bytes_field_names: Vec<_> = field_names.iter()
+        .map(|name| {
+            let name: &str = name;
+            builder.expr().lit().byte_str(name)
+        })
+        .collect();
+
+    // Match arms to extract a field from a string
+    let bytes_field_arms: Vec<_> = field_idents.iter().zip(bytes_field_names.iter())
+        .map(|(field_ident, field_name)| {
+            quote_arm!(cx, $field_name => { Ok(__Field::$field_ident) })
+        })
+        .collect();
+
+    let fallthrough_bytes_arm_expr = if !is_variant && !container_attrs.deny_unknown_fields() {
+        quote_expr!(cx, Ok(__Field::__ignore))
+    } else {
+        quote_expr!(cx, {
+            let value = ::std::string::String::from_utf8_lossy(value);
+            Err(::serde::de::Error::$unknown_ident(&value))
+        })
+    };
+
+    let bytes_body = quote_expr!(cx,
+        match value {
+            $bytes_field_arms
+            _ => $fallthrough_bytes_arm_expr
         }
     );
 
@@ -906,17 +950,7 @@ fn deserialize_field_visitor(
                     fn visit_bytes<E>(&mut self, value: &[u8]) -> ::std::result::Result<__Field, E>
                         where E: ::serde::de::Error,
                     {
-                        // TODO: would be better to generate a byte string literal match
+                        $bytes_body
-                        match ::std::str::from_utf8(value) {
-                            Ok(s) => self.visit_str(s),
-                            _ => {
-                                Err(
-                                    ::serde::de::Error::invalid_value(
-                                        "could not convert a byte string to a String"
-                                    )
-                                )
-                            }
-                        }
                     }
                 }
 
@@ -947,7 +981,7 @@ fn deserialize_struct_visitor(
                                              field,
                                              is_enum)
             );
-            Ok(field_attrs.deserialize_name_expr())
+            Ok(field_attrs.name().deserialize_name())
         })
         .collect();
 

serde_codegen/src/ser.rs

@@ -185,7 +185,7 @@ fn serialize_unit_struct(
     cx: &ExtCtxt,
     container_attrs: &attr::ContainerAttrs,
 ) -> Result<P<ast::Expr>, Error> {
-    let type_name = container_attrs.serialize_name_expr();
+    let type_name = container_attrs.name().serialize_name_expr();
 
     Ok(quote_expr!(cx,
         serializer.serialize_unit_struct($type_name)
@@ -196,7 +196,7 @@ fn serialize_newtype_struct(
     cx: &ExtCtxt,
     container_attrs: &attr::ContainerAttrs,
 ) -> Result<P<ast::Expr>, Error> {
-    let type_name = container_attrs.serialize_name_expr();
+    let type_name = container_attrs.name().serialize_name_expr();
 
     Ok(quote_expr!(cx,
         serializer.serialize_newtype_struct($type_name, &self.0)
@@ -224,7 +224,7 @@ fn serialize_tuple_struct(
         impl_generics,
     );
 
-    let type_name = container_attrs.serialize_name_expr();
+    let type_name = container_attrs.name().serialize_name_expr();
 
     Ok(quote_expr!(cx, {
         $visitor_struct
@@ -259,7 +259,7 @@ fn serialize_struct(
         false,
     ));
 
-    let type_name = container_attrs.serialize_name_expr();
+    let type_name = container_attrs.name().serialize_name_expr();
 
     Ok(quote_expr!(cx, {
         $visitor_struct
@@ -316,11 +316,11 @@ fn serialize_variant(
     variant_index: usize,
     container_attrs: &attr::ContainerAttrs,
 ) -> Result<ast::Arm, Error> {
-    let type_name = container_attrs.serialize_name_expr();
+    let type_name = container_attrs.name().serialize_name_expr();
 
     let variant_ident = variant.node.name;
     let variant_attrs = try!(attr::VariantAttrs::from_variant(cx, variant));
-    let variant_name = variant_attrs.serialize_name_expr();
+    let variant_name = variant_attrs.name().serialize_name_expr();
 
     match variant.node.data {
         ast::VariantData::Unit(_) => {
@@ -551,7 +551,7 @@ fn serialize_struct_variant(
         true,
     ));
 
-    let container_name = container_attrs.serialize_name_expr();
+    let container_name = container_attrs.name().serialize_name_expr();
 
     Ok(quote_expr!(cx, {
         $variant_struct
@@ -658,7 +658,7 @@ fn serialize_struct_visitor(
         .map(|(i, (ref field, ref field_attr))| {
             let name = field.node.ident().expect("struct has unnamed field");
 
-            let key_expr = field_attr.serialize_name_expr();
+            let key_expr = field_attr.name().serialize_name_expr();
 
             let stmt = if let Some(expr) = field_attr.skip_serializing_field_if() {
                     Some(quote_stmt!(cx, if $expr { continue; }))