Emit self ty for query debug name of assoc function queries #927
Conversation
✅ Deploy Preview for salsa-rs canceled.
|
| @@ -80,6 +80,6 @@ fn debug_name() { | |||
| assert_eq!(output.field(&db), 88); | |||
| db.assert_logs(expect
There was a problem hiding this comment.
You mean https://github.com/dtolnay/prettyplease
I don't think it's worth the dependency, given that this is for debugging only.
Veykril
force-pushed
the
veykril/push-mkounqurwslo
branch
from
b6e7444 to
0e616ee
Compare
CodSpeed Performance ReportMerging #927 will not alter performanceComparing Summary
|
| @@ -433,3 +455,82 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> { | |||
| Ok(options) | |||
| } | |||
| } | |||
| impl<A: AllowedOptions> quote::ToTokens for Options<A> { | |||
| fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { | |||
There was a problem hiding this comment.
I haven't read the diff to closely but what's this used for (should it replace some existing code?)
There was a problem hiding this comment.
This is used to inject arguments into the #[tracked] invocation supplied by the user, see https://github.com/salsa-rs/salsa/pull/927/files#diff-7091e579afd9cfe8ee883486f5951094096ebf6419e4e5fc5d1d9be926a858fdR86-R99
There was a problem hiding this comment.
Okay, this goes beyond what I understand about macros 😅
| syn::Meta::Path(..) => Default::default(), | ||
| _ => salsa_tracked_attr.parse_args()?, | ||
| }; | ||
| if args.self_ty.is_none() { | ||
| // If the user did not specify a self_ty, we use the impl's self_ty | ||
| args.self_ty = Some(self_ty.clone()); |
There was a problem hiding this comment.
What's the reason or use case where a user needs to provide a self type? Can you add a test for it (or remove the attribute)
There was a problem hiding this comment.
The user isn't really supposed to use this, this is solely needed for tracked impls to pass through their self type to the assoc tracked fn so that it can adjust the debug name appropriately
| @@ -80,6 +80,6 @@ fn debug_name() { | |||
| assert_eq!(output.field(&db), 88); | |||
| db.assert_logs(expect
|
Will cut a new release after this merges so we can update r-a (and see if things break for us given the generation changes) |
|
I believe this broke #925 |
No description provided.