internal: Migrate inference to next solver #20765
Conversation
rustbot
added
the
S-waiting-on-review
| // structurally normalize. We use `predicate_may_hold_opaque_types_jank` | ||
| // to support not-yet-defined opaque types. It will succeed for `impl Deref` | ||
| // but fail for `impl OtherTrait`. | ||
| if !self.table.infer_ctxt.predicate_may_hold_opaque_types_jank(&obligation) { |
There was a problem hiding this comment.
A bit of unrelated change, put it because I was clarified (#t-types > Do I need to prove T: Trait for <T as Trait>::Assoc = Foo?) that we do need to prove the obligation T: Deref first, before normalizing.
| ParamKind::Type => table.new_type_var().cast(Interner), | ||
| ParamKind::Const(ty) => table.new_const_var(ty.clone()).cast(Interner), | ||
| ParamKind::Lifetime => table.new_lifetime_var().cast(Interner), | ||
| self.fill(|x| { |
There was a problem hiding this comment.
There is a bunch of code that I migrated from TyBuilder to using methods on GenericArgs and the like; I regret this a bit, as TyBuilder is more principled and also checks against misuse, but it's too late now. We will want to consider what to do with the rest of the code that uses it though.
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq, Hash)] | ||
| pub struct GenericDefaults<'db>( | ||
| Option<Arc<[Option<EarlyBinder<'db, crate::next_solver::GenericArg<'db>>>]>>, |
There was a problem hiding this comment.
This query I changed to be a bit better: instead of putting errors there if there is no default, I made it Option, and instead of post-processing the types to remove references to params that come after, I introduced a flag to TyLoweringContext to disallow referencing them, which should also allow us to report diagnostic on that eventually.
| never: crate::next_solver::Ty<'db>, | ||
| i32: crate::next_solver::Ty<'db>, | ||
| f64: crate::next_solver::Ty<'db>, | ||
| struct InternedStandardTypes<'db> { |
There was a problem hiding this comment.
I used these in more places, to avoid re-interning of common types, and removed it from InferenceResult (moved to InferenceContext), except for the error type which can't be moved easily.
| /// in the two types. Used to filter out mismatch diagnostics that only differ in | ||
| /// `{unknown}` and unresolved projections. These mismatches are usually not helpful. | ||
| /// As the cause is usually an underlying name resolution problem | ||
| struct UnknownMismatch<'db>(&'db dyn HirDatabase); |
There was a problem hiding this comment.
The reason I removed this is because the next solver, unlike Chalk, does not have a trait for zipping (it has Relate but it has some things that prevent it from being used for that easily, such as variance). I didn't want to introduce our own trait just for that, and also considering our nameres got considerably better over the years perhaps this is no longer needed.
ChayimFriedman2
force-pushed
the
infer-ns-types
branch
from
1ccf6c6 to
95f74f5
Compare
| } | ||
|
|
||
| #[test] | ||
| fn infer_bad_lang_item() { |
There was a problem hiding this comment.
why the removed test? seems like we still should at least not crash on this
There was a problem hiding this comment.
It is a bit difficult (although not impossible) to support this due to not using TyBuilder, and this doesn't seem particularly important use case to support.
ChayimFriedman2
force-pushed
the
infer-ns-types
branch
from
95f74f5 to
d1288f6
Compare
| ty: replace_placeholder_with_binder(ctx, ty), | ||
| }; | ||
|
|
||
| fn replace_placeholder_with_binder(ctx: &mut InferenceContext<'_>, ty: Ty) -> Binders<Ty> { |
There was a problem hiding this comment.
This thing isn't needed anymore for EarlyBinder.
| sn return return expr [] | ||
| sn unsafe unsafe {} [] | ||
| sn while while expr {} [] | ||
| me not() fn(self) -> <Self as Not>::Output [requires_import] |
There was a problem hiding this comment.
This test regressing (no longer type_could_unify) is unfortunate, but unavoidable.
Previously it didn't work because Self::Output was normalized to bool and unified. Rather, it was kept unnormalized, and could_unify() just assumed it was bool.
I'm considering changing the new could_unify() (not could_unify_deeply()) to match (not sure), but we really shouldn't be leaking the TyKind::Param here. TyKind::Param has no meaning outside its home context. Even the fact that we know to render it as Self is because it carries a TypeParamId, which I want to get rid of eventually.
The real fix is to instantiate the method args in completion, but I don't want to do this in this PR (since it's already huge).
|
Merging then 🎉 |
rustbot
removed
the
S-waiting-on-review
The rule of thumb is: everything inside inference (plus method resolution) strives to be idiomatic, everything outside inference sprinkles
to_chalk/to_nextsolveras necessary and without much thinking to minimize the size of this already-gigantic PR.I still need to review thisedit: reviewed but this is ready for others to review.