Revamp / v0.4

[gui1117]

(This issue make use of example from substrate, for people not familiar, please ask I will use genuine example.)

1 [x] remove need for use_attr

• Remove need for use_attr and use_proc #5

in import_tokens_attr_internal, instead of generating 2 proc macros orig_sig and inner_sig, we could generate only orig_sig.

here is how: if there is a special first token then orig_sig will do as inner_sig. otherwise it does as orig_sig currently except at the end if generates the forward_tokens to call himself again but with a special first token.

Having only one function allow to get rid of use_attr because there is no longer any inner_sig function to keep along.

2 allow export_tokens to export the same ident but at different path

• New export_tokens path strategy #8

in export_tokens instead of generating just a macro_rules we can do similar to frame::pallet:

we create a unique_name with just a global counter in the macro_magic proc crate:

thread_local! {
	/// A global counter, can be used to generate a relatively unique identifier.
	static COUNTER: RefCell<Counter> = RefCell::new(Counter(0));
}
pub fn expand_tt_default_parts(def: &mut Def) -> proc_macro2::TokenStream {
	let count = COUNTER.with(|counter| counter.borrow_mut().inc());
	let unique_name = syn::Ident::new(&format!("__macro_magic_unique_id{}", count), def.item.span());

and then we generate the macro_rule with the unique_id and then make a use statement to assign with the correct id in the scope of the definition only (instead of root scope).

		#[macro_export]
		#[doc(hidden)]
		macro_rules! #unique_name {
		}
		pub use #unique_name as ident;

This is less important but it allows for instance many pallet in the same crate to register TestDefaultConfig for instance.
Currently if 2 pallet in the same crate register the same TestDefaultConfig then there is a conflict.

3 resolution of mm_override_path done dynamically

• Allow Path or Expr evaluating to Into<String> as arg for import_tokens_proc and import_tokens_attr #9

Instead of having the path to macro_magic requested as hardcoded: e.g. in #[import_tokens_attr(frame_support::macro_magic)] we should be able to give a code that would give the path to macro_magic later when the proc_macro orig_sig is called.
It would look like this:

diff --git a/core/src/lib.rs b/core/src/lib.rs
index 8ca6158..777f200 100644
--- a/core/src/lib.rs
+++ b/core/src/lib.rs
@@ -689,7 +689,7 @@ pub fn import_tokens_attr_internal<T1: Into<TokenStream2>, T2: Into<TokenStream2
     attr: T1,
     tokens: T2,
 ) -> Result<TokenStream2> {
-    let mm_override_path = match parse2::<Path>(attr.into()) {
+    let mm_override_path_expr = match parse2::<Expr>(attr.into()) {
         Ok(override_path) => override_path,
         Err(_) => macro_magic_root(),
     };
@@ -753,6 +753,7 @@ pub fn import_tokens_attr_internal<T1: Into<TokenStream2>, T2: Into<TokenStream2
                 escape_extra(path.to_token_stream().to_string().as_str()),
                 escape_extra(custom_parsed.to_token_stream().to_string().as_str())
             );
+            let mm_override_path = #mm_override_path_expr
             quote::quote! {
                 #mm_override_path::forward_tokens! {
                     #pound path,

then in substrate we can write derive_impl attribute like this:

#[import_tokens_attr(generate_crate_access_2018("frame-support").or_else(generate_crate_access_2018("frame")))]

4 add a parameter to import_tokens to dynamically fetch the path to self (e.g. DeriveImpl) thus removing the need to always import the macro in scope and allow to call frame_support::DeriveImpl directly.

not very needed but we could also give into attribute the expression to resolve the path to inner_macro_ident
so the call to forward_tokens would look like this:

diff --git a/core/src/lib.rs b/core/src/lib.rs
index 8ca6158..e3dbccb 100644
--- a/core/src/lib.rs
+++ b/core/src/lib.rs
@@ -753,10 +753,11 @@ pub fn import_tokens_attr_internal<T1: Into<TokenStream2>, T2: Into<TokenStream2
                 escape_extra(path.to_token_stream().to_string().as_str()),
                 escape_extra(custom_parsed.to_token_stream().to_string().as_str())
             );
+            let path_to_inner_macro = #inner_macro_ident_given_expr;
             quote::quote! {
                 #mm_override_path::forward_tokens! {
                     #pound path,
-                    #inner_macro_ident,
+                    #path_to_inner_macro::#inner_macro_ident,
                     #mm_override_path,
                     #pound extra
                 }

so that derive_impl can tell that it is expected to be found in frame-support and frame and so it doesn't need to be imported in scope.
Thought the drawback is that it means DeriveImpl cannot be re-exported either because it is only expected in frame_support and frame. That said, considering macro_magic::forward_token already require to give the path to be found at (e.g. frame or frame-support), the reexport is already impossible. (what I mean is that derive_impl already need to give an expression to fetch to path to forward_tokens to if there is a reexport in crate Foo caller of derive_impl will still need to depend on crate macro_magic or frame or frame_support)

[sam0x17]

This is fantastic! Love the solution of having the macro call itself in different situations. The original code for all of this was the product of a 3 week fever dream so it's great to see you got around the main problems I was unable to solve @thiolliere <3

[sam0x17]

I'll probably do a bump to 0.4.x when we release this btw

[sam0x17]

regarding 3., this is also great. It never occurred to me that we can just pass an expr around and then later evaluate it when we do the final expansion. That would be a lot more flexible 👍🏻

[gui1117]

regarding 3., this is also great. It never occurred to me that we can just pass an expr around and then later evaluate it when we do the final expansion. That would be a lot more flexible 👍🏻

for 3, I was wondering what would be the best API:

• just take any expression, and let the user code some stuff like generate_storage_2018("frame-support")
• or provide a higher level API and user can just give set of crate and path where macro_magic is expected to be found. e.g.

  {
    {crate: "frame-support", path: '''},
    {crate: "frame", path: ''dev::frame_support"}
  }
  

maybe best could be to have both so people can use what they want.

[sam0x17]

Yeah this sounds good. I was thinking of something like the following, if this still has the same flexibility you are talking about (I think it does?):

1-arg expr

#[import_tokens_attr(format!("{}::macro_magic", generate_crate_access_2018("frame-support")))]

we would expect the expr to expand to an AsRef<str> so it will take a String or a &String or a &str etc

1-arg path

#[import_tokens_attr(frame_support::macro_magic)]

This one would work essentially the way it does now

Are there any problems with that?

[gui1117]

yes it looks good.
because we need to parse either an expression or a path, maybe the error message can be less good. but looks good anyway.

[sam0x17]

Ok at this point items 1-3 have been merged and docs have been updated. I'm going to go ahead and release 0.4.0. May decide to do item 4 as well later.

Thanks again for all your help @thiolliere!

[gui1117]

thanks for the great crate, I hope I was a little helpful.

Yes 4 is not important and to be discussed maybe.