Extract struct DynSemantics<'db> from struct Semantics<'db, DB>

[regexident]

The Semantics<'db, DB> type is currently defined like this:

pub struct Semantics<'db, DB> {
    pub db: &'db DB,
    imp: SemanticsImpl<'db>,
}

I was surprised to find that Semantics<'db, DB> never actually accesses self.db (nor does any other code in ra_ap_hir). So I removed the pub from db to see who (if anybody) was actually using that field: Turns out the only crates that actually access sema.db are ra_ap_ide/ra_ap_ide_db/rust_analyzer.

As such it is rather unfortunate for all those useful methods on Semantics<'db, DB>, of which none actually need access to the type Db, are inaccessible in contexts where only db: &dyn HirDatabase is available (e.g. in salsa's tracked functions).

I would thus like to propose a refactoring of Semantics<'db, Db>, extracting the dyn-compatible logic into DynSemantics<'db> (with a plea to try to prefer the latter where applicable):

pub struct DynSemantics<'db> {
    imp: SemanticsImpl<'db>,
}

pub struct Semantics<'db, Db> {
    pub db: &'db DB,
    sema: DynSemantics<'db>,
}

The motivation behind this change is to make the resolution logic of Semantics<'db, DB> available in dyn contexts and to provide DynSemantics<'db> as an alternative to the tightly coupled Semantics<'db, DB>.

(the PR is motivated by an outside use of the ra_ap_hir crate that would benefit from being able to access Semantics<'db, Db> in dyn contexts)

[ChayimFriedman2]

First, that type already exists, it's SemanticsImpl.

Second, Semantics's methods are not useful above hir, nor they should be. Do you have a specific example where you'd like to use them in queries?

[regexident]

First, that type already exists, it's SemanticsImpl.

Yes and no. The Semantics<'db, Db> type adds an ergonomics layer on top of SemanticsImpl<'db> and iirc (from my previous PRs on Semantics<…>: #16703, #16705, #16707) then some APIs utilized by Semantics<'db, Db> simply aren't accessible to outside crates. So for outside users Semantics<'db, Db> often is the only option.

Second, Semantics's methods are not useful above hir, nor they should be. Do you have a specific example where you'd like to use them in queries?

Outside projects that are using rust-analyzer as a library.
My cargo-modules project would be one such example. It relies heavily on ra_ap_hir.

[ChayimFriedman2]

Yes and no. The Semantics<'db, Db> type adds an ergonomics layer on top of SemanticsImpl<'db> and iirc (from my previous PRs on Semantics<…>: #16703, #16705, #16707) then some APIs utilized by Semantics<'db, Db> simply aren't accessible to outside crates. So for outside users Semantics<'db, Db> often is the only option.

That means that if we want to have a no-db wrapper, we can transfer them to SemanticsImpl (and probably rename SemanticsImpl), and there is no need to a second wrapper.

Outside projects that are using rust-analyzer as a library. My cargo-modules project would be one such example. It relies heavily on ra_ap_hir.

And why can't they just use Semantics?

[regexident]

That means that if we want to have a no-db wrapper, we can transfer them to SemanticsImpl (and probably rename SemanticsImpl), and there is no need to a second wrapper.

That would work for me as well. Looks like the only one using SemanticsImpl is Semantics<…>, after all.

And why can't they just use Semantics?

In some contexts you only have access to the database via &dyn, such as in functions marked as #[salsa::tracked], to name one reason, other than lose coupling.

[ChayimFriedman2]

In some contexts you only have access to the database via &dyn, such as in functions marked as #[salsa::tracked], to name one reason, other than lose coupling.

That has a much simpler solution: add a ?Sized bound to Semantics's DB type parameter.

[regexident]

That has a much simpler solution: add a ?Sized bound to Semantics's DB type parameter.

Whatever works, works for me. 👍🏻
Happy to make the desired changes.

@Veykril any preference from your side?