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.14k stars 152 forks source link

Add `remove` to `MutQueryStorageOps`? #37

Open matklad opened 6 years ago

matklad commented 6 years ago

We have a set method to associate a value with a key, but we don't have a corresponding remove method. I need this because I try to model a set of files, known to rust-analyzer, as two queries:

salsa::query_group! {
    trait FilesDatabase: salsa::Database {
        fn file_text(key: FileId) -> Arc<String> {
            type FileText;
            storage input;
        }
        fn file_set(key: ()) -> Arc<Vec<File>> {
            type FileSet;
            storage input;
        }
    }
}

One query returns text, given file id. Another query returns a set of file ids. Currently, I can add and change files, but removing files is not really possible: a file can be removed from FileSet, but not from FileText.

Perhaps this all should be left to GC though?

nikomatsakis commented 6 years ago

I think remove makes sense for inputs but not derived queries. I hadn't expected the GC to prune the inputs.

nikomatsakis commented 6 years ago

So now that inputs panic if there is no value, I guess a remove would be fairly straightforward to implement. It would be "on you" though to ensure that when you remove something, no derived query will access it, right?

nikomatsakis commented 6 years ago

I imagine in that case we can literally just remove the key from the map and trigger the next revision, full stop.

matklad commented 6 years ago

I guess so!

We probably need to forbid removal of constants, and we need to review maybe_changed_since logic, though it seems correct.

samestep commented 3 years ago

Would a PR for this be welcome?