Make Semantics<'db, DB> support Semantics<'db, dyn HirDatabase>, take two

[regexident]

(This is a follow-up for #19850)

@ChayimFriedman2 it looks like I was a bit too quick trusting the suggested fix of just replacing DB with DB: ?Sized: it doesn't work, the Semantics<'_, dyn HirDatabase> isn't actually constructible.

I only did a quick type-level check like this on the previous PR, following your change request, which compiled just fine:

fn dummy(db: &'_ dyn HirDatabase) -> Semantics<'_, dyn HirDatabase> {
    todo!()
}

But replacing todo!() with Semantics::new(db) would have quickly shown that a couple of impls would require DB: ?Sized as well (iirc I had those initially, but clippy complained about them being unnecessary, so I removed them).

That said: even with those fixed, the "just change DB to DB: ?Sized doesn't work, due to this:

error[E0277]: the size for values of type `DB` cannot be known at compilation time
   --> crates/hir/src/semantics.rs:182:40
    |
180 | impl<DB: HirDatabase + ?Sized> Semantics<'_, DB> {
    |      -- this type parameter needs to be `Sized`
181 |     pub fn new(db: &DB) -> Semantics<'_, DB> {
182 |         let impl_ = SemanticsImpl::new(db as &dyn HirDatabase);
    |                                        ^^ doesn't have a size known at compile-time
    |
    = note: required for the cast from `&DB` to `&(dyn HirDatabase + 'static)`
help: consider removing the `?Sized` bound to make the type parameter `Sized`
    |
180 - impl<DB: HirDatabase + ?Sized> Semantics<'_, DB> {
180 + impl<DB: HirDatabase> Semantics<'_, DB> {
    |

So I'd like to re-propose either:

• a) introducing a DynSemantics<'db> in addition to the existing sema types (as I initially proposed), or
• b) replacing SemanticsImpl<'db> with DynSemantics<'db> (or rather refactoring it into that shape).

I've pushed the missing DB: ?Sized changes as a commit to this PR to illustrate the issue.
I'll replace it with whatever approach gets picked.

[ChayimFriedman2]

I think I prefer to have a new constructor fn new_dyn_db(db: &dyn HirDatabase).

[ChayimFriedman2]

But CC @rust-lang/rust-analyzer what is your opinion?

[regexident]

I think I prefer to have a new constructor fn new_dyn_db(db: &dyn HirDatabase).

Whatever enabled type-erased Semantics<'db, _> works for me. So if a new constructor does the trick, great! I wonder though, how would such a constructor look/be implemented?

[ChayimFriedman2]

Just taking &dyn HirDatabase should work.

[regexident]

Just taking &dyn HirDatabase should work.

Oh, you're right! I've made the corresponding changes.

[regexident]

@ChayimFriedman2 any chance to get this into the upcoming release?

[ChayimFriedman2]

I'm waiting for opinions from other team members. CC @rust-lang/rust-analyzer again.

[davidbarsky]

I think seems reasonable to me!