salsa-rs / salsa

A generic framework for on-demand, incrementalized computation. Inspired by adapton, glimmer, and rustc's query system.
https://salsa-rs.netlify.app/
Apache License 2.0
2.12k stars 150 forks source link

no_eq #521

Closed nikomatsakis closed 2 months ago

nikomatsakis commented 2 months ago

The no_eq flag is ignored in tracked functions after #518. @MichaReiser commented on Zulip.


There might also be an issue around interned or I'm using it incorrectly. But

#[salsa::tracked]
pub(crate) fn resolve_module_query<'db>(
    db: &'db dyn Db,
    module_name: ModuleNameIngredient<'db>,
) -> Option<Module> {
    let _span = tracing::trace_span!("resolve_module", ?module_name).entered();

    let name = module_name.name(db);

    let (search_path, module_file, kind) = resolve_name(db, name)?;

    let module = Module::new(name.clone(), kind, search_path, module_file);

    Some(module)
}

#[salsa::interned]
pub(crate) struct ModuleNameIngredient<'db> {
    #[return_ref]
    pub(super) name: ModuleName,
}

No longer compiles and fails with

error[E0277]: the trait bound `ModuleNameIngredient<'_>: LookupId<'_>` is not satisfied
  --> crates/red_knot_module_resolver/src/resolver.rs:30:1
   |
30 | #[salsa::tracked]
   | ^^^^^^^^^^^^^^^^^ the trait `FromId` is not implemented for `ModuleNameIngredient<'_>`, which is required by `ModuleNameIngredient<'_>: LookupId<'_>`
   |
   = note: required for `ModuleNameIngredient<'_>` to implement `LookupId<'_>`
   = note: this error originates in the macro `salsa::plumbing::setup_tracked_fn` which comes from the expansion of the attribute macro `salsa::tracked` (in Nightly builds, run with -Z macro-backtrace for more info)
nikomatsakis commented 2 months ago

Mentoring tips:

We can use an approach similar to the existing is_specifiable flag. Basically pass in a boolean flag indicating whether no_eq was set or not, and then in the expanded macro use $zalsa::macro_if!. We need to change what this code does...

https://github.com/salsa-rs/salsa/blob/1c69d3ba7b00b36fb1594edbb8b5de05d27b1419/components/salsa-macro-rules/src/setup_tracked_fn.rs#L161-L166

...so that it does something like...

$zalsa::macro_if! {
    if $no_eq {
        false
    } else {
        $zalsa::should_backdate_value(old_value, new_value)
    }
}
MichaReiser commented 2 months ago

Thanks. I try to have a look in the next few days