-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Macros to use path instead of ident #1474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7102f28
933fd3f
dc43fa5
16668ca
ebd13e3
270a6b7
8be65aa
850a77e
5ae6e07
04dc503
95444ae
6a773d0
bc17557
6d9887b
35747a1
330af4a
10bf65c
97bfec4
6a07bb3
f303276
963be94
76de90d
32f2f1e
877fdc3
c89c0c7
8184085
1ee920b
5050625
55daf79
43ca2c2
2a2b45f
b55615b
d253b66
d2009d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |
| //! Home of the parsing and expansion code for the new pallet benchmarking syntax | ||
|
|
||
| use derive_syn_parse::Parse; | ||
| use frame_support_procedural_tools::generate_crate_access_2018; | ||
| use frame_support_procedural_tools::generate_access_from_frame_or_crate; | ||
| use proc_macro::TokenStream; | ||
| use proc_macro2::{Ident, Span, TokenStream as TokenStream2}; | ||
| use quote::{quote, ToTokens}; | ||
|
|
@@ -418,7 +418,8 @@ pub fn benchmarks( | |
| true => quote!(T: Config<I>, I: 'static), | ||
| }; | ||
|
|
||
| let krate = generate_crate_access_2018("frame-benchmarking")?; | ||
| let krate = generate_access_from_frame_or_crate("frame-benchmarking")?; | ||
| let frame_system = generate_access_from_frame_or_crate("frame-system")?; | ||
|
|
||
| // benchmark name variables | ||
| let benchmark_names_str: Vec<String> = benchmark_names.iter().map(|n| n.to_string()).collect(); | ||
|
|
@@ -488,7 +489,7 @@ pub fn benchmarks( | |
| } | ||
| #[cfg(any(feature = "runtime-benchmarks", test))] | ||
| impl<#type_impl_generics> #krate::Benchmarking for Pallet<#type_use_generics> | ||
| where T: frame_system::Config, #where_clause | ||
| where T: #frame_system::Config, #where_clause | ||
| { | ||
| fn benchmarks( | ||
| extra: bool, | ||
|
|
@@ -535,7 +536,7 @@ pub fn benchmarks( | |
| _ => return Err("Could not find extrinsic.".into()), | ||
| }; | ||
| let mut whitelist = whitelist.to_vec(); | ||
| let whitelisted_caller_key = <frame_system::Account< | ||
| let whitelisted_caller_key = <#frame_system::Account< | ||
| T, | ||
| > as #krate::__private::storage::StorageMap<_, _,>>::hashed_key_for( | ||
| #krate::whitelisted_caller::<T::AccountId>() | ||
|
|
@@ -571,8 +572,8 @@ pub fn benchmarks( | |
| >::instance(&selected_benchmark, c, verify)?; | ||
|
|
||
| // Set the block number to at least 1 so events are deposited. | ||
| if #krate::__private::Zero::is_zero(&frame_system::Pallet::<T>::block_number()) { | ||
| frame_system::Pallet::<T>::set_block_number(1u32.into()); | ||
| if #krate::__private::Zero::is_zero(&#frame_system::Pallet::<T>::block_number()) { | ||
| #frame_system::Pallet::<T>::set_block_number(1u32.into()); | ||
| } | ||
|
|
||
| // Commit the externalities to the database, flushing the DB cache. | ||
|
|
@@ -654,7 +655,7 @@ pub fn benchmarks( | |
| } | ||
|
|
||
| #[cfg(test)] | ||
| impl<#type_impl_generics> Pallet<#type_use_generics> where T: ::frame_system::Config, #where_clause { | ||
| impl<#type_impl_generics> Pallet<#type_use_generics> where T: #frame_system::Config, #where_clause { | ||
| /// Test a particular benchmark by name. | ||
| /// | ||
| /// This isn't called `test_benchmark_by_name` just in case some end-user eventually | ||
|
|
@@ -719,10 +720,14 @@ fn expand_benchmark( | |
| where_clause: TokenStream2, | ||
| ) -> TokenStream2 { | ||
| // set up variables needed during quoting | ||
| let krate = match generate_crate_access_2018("frame-benchmarking") { | ||
| let krate = match generate_access_from_frame_or_crate("frame-benchmarking") { | ||
| Ok(ident) => ident, | ||
| Err(err) => return err.to_compile_error().into(), | ||
| }; | ||
| let frame_system = match generate_access_from_frame_or_crate("frame-system") { | ||
| Ok(path) => path, | ||
| Err(err) => return err.to_compile_error().into(), | ||
| }; | ||
| let codec = quote!(#krate::__private::codec); | ||
| let traits = quote!(#krate::__private::traits); | ||
| let setup_stmts = benchmark_def.setup_stmts; | ||
|
|
@@ -762,7 +767,7 @@ fn expand_benchmark( | |
| Expr::Cast(t) => { | ||
| let ty = t.ty.clone(); | ||
| quote! { | ||
| <<T as frame_system::Config>::RuntimeOrigin as From<#ty>>::from(#origin); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all of this seem to have been hardcoded 🙈 well, not like anything broke because of it, but indeed not a good practice.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unless if was intentional? but I can't see a reason.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see a reason either, and we need this unhardcoded moving forward to work with the frame umbrella crate |
||
| <<T as #frame_system::Config>::RuntimeOrigin as From<#ty>>::from(#origin); | ||
| } | ||
| }, | ||
| _ => quote! { | ||
|
|
@@ -932,7 +937,7 @@ fn expand_benchmark( | |
| } | ||
|
|
||
| #[cfg(test)] | ||
| impl<#type_impl_generics> Pallet<#type_use_generics> where T: ::frame_system::Config, #where_clause { | ||
| impl<#type_impl_generics> Pallet<#type_use_generics> where T: #frame_system::Config, #where_clause { | ||
| #[allow(unused)] | ||
| fn #test_ident() -> Result<(), #krate::BenchmarkError> { | ||
| let selected_benchmark = SelectedBenchmark::#name; | ||
|
|
@@ -951,8 +956,8 @@ fn expand_benchmark( | |
| >::instance(&selected_benchmark, &c, true)?; | ||
|
|
||
| // Set the block number to at least 1 so events are deposited. | ||
| if #krate::__private::Zero::is_zero(&frame_system::Pallet::<T>::block_number()) { | ||
| frame_system::Pallet::<T>::set_block_number(1u32.into()); | ||
| if #krate::__private::Zero::is_zero(&#frame_system::Pallet::<T>::block_number()) { | ||
| #frame_system::Pallet::<T>::set_block_number(1u32.into()); | ||
| } | ||
|
|
||
| // Run execution + verification | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -214,7 +214,7 @@ mod parse; | |
| use crate::pallet::parse::helper::two128_str; | ||
| use cfg_expr::Predicate; | ||
| use frame_support_procedural_tools::{ | ||
| generate_crate_access, generate_crate_access_2018, generate_hidden_includes, | ||
| generate_access_from_frame_or_crate, generate_crate_access, generate_hidden_includes, | ||
| }; | ||
| use itertools::Itertools; | ||
| use parse::{ExplicitRuntimeDeclaration, ImplicitRuntimeDeclaration, Pallet, RuntimeDeclaration}; | ||
|
|
@@ -272,7 +272,7 @@ fn construct_runtime_implicit_to_explicit( | |
| input: TokenStream2, | ||
| definition: ImplicitRuntimeDeclaration, | ||
| ) -> Result<TokenStream2> { | ||
| let frame_support = generate_crate_access_2018("frame-support")?; | ||
| let frame_support = generate_access_from_frame_or_crate("frame-support")?; | ||
| let mut expansion = quote::quote!( | ||
| #frame_support::construct_runtime! { #input } | ||
| ); | ||
|
|
@@ -283,7 +283,7 @@ fn construct_runtime_implicit_to_explicit( | |
| expansion = quote::quote!( | ||
| #frame_support::__private::tt_call! { | ||
| macro = [{ #pallet_path::tt_default_parts }] | ||
| frame_support = [{ #frame_support }] | ||
| your_tt_return = [{ #frame_support::__private::tt_return }] | ||
| ~~> #frame_support::match_and_insert! { | ||
| target = [{ #expansion }] | ||
| pattern = [{ #pallet_name: #pallet_path #pallet_instance }] | ||
|
|
@@ -308,7 +308,7 @@ fn construct_runtime_explicit_to_explicit_expanded( | |
| input: TokenStream2, | ||
| definition: ExplicitRuntimeDeclaration, | ||
| ) -> Result<TokenStream2> { | ||
| let frame_support = generate_crate_access_2018("frame-support")?; | ||
| let frame_support = generate_access_from_frame_or_crate("frame-support")?; | ||
| let mut expansion = quote::quote!( | ||
| #frame_support::construct_runtime! { #input } | ||
| ); | ||
|
|
@@ -319,7 +319,7 @@ fn construct_runtime_explicit_to_explicit_expanded( | |
| expansion = quote::quote!( | ||
| #frame_support::__private::tt_call! { | ||
| macro = [{ #pallet_path::tt_extra_parts }] | ||
| frame_support = [{ #frame_support }] | ||
| your_tt_return = [{ #frame_support::__private::tt_return }] | ||
| ~~> #frame_support::match_and_insert! { | ||
| target = [{ #expansion }] | ||
| pattern = [{ #pallet_name: #pallet_path #pallet_instance }] | ||
|
|
@@ -372,7 +372,7 @@ fn construct_runtime_final_expansion( | |
| let scrate = generate_crate_access(hidden_crate_name, "frame-support"); | ||
| let scrate_decl = generate_hidden_includes(hidden_crate_name, "frame-support"); | ||
|
|
||
| let frame_system = generate_crate_access_2018("frame-system")?; | ||
| let frame_system = generate_access_from_frame_or_crate("frame-system")?; | ||
| let block = quote!(<#name as #frame_system::Config>::Block); | ||
| let unchecked_extrinsic = quote!(<#block as #scrate::sp_runtime::traits::Block>::Extrinsic); | ||
|
|
||
|
|
@@ -799,7 +799,7 @@ fn decl_static_assertions( | |
| quote! { | ||
| #scrate::__private::tt_call! { | ||
| macro = [{ #path::tt_error_token }] | ||
| frame_support = [{ #scrate }] | ||
| your_tt_return = [{ #scrate::__private::tt_return }] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recall this was one of the BIG BIG fixes in this PR when I was working on it. Try this for yourself: if you are familiar with the tt_call pattern a bit, instead of passing in the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But, hearing what I just said, in the scope of this PR, it is pretty unclear why we are doing this. I recall my working example was
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, that's what looks at a glance... did you find out what's the reason?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@kianenigma Probably the best example is by using the frame-crate itself, which makes me reconsider if bringing this change here was a good idea and instead bring it in the 'main' frame umbrella PR. WDYT?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I highly suspect that it didn't work before because the "receiver" didn't expect to see any "arguments" named
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If my memory is good, the former didn't work because there was another error when giving a path instead of a single ident: frame_support = [{ $($frame_support:ident)::* }]
} => {
$($frame_support::)*__private::tt_return! {but it should have been like this for path to work frame_support = [{ $($frame_support:ident)::* }]
} => {
// see the semicolon is after the parentheses here
$($frame_support)::*__private::tt_return! {but when you implemented with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the specific workaround for passing paths to these btw is to do an interpolation of idents segmented by see: https://github.com/sam0x17/macro_magic/blob/main/core/src/lib.rs#L511 |
||
| ~~> #scrate::assert_error_encoded_size! { | ||
| path = [{ #path }] | ||
| runtime = [{ #runtime }] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.