From 660ea7bd7bd0525ae07b49ff67bdeb4238c7e741 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 20 May 2016 22:03:20 -0700 Subject: [PATCH 1/9] Attribute for handwritten where clauses --- serde_codegen/src/attr.rs | 147 ++++++++++++++++++++++++++++------ serde_codegen/src/bound.rs | 66 ++++++++++++--- serde_codegen/src/de.rs | 143 ++++++++++++++++++--------------- serde_codegen/src/ser.rs | 68 +++++++++------- serde_tests/tests/test_gen.rs | 36 +++++++++ 5 files changed, 328 insertions(+), 132 deletions(-) diff --git a/serde_codegen/src/attr.rs b/serde_codegen/src/attr.rs index cc5cfe38..ca6735cf 100644 --- a/serde_codegen/src/attr.rs +++ b/serde_codegen/src/attr.rs @@ -4,7 +4,7 @@ use syntax::attr; use syntax::codemap::Span; use syntax::ext::base::ExtCtxt; use syntax::fold::Folder; -use syntax::parse::parser::PathStyle; +use syntax::parse::parser::{Parser, PathStyle}; use syntax::parse::token::{self, InternedString}; use syntax::parse; use syntax::print::pprust::{lit_to_string, meta_item_to_string}; @@ -62,6 +62,8 @@ impl Name { pub struct ContainerAttrs { name: Name, deny_unknown_fields: bool, + ser_where: Option>, + de_where: Option>, } impl ContainerAttrs { @@ -70,6 +72,8 @@ impl ContainerAttrs { let mut container_attrs = ContainerAttrs { name: Name::new(item.ident), deny_unknown_fields: false, + ser_where: None, + de_where: None, }; for meta_items in item.attrs().iter().filter_map(get_serde_meta_items) { @@ -96,6 +100,20 @@ impl ContainerAttrs { container_attrs.deny_unknown_fields = true; } + // Parse `#[serde(where="D: Serialize")]` + ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"where" => { + let where_predicates = try!(parse_lit_into_where(cx, name, lit)); + container_attrs.ser_where = Some(where_predicates.clone()); + container_attrs.de_where = Some(where_predicates.clone()); + } + + // Parse `#[serde(where(serialize="D: Serialize", deserialize="D: Deserialize"))]` + ast::MetaItemKind::List(ref name, ref meta_items) if name == &"where" => { + let (ser_where, de_where) = try!(get_where_predicates(cx, meta_items)); + container_attrs.ser_where = ser_where; + container_attrs.de_where = de_where; + } + _ => { cx.span_err( meta_item.span, @@ -118,6 +136,14 @@ impl ContainerAttrs { pub fn deny_unknown_fields(&self) -> bool { self.deny_unknown_fields } + + pub fn ser_where(&self) -> Option<&Vec> { + self.ser_where.as_ref() + } + + pub fn de_where(&self) -> Option<&Vec> { + self.de_where.as_ref() + } } /// Represents variant attribute information @@ -181,6 +207,8 @@ pub struct FieldAttrs { default_expr_if_missing: Option>, serialize_with: Option, deserialize_with: Option, + ser_where: Option>, + de_where: Option>, } impl FieldAttrs { @@ -203,6 +231,8 @@ impl FieldAttrs { default_expr_if_missing: None, serialize_with: None, deserialize_with: None, + ser_where: None, + de_where: None, }; for meta_items in field.attrs.iter().filter_map(get_serde_meta_items) { @@ -274,6 +304,20 @@ impl FieldAttrs { field_attrs.deserialize_with = Some(path); } + // Parse `#[serde(where="D: Serialize")]` + ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"where" => { + let where_predicates = try!(parse_lit_into_where(cx, name, lit)); + field_attrs.ser_where = Some(where_predicates.clone()); + field_attrs.de_where = Some(where_predicates.clone()); + } + + // Parse `#[serde(where(serialize="D: Serialize", deserialize="D: Deserialize"))]` + ast::MetaItemKind::List(ref name, ref meta_items) if name == &"where" => { + let (ser_where, de_where) = try!(get_where_predicates(cx, meta_items)); + field_attrs.ser_where = ser_where; + field_attrs.de_where = de_where; + } + _ => { cx.span_err( meta_item.span, @@ -316,45 +360,59 @@ impl FieldAttrs { pub fn deserialize_with(&self) -> Option<&ast::Path> { self.deserialize_with.as_ref() } + + pub fn ser_where(&self) -> Option<&Vec> { + self.ser_where.as_ref() + } + + pub fn de_where(&self) -> Option<&Vec> { + self.de_where.as_ref() + } } /// Zip together fields and `#[serde(...)]` attributes on those fields. -pub fn fields_with_attrs<'a>( +pub fn fields_with_attrs( cx: &ExtCtxt, - fields: &'a [ast::StructField], -) -> Result, Error> { + fields: &[ast::StructField], +) -> Result, Error> { fields.iter() .enumerate() .map(|(i, field)| { let attrs = try!(FieldAttrs::from_field(cx, i, field)); - Ok((field, attrs)) + Ok((field.clone(), attrs)) }) .collect() } -fn get_renames(cx: &ExtCtxt, - items: &[P], - )-> Result<(Option, Option), Error> { - let mut ser_name = None; - let mut de_name = None; +fn get_ser_and_de( + cx: &ExtCtxt, + attribute: &str, + items: &[P], + f: F +) -> Result<(Option, Option), Error> + where F: Fn(&ExtCtxt, &str, &ast::Lit) -> Result, +{ + let mut ser_item = None; + let mut de_item = None; for item in items { match item.node { ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"serialize" => { - let s = try!(get_str_from_lit(cx, name, lit)); - ser_name = Some(s); + let s = try!(f(cx, name, lit)); + ser_item = Some(s); } ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"deserialize" => { - let s = try!(get_str_from_lit(cx, name, lit)); - de_name = Some(s); + let s = try!(f(cx, name, lit)); + de_item = Some(s); } _ => { cx.span_err( item.span, - &format!("unknown rename attribute `{}`", + &format!("unknown {} attribute `{}`", + attribute, meta_item_to_string(item))); return Err(Error); @@ -362,7 +420,21 @@ fn get_renames(cx: &ExtCtxt, } } - Ok((ser_name, de_name)) + Ok((ser_item, de_item)) +} + +fn get_renames( + cx: &ExtCtxt, + items: &[P], +) -> Result<(Option, Option), Error> { + get_ser_and_de(cx, "rename", items, get_str_from_lit) +} + +fn get_where_predicates( + cx: &ExtCtxt, + items: &[P], +) -> Result<(Option>, Option>), Error> { + get_ser_and_de(cx, "where", items, parse_lit_into_where) } pub fn get_serde_meta_items(attr: &ast::Attribute) -> Option<&[P]> { @@ -437,17 +509,18 @@ fn get_str_from_lit(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result Result { - 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 - // error reporting, we'll first parse the string into a token tree. Then we'll update those - // spans to say they're coming from a macro context that originally came from the attribute, - // and then finally parse them into an expression. +// If we just parse a string literal from an attibute, 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 error reporting, we'll first parse the string +// into a token tree. Then we'll update those spans to say they're coming from a +// macro context that originally came from the attribnute, and then finally +// parse them into an expression or where-clause. +fn parse_string_via_tts(cx: &ExtCtxt, name: &str, string: String, action: F) -> Result + where F: for<'a> Fn(&'a mut Parser) -> parse::PResult<'a, T>, +{ let tts = panictry!(parse::parse_tts_from_source_str( format!("", name), - (*source).to_owned(), + string, cx.cfg(), cx.parse_sess())); @@ -456,7 +529,7 @@ fn parse_lit_into_path(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result path, Err(mut e) => { e.emit(); @@ -476,6 +549,28 @@ fn parse_lit_into_path(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result Result { + let string = try!(get_str_from_lit(cx, name, lit)).to_string(); + + parse_string_via_tts(cx, name, string, |parser| { + parser.parse_path(PathStyle::Type) + }) +} + +fn parse_lit_into_where(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result, Error> { + let string = try!(get_str_from_lit(cx, name, lit)); + if string.is_empty() { + return Ok(Vec::new()); + } + + let where_string = format!("where {}", string); + + parse_string_via_tts(cx, name, where_string, |parser| { + let where_clause = try!(parser.parse_where_clause()); + Ok(where_clause.predicates) + }) +} + /// This function wraps the expression in `#[serde(default="...")]` in a function to prevent it /// from accessing the internal `Deserialize` state. fn wrap_default(path: ast::Path) -> P { diff --git a/serde_codegen/src/bound.rs b/serde_codegen/src/bound.rs index 4f563e9b..7ec11316 100644 --- a/serde_codegen/src/bound.rs +++ b/serde_codegen/src/bound.rs @@ -7,6 +7,9 @@ use syntax::ext::base::ExtCtxt; use syntax::ptr::P; use syntax::visit; +use attr; +use error::Error; + // Remove the default from every type parameter because in the generated impls // they look like associated types: "error: associated type bindings are not // allowed here". @@ -21,20 +24,50 @@ pub fn without_defaults(generics: &ast::Generics) -> ast::Generics { } } -pub fn with_bound( +pub fn with_where_predicates( + builder: &AstBuilder, + generics: &ast::Generics, + predicates: &Vec, +) -> ast::Generics { + builder.from_generics(generics.clone()) + .with_predicates(predicates.clone()) + .build() +} + +pub fn with_where_predicates_from_fields( cx: &ExtCtxt, builder: &AstBuilder, item: &ast::Item, generics: &ast::Generics, - filter: &Fn(&ast::StructField) -> bool, - bound: &ast::Path, -) -> ast::Generics { - builder.from_generics(generics.clone()) + from_field: F, +) -> Result + where F: Fn(&attr::FieldAttrs) -> Option<&Vec>, +{ + Ok(builder.from_generics(generics.clone()) .with_predicates( - all_variants(cx, item).iter() - .flat_map(|variant_data| all_struct_fields(variant_data)) - .filter(|field| filter(field)) - .map(|field| &field.ty) + try!(all_fields_with_attrs(cx, item)) + .iter() + .flat_map(|&(_, ref attrs)| from_field(attrs)) + .flat_map(|predicates| predicates.clone())) + .build()) +} + +pub fn with_bound( + cx: &ExtCtxt, + builder: &AstBuilder, + item: &ast::Item, + generics: &ast::Generics, + filter: F, + bound: &ast::Path, +) -> Result + where F: Fn(&ast::StructField, &attr::FieldAttrs) -> bool, +{ + Ok(builder.from_generics(generics.clone()) + .with_predicates( + try!(all_fields_with_attrs(cx, item)) + .iter() + .filter(|&&(ref field, ref attrs)| filter(field, attrs)) + .map(|&(ref field, _)| &field.ty) // TODO this filter can be removed later, see comment on function .filter(|ty| contains_generic(ty, generics)) .map(|ty| strip_reference(ty)) @@ -44,7 +77,20 @@ pub fn with_bound( // the bound e.g. Serialize .bound().trait_(bound.clone()).build() .build())) - .build() + .build()) +} + +fn all_fields_with_attrs( + cx: &ExtCtxt, + item: &ast::Item, +) -> Result, Error> { + let fields: Vec = + all_variants(cx, item).iter() + .flat_map(|variant_data| all_struct_fields(variant_data)) + .cloned() + .collect(); + + attr::fields_with_attrs(cx, &fields) } fn all_variants<'a>(cx: &ExtCtxt, item: &'a ast::Item) -> Vec<&'a ast::VariantData> { diff --git a/serde_codegen/src/de.rs b/serde_codegen/src/de.rs index a2779709..0970f26b 100644 --- a/serde_codegen/src/de.rs +++ b/serde_codegen/src/de.rs @@ -35,36 +35,53 @@ pub fn expand_derive_deserialize( let builder = aster::AstBuilder::new().span(span); - let generics = match item.node { - ast::ItemKind::Struct(_, ref generics) => generics, - ast::ItemKind::Enum(_, ref generics) => generics, - _ => { - cx.span_err( - meta_item.span, - "`#[derive(Deserialize)]` may only be applied to structs and enums"); - return; - } - }; - - let impl_generics = build_impl_generics(cx, &builder, item, generics); - - let ty = builder.ty().path() - .segment(item.ident).with_generics(impl_generics.clone()).build() - .build(); - - let body = match deserialize_body(cx, &builder, &item, &impl_generics, ty.clone()) { - Ok(body) => body, + let impl_item = match deserialize_item(cx, &builder, &item) { + Ok(item) => item, Err(Error) => { // An error occured, but it should have been reported already. return; } }; + push(Annotatable::Item(impl_item)) +} + +fn deserialize_item( + cx: &ExtCtxt, + builder: &aster::AstBuilder, + item: &Item, +) -> Result, Error> { + let generics = match item.node { + ast::ItemKind::Struct(_, ref generics) => generics, + ast::ItemKind::Enum(_, ref generics) => generics, + _ => { + cx.span_err( + item.span, + "`#[derive(Deserialize)]` may only be applied to structs and enums"); + return Err(Error); + } + }; + + let container_attrs = try!(attr::ContainerAttrs::from_item(cx, item)); + + let impl_generics = try!(build_impl_generics(cx, &builder, item, generics, &container_attrs)); + + let ty = builder.ty().path() + .segment(item.ident).with_generics(impl_generics.clone()).build() + .build(); + + let body = try!(deserialize_body(cx, + &builder, + &item, + &impl_generics, + ty.clone(), + &container_attrs)); + let where_clause = &impl_generics.where_clause; let dummy_const = builder.id(format!("_IMPL_DESERIALIZE_FOR_{}", item.ident)); - let impl_item = quote_item!(cx, + Ok(quote_item!(cx, #[allow(non_upper_case_globals, unused_attributes, unused_qualifications)] const $dummy_const: () = { extern crate serde as _serde; @@ -77,9 +94,7 @@ pub fn expand_derive_deserialize( } } }; - ).unwrap(); - - push(Annotatable::Item(impl_item)) + ).unwrap()) } // All the generics in the input, plus a bound `T: Deserialize` for each generic @@ -90,41 +105,43 @@ fn build_impl_generics( builder: &aster::AstBuilder, item: &Item, generics: &ast::Generics, -) -> ast::Generics { + container_attrs: &attr::ContainerAttrs, +) -> Result { let generics = bound::without_defaults(generics); - let generics = bound::with_bound(cx, builder, item, &generics, - &deserialized_by_us, - &builder.path().ids(&["_serde", "de", "Deserialize"]).build()); - let generics = bound::with_bound(cx, builder, item, &generics, - &requires_default, - &builder.path().global().ids(&["std", "default", "Default"]).build()); - generics + + let generics = try!(bound::with_where_predicates_from_fields( + cx, builder, item, &generics, + |attrs| attrs.de_where())); + + match container_attrs.de_where() { + Some(predicates) => { + let generics = bound::with_where_predicates(builder, &generics, predicates); + Ok(generics) + } + None => { + let generics = try!(bound::with_bound(cx, builder, item, &generics, + deserialized_by_us, + &builder.path().ids(&["_serde", "de", "Deserialize"]).build())); + let generics = try!(bound::with_bound(cx, builder, item, &generics, + requires_default, + &builder.path().global().ids(&["std", "default", "Default"]).build())); + Ok(generics) + } + } } // Fields with a `skip_deserializing` or `deserialize_with` attribute are not // deserialized by us. All other fields may need a `T: Deserialize` bound where // T is the type of the field. -fn deserialized_by_us(field: &ast::StructField) -> bool { - for meta_items in field.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_deserializing" => { - return false - } - ast::MetaItemKind::NameValue(ref name, _) if name == &"deserialize_with" => { - return false - } - _ => {} - } - } - } - true +fn deserialized_by_us(_: &ast::StructField, attrs: &attr::FieldAttrs) -> bool { + !attrs.skip_deserializing_field() + && attrs.deserialize_with().is_none() + && attrs.de_where().is_none() } // Fields with a `default` attribute (not `default=...`), and fields with a // `skip_deserializing` attribute that do not also have `default=...`. -fn requires_default(field: &ast::StructField) -> bool { - let mut has_skip_deserializing = false; +fn requires_default(field: &ast::StructField, attrs: &attr::FieldAttrs) -> bool { for meta_items in field.attrs.iter().filter_map(attr::get_serde_meta_items) { for meta_item in meta_items { match meta_item.node { @@ -134,14 +151,11 @@ fn requires_default(field: &ast::StructField) -> bool { ast::MetaItemKind::NameValue(ref name, _) if name == &"default" => { return false } - ast::MetaItemKind::Word(ref name) if name == &"skip_deserializing" => { - has_skip_deserializing = true - } _ => {} } } } - has_skip_deserializing + attrs.skip_deserializing_field() } fn deserialize_body( @@ -150,9 +164,8 @@ fn deserialize_body( item: &Item, impl_generics: &ast::Generics, ty: P, + container_attrs: &attr::ContainerAttrs, ) -> Result, Error> { - let container_attrs = try!(attr::ContainerAttrs::from_item(cx, item)); - match item.node { ast::ItemKind::Struct(ref variant_data, _) => { deserialize_item_struct( @@ -163,7 +176,7 @@ fn deserialize_body( ty, item.span, variant_data, - &container_attrs, + container_attrs, ) } ast::ItemKind::Enum(ref enum_def, _) => { @@ -174,7 +187,7 @@ fn deserialize_body( impl_generics, ty, enum_def, - &container_attrs, + container_attrs, ) } _ => { @@ -441,12 +454,12 @@ fn deserialize_seq( type_ident: Ident, type_path: ast::Path, impl_generics: &ast::Generics, - fields: &[(&ast::StructField, attr::FieldAttrs)], + fields: &[(ast::StructField, attr::FieldAttrs)], is_struct: bool, ) -> Result, Error> { let let_values: Vec<_> = fields.iter() .enumerate() - .map(|(i, &(field, ref attrs))| { + .map(|(i, &(ref field, ref attrs))| { let name = builder.id(format!("__field{}", i)); if attrs.skip_deserializing_field() { let default = expr_is_missing(cx, attrs); @@ -486,7 +499,7 @@ fn deserialize_seq( .with_id_exprs( fields.iter() .enumerate() - .map(|(i, &(field, _))| { + .map(|(i, &(ref field, _))| { ( match field.ident { Some(name) => name.clone(), @@ -521,9 +534,9 @@ fn deserialize_newtype_struct( type_ident: Ident, type_path: &ast::Path, impl_generics: &ast::Generics, - field: &(&ast::StructField, attr::FieldAttrs), + field: &(ast::StructField, attr::FieldAttrs), ) -> Result, Error> { - let &(field, ref attrs) = field; + let &(ref field, ref attrs) = field; let value = match attrs.deserialize_with() { None => { let field_ty = &field.ty; @@ -982,7 +995,7 @@ fn deserialize_struct_visitor( type_ident: Ident, struct_path: ast::Path, impl_generics: &ast::Generics, - fields: &[(&ast::StructField, attr::FieldAttrs)], + fields: &[(ast::StructField, attr::FieldAttrs)], container_attrs: &attr::ContainerAttrs, ) -> Result<(Vec>, ast::Stmt, P), Error> { let field_exprs = fields.iter() @@ -1010,7 +1023,7 @@ fn deserialize_struct_visitor( let fields_expr = builder.expr().ref_().slice() .with_exprs( fields.iter() - .map(|&(field, _)| { + .map(|&(ref field, _)| { match field.ident { Some(name) => builder.expr().str(name), None => { @@ -1034,7 +1047,7 @@ fn deserialize_map( type_ident: Ident, struct_path: ast::Path, impl_generics: &ast::Generics, - fields: &[(&ast::StructField, attr::FieldAttrs)], + fields: &[(ast::StructField, attr::FieldAttrs)], container_attrs: &attr::ContainerAttrs, ) -> Result, Error> { // Create the field names for the fields. @@ -1056,7 +1069,7 @@ fn deserialize_map( // Match arms to extract a value for a field. let value_arms = fields_attrs_names.iter() .filter(|&&(_, ref attrs, _)| !attrs.skip_deserializing_field()) - .map(|&(field, ref attrs, name)| { + .map(|&(ref field, ref attrs, name)| { let deser_name = attrs.name().deserialize_name(); let name_str = builder.expr().lit().str(deser_name); diff --git a/serde_codegen/src/ser.rs b/serde_codegen/src/ser.rs index a1220f69..815d84dc 100644 --- a/serde_codegen/src/ser.rs +++ b/serde_codegen/src/ser.rs @@ -60,7 +60,9 @@ fn serialize_item( } }; - let impl_generics = build_impl_generics(cx, builder, item, generics); + let container_attrs = try!(attr::ContainerAttrs::from_item(cx, item)); + + let impl_generics = try!(build_impl_generics(cx, builder, item, generics, &container_attrs)); let ty = builder.ty().path() .segment(item.ident).with_generics(impl_generics.clone()).build() @@ -70,7 +72,8 @@ fn serialize_item( &builder, &item, &impl_generics, - ty.clone())); + ty.clone(), + &container_attrs)); let where_clause = &impl_generics.where_clause; @@ -99,32 +102,36 @@ fn build_impl_generics( builder: &aster::AstBuilder, item: &Item, generics: &ast::Generics, -) -> ast::Generics { + container_attrs: &attr::ContainerAttrs, +) -> Result { let generics = bound::without_defaults(generics); - let generics = bound::with_bound(cx, builder, item, &generics, - &serialized_by_us, - &builder.path().ids(&["_serde", "ser", "Serialize"]).build()); - generics + + let generics = try!(bound::with_where_predicates_from_fields( + cx, builder, item, &generics, + |attrs| attrs.ser_where())); + + match container_attrs.ser_where() { + Some(predicates) => { + let generics = bound::with_where_predicates(builder, &generics, predicates); + Ok(generics) + } + None => { + let generics = try!(bound::with_bound(cx, builder, item, &generics, + needs_serialize_bound, + &builder.path().ids(&["_serde", "ser", "Serialize"]).build())); + Ok(generics) + } + } } // 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.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 +// serialized by us so we do not generate a bound. Fields with a `where` +// attribute specify their own bound so we do not generate one. All other fields +// may need a `T: Serialize` bound where T is the type of the field. +fn needs_serialize_bound(_: &ast::StructField, attrs: &attr::FieldAttrs) -> bool { + !attrs.skip_serializing_field() + && attrs.serialize_with().is_none() + && attrs.ser_where().is_none() } fn serialize_body( @@ -133,9 +140,8 @@ fn serialize_body( item: &Item, impl_generics: &ast::Generics, ty: P, + container_attrs: &attr::ContainerAttrs, ) -> Result, Error> { - let container_attrs = try!(attr::ContainerAttrs::from_item(cx, item)); - match item.node { ast::ItemKind::Struct(ref variant_data, _) => { serialize_item_struct( @@ -145,7 +151,7 @@ fn serialize_body( ty, item.span, variant_data, - &container_attrs, + container_attrs, ) } ast::ItemKind::Enum(ref enum_def, _) => { @@ -156,7 +162,7 @@ fn serialize_body( impl_generics, ty, enum_def, - &container_attrs, + container_attrs, ) } _ => { @@ -661,7 +667,7 @@ fn serialize_tuple_struct_visitor( let arms: Vec<_> = fields_with_attrs.iter() .enumerate() - .map(|(i, &(field, ref attrs))| { + .map(|(i, &(ref field, ref attrs))| { let mut field_expr = builder.expr().tup_field(i).field("value").self_(); if !is_enum { field_expr = quote_expr!(cx, &$field_expr); @@ -745,7 +751,7 @@ fn serialize_struct_visitor( let arms: Vec = fields_with_attrs.iter() .filter(|&&(_, ref attrs)| !attrs.skip_serializing_field()) .enumerate() - .map(|(i, &(field, ref attrs))| { + .map(|(i, &(ref field, ref attrs))| { let ident = field.ident.expect("struct has unnamed field"); let mut field_expr = quote_expr!(cx, self.value.$ident); if !is_enum { @@ -789,7 +795,7 @@ fn serialize_struct_visitor( let len = fields_with_attrs.iter() .filter(|&&(_, ref attrs)| !attrs.skip_serializing_field()) - .map(|&(field, ref attrs)| { + .map(|&(ref field, ref attrs)| { let ident = field.ident.expect("struct has unnamed fields"); let mut field_expr = quote_expr!(cx, self.value.$ident); if !is_enum { diff --git a/serde_tests/tests/test_gen.rs b/serde_tests/tests/test_gen.rs index 6f897e08..e6cbeff6 100644 --- a/serde_tests/tests/test_gen.rs +++ b/serde_tests/tests/test_gen.rs @@ -74,8 +74,44 @@ struct Tuple( X, ); +#[derive(Serialize, Deserialize)] +#[serde(where(serialize="D: Serialize", deserialize="D: Deserialize"))] +enum TreeNode { + Split { + left: Box>, + right: Box>, + }, + Leaf { + data: D, + }, +} + +#[derive(Serialize, Deserialize)] +struct ListNode { + data: D, + #[serde(where="")] + next: Box>, +} + +#[derive(Serialize, Deserialize)] +struct SerializeWithTrait { + #[serde(serialize_with="SerializeWith::serialize_with", + deserialize_with="DeserializeWith::deserialize_with", + where(serialize="D: SerializeWith", + deserialize="D: DeserializeWith"))] + data: D, +} + ////////////////////////////////////////////////////////////////////////// +trait SerializeWith { + fn serialize_with(_: &Self, _: &mut S) -> Result<(), S::Error>; +} + +trait DeserializeWith: Sized { + fn deserialize_with(_: &mut D) -> Result; +} + // Implements neither Serialize nor Deserialize struct X; fn ser_x(_: &X, _: &mut S) -> Result<(), S::Error> { panic!() } From 2256a049268dffb71e4d118c1aa9074bb87d54ba Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 4 Jun 2016 14:24:30 -0700 Subject: [PATCH 2/9] Address clippy lint "ptr_arg" --- serde_codegen/src/attr.rs | 16 ++++++++-------- serde_codegen/src/bound.rs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/serde_codegen/src/attr.rs b/serde_codegen/src/attr.rs index ca6735cf..10f77bf3 100644 --- a/serde_codegen/src/attr.rs +++ b/serde_codegen/src/attr.rs @@ -137,12 +137,12 @@ impl ContainerAttrs { self.deny_unknown_fields } - pub fn ser_where(&self) -> Option<&Vec> { - self.ser_where.as_ref() + pub fn ser_where(&self) -> Option<&[ast::WherePredicate]> { + self.ser_where.as_ref().map(Vec::as_slice) } - pub fn de_where(&self) -> Option<&Vec> { - self.de_where.as_ref() + pub fn de_where(&self) -> Option<&[ast::WherePredicate]> { + self.de_where.as_ref().map(Vec::as_slice) } } @@ -361,12 +361,12 @@ impl FieldAttrs { self.deserialize_with.as_ref() } - pub fn ser_where(&self) -> Option<&Vec> { - self.ser_where.as_ref() + pub fn ser_where(&self) -> Option<&[ast::WherePredicate]> { + self.ser_where.as_ref().map(Vec::as_slice) } - pub fn de_where(&self) -> Option<&Vec> { - self.de_where.as_ref() + pub fn de_where(&self) -> Option<&[ast::WherePredicate]> { + self.de_where.as_ref().map(Vec::as_slice) } } diff --git a/serde_codegen/src/bound.rs b/serde_codegen/src/bound.rs index 7ec11316..cdbe5fa1 100644 --- a/serde_codegen/src/bound.rs +++ b/serde_codegen/src/bound.rs @@ -27,10 +27,10 @@ pub fn without_defaults(generics: &ast::Generics) -> ast::Generics { pub fn with_where_predicates( builder: &AstBuilder, generics: &ast::Generics, - predicates: &Vec, + predicates: &[ast::WherePredicate], ) -> ast::Generics { builder.from_generics(generics.clone()) - .with_predicates(predicates.clone()) + .with_predicates(predicates.to_vec()) .build() } @@ -41,14 +41,14 @@ pub fn with_where_predicates_from_fields( generics: &ast::Generics, from_field: F, ) -> Result - where F: Fn(&attr::FieldAttrs) -> Option<&Vec>, + where F: Fn(&attr::FieldAttrs) -> Option<&[ast::WherePredicate]>, { Ok(builder.from_generics(generics.clone()) .with_predicates( try!(all_fields_with_attrs(cx, item)) .iter() .flat_map(|&(_, ref attrs)| from_field(attrs)) - .flat_map(|predicates| predicates.clone())) + .flat_map(|predicates| predicates.to_vec())) .build()) } From 4e6cd2d63f06bc4fca9ecec8d16b27960dfb78e0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 4 Jun 2016 15:48:23 -0700 Subject: [PATCH 3/9] Disable clippy lint "useless_let_if_seq" --- serde_codegen/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/serde_codegen/src/lib.rs b/serde_codegen/src/lib.rs index 52ca7912..36bb38c0 100644 --- a/serde_codegen/src/lib.rs +++ b/serde_codegen/src/lib.rs @@ -2,6 +2,7 @@ #![cfg_attr(feature = "nightly-testing", feature(plugin))] #![cfg_attr(feature = "nightly-testing", allow(too_many_arguments))] #![cfg_attr(feature = "nightly-testing", allow(used_underscore_binding))] +#![cfg_attr(feature = "nightly-testing", allow(useless_let_if_seq))] #![cfg_attr(not(feature = "with-syntex"), feature(rustc_private, plugin))] #![cfg_attr(not(feature = "with-syntex"), plugin(quasi_macros))] From bd408309055914710a93d8fcdcb81fc58e346693 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 4 Jun 2016 16:12:01 -0700 Subject: [PATCH 4/9] Do not generate bounds from recursive types --- serde_codegen/src/bound.rs | 45 +++++++++++++++++++++++++++++++++++ serde_tests/tests/test_gen.rs | 27 ++++++++++++++++----- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/serde_codegen/src/bound.rs b/serde_codegen/src/bound.rs index cdbe5fa1..2c659e33 100644 --- a/serde_codegen/src/bound.rs +++ b/serde_codegen/src/bound.rs @@ -70,6 +70,7 @@ pub fn with_bound( .map(|&(ref field, _)| &field.ty) // TODO this filter can be removed later, see comment on function .filter(|ty| contains_generic(ty, generics)) + .filter(|ty| !contains_recursion(ty, item.ident)) .map(|ty| strip_reference(ty)) .map(|ty| builder.where_predicate() // the type that is being bounded e.g. T @@ -159,6 +160,50 @@ fn contains_generic(ty: &ast::Ty, generics: &ast::Generics) -> bool { visitor.found_generic } +// We do not attempt to generate any bounds based on field types that are +// directly recursive, as in: +// +// struct Test { +// next: Box>, +// } +// +// This does not catch field types that are mutually recursive with some other +// type. For those, we require bounds to be specified by a `where` attribute if +// the inferred ones are not correct. +// +// struct Test { +// #[serde(where="D: Serialize + Deserialize")] +// next: Box>, +// } +// struct Other { +// #[serde(where="D: Serialize + Deserialize")] +// next: Box>, +// } +fn contains_recursion(ty: &ast::Ty, ident: ast::Ident) -> bool { + struct FindRecursion { + ident: ast::Ident, + found_recursion: bool, + } + impl<'v> visit::Visitor<'v> for FindRecursion { + fn visit_path(&mut self, path: &'v ast::Path, _id: ast::NodeId) { + if !path.global + && path.segments.len() == 1 + && path.segments[0].identifier == self.ident { + self.found_recursion = true; + } else { + visit::walk_path(self, path); + } + } + } + + let mut visitor = FindRecursion { + ident: ident, + found_recursion: false, + }; + visit::walk_ty(&mut visitor, ty); + visitor.found_recursion +} + // This is required to handle types that use both a reference and a value of // the same type, as in: // diff --git a/serde_tests/tests/test_gen.rs b/serde_tests/tests/test_gen.rs index e6cbeff6..6559b9b9 100644 --- a/serde_tests/tests/test_gen.rs +++ b/serde_tests/tests/test_gen.rs @@ -75,7 +75,6 @@ struct Tuple( ); #[derive(Serialize, Deserialize)] -#[serde(where(serialize="D: Serialize", deserialize="D: Deserialize"))] enum TreeNode { Split { left: Box>, @@ -89,17 +88,33 @@ enum TreeNode { #[derive(Serialize, Deserialize)] struct ListNode { data: D, - #[serde(where="")] next: Box>, } #[derive(Serialize, Deserialize)] -struct SerializeWithTrait { +#[serde(where="D: SerializeWith + DeserializeWith")] +struct WithTraits1 { + #[serde(serialize_with="SerializeWith::serialize_with", + deserialize_with="DeserializeWith::deserialize_with")] + d: D, #[serde(serialize_with="SerializeWith::serialize_with", deserialize_with="DeserializeWith::deserialize_with", - where(serialize="D: SerializeWith", - deserialize="D: DeserializeWith"))] - data: D, + where="E: SerializeWith + DeserializeWith")] + e: E, +} + +#[derive(Serialize, Deserialize)] +#[serde(where(serialize="D: SerializeWith", + deserialize="D: DeserializeWith"))] +struct WithTraits2 { + #[serde(serialize_with="SerializeWith::serialize_with", + deserialize_with="DeserializeWith::deserialize_with")] + d: D, + #[serde(serialize_with="SerializeWith::serialize_with", + deserialize_with="DeserializeWith::deserialize_with", + where(serialize="E: SerializeWith", + deserialize="E: DeserializeWith"))] + e: E, } ////////////////////////////////////////////////////////////////////////// From 45c51d319832720c1e8681800ae500932da84642 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 4 Jun 2016 16:53:45 -0700 Subject: [PATCH 5/9] Fix build on 1.5.0 which does not have Vec::as_slice --- serde_codegen/src/attr.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/serde_codegen/src/attr.rs b/serde_codegen/src/attr.rs index 10f77bf3..8a9e73fd 100644 --- a/serde_codegen/src/attr.rs +++ b/serde_codegen/src/attr.rs @@ -138,11 +138,11 @@ impl ContainerAttrs { } pub fn ser_where(&self) -> Option<&[ast::WherePredicate]> { - self.ser_where.as_ref().map(Vec::as_slice) + self.ser_where.as_ref().map(|vec| &vec[..]) } pub fn de_where(&self) -> Option<&[ast::WherePredicate]> { - self.de_where.as_ref().map(Vec::as_slice) + self.de_where.as_ref().map(|vec| &vec[..]) } } @@ -362,11 +362,11 @@ impl FieldAttrs { } pub fn ser_where(&self) -> Option<&[ast::WherePredicate]> { - self.ser_where.as_ref().map(Vec::as_slice) + self.ser_where.as_ref().map(|vec| &vec[..]) } pub fn de_where(&self) -> Option<&[ast::WherePredicate]> { - self.de_where.as_ref().map(Vec::as_slice) + self.de_where.as_ref().map(|vec| &vec[..]) } } From 578f34ecaff952efa97277f452b9a178acfc8f7f Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 5 Jun 2016 10:47:40 -0700 Subject: [PATCH 6/9] Use "bound" attribute instead of "where" --- serde_codegen/src/attr.rs | 70 +++++++++++++++++------------------ serde_codegen/src/bound.rs | 6 +-- serde_codegen/src/de.rs | 15 ++++---- serde_codegen/src/ser.rs | 8 ++-- serde_tests/tests/test_gen.rs | 8 ++-- 5 files changed, 54 insertions(+), 53 deletions(-) diff --git a/serde_codegen/src/attr.rs b/serde_codegen/src/attr.rs index 8a9e73fd..a78028d5 100644 --- a/serde_codegen/src/attr.rs +++ b/serde_codegen/src/attr.rs @@ -62,8 +62,8 @@ impl Name { pub struct ContainerAttrs { name: Name, deny_unknown_fields: bool, - ser_where: Option>, - de_where: Option>, + ser_bound: Option>, + de_bound: Option>, } impl ContainerAttrs { @@ -72,8 +72,8 @@ impl ContainerAttrs { let mut container_attrs = ContainerAttrs { name: Name::new(item.ident), deny_unknown_fields: false, - ser_where: None, - de_where: None, + ser_bound: None, + de_bound: None, }; for meta_items in item.attrs().iter().filter_map(get_serde_meta_items) { @@ -100,18 +100,18 @@ impl ContainerAttrs { container_attrs.deny_unknown_fields = true; } - // Parse `#[serde(where="D: Serialize")]` - ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"where" => { + // Parse `#[serde(bound="D: Serialize")]` + ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"bound" => { let where_predicates = try!(parse_lit_into_where(cx, name, lit)); - container_attrs.ser_where = Some(where_predicates.clone()); - container_attrs.de_where = Some(where_predicates.clone()); + container_attrs.ser_bound = Some(where_predicates.clone()); + container_attrs.de_bound = Some(where_predicates.clone()); } - // Parse `#[serde(where(serialize="D: Serialize", deserialize="D: Deserialize"))]` - ast::MetaItemKind::List(ref name, ref meta_items) if name == &"where" => { - let (ser_where, de_where) = try!(get_where_predicates(cx, meta_items)); - container_attrs.ser_where = ser_where; - container_attrs.de_where = de_where; + // Parse `#[serde(bound(serialize="D: Serialize", deserialize="D: Deserialize"))]` + ast::MetaItemKind::List(ref name, ref meta_items) if name == &"bound" => { + let (ser_bound, de_bound) = try!(get_where_predicates(cx, meta_items)); + container_attrs.ser_bound = ser_bound; + container_attrs.de_bound = de_bound; } _ => { @@ -137,12 +137,12 @@ impl ContainerAttrs { self.deny_unknown_fields } - pub fn ser_where(&self) -> Option<&[ast::WherePredicate]> { - self.ser_where.as_ref().map(|vec| &vec[..]) + pub fn ser_bound(&self) -> Option<&[ast::WherePredicate]> { + self.ser_bound.as_ref().map(|vec| &vec[..]) } - pub fn de_where(&self) -> Option<&[ast::WherePredicate]> { - self.de_where.as_ref().map(|vec| &vec[..]) + pub fn de_bound(&self) -> Option<&[ast::WherePredicate]> { + self.de_bound.as_ref().map(|vec| &vec[..]) } } @@ -207,8 +207,8 @@ pub struct FieldAttrs { default_expr_if_missing: Option>, serialize_with: Option, deserialize_with: Option, - ser_where: Option>, - de_where: Option>, + ser_bound: Option>, + de_bound: Option>, } impl FieldAttrs { @@ -231,8 +231,8 @@ impl FieldAttrs { default_expr_if_missing: None, serialize_with: None, deserialize_with: None, - ser_where: None, - de_where: None, + ser_bound: None, + de_bound: None, }; for meta_items in field.attrs.iter().filter_map(get_serde_meta_items) { @@ -304,18 +304,18 @@ impl FieldAttrs { field_attrs.deserialize_with = Some(path); } - // Parse `#[serde(where="D: Serialize")]` - ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"where" => { + // Parse `#[serde(bound="D: Serialize")]` + ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"bound" => { let where_predicates = try!(parse_lit_into_where(cx, name, lit)); - field_attrs.ser_where = Some(where_predicates.clone()); - field_attrs.de_where = Some(where_predicates.clone()); + field_attrs.ser_bound = Some(where_predicates.clone()); + field_attrs.de_bound = Some(where_predicates.clone()); } - // Parse `#[serde(where(serialize="D: Serialize", deserialize="D: Deserialize"))]` - ast::MetaItemKind::List(ref name, ref meta_items) if name == &"where" => { - let (ser_where, de_where) = try!(get_where_predicates(cx, meta_items)); - field_attrs.ser_where = ser_where; - field_attrs.de_where = de_where; + // Parse `#[serde(bound(serialize="D: Serialize", deserialize="D: Deserialize"))]` + ast::MetaItemKind::List(ref name, ref meta_items) if name == &"bound" => { + let (ser_bound, de_bound) = try!(get_where_predicates(cx, meta_items)); + field_attrs.ser_bound = ser_bound; + field_attrs.de_bound = de_bound; } _ => { @@ -361,12 +361,12 @@ impl FieldAttrs { self.deserialize_with.as_ref() } - pub fn ser_where(&self) -> Option<&[ast::WherePredicate]> { - self.ser_where.as_ref().map(|vec| &vec[..]) + pub fn ser_bound(&self) -> Option<&[ast::WherePredicate]> { + self.ser_bound.as_ref().map(|vec| &vec[..]) } - pub fn de_where(&self) -> Option<&[ast::WherePredicate]> { - self.de_where.as_ref().map(|vec| &vec[..]) + pub fn de_bound(&self) -> Option<&[ast::WherePredicate]> { + self.de_bound.as_ref().map(|vec| &vec[..]) } } @@ -434,7 +434,7 @@ fn get_where_predicates( cx: &ExtCtxt, items: &[P], ) -> Result<(Option>, Option>), Error> { - get_ser_and_de(cx, "where", items, parse_lit_into_where) + get_ser_and_de(cx, "bound", items, parse_lit_into_where) } pub fn get_serde_meta_items(attr: &ast::Attribute) -> Option<&[P]> { diff --git a/serde_codegen/src/bound.rs b/serde_codegen/src/bound.rs index 2c659e33..cabdb6ed 100644 --- a/serde_codegen/src/bound.rs +++ b/serde_codegen/src/bound.rs @@ -168,15 +168,15 @@ fn contains_generic(ty: &ast::Ty, generics: &ast::Generics) -> bool { // } // // This does not catch field types that are mutually recursive with some other -// type. For those, we require bounds to be specified by a `where` attribute if +// type. For those, we require bounds to be specified by a `bound` attribute if // the inferred ones are not correct. // // struct Test { -// #[serde(where="D: Serialize + Deserialize")] +// #[serde(bound="D: Serialize + Deserialize")] // next: Box>, // } // struct Other { -// #[serde(where="D: Serialize + Deserialize")] +// #[serde(bound="D: Serialize + Deserialize")] // next: Box>, // } fn contains_recursion(ty: &ast::Ty, ident: ast::Ident) -> bool { diff --git a/serde_codegen/src/de.rs b/serde_codegen/src/de.rs index 0970f26b..7a81cb3f 100644 --- a/serde_codegen/src/de.rs +++ b/serde_codegen/src/de.rs @@ -111,16 +111,16 @@ fn build_impl_generics( let generics = try!(bound::with_where_predicates_from_fields( cx, builder, item, &generics, - |attrs| attrs.de_where())); + |attrs| attrs.de_bound())); - match container_attrs.de_where() { + match container_attrs.de_bound() { Some(predicates) => { let generics = bound::with_where_predicates(builder, &generics, predicates); Ok(generics) } None => { let generics = try!(bound::with_bound(cx, builder, item, &generics, - deserialized_by_us, + needs_deserialize_bound, &builder.path().ids(&["_serde", "de", "Deserialize"]).build())); let generics = try!(bound::with_bound(cx, builder, item, &generics, requires_default, @@ -131,12 +131,13 @@ fn build_impl_generics( } // Fields with a `skip_deserializing` or `deserialize_with` attribute are not -// deserialized by us. All other fields may need a `T: Deserialize` bound where -// T is the type of the field. -fn deserialized_by_us(_: &ast::StructField, attrs: &attr::FieldAttrs) -> bool { +// deserialized by us so we do not generate a bound. Fields with a `bound` +// attribute specify their own bound so we do not generate one. All other fields +// may need a `T: Deserialize` bound where T is the type of the field. +fn needs_deserialize_bound(_: &ast::StructField, attrs: &attr::FieldAttrs) -> bool { !attrs.skip_deserializing_field() && attrs.deserialize_with().is_none() - && attrs.de_where().is_none() + && attrs.de_bound().is_none() } // Fields with a `default` attribute (not `default=...`), and fields with a diff --git a/serde_codegen/src/ser.rs b/serde_codegen/src/ser.rs index 815d84dc..3ab17fb8 100644 --- a/serde_codegen/src/ser.rs +++ b/serde_codegen/src/ser.rs @@ -108,9 +108,9 @@ fn build_impl_generics( let generics = try!(bound::with_where_predicates_from_fields( cx, builder, item, &generics, - |attrs| attrs.ser_where())); + |attrs| attrs.ser_bound())); - match container_attrs.ser_where() { + match container_attrs.ser_bound() { Some(predicates) => { let generics = bound::with_where_predicates(builder, &generics, predicates); Ok(generics) @@ -125,13 +125,13 @@ fn build_impl_generics( } // Fields with a `skip_serializing` or `serialize_with` attribute are not -// serialized by us so we do not generate a bound. Fields with a `where` +// serialized by us so we do not generate a bound. Fields with a `bound` // attribute specify their own bound so we do not generate one. All other fields // may need a `T: Serialize` bound where T is the type of the field. fn needs_serialize_bound(_: &ast::StructField, attrs: &attr::FieldAttrs) -> bool { !attrs.skip_serializing_field() && attrs.serialize_with().is_none() - && attrs.ser_where().is_none() + && attrs.ser_bound().is_none() } fn serialize_body( diff --git a/serde_tests/tests/test_gen.rs b/serde_tests/tests/test_gen.rs index 6559b9b9..28e3533e 100644 --- a/serde_tests/tests/test_gen.rs +++ b/serde_tests/tests/test_gen.rs @@ -92,19 +92,19 @@ struct ListNode { } #[derive(Serialize, Deserialize)] -#[serde(where="D: SerializeWith + DeserializeWith")] +#[serde(bound="D: SerializeWith + DeserializeWith")] struct WithTraits1 { #[serde(serialize_with="SerializeWith::serialize_with", deserialize_with="DeserializeWith::deserialize_with")] d: D, #[serde(serialize_with="SerializeWith::serialize_with", deserialize_with="DeserializeWith::deserialize_with", - where="E: SerializeWith + DeserializeWith")] + bound="E: SerializeWith + DeserializeWith")] e: E, } #[derive(Serialize, Deserialize)] -#[serde(where(serialize="D: SerializeWith", +#[serde(bound(serialize="D: SerializeWith", deserialize="D: DeserializeWith"))] struct WithTraits2 { #[serde(serialize_with="SerializeWith::serialize_with", @@ -112,7 +112,7 @@ struct WithTraits2 { d: D, #[serde(serialize_with="SerializeWith::serialize_with", deserialize_with="DeserializeWith::deserialize_with", - where(serialize="E: SerializeWith", + bound(serialize="E: SerializeWith", deserialize="E: DeserializeWith"))] e: E, } From 2e067862624b8c0f24d5c017c4e56ecebdc2bb67 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 5 Jun 2016 11:23:01 -0700 Subject: [PATCH 7/9] Remove unnecessary clones --- serde_codegen/src/attr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/serde_codegen/src/attr.rs b/serde_codegen/src/attr.rs index a78028d5..2a3bd497 100644 --- a/serde_codegen/src/attr.rs +++ b/serde_codegen/src/attr.rs @@ -104,7 +104,7 @@ impl ContainerAttrs { ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"bound" => { let where_predicates = try!(parse_lit_into_where(cx, name, lit)); container_attrs.ser_bound = Some(where_predicates.clone()); - container_attrs.de_bound = Some(where_predicates.clone()); + container_attrs.de_bound = Some(where_predicates); } // Parse `#[serde(bound(serialize="D: Serialize", deserialize="D: Deserialize"))]` @@ -308,7 +308,7 @@ impl FieldAttrs { ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"bound" => { let where_predicates = try!(parse_lit_into_where(cx, name, lit)); field_attrs.ser_bound = Some(where_predicates.clone()); - field_attrs.de_bound = Some(where_predicates.clone()); + field_attrs.de_bound = Some(where_predicates); } // Parse `#[serde(bound(serialize="D: Serialize", deserialize="D: Deserialize"))]` From f197c3ce96470bba56885621d22b896cea5f5aa6 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 5 Jun 2016 11:54:36 -0700 Subject: [PATCH 8/9] Readme for "bound" attribute --- README.md | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 7af59589..03093081 100644 --- a/README.md +++ b/README.md @@ -688,12 +688,15 @@ how types are serialized. Here are the supported annotations: Container Annotations: -| Annotation | Function | -| ---------- | -------- | -| `#[serde(rename="name")]` | Serialize and deserialize this container with the given name | -| `#[serde(rename(serialize="name1"))]` | Serialize this container with the given name | -| `#[serde(rename(deserialize="name1"))]` | Deserialize this container with the given name | -| `#[serde(deny_unknown_fields)]` | Always error during serialization when encountering unknown fields. When absent, unknown fields are ignored for self-describing formats like JSON. | +| Annotation | Function | +| ---------- | -------- | +| `#[serde(rename="name")]` | Serialize and deserialize this container with the given name | +| `#[serde(rename(serialize="name1"))]` | Serialize this container with the given name | +| `#[serde(rename(deserialize="name1"))]` | Deserialize this container with the given name | +| `#[serde(deny_unknown_fields)]` | Always error during serialization when encountering unknown fields. When absent, unknown fields are ignored for self-describing formats like JSON. | +| `#[serde(bound="T: MyTrait")]` | Where-clause for the Serialize and Deserialize impls. This replaces any bounds inferred by Serde. | +| `#[serde(bound(serialize="T: MyTrait"))]` | Where-clause for the Serialize impl. | +| `#[serde(bound(deserialize="T: MyTrait"))]` | Where-clause for the Deserialize impl. | Variant Annotations: @@ -705,18 +708,21 @@ Variant Annotations: Field Annotations: -| Annotation | Function | -| ---------- | -------- | -| `#[serde(rename="name")]` | Serialize and deserialize this field with the given name | -| `#[serde(rename(serialize="name1"))]` | Serialize this field with the given name | -| `#[serde(rename(deserialize="name1"))]` | Deserialize this field with the given name | -| `#[serde(default)]` | If the value is not specified, use the `Default::default()` | -| `#[serde(default="$path")]` | Call the path to a function `fn() -> T` to build the value | -| `#[serde(skip_serializing)]` | Do not serialize this value | -| `#[serde(skip_deserializing)]` | Always use `Default::default()` or `#[serde(default="$path")]` instead of deserializing this value | -| `#[serde(skip_serializing_if="$path")]` | Do not serialize this value if this function `fn(&T) -> bool` returns `true` | -| `#[serde(serialize_with="$path")]` | Call a function `fn(&T, &mut S) -> Result<(), S::Error> where S: Serializer` to serialize this value of type `T` | -| `#[serde(deserialize_with="$path")]` | Call a function `fn(&mut D) -> Result where D: Deserializer` to deserialize this value of type `T` | +| Annotation | Function | +| ---------- | -------- | +| `#[serde(rename="name")]` | Serialize and deserialize this field with the given name | +| `#[serde(rename(serialize="name1"))]` | Serialize this field with the given name | +| `#[serde(rename(deserialize="name1"))]` | Deserialize this field with the given name | +| `#[serde(default)]` | If the value is not specified, use the `Default::default()` | +| `#[serde(default="$path")]` | Call the path to a function `fn() -> T` to build the value | +| `#[serde(skip_serializing)]` | Do not serialize this value | +| `#[serde(skip_deserializing)]` | Always use `Default::default()` or `#[serde(default="$path")]` instead of deserializing this value | +| `#[serde(skip_serializing_if="$path")]` | Do not serialize this value if this function `fn(&T) -> bool` returns `true` | +| `#[serde(serialize_with="$path")]` | Call a function `fn(&T, &mut S) -> Result<(), S::Error> where S: Serializer` to serialize this value of type `T` | +| `#[serde(deserialize_with="$path")]` | Call a function `fn(&mut D) -> Result where D: Deserializer` to deserialize this value of type `T` | +| `#[serde(bound="T: MyTrait")]` | Where-clause for the Serialize and Deserialize impls. This replaces any bounds inferred by Serde for the current field. | +| `#[serde(bound(serialize="T: MyTrait"))]` | Where-clause for the Serialize impl. | +| `#[serde(bound(deserialize="T: MyTrait"))]` | Where-clause for the Deserialize impl. | Using in `no_std` crates ======================== From bdffaf3ea160522255766f82d1439df38bca426d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 5 Jun 2016 13:00:44 -0700 Subject: [PATCH 9/9] Re-enable clippy lint "useless_let_if_seq" This reverts commit 4e6cd2d63f06bc4fca9ecec8d16b27960dfb78e0. --- serde_codegen/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/serde_codegen/src/lib.rs b/serde_codegen/src/lib.rs index 36bb38c0..52ca7912 100644 --- a/serde_codegen/src/lib.rs +++ b/serde_codegen/src/lib.rs @@ -2,7 +2,6 @@ #![cfg_attr(feature = "nightly-testing", feature(plugin))] #![cfg_attr(feature = "nightly-testing", allow(too_many_arguments))] #![cfg_attr(feature = "nightly-testing", allow(used_underscore_binding))] -#![cfg_attr(feature = "nightly-testing", allow(useless_let_if_seq))] #![cfg_attr(not(feature = "with-syntex"), feature(rustc_private, plugin))] #![cfg_attr(not(feature = "with-syntex"), plugin(quasi_macros))]