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

Disambiguate BlockNumber type in decl_module #7061

Merged
bkchr merged 4 commits into from
Sep 9, 2020
Merged

Disambiguate BlockNumber type in decl_module #7061

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c080be0
Disambiguate `BlockNumber` type in `decl_module`
shawntabrizi Sep 9, 2020
83b485f
fix `frame-support-tests`
shawntabrizi Sep 9, 2020
69351de
fix ui tests
shawntabrizi Sep 9, 2020
57b7ed0
fix trait order
shawntabrizi Sep 9, 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
65 changes: 37 additions & 28 deletions frame/support/src/dispatch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1265,28 +1265,30 @@ macro_rules! decl_module {
};

(@impl_on_initialize
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
fn on_initialize() -> $return:ty { $( $impl:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OnInitialize<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
Copy link
Contributor

@gui1117 gui1117 Sep 9, 2020

Choose a reason for hiding this comment

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

this bound doesn't work for instantiable module it basically expand to Trait<I>, I: Instance + system::Trait

Suggested change
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
impl<$trait_instance: $system::Trait + $trait_name$(<I>, $instance: $instantiable)?>

Copy link
Member Author

@shawntabrizi shawntabrizi Sep 9, 2020

Choose a reason for hiding this comment

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

@thiolliere why do we use $instance in one place but <I> in another? Should be:

(<$instance>, $instance: $instantiable)?

Copy link
Contributor

@gui1117 gui1117 Sep 10, 2020

Choose a reason for hiding this comment

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

indeed it should be done as you say. Currently we don't support other instance name than I.

$crate::traits::OnInitialize<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{
fn on_initialize(_block_number_not_used: $trait_instance::BlockNumber) -> $return {
fn on_initialize(_block_number_not_used: <$trait_instance as $system::Trait>::BlockNumber) -> $return {
$crate::sp_tracing::enter_span!("on_initialize");
{ $( $impl )* }
}
}
};

(@impl_on_initialize
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
fn on_initialize($param:ident : $param_ty:ty) -> $return:ty { $( $impl:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OnInitialize<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OnInitialize<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{
fn on_initialize($param: $param_ty) -> $return {
Expand All @@ -1297,11 +1299,12 @@ macro_rules! decl_module {
};

(@impl_on_initialize
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OnInitialize<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OnInitialize<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{}
};
Expand Down Expand Up @@ -1362,28 +1365,30 @@ macro_rules! decl_module {
};

(@impl_on_finalize
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
fn on_finalize() { $( $impl:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OnFinalize<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OnFinalize<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{
fn on_finalize(_block_number_not_used: $trait_instance::BlockNumber) {
fn on_finalize(_block_number_not_used: <$trait_instance as $system::Trait>::BlockNumber) {
$crate::sp_tracing::enter_span!("on_finalize");
{ $( $impl )* }
}
}
};

(@impl_on_finalize
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
fn on_finalize($param:ident : $param_ty:ty) { $( $impl:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OnFinalize<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OnFinalize<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{
fn on_finalize($param: $param_ty) {
Expand All @@ -1394,48 +1399,52 @@ macro_rules! decl_module {
};

(@impl_on_finalize
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OnFinalize<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OnFinalize<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{
}
};

(@impl_offchain
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
fn offchain_worker() { $( $impl:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OffchainWorker<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OffchainWorker<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{
fn offchain_worker(_block_number_not_used: $trait_instance::BlockNumber) { $( $impl )* }
fn offchain_worker(_block_number_not_used: <$trait_instance as $system::Trait>::BlockNumber) { $( $impl )* }
}
};

(@impl_offchain
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
fn offchain_worker($param:ident : $param_ty:ty) { $( $impl:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OffchainWorker<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OffchainWorker<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{
fn offchain_worker($param: $param_ty) { $( $impl )* }
}
};

(@impl_offchain
{ $system:ident }
$module:ident<$trait_instance:ident: $trait_name:ident$(<I>, $instance:ident: $instantiable:path)?>;
{ $( $other_where_bounds:tt )* }
) => {
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)?>
$crate::traits::OffchainWorker<$trait_instance::BlockNumber>
impl<$trait_instance: $trait_name$(<I>, $instance: $instantiable)? + $system::Trait>
$crate::traits::OffchainWorker<<$trait_instance as $system::Trait>::BlockNumber>
for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
{}
};
Expand Down Expand Up @@ -1635,6 +1644,7 @@ macro_rules! decl_module {

$crate::decl_module! {
@impl_on_initialize
{ $system }
$mod_type<$trait_instance: $trait_name $(<I>, $instance: $instantiable)?>;
{ $( $other_where_bounds )* }
$( $on_initialize )*
Expand All @@ -1649,13 +1659,15 @@ macro_rules! decl_module {

$crate::decl_module! {
@impl_on_finalize
{ $system }
$mod_type<$trait_instance: $trait_name $(<I>, $instance: $instantiable)?>;
{ $( $other_where_bounds )* }
$( $on_finalize )*
}

$crate::decl_module! {
@impl_offchain
{ $system }
$mod_type<$trait_instance: $trait_name $(<I>, $instance: $instantiable)?>;
{ $( $other_where_bounds )* }
$( $offchain )*
Expand Down Expand Up @@ -2345,9 +2357,7 @@ mod tests {
IntegrityTest,
};

pub trait Trait: system::Trait + Sized where Self::AccountId: From<u32> {
type BlockNumber: Into<u32>;
}
pub trait Trait: system::Trait + Sized where Self::AccountId: From<u32> { }

pub mod system {
use codec::{Encode, Decode};
Expand All @@ -2357,6 +2367,7 @@ mod tests {
type Call;
type BaseCallFilter;
type Origin: crate::traits::OriginTrait<Call = Self::Call>;
type BlockNumber: Into<u32>;
}

#[derive(Clone, PartialEq, Eq, Debug, Encode, Decode)]
Expand Down Expand Up @@ -2480,10 +2491,7 @@ mod tests {
];

pub struct TraitImpl {}

impl Trait for TraitImpl {
type BlockNumber = u32;
}
impl Trait for TraitImpl { }

type Test = Module<TraitImpl>;

Expand All @@ -2502,6 +2510,7 @@ mod tests {
type AccountId = u32;
type Call = OuterCall;
type BaseCallFilter = ();
type BlockNumber = u32;
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion frame/system/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ decl_error! {
}

decl_module! {
pub struct Module<T: Trait> for enum Call where origin: T::Origin {
pub struct Module<T: Trait> for enum Call where origin: T::Origin, system=self {
type Error = Error<T>;

/// The maximum number of blocks to allow in mortal eras.
Expand Down