Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3556df3
implement index for pallet + some tests
gui1117 Aug 26, 2020
766f510
add test and doc
gui1117 Sep 1, 2020
d246bdf
remove deprecated and document behavior
gui1117 Sep 1, 2020
11a3af2
update internal doc
gui1117 Sep 1, 2020
ce2d1bd
Apply suggestions from code review
gui1117 Sep 7, 2020
b56cbd7
address review
gui1117 Sep 7, 2020
da702ae
use index for all module, break construct_runtime
gui1117 Sep 8, 2020
0290c47
fix line length
gui1117 Sep 8, 2020
82e36e4
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 9, 2020
6c8e80a
implement migration helper funciton in scheduler
gui1117 Sep 9, 2020
4e5b4f8
fix start at index 0
gui1117 Sep 9, 2020
95a1eb7
Update frame/scheduler/src/lib.rs
gui1117 Sep 10, 2020
9518e5b
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 10, 2020
a830e2f
Update frame/support/procedural/src/lib.rs
gui1117 Sep 16, 2020
9812da2
bump frame-metadata crate
gui1117 Sep 16, 2020
5d86201
factorize
gui1117 Sep 16, 2020
078a543
avoid some unwrap and remove nightly join
gui1117 Sep 16, 2020
4bba712
Update frame/support/src/event.rs
gui1117 Sep 16, 2020
c6f073b
fix test
gui1117 Sep 16, 2020
43856fd
add test and improve error message
gui1117 Sep 16, 2020
7da2738
factorize test
gui1117 Sep 16, 2020
b9fca7e
keep iterator, and use slice instead of vec
gui1117 Sep 16, 2020
61019e7
refactor to avoid to have expects
gui1117 Sep 16, 2020
a9d454a
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 16, 2020
ae0f859
small refactor
gui1117 Sep 16, 2020
85a65f2
Test something
bkchr Sep 17, 2020
ab0c998
Make sure we update the `Cargo.lock`
bkchr Sep 17, 2020
fda8a1a
Apply suggestions from code review
gui1117 Sep 18, 2020
95ef421
return 2 error
gui1117 Sep 18, 2020
68b472e
Apply suggestions from code review
gui1117 Sep 21, 2020
21510d4
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 21, 2020
6feb460
Update frame/scheduler/src/lib.rs
gui1117 Sep 21, 2020
f2de8f2
fix typo
gui1117 Sep 21, 2020
3f38e1a
Revert "fix typo"
gui1117 Sep 21, 2020
492a83f
Revert "Update frame/scheduler/src/lib.rs"
gui1117 Sep 21, 2020
d04f8cd
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 22, 2020
986155b
Merge remote-tracking branch 'origin/master' into gui-construct-runti…
gui1117 Sep 22, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor to avoid to have expects
  • Loading branch information
gui1117 committed Sep 16, 2020
commit 61019e704f211ef400db9262d99fbf635dba7997
140 changes: 86 additions & 54 deletions frame/support/procedural/src/construct_runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ mod parse;

use frame_support_procedural_tools::syn_ext as ext;
use frame_support_procedural_tools::{generate_crate_access, generate_hidden_includes};
use parse::{ModuleDeclaration, RuntimeDefinition, WhereSection};
use parse::{ModuleDeclaration, RuntimeDefinition, WhereSection, ModulePart};
use proc_macro::TokenStream;
use proc_macro2::{TokenStream as TokenStream2, Span};
use quote::quote;
Expand All @@ -29,6 +29,74 @@ use std::collections::HashMap;
/// The fixed name of the system module.
const SYSTEM_MODULE_NAME: &str = "System";

/// The complete definition of a module with the resulting fixed index.
#[derive(Debug, Clone)]
pub struct Module {
pub name: Ident,
pub index: u8,
pub module: Ident,
pub instance: Option<Ident>,
pub module_parts: Vec<ModulePart>,
}

impl Module {
/// Get resolved module parts
fn module_parts(&self) -> &[ModulePart] {
&self.module_parts
}

/// Find matching parts
fn find_part(&self, name: &str) -> Option<&ModulePart> {
self.module_parts.iter().find(|part| part.name() == name)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given assumption that System is always there, maybe you can maybe simplify a thing or two by fn get_system() -> Option<&ModulePart> or fn is_system() -> bool but just an idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, for now system is only queried one time and filtered out only one time too, but if I would refactor such functions looks more readable.

/// Return whether module contains part
fn exists_part(&self, name: &str) -> bool {
self.find_part(name).is_some()
}
}

/// Convert from the parsed module to their final information.
/// Assign index to each modules using same rules as rust for fieldless enum.
/// I.e. implicit are assigned number incrementedly from last explicit or 0.
fn complete_modules(decl: impl Iterator<Item = ModuleDeclaration>) -> syn::Result<Vec<Module>> {
let mut indices = HashMap::new();
let mut last_index: Option<u8> = None;

decl
.map(|module| {
let final_index = match module.index {
Some(i) => i,
None => last_index.map_or(Some(0), |i| i.checked_add(1))
.ok_or_else(|| {
let msg = "Module index doesn't fit into u8, index is 256";
syn::Error::new(module.name.span(), msg)
})?,
};

last_index = Some(final_index);

if let Some(used_module) = indices.insert(final_index, module.name.clone()) {
let msg = format!(
"Module index are conflicting both modules {} and {} are at index {}",
module.name,
used_module,
final_index,
);
return Err(syn::Error::new(module.name.span(), msg));
}

Ok(Module {
name: module.name,
index: final_index,
module: module.module,
instance: module.instance,
module_parts: module.module_parts,
})
})
.collect()
}

pub fn construct_runtime(input: TokenStream) -> TokenStream {
let definition = syn::parse_macro_input!(input as RuntimeDefinition);
construct_runtime_parsed(definition)
Expand All @@ -53,10 +121,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result<TokenStream
..
} = definition;

let mut modules = modules.into_iter().collect::<Vec<_>>();

// Automatically assign implicit index:
assign_implicit_index(&mut modules)?;
let modules = complete_modules(modules.into_iter())?;

let system_module = modules.iter()
.find(|decl| decl.name == SYSTEM_MODULE_NAME)
Expand Down Expand Up @@ -139,7 +204,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result<TokenStream

fn decl_validate_unsigned<'a>(
runtime: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: impl Iterator<Item = &'a Module>,
scrate: &'a TokenStream2,
) -> TokenStream2 {
let modules_tokens = module_declarations
Expand All @@ -157,7 +222,7 @@ fn decl_validate_unsigned<'a>(
fn decl_outer_inherent<'a>(
block: &'a syn::TypePath,
unchecked_extrinsic: &'a syn::TypePath,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: impl Iterator<Item = &'a Module>,
scrate: &'a TokenStream2,
) -> TokenStream2 {
let modules_tokens = module_declarations.filter_map(|module_declaration| {
Expand All @@ -181,7 +246,7 @@ fn decl_outer_inherent<'a>(

fn decl_outer_config<'a>(
runtime: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: impl Iterator<Item = &'a Module>,
scrate: &'a TokenStream2,
) -> TokenStream2 {
let modules_tokens = module_declarations
Expand Down Expand Up @@ -219,7 +284,7 @@ fn decl_outer_config<'a>(

fn decl_runtime_metadata<'a>(
runtime: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: impl Iterator<Item = &'a Module>,
scrate: &'a TokenStream2,
extrinsic: &TypePath,
) -> TokenStream2 {
Expand All @@ -244,7 +309,7 @@ fn decl_runtime_metadata<'a>(
.map(|name| quote!(<#name>))
.into_iter();

let index = module_declaration.index.expect("All index have been assigned");
let index = module_declaration.index;

quote!(
#module::Module #(#instance)* as #name { index #index } with #(#filtered_names)*,
Expand All @@ -260,15 +325,15 @@ fn decl_runtime_metadata<'a>(

fn decl_outer_dispatch<'a>(
runtime: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: impl Iterator<Item = &'a Module>,
scrate: &'a TokenStream2,
) -> TokenStream2 {
let modules_tokens = module_declarations
.filter(|module_declaration| module_declaration.exists_part("Call"))
.map(|module_declaration| {
let module = &module_declaration.module;
let name = &module_declaration.name;
let index = module_declaration.index.expect("All index have been assigned");
let index = module_declaration.index;
let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site());
quote!(#[codec(index = #index_string)] #module::#name)
});
Expand All @@ -284,8 +349,8 @@ fn decl_outer_dispatch<'a>(

fn decl_outer_origin<'a>(
runtime_name: &'a Ident,
modules_except_system: impl Iterator<Item = &'a ModuleDeclaration>,
system_module: &'a ModuleDeclaration,
modules_except_system: impl Iterator<Item = &'a Module>,
system_module: &'a Module,
scrate: &'a TokenStream2,
) -> syn::Result<TokenStream2> {
let mut modules_tokens = TokenStream2::new();
Expand All @@ -303,7 +368,7 @@ fn decl_outer_origin<'a>(
);
return Err(syn::Error::new(module_declaration.name.span(), msg));
}
let index = module_declaration.index.expect("All index have been assigned");
let index = module_declaration.index;
let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site());
let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,);
modules_tokens.extend(tokens);
Expand All @@ -313,7 +378,7 @@ fn decl_outer_origin<'a>(
}

let system_name = &system_module.module;
let system_index = system_module.index.expect("All index have been assigned");
let system_index = system_module.index;
let system_index_string = syn::LitStr::new(&format!("{}", system_index), Span::call_site());

Ok(quote!(
Expand All @@ -330,7 +395,7 @@ fn decl_outer_origin<'a>(

fn decl_outer_event<'a>(
runtime_name: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: impl Iterator<Item = &'a Module>,
scrate: &'a TokenStream2,
) -> syn::Result<TokenStream2> {
let mut modules_tokens = TokenStream2::new();
Expand All @@ -349,7 +414,7 @@ fn decl_outer_event<'a>(
return Err(syn::Error::new(module_declaration.name.span(), msg));
}

let index = module_declaration.index.expect("All index have been assigned");
let index = module_declaration.index;
let index_string = syn::LitStr::new(&format!("{}", index), Span::call_site());
let tokens = quote!(#[codec(index = #index_string)] #module #instance #generics,);
modules_tokens.extend(tokens);
Expand All @@ -369,7 +434,7 @@ fn decl_outer_event<'a>(

fn decl_all_modules<'a>(
runtime: &'a Ident,
module_declarations: impl Iterator<Item = &'a ModuleDeclaration>,
module_declarations: impl Iterator<Item = &'a Module>,
) -> TokenStream2 {
let mut types = TokenStream2::new();
let mut names = Vec::new();
Expand Down Expand Up @@ -402,12 +467,12 @@ fn decl_all_modules<'a>(
}

fn decl_module_to_index<'a>(
module_declarations: &[ModuleDeclaration],
module_declarations: &[Module],
scrate: &TokenStream2,
) -> TokenStream2 {
let names = module_declarations.iter().map(|d| &d.name);
let indices = module_declarations.iter()
.map(|module| module.index.expect("All index have been assigned") as usize);
.map(|module| module.index as usize);

quote!(
/// Provides an implementation of `ModuleToIndex` to map a module
Expand Down Expand Up @@ -442,36 +507,3 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 {
}
)
}

/// Assign index to each modules using same rules as rust for fieldless enum.
/// I.e. implicit are assigned number incrementedly from last explicit or 0.
fn assign_implicit_index(modules: &mut [ModuleDeclaration]) -> syn::Result<()> {
let mut indices = HashMap::new();
let mut last_index: Option<u8> = None;

for module in modules {
let final_index = match module.index {
Some(i) => i,
None => last_index.map_or(Some(0), |i| i.checked_add(1))
.ok_or_else(|| {
let msg = "Module index doesn't fit into u8, index is 256";
syn::Error::new(module.name.span(), msg)
})?,
};

module.index = Some(final_index);
last_index = Some(final_index);

if let Some(used_module) = indices.insert(final_index, module.name.clone()) {
let msg = format!(
"Module index are conflicting both modules {} and {} are at index {}",
module.name,
used_module,
final_index,
);
return Err(syn::Error::new(module.name.span(), msg));
}
}

Ok(())
}
15 changes: 0 additions & 15 deletions frame/support/procedural/src/construct_runtime/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,21 +198,6 @@ impl Parse for ModuleDeclaration {
}
}

impl ModuleDeclaration {
/// Get resolved module parts
pub fn module_parts(&self) -> &[ModulePart] {
&self.module_parts
}

pub fn find_part(&self, name: &str) -> Option<&ModulePart> {
self.module_parts.iter().find(|part| part.name() == name)
}

pub fn exists_part(&self, name: &str) -> bool {
self.find_part(name).is_some()
}
}

/// Parse [`ModulePart`]'s from a braces enclosed list that is split by commas, e.g.
///
/// `{ Call, Event }`
Expand Down