# Salsa 1.0 versus 3.0

## Background/Takeaways

I tried to port rust-analyzer's `base-db` yesterday to new Salsa. I initially expected a 1:1 mapping, but I was (very!) wrong to: some concepts in the old Salsa do not map cleanly to the new Salsa, which may lead to an overall decrease in LoC in rust-analyzer. Here are some potentially incorrect takeaways:
- Old Salsa has everything in a database/trait: there are no free functions or structures. Old Salsa requires placing everything into a trait that masquarades as a query group
- New Salsa is a more fine-grained: it has databases (often traits), free tracked functions, and standalone, tracked or interned structs. It is _not_ necessary to group all queries into a single trait/[query group](https://github.com/salsa-rs/salsa-rfcs/blob/master/RFC0001-Query-Group-Traits.md) in the new Salsa.

## Comparison Between Old and New Salsa

## Basic, Hello World Comparison

Given the following in old Salsa:

struct InputId(u32);

#[salsa::query_group(OgSalsaDatabaseStorage)]
trait OgSalsaDatabase {
    #[salsa::input]
    fn field(&self, input_id: InputId) -> u32;

    fn foo(&self, input: InputId) -> u32 {
        self.field(input)
    }
}

...this would be rewritten as the following in new Salsa:

#[salsa::input]
struct Input {
    field: u32
}

#[salsa::tracked]
fn foo(db: &dyn crate::Db, input: Input) -> u32 {
    input.field(db)
}

### Concrete `DefDatabase` Example

Let's consider a snippet of `DefDatabase` from `crates/hir-def/src/db.rs`:

#[salsa::query_group(DefDatabaseStorage)]
pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast<dyn ExpandDatabase> {
    fn crate_supports_no_std(&self, crate_id: CrateId) -> bool;
    
    #[salsa::input]
    fn expand_proc_attr_macros(&self) -> bool;

    #[salsa::invoke(DefMap::crate_def_map_query)]
    fn crate_def_map(&self, krate: CrateId) -> Arc<DefMap>;
}

fn crate_supports_no_std(db: &dyn DefDatabase, crate_id: CrateId) -> bool {
    let file = db.crate_graph()[crate_id].root_file_id();
    // todo...
}

### `#[salsa::query_group]`

Like the name suggests, this annotation defines a query group, which contains *everything* that a block of queries need to execute (storage, namely), as well as the queries themselves. In the `DefDatabase` example, the queries are `expand_proc_attr_macros` and `crate_def_map`. As best as I can tell, this is necessary boilerplate in old Salsa and there is no equivalent in new Salsa.

This is how most databases and queries are defined (the above snippet is `DefDatabase::crate_supports_no_std`, but another [example is from`ide-db`](https://github.com/rust-lang/rust-analyzer/blob/3723e5910c14f0ffbd13de474b8a8fcc74db04ce/crates/ide-db/src/symbol_index.rs#L102-L149)):

#[salsa::query_group(SymbolsDatabaseStorage)]
pub trait SymbolsDatabase: HirDatabase + SourceRootDatabase + Upcast<dyn HirDatabase> {
    /// The symbol index for a given module. These modules should only be in source roots that
    /// are inside local_roots.
    fn module_symbols(&self, module: Module) -> Arc<SymbolIndex>;
}

fn module_symbols(db: &dyn SymbolsDatabase, module: Module) -> Arc<SymbolIndex> {
    let _p = tracing::info_span!("module_symbols").entered();

    let symbols = SymbolCollector::collect_module(db.upcast(), module);
    Arc::new(SymbolIndex::new(symbols))
}

There isn't an 1:1 equivalent in new Salsa, but I think the closest are [_tracked_ functions](https://salsa-rs.netlify.app/tutorial/parser#the-parse_statements-function):

#[salsa::input]
struct Module {
    field: u32,
}

#[salsa::tracked]
fn module_symbols(db: &dyn SymbolsDatabase, module: Module) -> Arc<SymbolIndex> {
    let _p = tracing::info_span!("module_symbols").entered();

    let symbols = SymbolCollector::collect_module(db.upcast(), module);
    Arc::new(SymbolIndex::new(symbols))
}

The primary syntactic difference is that a trait is not required to define or group queries: the plumbing happens automagically, upon first use.

### `#[salsa::input]`
Unlike derived queries, whose value is given by a function, input queries are explicitly set by doing `db.query(QueryType).set(key, value)`.
- For `expand_proc_attr_macros`, an example setter would be `self.db.set_expand_proc_attr_macros(true)`
- rust-analyzer tends to use the generated durabilty variant (`self.db.set_expand_proc_attr_macros_with_durability(true, Durability::HIGH)`) more often.

In the new Salsa, the equivalent API is [a builder API](https://github.com/salsa-rs/salsa/blob/d5018d5614a58a473825d65faacac1fa329faf4c/tests/tracked_fn_high_durability_dependency.rs#L21-L25):

let mut db = salsa::DatabaseImpl::default();

let input_high = MyInput::new(&mut db, 0);
input_high
    .set_field(&mut db)
    .with_durability(Durability::HIGH)
    .to(2200);

### `#[salsa::invoke]`

For a non-input query, this indicates the function to call when a query must be recomputed. For `DefDatabase`, `DefDatabase::crate_def_map` will be invoking `DefMap::crate_def_map_query` instead. There isn't an equivalent in Salsa 3.0 due to the absence of `#[salsa::query_group]` and prominence of `#[salsa::tracked]`.

## Questions

- The version of Salsa that rust-analyzer uses requires placing everything into a trait/query group/database. What is the expected pattern for new Salsa?
- Naively, I'd expect that there'd be substantially more free functions annotated with `#[salsa::tracked]`. Is that correct?
- When should be be creating a new database? My impression from reading [this commnet](https://github.com/salsa-rs/salsa/pull/538#discussion_r1694250775) is that creating a database is useful for two things:
- Passing around state that _isn't_ tracked by Salsa (logs, maybe file IO, etc.)
- Faster compile times/better encapsulation, by keeping each database local to the crate that uses it.
- Does the Salsa entity changes ([PR](https://github.com/salsa-rs/salsa/pull/304), [document](https://hackmd.io/8pNav1yXQHiUmS6V2Bmstg)) (I assume it is the current precursor to the new Salsa) address [matklad's complaints](https://hackmd.io/o07mw34ITEiXgRaouTb4Yw#Matklads-complaints), especially around "[u]ser is reponsible for managing storage"?
- He says: "managing storage takes a fair amount of code, it would be cool to have ECS-style uniform storage, when you don't have to manually add fields to parent entities". It seems like new Salsa, with tracked/interned structs, accomplishes this. Is that correct?
- Old Salsa generates setters for values. For instance, `SourceDatabase::file_text` will have an automatically generated setter called `SourceDatabase::set_file_text_with_durability`. While it might be nice to keep a semantic equivalent, I think using an input/tracked struct's builder is preferrable. See this example: https://github.com/salsa-rs/salsa/blob/af2ec49d8060c16dab28e25111950eb7b09ff590/tests/input_field_durability.rs. This has a few interesting implications:
- I think we need one big trait of databases in order to make this feel more "right". I think `db: &dyn salsa::Db` doesn't cut the mustard.
- This means we can't have queries return plain, unadorned `Arc<[u8]>` or `Parse<ast::SourceFile>`: they need to wrapped with Salsa structs.
- The entire VFS module/functionality seems superfluous under new Salsa, but I think to make migration easier, we should bridge to it.