Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
construct_runtime: Fix generation of types behind features (#12229)
Browse files Browse the repository at this point in the history 
* construct_runtime: Fix generation of types behind features

With the recent addition of supporting features in `construct_runtime!` there was a bug overseen.
The `AllPalletsWithSystem` etc type declarations would be declared twice when a certain was enabled.
The problem was that in the macro we didn't feature gate the types that should be declared when
there is no feature enabled. This pull request now takes care of feature gating this type behind
`all(#( not(feature) ))`. So, these types will only be enabled if no of the configured features is enabled.

* Fix tests

* FMT
  • Loading branch information
@bkchr
bkchr authored Sep 10, 2022
1 parent 5a48536 commit 51f60d0
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 92 deletions.
138 changes: 67 additions & 71 deletions frame/support/procedural/src/construct_runtime/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,7 @@ use parse::{
use proc_macro::TokenStream;
use proc_macro2::TokenStream as TokenStream2;
use quote::quote;
use std::{
collections::{HashMap, HashSet},
str::FromStr,
};
use std::{collections::HashSet, str::FromStr};
use syn::{Ident, Result};

/// The fixed name of the system pallet.
Expand Down Expand Up @@ -324,11 +321,15 @@ fn decl_all_pallets<'a>(
features: &HashSet<&str>,
) -> TokenStream2 {
let mut types = TokenStream2::new();
let mut names_by_feature = features

// Every feature set to the pallet names that should be included by this feature set.
let mut features_to_names = features
.iter()
.map(|f| *f)
.powerset()
.map(|feat| (feat, Vec::new()))
.collect::<HashMap<_, _>>();
.map(|feat| (HashSet::from_iter(feat), Vec::new()))
.collect::<Vec<(HashSet<_>, Vec<_>)>>();

for pallet_declaration in pallet_declarations {
let type_name = &pallet_declaration.name;
let pallet = &pallet_declaration.path;
Expand All @@ -346,33 +347,57 @@ fn decl_all_pallets<'a>(
types.extend(type_decl);

if pallet_declaration.cfg_pattern.is_empty() {
for names in names_by_feature.values_mut() {
for (_, names) in features_to_names.iter_mut() {
names.push(&pallet_declaration.name);
}
} else {
for (feature_set, names) in &mut names_by_feature {
for (feature_set, names) in &mut features_to_names {
// Rust tidbit: if we have multiple `#[cfg]` feature on the same item, then the
// predicates listed in all `#[cfg]` attributes are effectively joined by `and()`,
// meaning that all of them must match in order to activate the item
let is_feature_active = pallet_declaration.cfg_pattern.iter().all(|expr| {
expr.eval(|pred| match pred {
Predicate::Feature(f) => feature_set.contains(&f),
Predicate::Test => feature_set.contains(&&"test"),
Predicate::Feature(f) => feature_set.contains(f),
Predicate::Test => feature_set.contains(&"test"),
_ => false,
})
});

if is_feature_active {
names.push(&pallet_declaration.name);
}
}
}
}

let all_pallets_without_system = names_by_feature.iter().map(|(feature_set, names)| {
let mut feature_set = feature_set.iter().collect::<HashSet<_>>();
let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter();
let feature_set = feature_set.into_iter();
let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]);
// All possible features. This will be used below for the empty feature set.
let mut all_features = features_to_names
.iter()
.flat_map(|f| f.0.iter().cloned())
.collect::<HashSet<_>>();
let attribute_to_names = features_to_names
.into_iter()
.map(|(mut features, names)| {
// If this is the empty feature set, it needs to be changed to negate all available
// features. So, we ensure that there is some type declared when all features are not
// enabled.
if features.is_empty() {
let test_cfg = all_features.remove("test").then_some(quote!(test)).into_iter();
let features = all_features.iter();
let attr = quote!(#[cfg(all( #(not(#test_cfg)),* #(not(feature = #features)),* ))]);

(attr, names)
} else {
let test_cfg = features.remove("test").then_some(quote!(test)).into_iter();
let features = features.iter();
let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #features),* ))]);

(attr, names)
}
})
.collect::<Vec<_>>();

let all_pallets_without_system = attribute_to_names.iter().map(|(attr, names)| {
let names = names.iter().filter(|n| **n != SYSTEM_PALLET_NAME);
quote! {
#attr
Expand All @@ -382,40 +407,27 @@ fn decl_all_pallets<'a>(
}
});

let all_pallets_with_system = names_by_feature.iter().map(|(feature_set, names)| {
let mut feature_set = feature_set.iter().collect::<HashSet<_>>();
let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter();
let feature_set = feature_set.into_iter();
let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]);
let all_pallets_with_system = attribute_to_names.iter().map(|(attr, names)| {
quote! {
#attr
/// All pallets included in the runtime as a nested tuple of types.
pub type AllPalletsWithSystem = ( #(#names,)* );
}
});

let all_pallets_without_system_reversed =
names_by_feature.iter().map(|(feature_set, names)| {
let mut feature_set = feature_set.iter().collect::<HashSet<_>>();
let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter();
let feature_set = feature_set.into_iter();
let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]);
let names = names.iter().filter(|n| **n != SYSTEM_PALLET_NAME).rev();
quote! {
#attr
/// All pallets included in the runtime as a nested tuple of types in reversed order.
/// Excludes the System pallet.
#[deprecated(note = "Using reverse pallet orders is deprecated. use only \
`AllPalletWithSystem or AllPalletsWithoutSystem`")]
pub type AllPalletsWithoutSystemReversed = ( #(#names,)* );
}
});
let all_pallets_without_system_reversed = attribute_to_names.iter().map(|(attr, names)| {
let names = names.iter().filter(|n| **n != SYSTEM_PALLET_NAME).rev();
quote! {
#attr
/// All pallets included in the runtime as a nested tuple of types in reversed order.
/// Excludes the System pallet.
#[deprecated(note = "Using reverse pallet orders is deprecated. use only \
`AllPalletWithSystem or AllPalletsWithoutSystem`")]
pub type AllPalletsWithoutSystemReversed = ( #(#names,)* );
}
});

let all_pallets_with_system_reversed = names_by_feature.iter().map(|(feature_set, names)| {
let mut feature_set = feature_set.iter().collect::<HashSet<_>>();
let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter();
let feature_set = feature_set.into_iter();
let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]);
let all_pallets_with_system_reversed = attribute_to_names.iter().map(|(attr, names)| {
let names = names.iter().rev();
quote! {
#attr
Expand All @@ -426,35 +438,19 @@ fn decl_all_pallets<'a>(
}
});

let system_pallet =
match names_by_feature[&Vec::new()].iter().find(|n| **n == SYSTEM_PALLET_NAME) {
Some(name) => name,
None =>
return syn::Error::new(
proc_macro2::Span::call_site(),
"`System` pallet declaration is missing. \
Please add this line: `System: frame_system,`",
)
.into_compile_error(),
};

let all_pallets_reversed_with_system_first =
names_by_feature.iter().map(|(feature_set, names)| {
let mut feature_set = feature_set.iter().collect::<HashSet<_>>();
let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter();
let feature_set = feature_set.into_iter();
let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]);
let names = std::iter::once(system_pallet)
.chain(names.iter().rev().filter(|n| **n != SYSTEM_PALLET_NAME));
quote! {
#attr
/// All pallets included in the runtime as a nested tuple of types in reversed order.
/// With the system pallet first.
#[deprecated(note = "Using reverse pallet orders is deprecated. use only \
`AllPalletWithSystem or AllPalletsWithoutSystem`")]
pub type AllPalletsReversedWithSystemFirst = ( #(#names,)* );
}
});
let all_pallets_reversed_with_system_first = attribute_to_names.iter().map(|(attr, names)| {
let system = quote::format_ident!("{}", SYSTEM_PALLET_NAME);
let names = std::iter::once(&system)
.chain(names.iter().rev().filter(|n| **n != SYSTEM_PALLET_NAME).cloned());
quote! {
#attr
/// All pallets included in the runtime as a nested tuple of types in reversed order.
/// With the system pallet first.
#[deprecated(note = "Using reverse pallet orders is deprecated. use only \
`AllPalletWithSystem or AllPalletsWithoutSystem`")]
pub type AllPalletsReversedWithSystemFirst = ( #(#names,)* );
}
});

quote!(
#types
Expand Down
7 changes: 4 additions & 3 deletions frame/support/test/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,10 @@ std = [
"sp-version/std",
]
try-runtime = ["frame-support/try-runtime"]
# WARNING: CI only execute pallet test with this feature,
# if the feature intended to be used outside, CI and this message need to be updated.
conditional-storage = []
# WARNING:
# Only CI runs with this feature enabled. This feature is for testing stuff related to the FRAME macros
# in conjunction with rust features.
frame-feature-testing = []
# Disable ui tests
disable-ui-tests = []
no-metadata-docs = ["frame-support/no-metadata-docs"]
Loading

0 comments on commit 51f60d0

Please sign in to comment.