From 14accf751801d34a2c015efcce31c0743622e63e Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 23 Aug 2021 10:11:18 -0700 Subject: [PATCH 1/2] Add test of remote with a packed struct Currently fails to build: error: reference to packed field is unaligned --> test_suite/tests/test_gen.rs:858:10 | 858 | #[derive(Serialize, Deserialize)] | ^^^^^^^^^ | note: the lint level is defined here --> test_suite/tests/test_gen.rs:5:9 | 5 | #![deny(warnings)] | ^^^^^^^^ = note: `#[deny(unaligned_references)]` implied by `#[deny(warnings)]` = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) = note: this error originates in the derive macro `Serialize` (in Nightly builds, run with -Z macro-backtrace for more info) error: reference to packed field is unaligned --> test_suite/tests/test_gen.rs:858:21 | 858 | #[derive(Serialize, Deserialize)] | ^^^^^^^^^^^ | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info) --- test_suite/tests/test_gen.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test_suite/tests/test_gen.rs b/test_suite/tests/test_gen.rs index 93d5cd3a5..3e19d3c4b 100644 --- a/test_suite/tests/test_gen.rs +++ b/test_suite/tests/test_gen.rs @@ -846,3 +846,19 @@ where { T::deserialize(deserializer) } + +////////////////////////////////////////////////////////////////////////// + +#[repr(packed)] +pub struct RemotePacked { + pub a: u8, + pub b: u16, +} + +#[derive(Serialize, Deserialize)] +#[repr(packed)] +#[serde(remote = "RemotePacked")] +pub struct RemotePackedDef { + a: u8, + b: u16, +} From 54102ee7d08cd2037eba8c3f9ad087e3a04d1e72 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 23 Aug 2021 10:17:44 -0700 Subject: [PATCH 2/2] Avoid generating ref patterns for fields of packed remote struct --- serde_derive/src/de.rs | 7 ++++++- serde_derive/src/pretend.rs | 19 ++++++++++++------- serde_derive/src/ser.rs | 2 +- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 0903a084c..7f4d7c441 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -36,7 +36,7 @@ pub fn expand_derive_deserialize( let impl_block = if let Some(remote) = cont.attrs.remote() { let vis = &input.vis; - let used = pretend::pretend_used(&cont); + let used = pretend::pretend_used(&cont, params.is_packed); quote! { impl #de_impl_generics #ident #ty_generics #where_clause { #vis fn deserialize<__D>(__deserializer: __D) -> #serde::__private::Result<#remote #ty_generics, __D::Error> @@ -125,6 +125,9 @@ struct Parameters { /// At least one field has a serde(getter) attribute, implying that the /// remote type has a private field. has_getter: bool, + + /// Type has a repr(packed) attribute. + is_packed: bool, } impl Parameters { @@ -137,6 +140,7 @@ impl Parameters { let borrowed = borrowed_lifetimes(cont); let generics = build_generics(cont, &borrowed); let has_getter = cont.data.has_getter(); + let is_packed = cont.attrs.is_packed(); Parameters { local, @@ -144,6 +148,7 @@ impl Parameters { generics, borrowed, has_getter, + is_packed, } } diff --git a/serde_derive/src/pretend.rs b/serde_derive/src/pretend.rs index 955ce3d1c..7ec894446 100644 --- a/serde_derive/src/pretend.rs +++ b/serde_derive/src/pretend.rs @@ -20,8 +20,8 @@ use internals::ast::{Container, Data, Field, Style}; // 8 | enum EnumDef { V } // | ^ // -pub fn pretend_used(cont: &Container) -> TokenStream { - let pretend_fields = pretend_fields_used(cont); +pub fn pretend_used(cont: &Container, is_packed: bool) -> TokenStream { + let pretend_fields = pretend_fields_used(cont, is_packed); let pretend_variants = pretend_variants_used(cont); quote! { @@ -48,7 +48,7 @@ pub fn pretend_used(cont: &Container) -> TokenStream { // The `ref` is important in case the user has written a Drop impl on their // type. Rust does not allow destructuring a struct or enum that has a Drop // impl. -fn pretend_fields_used(cont: &Container) -> TokenStream { +fn pretend_fields_used(cont: &Container, is_packed: bool) -> TokenStream { let type_ident = &cont.ident; let (_, ty_generics, _) = cont.generics.split_for_impl(); @@ -58,14 +58,14 @@ fn pretend_fields_used(cont: &Container) -> TokenStream { .filter_map(|variant| match variant.style { Style::Struct => { let variant_ident = &variant.ident; - let pat = struct_pattern(&variant.fields); + let pat = struct_pattern(&variant.fields, is_packed); Some(quote!(#type_ident::#variant_ident #pat)) } _ => None, }) .collect::>(), Data::Struct(Style::Struct, fields) => { - let pat = struct_pattern(fields); + let pat = struct_pattern(fields, is_packed); vec![quote!(#type_ident #pat)] } Data::Struct(_, _) => { @@ -132,9 +132,14 @@ fn pretend_variants_used(cont: &Container) -> TokenStream { quote!(#(#cases)*) } -fn struct_pattern(fields: &[Field]) -> TokenStream { +fn struct_pattern(fields: &[Field], is_packed: bool) -> TokenStream { let members = fields.iter().map(|field| &field.member); let placeholders = (0..fields.len()).map(|i| Ident::new(&format!("__v{}", i), Span::call_site())); - quote!({ #(#members: ref #placeholders),* }) + let take_reference = if is_packed { + None + } else { + Some(quote!(ref)) + }; + quote!({ #(#members: #take_reference #placeholders),* }) } diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 4f3e5ff08..529a20d79 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -30,7 +30,7 @@ pub fn expand_derive_serialize( let impl_block = if let Some(remote) = cont.attrs.remote() { let vis = &input.vis; - let used = pretend::pretend_used(&cont); + let used = pretend::pretend_used(&cont, params.is_packed); quote! { impl #impl_generics #ident #ty_generics #where_clause { #vis fn serialize<__S>(__self: &#remote #ty_generics, __serializer: __S) -> #serde::__private::Result<__S::Ok, __S::Error>