attributes: remove direct dependency on proc-macro2 #296
Conversation
- remove `proc-macro2` as a direct dependency - replace usage of `TokenStream` from `proc-macro2` with `impl ToTokens` from `quote` - remove usage of `Span::call_site` as that is used by default for the `quote!` macro
There was a problem hiding this comment.
I think we should continue propagating the span from the original instrumented function in the quoted output so that we get nice compiler diagnostics if there are errors in the instrumented function. I mentioned in a comment how we could do that.
Beyond that, this looks great, and I'd be happy to merge once you make that change. Thanks!
| @@ -187,7 +185,7 @@ pub fn instrument(args: TokenStream, item: TokenStream) -> TokenStream { | |||
| let level = level(&args); | |||
| let target = target(&args); | |||
|
|
|||
| quote_spanned!(call_site=> | |||
| quote!( | |||
There was a problem hiding this comment.
I think propagating the span of the original function is important, so that any errors are displayed correctly. Is there a way to do that using syn::spanned::Spanned, so that we don't need to import the Span type explicitly?
I believe that we can use the Spanned impl for ItemFn for this.
There was a problem hiding this comment.
I may be misunderstanding the documentation, but this seems to indicate that quote!(…) does the same thing as quote_spanned!(Span::call_site()=> …)
There was a problem hiding this comment.
Oh, I think I missed that in the docs, that's great. Nevermind my previous comment!
| @@ -204,7 +202,7 @@ pub fn instrument(args: TokenStream, item: TokenStream) -> TokenStream { | |||
| .into() | |||
| } | |||
|
|
|||
| fn level(args: &AttributeArgs) -> proc_macro2::TokenStream { | |||
| fn level(args: &AttributeArgs) -> impl ToTokens { | |||
| @@ -187,7 +185,7 @@ pub fn instrument(args: TokenStream, item: TokenStream) -> TokenStream { | |||
| let level = level(&args); | |||
| let target = target(&args); | |||
|
|
|||
| quote_spanned!(call_site=> | |||
| quote!( | |||
There was a problem hiding this comment.
Oh, I think I missed that in the docs, that's great. Nevermind my previous comment!
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Motivation
As discussed in #292:
Solution
Remove
proc-macro2as a direct dependency and replace usages of types from it accordingly.