Skip to content
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

Support tracked methods without self #585

Open
nikomatsakis opened this issue Sep 29, 2024 · 1 comment
Open

Support tracked methods without self #585

nikomatsakis opened this issue Sep 29, 2024 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@nikomatsakis
Copy link
Member

This is an error today:

#[salsa::tracked]
struct SymTy<'db> {
    ...
}

#[salsa::tracked]
impl<'db> SymTy<'db> {
    #[salsa::tracked]
    pub fn unit(db: &'db dyn Db) -> Self {}
}

...but why? I can still desugar this relatively easily to a regular function calling __unit (see below), so why can't the macro do it?

#[salsa::tracked]
fn __unit(db: &'db dyn Db) -> SymTy<'db> { ... }
@nikomatsakis nikomatsakis added bug Something isn't working good first issue Good for newcomers labels Sep 29, 2024
@nikomatsakis
Copy link
Member Author

Mentoring instructions

Tracked methods are defined in the proc macro here:

fn try_generate(&self, mut impl_item: syn::ItemImpl) -> syn::Result<TokenStream> {
let mut member_items = std::mem::take(&mut impl_item.items);
for member_item in &mut member_items {
self.modify_member(&impl_item, member_item)?;
}
impl_item.items = member_items;
Ok(crate::debug::dump_tokens(
format!("impl {:?}", impl_item.self_ty),
impl_item.into_token_stream(),
))
}

It works by modifying tracked methods in place with this method:

#[allow(non_snake_case)]
fn modify_member(
&self,
impl_item: &syn::ItemImpl,
member_item: &mut syn::ImplItem,
) -> syn::Result<()> {
let syn::ImplItem::Fn(fn_item) = member_item else {
return Ok(());
};

so that they expand to a call to the setup_method_body! macro-rules macro:

{
trait $InnerTrait<$($db_lt)?> {
fn $inner_fn_name($self, db: $($db_ty)*, $($input_id: $input_ty),*) -> $output_ty;
}
impl<$($db_lt)?> $InnerTrait<$($db_lt)?> for $self_ty {
$inner_fn
}
#[$salsa_tracked_attr]
fn $inner_fn_name<$($db_lt)?>(db: $($db_ty)*, this: $self_ty, $($input_id: $input_ty),*) -> $output_ty {
<$self_ty as $InnerTrait>::$inner_fn_name(this, db, $($input_id),*)
}
$inner_fn_name($db, $self, $($input_id),*)
}

We need to modify these to be a bit more general.

@nikomatsakis nikomatsakis added this to the Salsa 3.0 milestone Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant