Skip to content

Commit ddf9439

Browse files
pgherveoubkchr
pgherveou
and
bkchr
authored
Fix max_encoded_len for Compact fields (#508)
* Fix max_encoded_len for Compact fields * fix * Add missing test * nit * Apply suggestions from code review Co-authored-by: Bastian Köcher <git@kchr.de> * fix ui-test output --------- Co-authored-by: Bastian Köcher <git@kchr.de>
1 parent 1516bb9 commit ddf9439

3 files changed

Lines changed: 65 additions & 31 deletions

File tree

‎derive/src/max_encoded_len.rs‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717

1818
use crate::{
1919
trait_bounds,
20-
utils::{codec_crate_path, custom_mel_trait_bound, has_dumb_trait_bound, should_skip},
20+
utils::{self, codec_crate_path, custom_mel_trait_bound, has_dumb_trait_bound, should_skip},
2121
};
2222
use quote::{quote, quote_spanned};
23-
use syn::{parse_quote, spanned::Spanned, Data, DeriveInput, Fields, Type};
23+
use syn::{parse_quote, spanned::Spanned, Data, DeriveInput, Field, Fields};
2424

2525
/// impl for `#[derive(MaxEncodedLen)]`
2626
pub fn derive_max_encoded_len(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
@@ -43,13 +43,13 @@ pub fn derive_max_encoded_len(input: proc_macro::TokenStream) -> proc_macro::Tok
4343
parse_quote!(#crate_path::MaxEncodedLen),
4444
None,
4545
has_dumb_trait_bound(&input.attrs),
46-
&crate_path
46+
&crate_path,
4747
) {
4848
return e.to_compile_error().into()
4949
}
5050
let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
5151

52-
let data_expr = data_length_expr(&input.data);
52+
let data_expr = data_length_expr(&input.data, &crate_path);
5353

5454
quote::quote!(
5555
const _: () = {
@@ -64,22 +64,22 @@ pub fn derive_max_encoded_len(input: proc_macro::TokenStream) -> proc_macro::Tok
6464
}
6565

6666
/// generate an expression to sum up the max encoded length from several fields
67-
fn fields_length_expr(fields: &Fields) -> proc_macro2::TokenStream {
68-
let type_iter: Box<dyn Iterator<Item = &Type>> = match fields {
69-
Fields::Named(ref fields) => Box::new(
70-
fields.named.iter().filter_map(|field| if should_skip(&field.attrs) {
67+
fn fields_length_expr(fields: &Fields, crate_path: &syn::Path) -> proc_macro2::TokenStream {
68+
let fields_iter: Box<dyn Iterator<Item = &Field>> = match fields {
69+
Fields::Named(ref fields) => Box::new(fields.named.iter().filter_map(|field| {
70+
if should_skip(&field.attrs) {
7171
None
7272
} else {
73-
Some(&field.ty)
74-
})
75-
),
76-
Fields::Unnamed(ref fields) => Box::new(
77-
fields.unnamed.iter().filter_map(|field| if should_skip(&field.attrs) {
73+
Some(field)
74+
}
75+
})),
76+
Fields::Unnamed(ref fields) => Box::new(fields.unnamed.iter().filter_map(|field| {
77+
if should_skip(&field.attrs) {
7878
None
7979
} else {
80-
Some(&field.ty)
81-
})
82-
),
80+
Some(field)
81+
}
82+
})),
8383
Fields::Unit => Box::new(std::iter::empty()),
8484
};
8585
// expands to an expression like
@@ -92,9 +92,16 @@ fn fields_length_expr(fields: &Fields) -> proc_macro2::TokenStream {
9292
// `max_encoded_len` call. This way, if one field's type doesn't implement
9393
// `MaxEncodedLen`, the compiler's error message will underline which field
9494
// caused the issue.
95-
let expansion = type_iter.map(|ty| {
96-
quote_spanned! {
97-
ty.span() => .saturating_add(<#ty>::max_encoded_len())
95+
let expansion = fields_iter.map(|field| {
96+
let ty = &field.ty;
97+
if utils::is_compact(&field) {
98+
quote_spanned! {
99+
ty.span() => .saturating_add(<#crate_path::Compact::<#ty> as #crate_path::MaxEncodedLen>::max_encoded_len())
100+
}
101+
} else {
102+
quote_spanned! {
103+
ty.span() => .saturating_add(<#ty as #crate_path::MaxEncodedLen>::max_encoded_len())
104+
}
98105
}
99106
});
100107
quote! {
@@ -103,9 +110,9 @@ fn fields_length_expr(fields: &Fields) -> proc_macro2::TokenStream {
103110
}
104111

105112
// generate an expression to sum up the max encoded length of each field
106-
fn data_length_expr(data: &Data) -> proc_macro2::TokenStream {
113+
fn data_length_expr(data: &Data, crate_path: &syn::Path) -> proc_macro2::TokenStream {
107114
match *data {
108-
Data::Struct(ref data) => fields_length_expr(&data.fields),
115+
Data::Struct(ref data) => fields_length_expr(&data.fields, crate_path),
109116
Data::Enum(ref data) => {
110117
// We need an expression expanded for each variant like
111118
//
@@ -121,7 +128,7 @@ fn data_length_expr(data: &Data) -> proc_macro2::TokenStream {
121128
// Each variant expression's sum is computed the way an equivalent struct's would be.
122129

123130
let expansion = data.variants.iter().map(|variant| {
124-
let variant_expression = fields_length_expr(&variant.fields);
131+
let variant_expression = fields_length_expr(&variant.fields, crate_path);
125132
quote! {
126133
.max(#variant_expression)
127134
}

‎tests/max_encoded_len.rs‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
//! Tests for MaxEncodedLen derive macro
1717
#![cfg(all(feature = "derive", feature = "max-encoded-len"))]
1818

19-
use parity_scale_codec::{MaxEncodedLen, Compact, Decode, Encode};
19+
use parity_scale_codec::{Compact, Decode, Encode, MaxEncodedLen};
2020

2121
#[derive(Encode, MaxEncodedLen)]
2222
struct Primitives {
@@ -64,6 +64,29 @@ fn generic_max_length() {
6464
assert_eq!(Generic::<u32>::max_encoded_len(), u32::max_encoded_len() * 2);
6565
}
6666

67+
#[derive(Encode, MaxEncodedLen)]
68+
struct CompactField {
69+
#[codec(compact)]
70+
t: u64,
71+
v: u64,
72+
}
73+
74+
#[test]
75+
fn compact_field_max_length() {
76+
assert_eq!(
77+
CompactField::max_encoded_len(),
78+
Compact::<u64>::max_encoded_len() + u64::max_encoded_len()
79+
);
80+
}
81+
82+
#[derive(Encode, MaxEncodedLen)]
83+
struct CompactStruct(#[codec(compact)] u64);
84+
85+
#[test]
86+
fn compact_struct_max_length() {
87+
assert_eq!(CompactStruct::max_encoded_len(), Compact::<u64>::max_encoded_len());
88+
}
89+
6790
#[derive(Encode, MaxEncodedLen)]
6891
struct TwoGenerics<T, U> {
6992
t: T,

‎tests/max_encoded_len_ui/unsupported_variant.stderr‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
1-
error[E0599]: no function or associated item named `max_encoded_len` found for struct `NotMel` in the current scope
1+
error[E0277]: the trait bound `NotMel: MaxEncodedLen` is not satisfied
22
--> tests/max_encoded_len_ui/unsupported_variant.rs:8:9
33
|
4-
4 | struct NotMel;
5-
| ------------- function or associated item `max_encoded_len` not found for this struct
6-
...
74
8 | NotMel(NotMel),
8-
| ^^^^^^ function or associated item not found in `NotMel`
5+
| ^^^^^^ the trait `MaxEncodedLen` is not implemented for `NotMel`
96
|
10-
= help: items from traits can only be used if the trait is implemented and in scope
11-
= note: the following trait defines an item `max_encoded_len`, perhaps you need to implement it:
12-
candidate #1: `MaxEncodedLen`
7+
= help: the following other types implement trait `MaxEncodedLen`:
8+
()
9+
(TupleElement0, TupleElement1)
10+
(TupleElement0, TupleElement1, TupleElement2)
11+
(TupleElement0, TupleElement1, TupleElement2, TupleElement3)
12+
(TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4)
13+
(TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5)
14+
(TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6)
15+
(TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7)
16+
and $N others

0 commit comments

Comments
 (0)