Skip to content

Commit

Permalink
Fixes parsing for config attrs in pallet macros (paritytech#2677)
Browse files Browse the repository at this point in the history
  • Loading branch information
gupnik committed Dec 13, 2023
1 parent 057db08 commit 301db49
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 25 deletions.
7 changes: 7 additions & 0 deletions substrate/frame/support/procedural/src/derive_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,15 @@ fn combine_impls(
return None
}
if let ImplItem::Type(typ) = item.clone() {
let cfg_attrs = typ
.attrs
.iter()
.filter(|attr| attr.path().get_ident().map_or(false, |ident| ident == "cfg"))
.map(|attr| attr.to_token_stream());
if is_runtime_type(&typ) {
let item: ImplItem = if inject_runtime_types {
parse_quote! {
#( #cfg_attrs )*
type #ident = #ident;
}
} else {
Expand All @@ -148,6 +154,7 @@ fn combine_impls(
}
// modify and insert uncolliding type items
let modified_item: ImplItem = parse_quote! {
#( #cfg_attrs )*
type #ident = <#default_impl_path as #disambiguation_path>::#ident;
};
return Some(modified_item)
Expand Down
22 changes: 18 additions & 4 deletions substrate/frame/support/procedural/src/pallet/expand/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,15 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
})
.collect::<Vec<_>>();

let cfg_attrs = methods
.iter()
.map(|method| {
let attrs =
method.cfg_attrs.iter().map(|attr| attr.to_token_stream()).collect::<Vec<_>>();
quote::quote!( #( #attrs )* )
})
.collect::<Vec<_>>();

let feeless_check = methods.iter().map(|method| &method.feeless_check).collect::<Vec<_>>();
let feeless_check_result =
feeless_check.iter().zip(args_name.iter()).map(|(feeless_check, arg_name)| {
Expand Down Expand Up @@ -297,6 +306,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
#frame_support::Never,
),
#(
#cfg_attrs
#[doc = #fn_doc]
#[codec(index = #call_index)]
#fn_name {
Expand All @@ -310,6 +320,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {

impl<#type_impl_gen> #call_ident<#type_use_gen> #where_clause {
#(
#cfg_attrs
#[doc = #new_call_variant_doc]
pub fn #new_call_variant_fn_name(
#( #args_name_stripped: #args_type ),*
Expand All @@ -328,6 +339,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
fn get_dispatch_info(&self) -> #frame_support::dispatch::DispatchInfo {
match *self {
#(
#cfg_attrs
Self::#fn_name { #( #args_name_pattern_ref, )* } => {
let __pallet_base_weight = #fn_weight;

Expand Down Expand Up @@ -365,6 +377,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
fn is_feeless(&self, origin: &Self::Origin) -> bool {
match *self {
#(
#cfg_attrs
Self::#fn_name { #( #args_name_pattern_ref, )* } => {
#feeless_check_result
},
Expand All @@ -379,13 +392,13 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
{
fn get_call_name(&self) -> &'static str {
match *self {
#( Self::#fn_name { .. } => stringify!(#fn_name), )*
#( #cfg_attrs Self::#fn_name { .. } => stringify!(#fn_name), )*
Self::__Ignore(_, _) => unreachable!("__PhantomItem cannot be used."),
}
}

fn get_call_names() -> &'static [&'static str] {
&[ #( stringify!(#fn_name), )* ]
&[ #( #cfg_attrs stringify!(#fn_name), )* ]
}
}

Expand All @@ -394,13 +407,13 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
{
fn get_call_index(&self) -> u8 {
match *self {
#( Self::#fn_name { .. } => #call_index, )*
#( #cfg_attrs Self::#fn_name { .. } => #call_index, )*
Self::__Ignore(_, _) => unreachable!("__PhantomItem cannot be used."),
}
}

fn get_call_indices() -> &'static [u8] {
&[ #( #call_index, )* ]
&[ #( #cfg_attrs #call_index, )* ]
}
}

Expand All @@ -416,6 +429,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
#frame_support::dispatch_context::run_in_context(|| {
match self {
#(
#cfg_attrs
Self::#fn_name { #( #args_name_pattern, )* } => {
#frame_support::__private::sp_tracing::enter_span!(
#frame_support::__private::sp_tracing::trace_span!(stringify!(#fn_name))
Expand Down
37 changes: 22 additions & 15 deletions substrate/frame/support/procedural/src/pallet/expand/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@
// limitations under the License.

use crate::{
pallet::{parse::error::VariantField, Def},
pallet::{
parse::error::{VariantDef, VariantField},
Def,
},
COUNTER,
};
use frame_support_procedural_tools::get_doc_literals;
use quote::ToTokens;
use syn::spanned::Spanned;

///
Expand Down Expand Up @@ -67,20 +71,23 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream {
)
);

let as_str_matches = error.variants.iter().map(|(variant, field_ty, _)| {
let variant_str = variant.to_string();
match field_ty {
Some(VariantField { is_named: true }) => {
quote::quote_spanned!(error.attr_span => Self::#variant { .. } => #variant_str,)
},
Some(VariantField { is_named: false }) => {
quote::quote_spanned!(error.attr_span => Self::#variant(..) => #variant_str,)
},
None => {
quote::quote_spanned!(error.attr_span => Self::#variant => #variant_str,)
},
}
});
let as_str_matches = error.variants.iter().map(
|VariantDef { ident: variant, field: field_ty, docs: _, cfg_attrs }| {
let variant_str = variant.to_string();
let cfg_attrs = cfg_attrs.iter().map(|attr| attr.to_token_stream());
match field_ty {
Some(VariantField { is_named: true }) => {
quote::quote_spanned!(error.attr_span => #( #cfg_attrs )* Self::#variant { .. } => #variant_str,)
},
Some(VariantField { is_named: false }) => {
quote::quote_spanned!(error.attr_span => #( #cfg_attrs )* Self::#variant(..) => #variant_str,)
},
None => {
quote::quote_spanned!(error.attr_span => #( #cfg_attrs )* Self::#variant => #variant_str,)
},
}
},
);

let error_item = {
let item = &mut def.item.content.as_mut().expect("Checked by def parser").1[error.index];
Expand Down
4 changes: 4 additions & 0 deletions substrate/frame/support/procedural/src/pallet/parse/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ pub struct CallVariantDef {
pub docs: Vec<syn::Expr>,
/// Attributes annotated at the top of the dispatchable function.
pub attrs: Vec<syn::Attribute>,
/// The `cfg` attributes.
pub cfg_attrs: Vec<syn::Attribute>,
/// The optional `feeless_if` attribute on the `pallet::call`.
pub feeless_check: Option<syn::ExprClosure>,
}
Expand Down Expand Up @@ -266,6 +268,7 @@ impl CallDef {
return Err(syn::Error::new(method.sig.span(), msg))
}

let cfg_attrs: Vec<syn::Attribute> = helper::get_item_cfg_attrs(&method.attrs);
let mut call_idx_attrs = vec![];
let mut weight_attrs = vec![];
let mut feeless_attrs = vec![];
Expand Down Expand Up @@ -442,6 +445,7 @@ impl CallDef {
args,
docs,
attrs: method.attrs.clone(),
cfg_attrs,
feeless_check,
});
} else {
Expand Down
26 changes: 22 additions & 4 deletions substrate/frame/support/procedural/src/pallet/parse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,31 @@ mod keyword {
syn::custom_keyword!(Error);
}

/// Records information about the error enum variants.
/// Records information about the error enum variant field.
pub struct VariantField {
/// Whether or not the field is named, i.e. whether it is a tuple variant or struct variant.
pub is_named: bool,
}

/// Records information about the error enum variants.
pub struct VariantDef {
/// The variant ident.
pub ident: syn::Ident,
/// The variant field, if any.
pub field: Option<VariantField>,
/// The variant doc literals.
pub docs: Vec<syn::Expr>,
/// The `cfg` attributes.
pub cfg_attrs: Vec<syn::Attribute>,
}

/// This checks error declaration as a enum declaration with only variants without fields nor
/// discriminant.
pub struct ErrorDef {
/// The index of error item in pallet module.
pub index: usize,
/// Variants ident, optional field and doc literals (ordered as declaration order)
pub variants: Vec<(syn::Ident, Option<VariantField>, Vec<syn::Expr>)>,
/// Variant definitions.
pub variants: Vec<VariantDef>,
/// A set of usage of instance, must be check for consistency with trait.
pub instances: Vec<helper::InstanceUsage>,
/// The keyword error used (contains span).
Expand Down Expand Up @@ -87,8 +99,14 @@ impl ErrorDef {
let span = variant.discriminant.as_ref().unwrap().0.span();
return Err(syn::Error::new(span, msg))
}
let cfg_attrs: Vec<syn::Attribute> = helper::get_item_cfg_attrs(&variant.attrs);

Ok((variant.ident.clone(), field_ty, get_doc_literals(&variant.attrs)))
Ok(VariantDef {
ident: variant.ident.clone(),
field: field_ty,
docs: get_doc_literals(&variant.attrs),
cfg_attrs,
})
})
.collect::<Result<_, _>>()?;

Expand Down
52 changes: 52 additions & 0 deletions substrate/frame/support/test/tests/derive_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// This file is part of Substrate.

// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// 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.

use frame_support::derive_impl;

trait Shape {
fn area(&self) -> u32;
}

struct SomeRectangle {}

#[frame_support::register_default_impl(SomeRectangle)]
impl Shape for SomeRectangle {
#[cfg(not(feature = "feature-frame-testing"))]
fn area(&self) -> u32 {
10
}

#[cfg(feature = "feature-frame-testing")]
fn area(&self) -> u32 {
0
}
}

struct SomeSquare {}

#[derive_impl(SomeRectangle)]
impl Shape for SomeSquare {}

#[test]
fn test_feature_parsing() {
let square = SomeSquare {};
#[cfg(not(feature = "feature-frame-testing"))]
assert_eq!(square.area(), 10);

#[cfg(feature = "feature-frame-testing")]
assert_eq!(square.area(), 0);
}
Loading

0 comments on commit 301db49

Please sign in to comment.