From bc82eb6ec9888e0598df9ecdc2d17bcc1f3cd8d4 Mon Sep 17 00:00:00 2001
From: gupnik <nikhilgupta.iitk@gmail.com>
Date: Wed, 13 Dec 2023 15:30:24 +0530
Subject: [PATCH] Fixes parsing for config attrs in pallet macros (#2677)
---
.../support/procedural/src/derive_impl.rs | 7 ++
.../procedural/src/pallet/expand/call.rs | 22 +++++--
.../procedural/src/pallet/expand/error.rs | 37 ++++++-----
.../procedural/src/pallet/parse/call.rs | 4 ++
.../procedural/src/pallet/parse/error.rs | 26 ++++++--
.../frame/support/test/tests/derive_impl.rs | 52 +++++++++++++++
substrate/frame/support/test/tests/pallet.rs | 64 ++++++++++++++++++-
7 files changed, 187 insertions(+), 25 deletions(-)
create mode 100644 substrate/frame/support/test/tests/derive_impl.rs
diff --git a/substrate/frame/support/procedural/src/derive_impl.rs b/substrate/frame/support/procedural/src/derive_impl.rs
index 3e044053116..d6d5bf68efd 100644
--- a/substrate/frame/support/procedural/src/derive_impl.rs
+++ b/substrate/frame/support/procedural/src/derive_impl.rs
@@ -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 {
@@ -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)
diff --git a/substrate/frame/support/procedural/src/pallet/expand/call.rs b/substrate/frame/support/procedural/src/pallet/expand/call.rs
index cf302faafc7..624cde018dc 100644
--- a/substrate/frame/support/procedural/src/pallet/expand/call.rs
+++ b/substrate/frame/support/procedural/src/pallet/expand/call.rs
@@ -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)| {
@@ -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 {
@@ -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 ),*
@@ -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;
@@ -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
},
@@ -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), )* ]
}
}
@@ -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, )* ]
}
}
@@ -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))
diff --git a/substrate/frame/support/procedural/src/pallet/expand/error.rs b/substrate/frame/support/procedural/src/pallet/expand/error.rs
index 877489fd605..72fb6e92357 100644
--- a/substrate/frame/support/procedural/src/pallet/expand/error.rs
+++ b/substrate/frame/support/procedural/src/pallet/expand/error.rs
@@ -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;
///
@@ -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];
diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs
index 58e14365e76..4e09b86fdde 100644
--- a/substrate/frame/support/procedural/src/pallet/parse/call.rs
+++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs
@@ -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>,
}
@@ -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![];
@@ -442,6 +445,7 @@ impl CallDef {
args,
docs,
attrs: method.attrs.clone(),
+ cfg_attrs,
feeless_check,
});
} else {
diff --git a/substrate/frame/support/procedural/src/pallet/parse/error.rs b/substrate/frame/support/procedural/src/pallet/parse/error.rs
index 6f82ce61fc9..362df8d7340 100644
--- a/substrate/frame/support/procedural/src/pallet/parse/error.rs
+++ b/substrate/frame/support/procedural/src/pallet/parse/error.rs
@@ -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).
@@ -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<_, _>>()?;
diff --git a/substrate/frame/support/test/tests/derive_impl.rs b/substrate/frame/support/test/tests/derive_impl.rs
new file mode 100644
index 00000000000..675e85f4bfc
--- /dev/null
+++ b/substrate/frame/support/test/tests/derive_impl.rs
@@ -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);
+}
diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs
index d2e5b185163..0223979d7f0 100644
--- a/substrate/frame/support/test/tests/pallet.rs
+++ b/substrate/frame/support/test/tests/pallet.rs
@@ -257,6 +257,13 @@ pub mod pallet {
pub fn check_for_dispatch_context(_origin: OriginFor<T>) -> DispatchResult {
with_context::<(), _>(|_| ()).ok_or_else(|| DispatchError::Unavailable)
}
+
+ #[cfg(feature = "frame-feature-testing")]
+ #[pallet::call_index(5)]
+ #[pallet::weight({1})]
+ pub fn foo_feature_test(_origin: OriginFor<T>) -> DispatchResult {
+ Ok(())
+ }
}
#[pallet::error]
@@ -269,6 +276,8 @@ pub mod pallet {
#[codec(skip)]
Skipped(u128),
CompactU8(#[codec(compact)] u8),
+ #[cfg(feature = "frame-feature-testing")]
+ FeatureTest,
}
#[pallet::event]
@@ -796,6 +805,7 @@ fn call_expand() {
}
);
assert_eq!(call_foo.get_call_name(), "foo");
+ #[cfg(not(feature = "frame-feature-testing"))]
assert_eq!(
pallet::Call::<Runtime>::get_call_names(),
&[
@@ -806,9 +816,24 @@ fn call_expand() {
"check_for_dispatch_context"
],
);
+ #[cfg(feature = "frame-feature-testing")]
+ assert_eq!(
+ pallet::Call::<Runtime>::get_call_names(),
+ &[
+ "foo",
+ "foo_storage_layer",
+ "foo_index_out_of_order",
+ "foo_no_post_info",
+ "check_for_dispatch_context",
+ "foo_feature_test"
+ ],
+ );
assert_eq!(call_foo.get_call_index(), 0u8);
- assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8])
+ #[cfg(not(feature = "frame-feature-testing"))]
+ assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8]);
+ #[cfg(feature = "frame-feature-testing")]
+ assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8, 5u8]);
}
#[test]
@@ -816,7 +841,10 @@ fn call_expand_index() {
let call_foo = pallet::Call::<Runtime>::foo_index_out_of_order {};
assert_eq!(call_foo.get_call_index(), 4u8);
- assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8])
+ #[cfg(not(feature = "frame-feature-testing"))]
+ assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8]);
+ #[cfg(feature = "frame-feature-testing")]
+ assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8, 5u8]);
}
#[test]
@@ -838,6 +866,8 @@ fn error_expand() {
}),
);
assert_eq!(<pallet::Error::<Runtime> as PalletError>::MAX_ENCODED_SIZE, 3);
+ #[cfg(feature = "frame-feature-testing")]
+ assert_eq!(format!("{:?}", pallet::Error::<Runtime>::FeatureTest), String::from("FeatureTest"),);
}
#[test]
@@ -2379,3 +2409,33 @@ fn test_dispatch_context() {
.dispatch(RuntimeOrigin::root()));
});
}
+
+#[test]
+fn test_call_feature_parsing() {
+ let call = pallet::Call::<Runtime>::check_for_dispatch_context {};
+ match call {
+ pallet::Call::<Runtime>::check_for_dispatch_context {} |
+ pallet::Call::<Runtime>::foo { .. } |
+ pallet::Call::foo_storage_layer { .. } |
+ pallet::Call::foo_index_out_of_order {} |
+ pallet::Call::foo_no_post_info {} => (),
+ #[cfg(feature = "frame-feature-testing")]
+ pallet::Call::foo_feature_test {} => (),
+ pallet::Call::__Ignore(_, _) => (),
+ }
+}
+
+#[test]
+fn test_error_feature_parsing() {
+ let err = pallet::Error::<Runtime>::InsufficientProposersBalance;
+ match err {
+ pallet::Error::InsufficientProposersBalance |
+ pallet::Error::NonExistentStorageValue |
+ pallet::Error::Code(_) |
+ pallet::Error::Skipped(_) |
+ pallet::Error::CompactU8(_) => (),
+ #[cfg(feature = "frame-feature-testing")]
+ pallet::Error::FeatureTest => (),
+ pallet::Error::__Ignore(_, _) => (),
+ }
+}
--
GitLab