Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shrink derive output size #2250

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 41 additions & 0 deletions serde/src/de/mod.rs
Expand Up @@ -1732,6 +1732,25 @@ pub trait SeqAccess<'de> {
self.next_element_seed(PhantomData)
}

/// Equivalent to calling `next_element`, and then if there are no more remaining items,
/// calling `Error::invalid_length`.
///
/// This method exists purely to reduce the size of the generated code,
/// reducing compile times. `SeqAccess` implementations should not override

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to implement is specifically as a method rather than a free function? Using a free function gives the desired no-overriding guarantee, and method call syntax is not used in derived code anyway.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A method was easier, being more similar to the original function. But it's not fundamental.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @AnthonyMikh here, that this would be better as free functions in serde::__private::de module

/// the default behavior.
#[doc(hidden)]
#[inline]
fn next_element_checked<T>(&mut self, len: usize, exp: &Expected) -> Result<T, Self::Error>
where
T: Deserialize<'de>,
{
match self.next_element() {
Ok(Some(val)) => Ok(val),
Ok(None) => Err(Error::invalid_length(len, exp)),
Err(err) => Err(err),
}
}

/// Returns the number of elements remaining in the sequence, if known.
#[inline]
fn size_hint(&self) -> Option<usize> {
Expand Down Expand Up @@ -1871,6 +1890,28 @@ pub trait MapAccess<'de> {
self.next_value_seed(PhantomData)
}

/// Equivalent to calling `Error::duplicate_field` if the field has already
/// been deserialized, and to calling `next_value` otherwise.
///
/// This method exists purely to reduce the size of the generated code,
/// reducing compile times. `MapAccess` implementations should not override
/// the default behavior.
#[doc(hidden)]
#[inline]
fn next_value_checked<V>(
&mut self,
field: &Option<V>,
field_name: &'static str,
) -> Result<V, Self::Error>
where
V: Deserialize<'de>,
{
if field.is_some() {
return Err(Error::duplicate_field(field_name));
}
self.next_value()
}

/// This returns `Ok(Some((key, value)))` for the next (key-value) pair in
/// the map, or `Ok(None)` if there are no more remaining items.
///
Expand Down
19 changes: 18 additions & 1 deletion serde/src/private/de.rs
Expand Up @@ -15,7 +15,7 @@ pub use self::content::{

pub use seed::InPlaceSeed;

/// If the missing field is of type `Option<T>` then treat is as `None`,
/// If the missing field is of type `Option<T>` then treat it as `None`,
/// otherwise it is an error.
pub fn missing_field<'de, V, E>(field: &'static str) -> Result<V, E>
where
Expand Down Expand Up @@ -55,6 +55,23 @@ where
Deserialize::deserialize(deserializer)
}

/// Equivalent to calling `missing_field` if the field has been missed, and
/// returning its value otherwise.
///
/// This method exists purely to reduce the size of the generated code,
/// reducing compile times.
#[inline]
pub fn missing_field_checked<'de, V, E>(value: Option<V>, name: &'static str) -> Result<V, E>
where
V: Deserialize<'de>,
E: Error,
{
match value {
Some(value) => Ok(value),
None => missing_field(name),
}
}

#[cfg(any(feature = "std", feature = "alloc"))]
pub fn borrow_cow_str<'de: 'a, 'a, D, R>(deserializer: D) -> Result<R, D::Error>
where
Expand Down
73 changes: 52 additions & 21 deletions serde_derive/src/de.rs
Expand Up @@ -654,6 +654,18 @@ fn deserialize_seq(
quote! {
let #var = #default;
}
} else if field.attrs.deserialize_with().is_none() && field.attrs.default().is_none() {
// Specialize the common case, to make the expanded code shorter.
let field_ty = field.ty;
let span = field.original.span();
let func = quote_spanned! {
span=> _serde::de::SeqAccess::next_element_checked::<#field_ty>
nnethercote marked this conversation as resolved.
Show resolved Hide resolved
};
let assign = quote! {
let #var = try!(#func(&mut __seq, #index_in_seq, &expecting));
};
index_in_seq += 1;
assign
} else {
let visit = match field.attrs.deserialize_with() {
None => {
Expand All @@ -677,7 +689,7 @@ fn deserialize_seq(
attr::Default::Default => quote!(_serde::__private::Default::default()),
attr::Default::Path(path) => quote!(#path()),
attr::Default::None => quote!(
return _serde::__private::Err(_serde::de::Error::invalid_length(#index_in_seq, &#expecting));
return _serde::__private::Err(_serde::de::Error::invalid_length(#index_in_seq, &expecting));
),
};
let assign = quote! {
Expand Down Expand Up @@ -727,6 +739,7 @@ fn deserialize_seq(

quote_block! {
#let_default
let expecting = #expecting;
#(#let_values)*
_serde::__private::Ok(#result)
}
Expand Down Expand Up @@ -768,7 +781,7 @@ fn deserialize_seq_in_place(
self.place.#member = #path();
),
attr::Default::None => quote!(
return _serde::__private::Err(_serde::de::Error::invalid_length(#index_in_seq, &#expecting));
return _serde::__private::Err(_serde::de::Error::invalid_length(#index_in_seq, &expecting));
),
};
let write = match field.attrs.deserialize_with() {
Expand Down Expand Up @@ -819,6 +832,7 @@ fn deserialize_seq_in_place(

quote_block! {
#let_default
let expecting = #expecting;
#(#write_values)*
_serde::__private::Ok(())
}
Expand Down Expand Up @@ -2473,35 +2487,40 @@ fn deserialize_map(
.map(|(field, name)| {
let deser_name = field.attrs.name().deserialize_name();

let visit = match field.attrs.deserialize_with() {
match field.attrs.deserialize_with() {
None => {
// Specialize the common case, to make the expanded code shorter.
let field_ty = field.ty;
let span = field.original.span();
let func =
quote_spanned!(span=> _serde::de::MapAccess::next_value::<#field_ty>);
quote_spanned!(span=> _serde::de::MapAccess::next_value_checked::<#field_ty>);
quote! {
try!(#func(&mut __map))
__Field::#name => {
#name = _serde::__private::Some(
try!(#func(&mut __map, &#name, #deser_name))
);
}
}
}
Some(path) => {
let (wrapper, wrapper_ty) = wrap_deserialize_field_with(params, field.ty, path);
quote!({
let visit = quote!({
#wrapper
match _serde::de::MapAccess::next_value::<#wrapper_ty>(&mut __map) {
_serde::__private::Ok(__wrapper) => __wrapper.value,
_serde::__private::Err(__err) => {
return _serde::__private::Err(__err);
}
}
})
}
};
quote! {
__Field::#name => {
if _serde::__private::Option::is_some(&#name) {
return _serde::__private::Err(<__A::Error as _serde::de::Error>::duplicate_field(#deser_name));
});
quote! {
__Field::#name => {
if _serde::__private::Option::is_some(&#name) {
return _serde::__private::Err(<__A::Error as _serde::de::Error>::duplicate_field(#deser_name));
}
#name = _serde::__private::Some(#visit);
}
}
#name = _serde::__private::Some(#visit);
}
}
});
Expand Down Expand Up @@ -2547,13 +2566,25 @@ fn deserialize_map(
.iter()
.filter(|&&(field, _)| !field.attrs.skip_deserializing() && !field.attrs.flatten())
.map(|(field, name)| {
let missing_expr = Match(expr_is_missing(field, cattrs));

quote! {
let #name = match #name {
_serde::__private::Some(#name) => #name,
_serde::__private::None => #missing_expr
};
if field.attrs.default().is_none()
&& cattrs.default().is_none()
&& field.attrs.deserialize_with().is_none()
{
// Specialize the common case, to make the expanded code shorter.
let span = field.original.span();
let func = quote_spanned!(span=> _serde::__private::de::missing_field_checked);
let name_str = field.attrs.name().deserialize_name();
quote! {
let #name = try!(#func(#name, #name_str));
}
} else {
let missing_expr = Match(expr_is_missing(field, cattrs));
quote! {
let #name = match #name {
_serde::__private::Some(#name) => #name,
_serde::__private::None => #missing_expr
};
}
}
});

Expand Down
18 changes: 10 additions & 8 deletions serde_derive/src/ser.rs
Expand Up @@ -336,18 +336,20 @@ fn serialize_struct_as_struct(

let let_mut = mut_if(serialized_fields.peek().is_some() || tag_field_exists);

let len = serialized_fields
.map(|field| match field.attrs.skip_serializing_if() {
None => quote!(1),
let mut num_normals: usize = if tag_field_exists { 1 } else { 0 };
let adds_for_skippables = serialized_fields
.filter_map(|field| match field.attrs.skip_serializing_if() {
None => {
num_normals += 1;
None
}
Some(path) => {
let field_expr = get_member(params, field, &field.member);
quote!(if #path(#field_expr) { 0 } else { 1 })
Some(quote!(if #path(#field_expr) { 0 } else { 1 }))
}
})
.fold(
quote!(#tag_field_exists as usize),
|sum, expr| quote!(#sum + #expr),
);
.fold(TokenStream::new(), |sum, expr| quote!(#sum + #expr));
let len = quote!(#num_normals #adds_for_skippables);

quote_block! {
let #let_mut __serde_state = try!(_serde::Serializer::serialize_struct(__serializer, #type_name, #len));
Expand Down