Skip to content

Commit

Permalink
Handle more SCALE attributes: skip, index (#44)
Browse files Browse the repository at this point in the history
* Parameterize CompactForm String for optional SCALE impl

By default, parity-scale-codec does not provide Encode/Decode impls for an owned String.
This is only provided under the "full" feature which is not used by the substrate runtime,
because it should not be used for consensus critical code. So in order for the CompactForm
to be integrated into the substrate runtime, or wherever the "full" feature cannot be used,
then we must parameterize the `String` type so that it can be both an `&'static str` on the
runtime side where it is encoded, and a `String` in client/consuming code where it is decoded.

* Fix no-std compilation

* Obey the fmt

* Introduce String trait for Form

* Rename "Compact" to "Frozen" (and associated fallout)
Add a `compact` member to `Field` to indicate it is `scale_codec::Compact`

* Docs cleanup and more renames

* Cleanup

* More cleanup

* obey the fmt

* Add a `compact` flag to `Field` to indicate that this type is to be encoded/decoded as a SCALE Compact type

* Clippy warnings

* Acommodate older clippy

* Derive (scale) compact fields

* Use utils from parity-scale-codec-derive
Handle `codec(skip)` and `codec(index = $int)` attributes

* fmt

* Attempt to fix CI

* FIx CI take 2

* Use is_compact from utils
Ensure we're working with an outer attribute

* Fn is enough

* Doc tweaks

* Add tests for enums

* Add test for indexed enum

* Oops

* Update derive/src/utils.rs

Co-authored-by: Andrew Jones <[email protected]>

* Review feedback
  Better error message
  Remove redundant tests
  Test more enum cases to document index/discriminant interaction

* fmt

* Better error message, clearer bad syntax trubuild-test

Co-authored-by: Andrew Jones <[email protected]>
  • Loading branch information
dvdplm and ascjones authored Mar 30, 2021
1 parent c51a1d5 commit a02b151
Show file tree
Hide file tree
Showing 8 changed files with 292 additions and 89 deletions.
119 changes: 45 additions & 74 deletions derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ extern crate alloc;
extern crate proc_macro;

mod trait_bounds;
mod utils;

use alloc::{
string::{
Expand All @@ -41,21 +42,14 @@ use syn::{
punctuated::Punctuated,
token::Comma,
visit_mut::VisitMut,
AttrStyle,
Data,
DataEnum,
DataStruct,
DeriveInput,
Expr,
ExprLit,
Field,
Fields,
Ident,
Lifetime,
Lit,
Meta,
MetaList,
NestedMeta,
Variant,
};

Expand Down Expand Up @@ -140,6 +134,7 @@ type FieldsList = Punctuated<Field, Comma>;
fn generate_fields(fields: &FieldsList) -> Vec<TokenStream2> {
fields
.iter()
.filter(|f| !utils::should_skip(&f.attrs))
.map(|f| {
let (ty, ident) = (&f.ty, &f.ident);
// Replace any field lifetime params with `static to prevent "unnecessary lifetime parameter"
Expand All @@ -154,7 +149,7 @@ fn generate_fields(fields: &FieldsList) -> Vec<TokenStream2> {
StaticLifetimesReplace.visit_type_mut(&mut ty);

let type_name = clean_type_string(&quote!(#ty).to_string());
let method_call = if is_compact(f) {
let method_call = if utils::is_compact(f) {
quote!(.compact_of::<#ty>)
} else {
quote!(.field_of::<#ty>)
Expand All @@ -168,23 +163,6 @@ fn generate_fields(fields: &FieldsList) -> Vec<TokenStream2> {
.collect()
}

/// Look for a `#[codec(compact)]` outer attribute.
fn is_compact(f: &Field) -> bool {
f.attrs.iter().any(|attr| {
let mut is_compact = false;
if attr.style == AttrStyle::Outer && attr.path.is_ident("codec") {
if let Ok(Meta::List(MetaList { nested, .. })) = attr.parse_meta() {
if let Some(NestedMeta::Meta(Meta::Path(path))) = nested.iter().next() {
if path.is_ident("compact") {
is_compact = true;
}
}
}
}
is_compact
})
}

fn clean_type_string(input: &str) -> String {
input
.replace(" ::", "::")
Expand Down Expand Up @@ -229,27 +207,17 @@ fn generate_composite_type(data_struct: &DataStruct, scale_info: &Ident) -> Toke
type VariantList = Punctuated<Variant, Comma>;

fn generate_c_like_enum_def(variants: &VariantList, scale_info: &Ident) -> TokenStream2 {
let variants = variants.into_iter().enumerate().map(|(i, v)| {
let name = &v.ident;
let discriminant = if let Some((
_,
Expr::Lit(ExprLit {
lit: Lit::Int(lit_int),
..
}),
)) = &v.discriminant
{
match lit_int.base10_parse::<u64>() {
Ok(i) => i,
Err(err) => return err.to_compile_error(),
let variants = variants
.into_iter()
.enumerate()
.filter(|(_, v)| !utils::should_skip(&v.attrs))
.map(|(i, v)| {
let name = &v.ident;
let discriminant = utils::variant_index(v, i);
quote! {
.variant(stringify!(#name), #discriminant as u64)
}
} else {
i as u64
};
quote! {
.variant(stringify!(#name), #discriminant)
}
});
});
quote! {
variant(
:: #scale_info ::build::Variants::fieldless()
Expand All @@ -259,9 +227,9 @@ fn generate_c_like_enum_def(variants: &VariantList, scale_info: &Ident) -> Token
}

fn is_c_like_enum(variants: &VariantList) -> bool {
// any variant has an explicit discriminant
// One of the variants has an explicit discriminant, or…
variants.iter().any(|v| v.discriminant.is_some()) ||
// all variants are unit
// all variants are unit
variants.iter().all(|v| matches!(v.fields, Fields::Unit))
}

Expand All @@ -272,37 +240,40 @@ fn generate_variant_type(data_enum: &DataEnum, scale_info: &Ident) -> TokenStrea
return generate_c_like_enum_def(variants, scale_info)
}

let variants = variants.into_iter().map(|v| {
let ident = &v.ident;
let v_name = quote! {stringify!(#ident) };
match v.fields {
Fields::Named(ref fs) => {
let fields = generate_fields(&fs.named);
quote! {
.variant(
#v_name,
:: #scale_info ::build::Fields::named()
#( #fields)*
)
let variants = variants
.into_iter()
.filter(|v| !utils::should_skip(&v.attrs))
.map(|v| {
let ident = &v.ident;
let v_name = quote! {stringify!(#ident) };
match v.fields {
Fields::Named(ref fs) => {
let fields = generate_fields(&fs.named);
quote! {
.variant(
#v_name,
:: #scale_info::build::Fields::named()
#( #fields)*
)
}
}
}
Fields::Unnamed(ref fs) => {
let fields = generate_fields(&fs.unnamed);
quote! {
.variant(
#v_name,
:: #scale_info ::build::Fields::unnamed()
#( #fields)*
)
Fields::Unnamed(ref fs) => {
let fields = generate_fields(&fs.unnamed);
quote! {
.variant(
#v_name,
:: #scale_info::build::Fields::unnamed()
#( #fields)*
)
}
}
}
Fields::Unit => {
quote! {
.variant_unit(#v_name)
Fields::Unit => {
quote! {
.variant_unit(#v_name)
}
}
}
}
});
});
quote! {
variant(
:: #scale_info ::build::Variants::with_fields()
Expand Down
4 changes: 3 additions & 1 deletion derive/src/trait_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use syn::{
WhereClause,
};

use crate::utils;

/// Generates a where clause for a `TypeInfo` impl, adding `TypeInfo + 'static` bounds to all
/// relevant generic types including associated types (e.g. `T::A: TypeInfo`), correctly dealing
/// with self-referential types.
Expand Down Expand Up @@ -164,7 +166,7 @@ fn collect_types_to_bind(
// to not have them in the where clause.
!type_or_sub_type_path_starts_with_ident(&field.ty, &input_ident)
})
.map(|f| (f.ty.clone(), super::is_compact(f)))
.map(|f| (f.ty.clone(), utils::is_compact(f)))
.collect()
};

Expand Down
109 changes: 109 additions & 0 deletions derive/src/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright 2019-2021 Parity Technologies (UK) Ltd.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//! Utility methods to work with `SCALE` attributes relevant for the `TypeInfo` derive..
//!
//! NOTE: The code here is copied verbatim from `parity-scale-codec-derive`.

use proc_macro2::TokenStream;
use quote::quote;
use syn::{
spanned::Spanned,
AttrStyle,
Attribute,
Lit,
Meta,
NestedMeta,
Variant,
};

/// Look for a `#[codec(index = $int)]` attribute on a variant. If no attribute
/// is found, fall back to the discriminant or just the variant index.
pub fn variant_index(v: &Variant, i: usize) -> TokenStream {
// first look for an attribute
let index = find_meta_item(v.attrs.iter(), |meta| {
if let NestedMeta::Meta(Meta::NameValue(ref nv)) = meta {
if nv.path.is_ident("index") {
if let Lit::Int(ref v) = nv.lit {
let byte = v
.base10_parse::<u8>()
.expect("Internal error. `#[codec(index = …)]` attribute syntax must be checked in `parity-scale-codec`. This is a bug.");
return Some(byte)
}
}
}

None
});

// then fallback to discriminant or just index
index.map(|i| quote! { #i }).unwrap_or_else(|| {
v.discriminant
.as_ref()
.map(|&(_, ref expr)| quote! { #expr })
.unwrap_or_else(|| quote! { #i })
})
}

/// Look for a `#[codec(compact)]` outer attribute on the given `Field`.
pub fn is_compact(field: &syn::Field) -> bool {
let outer_attrs = field
.attrs
.iter()
.filter(|attr| attr.style == AttrStyle::Outer);
find_meta_item(outer_attrs, |meta| {
if let NestedMeta::Meta(Meta::Path(ref path)) = meta {
if path.is_ident("compact") {
return Some(())
}
}

None
})
.is_some()
}

/// Look for a `#[codec(skip)]` in the given attributes.
pub fn should_skip(attrs: &[Attribute]) -> bool {
find_meta_item(attrs.iter(), |meta| {
if let NestedMeta::Meta(Meta::Path(ref path)) = meta {
if path.is_ident("skip") {
return Some(path.span())
}
}

None
})
.is_some()
}

fn find_meta_item<'a, F, R, I>(itr: I, pred: F) -> Option<R>
where
F: Fn(&NestedMeta) -> Option<R> + Clone,
I: Iterator<Item = &'a Attribute>,
{
itr.filter_map(|attr| {
if attr.path.is_ident("codec") {
if let Meta::List(ref meta_list) = attr
.parse_meta()
.expect("scale-info: Bad index in `#[codec(index = …)]`, see `parity-scale-codec` error")
{
return meta_list.nested.iter().filter_map(pred.clone()).next()
}
}

None
})
.next()
}
2 changes: 1 addition & 1 deletion src/ty/variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use serde::{
/// Monday,
/// Tuesday,
/// Wednesday,
/// Thursday = 42, // Also allows to manually set the discriminant!
/// Thursday = 42, // Allows setting the discriminant explicitly
/// Friday,
/// Saturday,
/// Sunday,
Expand Down
Loading

0 comments on commit a02b151

Please sign in to comment.