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

Ingredient debug name are uninformative for tracked trait impl #587

Open
arialpew opened this issue Oct 3, 2024 · 1 comment
Open

Ingredient debug name are uninformative for tracked trait impl #587

arialpew opened this issue Oct 3, 2024 · 1 comment

Comments

@arialpew
Copy link

arialpew commented Oct 3, 2024

A tracked trait impl give inner_fn_name_ as ingredient debug name. We should pick the name of the original trait impl, not the InnerTrait trait impl generated by salsa::tracked macro.

#[salsa::db]
struct Database;

#[salsa::db]
impl salsa::Database for Database {
    fn salsa_event(&self, event: &dyn Fn() -> salsa::Event) {
        let event = event();

        if let salsa::EventKind::WillExecute { database_key } = event.kind {
            dbg!(self.ingredient_debug_name(database_key.ingredient_index()));
        }
    }
}

/// Given a tracked trait impl (which is implemented for some tracked input or tracked struct).
#[salsa::tracked]
impl MyTrait for MyStruct {
    #[salsa::tracked]
    fn parse(self, db: &dyn salsa::Database) -> Something<'_> {
      // impl
    }
}

/// EXPANDED
///
/// The macro generate this InnerTrait code, which take over `parse`,
/// when resolving the ingredient_debug_name.
/// All tracked trait impl result in this uninformative `inner_fn_name_`.
/// `<MyStruct as MyTrait>::parse` would be the best possible debug name output,
/// or `parse` or `MyTrait::parse`, but that would make it harder to distinguish multiple trait implementors.
trait InnerTrait_ {
    fn inner_fn_name_(self, db: &dyn salsa::Database) -> Something<'_>;
}

impl InnerTrait_ for SourceFile {
    fn inner_fn_name_(self, db: &dyn salsa::Database) -> Something<'_> {
      // impl
    }
}
@puuuuh
Copy link
Contributor

puuuuh commented Oct 7, 2024

I can implement this, but its a little bit hacky due to absent of ToString on syn::Type. Only way i could see is to take impl.ty.span().source_text() and normalize it somehow and the same for impl.self_ty. Idk if its ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants