From b313f947dc3cbc57dc24dcd49d39be2784463ecd Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 9 Jan 2018 20:28:23 -0800 Subject: [PATCH 1/4] Use call_site as the span of unnamed member access --- serde_derive/src/de.rs | 12 +++++++++--- serde_derive/src/ser.rs | 17 +++++++++++++---- test_suite/Cargo.toml | 1 + 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index c9154321..a5d6bd4d 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -6,7 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use syn::{self, Ident, Member}; +use syn::{self, Ident, Index, Member}; use syn::punctuated::Punctuated; use quote::{ToTokens, Tokens}; use proc_macro2::{Literal, Span, Term}; @@ -624,7 +624,10 @@ fn deserialize_seq_in_place( let field_name = field .ident .map(Member::Named) - .unwrap_or_else(|| Member::Unnamed(field_index.into())); + .unwrap_or_else(|| Member::Unnamed(Index { + index: field_index as u32, + span: Span::call_site(), + })); if field.attrs.skip_deserializing() { let default = Expr(expr_is_missing(field, cattrs)); @@ -2314,7 +2317,10 @@ fn wrap_deserialize_variant_with( let (wrapper, wrapper_ty) = wrap_deserialize_with(params, "e!((#(#field_tys),*)), deserialize_with); - let field_access = (0..variant.fields.len()).map(|n| Member::Unnamed(n.into())); + let field_access = (0..variant.fields.len()).map(|n| Member::Unnamed(Index { + index: n as u32, + span: Span::call_site(), + })); let unwrap_fn = match variant.style { Style::Struct => { let field_idents = variant diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 32a6b1d4..5f190551 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -6,7 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use syn::{self, Ident, Member}; +use syn::{self, Ident, Index, Member}; use quote::Tokens; use proc_macro2::Span; @@ -202,7 +202,10 @@ fn serialize_newtype_struct( ) -> Fragment { let type_name = cattrs.name().serialize_name(); - let mut field_expr = get_member(params, field, &Member::Unnamed(0.into())); + let mut field_expr = get_member(params, field, &Member::Unnamed(Index { + index: 0, + span: Span::call_site(), + })); if let Some(path) = field.attrs.serialize_with() { field_expr = wrap_serialize_field_with(params, field.ty, path, &field_expr); } @@ -817,7 +820,10 @@ fn serialize_tuple_struct_visitor( let id = Ident::new(&format!("__field{}", i), Span::def_site()); quote!(#id) } else { - get_member(params, field, &Member::Unnamed(i.into())) + get_member(params, field, &Member::Unnamed(Index { + index: i as u32, + span: Span::call_site(), + })) }; let skip = field @@ -940,7 +946,10 @@ fn wrap_serialize_with( }; let (wrapper_impl_generics, wrapper_ty_generics, _) = wrapper_generics.split_for_impl(); - let field_access = (0..field_exprs.len()).map(|n| Member::Unnamed(n.into())); + let field_access = (0..field_exprs.len()).map(|n| Member::Unnamed(Index { + index: n as u32, + span: Span::call_site(), + })); quote!({ struct __SerializeWith #wrapper_impl_generics #where_clause { diff --git a/test_suite/Cargo.toml b/test_suite/Cargo.toml index 50e10e36..23a9fd79 100644 --- a/test_suite/Cargo.toml +++ b/test_suite/Cargo.toml @@ -9,6 +9,7 @@ unstable = ["serde/unstable", "compiletest_rs"] [dev-dependencies] fnv = "1.0" +proc-macro2 = "0.2" rustc-serialize = "0.3.16" serde = { path = "../serde", features = ["rc"] } serde_derive = { path = "../serde_derive", features = ["deserialize_in_place"] } From ddc4b50d4dc58f76e437f420237865a397aacd5a Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 9 Jan 2018 20:34:29 -0800 Subject: [PATCH 2/4] Use call_site in 'with' attribute --- serde_derive_internals/Cargo.toml | 1 + serde_derive_internals/src/attr.rs | 10 ++++++---- serde_derive_internals/src/lib.rs | 2 ++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/serde_derive_internals/Cargo.toml b/serde_derive_internals/Cargo.toml index ac601190..74888f37 100644 --- a/serde_derive_internals/Cargo.toml +++ b/serde_derive_internals/Cargo.toml @@ -12,6 +12,7 @@ readme = "README.md" include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE-APACHE", "LICENSE-MIT"] [dependencies] +proc-macro2 = "0.2" syn = { version = "0.12", default-features = false, features = ["derive", "parsing", "clone-impls"] } [badges] diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index bfb7811e..1d289bd3 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -8,12 +8,14 @@ use Ctxt; use syn; +use syn::Ident; use syn::Meta::{List, NameValue, Word}; use syn::NestedMeta::{Literal, Meta}; use syn::punctuated::Punctuated; use syn::synom::Synom; use std::collections::BTreeSet; use std::str::FromStr; +use proc_macro2::Span; // This module handles parsing of `#[serde(...)]` attributes. The entrypoints // are `attr::Container::from_ast`, `attr::Variant::from_ast`, and @@ -577,10 +579,10 @@ impl Variant { Meta(NameValue(ref m)) if m.ident == "with" => { if let Ok(path) = parse_lit_into_path(cx, m.ident.as_ref(), &m.lit) { let mut ser_path = path.clone(); - ser_path.segments.push("serialize".into()); + ser_path.segments.push(Ident::new("serialize", Span::call_site()).into()); serialize_with.set(ser_path); let mut de_path = path; - de_path.segments.push("deserialize".into()); + de_path.segments.push(Ident::new("deserialize", Span::call_site()).into()); deserialize_with.set(de_path); } } @@ -819,10 +821,10 @@ impl Field { Meta(NameValue(ref m)) if m.ident == "with" => { if let Ok(path) = parse_lit_into_path(cx, m.ident.as_ref(), &m.lit) { let mut ser_path = path.clone(); - ser_path.segments.push("serialize".into()); + ser_path.segments.push(Ident::new("serialize", Span::call_site()).into()); serialize_with.set(ser_path); let mut de_path = path; - de_path.segments.push("deserialize".into()); + de_path.segments.push(Ident::new("deserialize", Span::call_site()).into()); deserialize_with.set(de_path); } } diff --git a/serde_derive_internals/src/lib.rs b/serde_derive_internals/src/lib.rs index dd19ecaf..c1630a9e 100644 --- a/serde_derive_internals/src/lib.rs +++ b/serde_derive_internals/src/lib.rs @@ -12,6 +12,8 @@ #[macro_use] extern crate syn; +extern crate proc_macro2; + pub mod ast; pub mod attr; From 63623eb3b3d3b85307a88a33142fe3767e7a4ae3 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 9 Jan 2018 22:22:08 -0800 Subject: [PATCH 3/4] Hygiene fixes --- serde_derive/src/de.rs | 35 +++++++++++++++++++----------- serde_derive/src/ser.rs | 8 ++++--- serde_derive_internals/src/attr.rs | 18 +++++++++++++-- 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index a5d6bd4d..1e78d52a 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -32,9 +32,10 @@ pub fn expand_derive_deserialize(input: &syn::DeriveInput) -> Result deserialize); quote! { impl #de_impl_generics #ident #ty_generics #where_clause { - #vis fn deserialize<__D>(__deserializer: __D) -> _serde::export::Result<#remote #ty_generics, __D::Error> + #vis fn #fun<__D>(__deserializer: __D) -> _serde::export::Result<#remote #ty_generics, __D::Error> where __D: _serde::Deserializer<#delife> { #body @@ -562,14 +563,16 @@ fn deserialize_seq( } }); + // FIXME: parentheses around field values are because of + // https://github.com/rust-lang/rust/issues/47311 let mut result = if is_struct { let names = fields.iter().map(|f| &f.ident); - quote! { - #type_path { #( #names: #vars ),* } + quote_spanned! {Span::call_site()=> + #type_path { #( #names: (#vars) ),* } } } else { - quote! { - #type_path ( #(#vars),* ) + quote_spanned! {Span::call_site()=> + #type_path ( #((#vars)),* ) } }; @@ -629,10 +632,11 @@ fn deserialize_seq_in_place( span: Span::call_site(), })); + let dot = quote_spanned!(Span::call_site()=> .); if field.attrs.skip_deserializing() { let default = Expr(expr_is_missing(field, cattrs)); quote! { - self.place.#field_name = #default; + self.place #dot #field_name = #default; } } else { let return_invalid_length = quote! { @@ -642,7 +646,7 @@ fn deserialize_seq_in_place( None => { quote! { if let _serde::export::None = try!(_serde::de::SeqAccess::next_element_seed(&mut __seq, - _serde::private::de::InPlaceSeed(&mut self.place.#field_name))) + _serde::private::de::InPlaceSeed(&mut self.place #dot #field_name))) { #return_invalid_length } @@ -655,7 +659,7 @@ fn deserialize_seq_in_place( #wrapper match try!(_serde::de::SeqAccess::next_element::<#wrapper_ty>(&mut __seq)) { _serde::export::Some(__wrap) => { - self.place.#field_name = __wrap.value; + self.place #dot #field_name = __wrap.value; } _serde::export::None => { #return_invalid_length @@ -711,7 +715,9 @@ fn deserialize_newtype_struct(type_path: &Tokens, params: &Parameters, field: &F } }; - let mut result = quote!(#type_path(#value)); + // FIXME: parentheses around field values are because of + // https://github.com/rust-lang/rust/issues/47311 + let mut result = quote_spanned!(Span::call_site()=> #type_path((#value))); if params.has_getter { let this = ¶ms.this; result = quote! { @@ -736,12 +742,13 @@ fn deserialize_newtype_struct_in_place(params: &Parameters, field: &Field) -> To let delife = params.borrowed.de_lifetime(); + let elem = quote_spanned!(Span::call_site()=> .0); quote! { #[inline] fn visit_newtype_struct<__E>(self, __e: __E) -> _serde::export::Result where __E: _serde::Deserializer<#delife> { - _serde::Deserialize::deserialize_in_place(__e, &mut self.place.0) + _serde::Deserialize::deserialize_in_place(__e, &mut self.place #elem) } } } @@ -2042,13 +2049,15 @@ fn deserialize_map( } }); + // FIXME: parentheses around field values are because of + // https://github.com/rust-lang/rust/issues/47311 let result = fields_names.iter().map(|&(field, ref name)| { let ident = field.ident.expect("struct contains unnamed fields"); if field.attrs.skip_deserializing() { let value = Expr(expr_is_missing(field, cattrs)); - quote!(#ident: #value) + quote_spanned!(Span::call_site()=> #ident: (#value)) } else { - quote!(#ident: #name) + quote_spanned!(Span::call_site()=> #ident: (#name)) } }); @@ -2066,7 +2075,7 @@ fn deserialize_map( } }; - let mut result = quote!(#struct_path { #(#result),* }); + let mut result = quote_spanned!(Span::call_site()=> #struct_path { #(#result),* }); if params.has_getter { let this = ¶ms.this; result = quote! { diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 5f190551..55b33f73 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -31,9 +31,10 @@ pub fn expand_derive_serialize(input: &syn::DeriveInput) -> Result serialize); quote! { impl #impl_generics #ident #ty_generics #where_clause { - #vis fn serialize<__S>(__self: &#remote #ty_generics, __serializer: __S) -> _serde::export::Result<__S::Ok, __S::Error> + #vis fn #fun<__S>(__self: &#remote #ty_generics, __serializer: __S) -> _serde::export::Result<__S::Ok, __S::Error> where __S: _serde::Serializer { #body @@ -990,11 +991,12 @@ fn get_member(params: &Parameters, field: &Field, member: &Member) -> Tokens { let self_var = ¶ms.self_var; match (params.is_remote, field.attrs.getter()) { (false, None) => { - quote!(&#self_var.#member) + quote_spanned!(Span::call_site()=> &#self_var.#member) } (true, None) => { + let inner = quote_spanned!(Span::call_site()=> &#self_var.#member); let ty = field.ty; - quote!(_serde::private::ser::constrain::<#ty>(&#self_var.#member)) + quote!(_serde::private::ser::constrain::<#ty>(#inner)) } (true, Some(getter)) => { let ty = field.ty; diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index 1d289bd3..af1fbdec 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -913,10 +913,24 @@ impl Field { // impl<'de: 'a, 'a> Deserialize<'de> for Cow<'a, str> // impl<'de: 'a, 'a> Deserialize<'de> for Cow<'a, [u8]> if is_cow(&field.ty, is_str) { - let path = syn::parse_str("_serde::private::de::borrow_cow_str").unwrap(); + let mut path = syn::Path { + leading_colon: None, + segments: Punctuated::new(), + }; + path.segments.push(Ident::new("_serde", Span::def_site()).into()); + path.segments.push(Ident::new("private", Span::def_site()).into()); + path.segments.push(Ident::new("de", Span::def_site()).into()); + path.segments.push(Ident::new("borrow_cow_str", Span::def_site()).into()); deserialize_with.set_if_none(path); } else if is_cow(&field.ty, is_slice_u8) { - let path = syn::parse_str("_serde::private::de::borrow_cow_bytes").unwrap(); + let mut path = syn::Path { + leading_colon: None, + segments: Punctuated::new(), + }; + path.segments.push(Ident::new("_serde", Span::def_site()).into()); + path.segments.push(Ident::new("private", Span::def_site()).into()); + path.segments.push(Ident::new("de", Span::def_site()).into()); + path.segments.push(Ident::new("borrow_cow_bytes", Span::def_site()).into()); deserialize_with.set_if_none(path); } } else if is_rptr(&field.ty, is_str) || is_rptr(&field.ty, is_slice_u8) { From dd6914a203b05017e78470e932eebbd71476a7b1 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 9 Jan 2018 22:23:19 -0800 Subject: [PATCH 4/4] Build the test suite in CI using proc-macro2/nightly --- travis.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/travis.sh b/travis.sh index c498462e..4ef4931b 100755 --- a/travis.sh +++ b/travis.sh @@ -55,6 +55,7 @@ else channel build cd "$DIR/test_suite" channel test --features unstable + channel build --tests --features proc-macro2/nightly if [ -z "${APPVEYOR}" ]; then cd "$DIR/test_suite/no_std" channel build