Restructure frame_support macro related exports#14745
Restructure frame_support macro related exports#14745paritytech-processbot[bot] merged 33 commits intomasterfrom
frame_support macro related exports#14745Conversation
|
bot fmt |
|
@juangirini https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3379775 was started for your command Comment |
|
@juangirini Command |
juangirini
changed the title
|
bot fmt |
|
@juangirini https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3401090 was started for your command Comment |
|
@juangirini Command |
@gilescope it should not break downstream - unless downstream used internal re-exports, which is a crime anyway… |
liamaharon
left a comment
liamaharon
left a comment
There was a problem hiding this comment.
Looks fine. Although I am a bit surprised to see this __private syntax. Doesn't feel very Rust-like. Why do we need to use it so much in macros?
| let __benchmarked_call_encoded = $crate::frame_support::__private::codec::Encode::encode( | ||
| &__call | ||
| ); | ||
| }: { | ||
| let __call_decoded = < | ||
| Call<T $(, $instance )?> | ||
| as $crate::frame_support::codec::Decode | ||
| as $crate::frame_support::__private::codec::Decode |
There was a problem hiding this comment.
this feels..wrong.
curious why we need to do it this way, and can't just pull Encode and Decode from their codec crate.
There was a problem hiding this comment.
To make it correct, you would need to re-export codec from frame-benchmarking.
There was a problem hiding this comment.
yes, benchmarking re-exports will follow in the parent issue https://github.com/paritytech/substrate/issues/14213
There was a problem hiding this comment.
Because its a macro that emits code. The emitted code cannot make the assumption that codec is in the Cargo.toml of the crate that it is being used by.
We therefore need to ship a macro will all exports that it needs.
| #scrate::__private::codec::Encode, | ||
| #scrate::__private::codec::Decode, | ||
| #scrate::__private::scale_info::TypeInfo, | ||
| #scrate::__private::RuntimeDebug, |
There was a problem hiding this comment.
Is there a better way? 😄
There was a problem hiding this comment.
I know it looks weird, but it seems like the "less bad" solution to make it obvious that those re-exports are not supposed to be used externally
There was a problem hiding this comment.
This actually isn't that bad, since macro-generated code is not part of "surface syntax". I think most people do expect code expanded this way to be sort of internal after all -- if you want to convince yourself more, try looking at the __is_call_part_defined_{num} macros that gets automatically generated and exported while we process #[pallet::call].
| pub use codec::{ | ||
| Codec, Decode, Encode, EncodeAsRef, EncodeLike, HasCompact, Input, MaxEncodedLen, Output, | ||
| }; | ||
| pub use scale_info::TypeInfo; |
There was a problem hiding this comment.
Most of these exports here are should also be private, but yeah do this later.
| let __benchmarked_call_encoded = $crate::frame_support::__private::codec::Encode::encode( | ||
| &__call | ||
| ); | ||
| }: { | ||
| let __call_decoded = < | ||
| Call<T $(, $instance )?> | ||
| as $crate::frame_support::codec::Decode | ||
| as $crate::frame_support::__private::codec::Decode |
There was a problem hiding this comment.
To make it correct, you would need to re-export codec from frame-benchmarking.
Co-authored-by: Bastian Köcher <git@kchr.de>
|
bot rebase |
…o-related-exports
|
Rebased |
|
bot rebase |
…o-related-exports
|
Rebased |
|
bot rebase |
…o-related-exports
|
Rebased |
|
bot merge |
|
Waiting for commit status. |
paritytech-processbot
bot
deleted the
jg/restructure-macro-related-exports
branch
6 participants
Partially for paritytech/polkadot-sdk#172