feat(codegen): Inhibit generic bounds if skip_serializing

serde_codegen/src/attr.rs

@@ -382,7 +382,7 @@ fn get_renames(cx: &ExtCtxt,
     Ok((ser_name, de_name))
 }
 
-fn get_serde_meta_items(attr: &ast::Attribute) -> Option<&[P<ast::MetaItem>]> {
+pub fn get_serde_meta_items(attr: &ast::Attribute) -> Option<&[P<ast::MetaItem>]> {
     match attr.node.value.node {
         ast::MetaItemKind::List(ref name, ref items) if name == &"serde" => {
             attr::mark_used(&attr);

serde_codegen/src/ser.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use aster;
 
 use syntax::ast::{
@@ -10,6 +12,7 @@ use syntax::codemap::Span;
 use syntax::ext::base::{Annotatable, ExtCtxt};
 use syntax::ext::build::AstBuilder;
 use syntax::ptr::P;
+use syntax::visit;
 
 use attr;
 use error::Error;
@@ -60,11 +63,7 @@ fn serialize_item(
         }
     };
 
-    let impl_generics = builder.from_generics(generics.clone())
-        .add_ty_param_bound(
-            builder.path().global().ids(&["serde", "ser", "Serialize"]).build()
-        )
-        .build();
+    let impl_generics = build_impl_generics(cx, builder, item, generics);
 
     let ty = builder.ty().path()
         .segment(item.ident).with_generics(impl_generics.clone()).build()
@@ -89,6 +88,157 @@ fn serialize_item(
     ).unwrap())
 }
 
+// All the generics in the input, plus a bound `T: Serialize` for each generic
+// field type that will be serialized by us.
+fn build_impl_generics(
+    cx: &ExtCtxt,
+    builder: &aster::AstBuilder,
+    item: &Item,
+    generics: &ast::Generics,
+) -> ast::Generics {
+    let serialize_path = builder.path()
+        .global()
+        .ids(&["serde", "ser", "Serialize"])
+        .build();
+
+    builder.from_generics(generics.clone())
+        .with_predicates(
+            all_variants(cx, item).iter()
+                .flat_map(|variant_data| all_struct_fields(variant_data))
+                .filter(|field| serialized_by_us(field))
+                .map(|field| &field.node.ty)
+                // TODO this filter can be removed later, see comment on function
+                .filter(|ty| contains_generic(ty, generics))
+                .map(|ty| strip_reference(ty))
+                .map(|ty| builder.where_predicate()
+                    // the type that is being bounded i.e. T
+                    .bound().build(ty.clone())
+                    // the bound i.e. Serialize
+                    .bound().trait_(serialize_path.clone()).build()
+                    .build()))
+        .build()
+}
+
+fn all_variants<'a>(cx: &ExtCtxt, item: &'a Item) -> Vec<&'a ast::VariantData> {
+    match item.node {
+        ast::ItemKind::Struct(ref variant_data, _) => {
+            vec![variant_data]
+        }
+        ast::ItemKind::Enum(ref enum_def, _) => {
+            enum_def.variants.iter()
+                .map(|variant| &variant.node.data)
+                .collect()
+        }
+        _ => {
+            cx.span_bug(item.span,
+                        "expected Item to be Struct or Enum in #[derive(Serialize)]");
+        }
+    }
+}
+
+fn all_struct_fields(variant_data: &ast::VariantData) -> &[ast::StructField] {
+    match *variant_data {
+        ast::VariantData::Struct(ref fields, _) |
+        ast::VariantData::Tuple(ref fields, _) => {
+            fields
+        }
+        ast::VariantData::Unit(_) => {
+            &[]
+        }
+    }
+}
+
+// Fields with a `skip_serializing` or `serialize_with` attribute are not
+// serialized by us. All other fields may need a `T: Serialize` bound where T is
+// the type of the field.
+fn serialized_by_us(field: &ast::StructField) -> bool {
+    for meta_items in field.node.attrs.iter().filter_map(attr::get_serde_meta_items) {
+        for meta_item in meta_items {
+            match meta_item.node {
+                ast::MetaItemKind::Word(ref name) if name == &"skip_serializing" => {
+                    return false
+                }
+                ast::MetaItemKind::NameValue(ref name, _) if name == &"serialize_with" => {
+                    return false
+                }
+                _ => {}
+            }
+        }
+    }
+    true
+}
+
+// Rust <1.7 enforces that `where` clauses involve generic type parameters. The
+// corresponding compiler error is E0193. It is no longer enforced in Rust >=1.7
+// so this filtering can be removed in the future when we stop supporting <1.7.
+//
+// E0193 means we must not generate a `where` clause like `i32: Serialize`
+// because even though i32 implements Serialize, i32 is not a generic type
+// parameter. Clauses like `T: Serialize` and `Option<T>: Serialize` are okay.
+// This function decides whether a given type references any of the generic type
+// parameters in the input `Generics`.
+fn contains_generic(ty: &ast::Ty, generics: &ast::Generics) -> bool {
+    struct FindGeneric<'a> {
+        generic_names: &'a HashSet<ast::Name>,
+        found_generic: bool,
+    }
+    impl<'a, 'v> visit::Visitor<'v> for FindGeneric<'a> {
+        fn visit_path(&mut self, path: &'v ast::Path, _id: ast::NodeId) {
+            if !path.global
+                    && path.segments.len() == 1
+                    && self.generic_names.contains(&path.segments[0].identifier.name) {
+                self.found_generic = true;
+            } else {
+                visit::walk_path(self, path);
+            }
+        }
+    }
+
+    let generic_names: HashSet<_> = generics.ty_params.iter()
+        .map(|ty_param| ty_param.ident.name)
+        .collect();
+
+    let mut visitor = FindGeneric {
+        generic_names: &generic_names,
+        found_generic: false,
+    };
+    visit::walk_ty(&mut visitor, ty);
+    visitor.found_generic
+}
+
+// This is required to handle types that use both a reference and a value of
+// the same type, as in:
+//
+//    enum Test<'a, T> where T: 'a {
+//        Lifetime(&'a T),
+//        NoLifetime(T),
+//    }
+//
+// Preserving references, we would generate an impl like:
+//
+//    impl<'a, T> Serialize for Test<'a, T>
+//        where &'a T: Serialize,
+//              T: Serialize { ... }
+//
+// And taking a reference to one of the elements would fail with:
+//
+//    error: cannot infer an appropriate lifetime for pattern due
+//    to conflicting requirements [E0495]
+//        Test::NoLifetime(ref v) => { ... }
+//                         ^~~~~
+//
+// Instead, we strip references before adding `T: Serialize` bounds in order to
+// generate:
+//
+//    impl<'a, T> Serialize for Test<'a, T>
+//        where T: Serialize { ... }
+fn strip_reference(ty: &P<ast::Ty>) -> &P<ast::Ty> {
+    match ty.node {
+        ast::TyKind::Rptr(_, ref mut_ty) => &mut_ty.ty,
+        _ => ty
+    }
+}
+
 fn serialize_body(
     cx: &ExtCtxt,
     builder: &aster::AstBuilder,