Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #14971 - lowr:fix/captured-item-ty-outer-binder, r=HKalbasi
fix: consider outer binders when folding captured items' type
Fixes #14966
Basically, the crash is caused by us producing a broken type and passing it to chalk: `&dyn for<type> [for<> Implemented(^1.0: A<^0.0>)]` (notice the innermost bound var `^0.0` has no corresponding binder). It's created in `CapturedItemWithoutTy::with_ty()`, which didn't consider outer binders when folding types to replace placeholders with bound variables.
The fix is one-liner, but I've also refactored the surrounding code a little.
| -rw-r--r-- | crates/hir-ty/src/infer.rs | 50 | ||||
| -rw-r--r-- | crates/hir-ty/src/infer/closure.rs | 62 | ||||
| -rw-r--r-- | crates/hir-ty/src/mir/eval/tests.rs | 34 | ||||
| -rw-r--r-- | crates/hir-ty/src/mir/monomorphization.rs | 24 |
4 files changed, 102 insertions, 68 deletions
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index fa81fe39aa..80f32e96ee 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -367,6 +367,10 @@ pub enum PointerCast { } /// The result of type inference: A mapping from expressions and patterns to types. +/// +/// When you add a field that stores types (including `Substitution` and the like), don't forget +/// `resolve_completely()`'ing them in `InferenceContext::resolve_all()`. Inference variables must +/// not appear in the final inference result. #[derive(Clone, PartialEq, Eq, Debug, Default)] pub struct InferenceResult { /// For each method call expr, records the function it resolves to. @@ -576,6 +580,30 @@ impl<'a> InferenceContext<'a> { // there is no problem in it being `pub(crate)`, remove this comment. pub(crate) fn resolve_all(self) -> InferenceResult { let InferenceContext { mut table, mut result, .. } = self; + // Destructure every single field so whenever new fields are added to `InferenceResult` we + // don't forget to handle them here. + let InferenceResult { + method_resolutions, + field_resolutions: _, + variant_resolutions: _, + assoc_resolutions, + diagnostics, + type_of_expr, + type_of_pat, + type_of_binding, + type_of_rpit, + type_of_for_iterator, + type_mismatches, + standard_types: _, + pat_adjustments, + binding_modes: _, + expr_adjustments, + // Types in `closure_info` have already been `resolve_completely()`'d during + // `InferenceContext::infer_closures()` (in `HirPlace::ty()` specifically), so no need + // to resolve them here. + closure_info: _, + mutated_bindings_in_closure: _, + } = &mut result; table.fallback_if_possible(); @@ -584,26 +612,26 @@ impl<'a> InferenceContext<'a> { // make sure diverging type variables are marked as such table.propagate_diverging_flag(); - for ty in result.type_of_expr.values_mut() { + for ty in type_of_expr.values_mut() { *ty = table.resolve_completely(ty.clone()); } - for ty in result.type_of_pat.values_mut() { + for ty in type_of_pat.values_mut() { *ty = table.resolve_completely(ty.clone()); } - for ty in result.type_of_binding.values_mut() { + for ty in type_of_binding.values_mut() { *ty = table.resolve_completely(ty.clone()); } - for ty in result.type_of_rpit.values_mut() { + for ty in type_of_rpit.values_mut() { *ty = table.resolve_completely(ty.clone()); } - for ty in result.type_of_for_iterator.values_mut() { + for ty in type_of_for_iterator.values_mut() { *ty = table.resolve_completely(ty.clone()); } - for mismatch in result.type_mismatches.values_mut() { + for mismatch in type_mismatches.values_mut() { mismatch.expected = table.resolve_completely(mismatch.expected.clone()); mismatch.actual = table.resolve_completely(mismatch.actual.clone()); } - result.diagnostics.retain_mut(|diagnostic| { + diagnostics.retain_mut(|diagnostic| { use InferenceDiagnostic::*; match diagnostic { ExpectedFunction { found: ty, .. } @@ -631,16 +659,16 @@ impl<'a> InferenceContext<'a> { } true }); - for (_, subst) in result.method_resolutions.values_mut() { + for (_, subst) in method_resolutions.values_mut() { *subst = table.resolve_completely(subst.clone()); } - for (_, subst) in result.assoc_resolutions.values_mut() { + for (_, subst) in assoc_resolutions.values_mut() { *subst = table.resolve_completely(subst.clone()); } - for adjustment in result.expr_adjustments.values_mut().flatten() { + for adjustment in expr_adjustments.values_mut().flatten() { adjustment.target = table.resolve_completely(adjustment.target.clone()); } - for adjustment in result.pat_adjustments.values_mut().flatten() { + for adjustment in pat_adjustments.values_mut().flatten() { *adjustment = table.resolve_completely(adjustment.clone()); } result diff --git a/crates/hir-ty/src/infer/closure.rs b/crates/hir-ty/src/infer/closure.rs index 754ac88bb5..23189f383e 100644 --- a/crates/hir-ty/src/infer/closure.rs +++ b/crates/hir-ty/src/infer/closure.rs @@ -5,7 +5,7 @@ use std::{cmp, collections::HashMap, convert::Infallible, mem}; use chalk_ir::{ cast::Cast, fold::{FallibleTypeFolder, TypeFoldable}, - AliasEq, AliasTy, BoundVar, ConstData, DebruijnIndex, FnSubst, Mutability, TyKind, WhereClause, + AliasEq, AliasTy, BoundVar, DebruijnIndex, FnSubst, Mutability, TyKind, WhereClause, }; use hir_def::{ data::adt::VariantData, @@ -26,8 +26,8 @@ use crate::{ static_lifetime, to_chalk_trait_id, traits::FnTrait, utils::{self, generics, Generics}, - Adjust, Adjustment, Binders, BindingMode, ChalkTraitId, ClosureId, ConstValue, DynTy, - FnPointer, FnSig, Interner, Substitution, Ty, TyExt, + Adjust, Adjustment, Binders, BindingMode, ChalkTraitId, ClosureId, DynTy, FnPointer, FnSig, + Interner, Substitution, Ty, TyExt, }; use super::{Expectation, InferenceContext}; @@ -236,6 +236,24 @@ pub(crate) struct CapturedItemWithoutTy { impl CapturedItemWithoutTy { fn with_ty(self, ctx: &mut InferenceContext<'_>) -> CapturedItem { + let ty = self.place.ty(ctx).clone(); + let ty = match &self.kind { + CaptureKind::ByValue => ty, + CaptureKind::ByRef(bk) => { + let m = match bk { + BorrowKind::Mut { .. } => Mutability::Mut, + _ => Mutability::Not, + }; + TyKind::Ref(m, static_lifetime(), ty).intern(Interner) + } + }; + return CapturedItem { + place: self.place, + kind: self.kind, + span: self.span, + ty: replace_placeholder_with_binder(ctx.db, ctx.owner, ty), + }; + fn replace_placeholder_with_binder( db: &dyn HirDatabase, owner: DefWithBodyId, @@ -266,56 +284,28 @@ impl CapturedItemWithoutTy { let Some(idx) = self.generics.param_idx(x) else { return Err(()); }; - Ok(ConstData { - ty, - value: ConstValue::BoundVar(BoundVar::new(outer_binder, idx)), - } - .intern(Interner)) + Ok(BoundVar::new(outer_binder, idx).to_const(Interner, ty)) } fn try_fold_free_placeholder_ty( &mut self, idx: chalk_ir::PlaceholderIndex, - _outer_binder: DebruijnIndex, + outer_binder: DebruijnIndex, ) -> std::result::Result<Ty, Self::Error> { let x = from_placeholder_idx(self.db, idx); let Some(idx) = self.generics.param_idx(x) else { return Err(()); }; - Ok(TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, idx)) - .intern(Interner)) + Ok(BoundVar::new(outer_binder, idx).to_ty(Interner)) } } - let g_def = match owner { - DefWithBodyId::FunctionId(f) => Some(f.into()), - DefWithBodyId::StaticId(_) => None, - DefWithBodyId::ConstId(f) => Some(f.into()), - DefWithBodyId::VariantId(f) => Some(f.into()), - }; - let Some(generics) = g_def.map(|g_def| generics(db.upcast(), g_def)) else { + let Some(generic_def) = owner.as_generic_def_id() else { return Binders::empty(Interner, ty); }; - let filler = &mut Filler { db, generics }; + let filler = &mut Filler { db, generics: generics(db.upcast(), generic_def) }; let result = ty.clone().try_fold_with(filler, DebruijnIndex::INNERMOST).unwrap_or(ty); make_binders(db, &filler.generics, result) } - let ty = self.place.ty(ctx).clone(); - let ty = match &self.kind { - CaptureKind::ByValue => ty, - CaptureKind::ByRef(bk) => { - let m = match bk { - BorrowKind::Mut { .. } => Mutability::Mut, - _ => Mutability::Not, - }; - TyKind::Ref(m, static_lifetime(), ty).intern(Interner) - } - }; - CapturedItem { - place: self.place, - kind: self.kind, - span: self.span, - ty: replace_placeholder_with_binder(ctx.db, ctx.owner, ty), - } } } diff --git a/crates/hir-ty/src/mir/eval/tests.rs b/crates/hir-ty/src/mir/eval/tests.rs index dabc76ba15..ca4268b8fb 100644 --- a/crates/hir-ty/src/mir/eval/tests.rs +++ b/crates/hir-ty/src/mir/eval/tests.rs @@ -640,3 +640,37 @@ fn main() { "#, ); } + +#[test] +fn regression_14966() { + check_pass( + r#" +//- minicore: fn, copy, coerce_unsized +trait A<T> { + fn a(&self) {} +} +impl A<()> for () {} + +struct B; +impl B { + pub fn b<T>(s: &dyn A<T>) -> Self { + B + } +} +struct C; +impl C { + fn c<T>(a: &dyn A<T>) -> Self { + let mut c = C; + let b = B::b(a); + c.d(|| a.a()); + c + } + fn d(&mut self, f: impl FnOnce()) {} +} + +fn main() { + C::c(&()); +} +"#, + ); +} diff --git a/crates/hir-ty/src/mir/monomorphization.rs b/crates/hir-ty/src/mir/monomorphization.rs index b17ac58365..ce3f7a8e51 100644 --- a/crates/hir-ty/src/mir/monomorphization.rs +++ b/crates/hir-ty/src/mir/monomorphization.rs @@ -303,13 +303,7 @@ pub fn monomorphized_mir_body_query( subst: Substitution, trait_env: Arc<crate::TraitEnvironment>, ) -> Result<Arc<MirBody>, MirLowerError> { - let g_def = match owner { - DefWithBodyId::FunctionId(f) => Some(f.into()), - DefWithBodyId::StaticId(_) => None, - DefWithBodyId::ConstId(f) => Some(f.into()), - DefWithBodyId::VariantId(f) => Some(f.into()), - }; - let generics = g_def.map(|g_def| generics(db.upcast(), g_def)); + let generics = owner.as_generic_def_id().map(|g_def| generics(db.upcast(), g_def)); let filler = &mut Filler { db, subst: &subst, trait_env, generics, owner }; let body = db.mir_body(owner)?; let mut body = (*body).clone(); @@ -334,13 +328,7 @@ pub fn monomorphized_mir_body_for_closure_query( trait_env: Arc<crate::TraitEnvironment>, ) -> Result<Arc<MirBody>, MirLowerError> { let (owner, _) = db.lookup_intern_closure(closure.into()); - let g_def = match owner { - DefWithBodyId::FunctionId(f) => Some(f.into()), - DefWithBodyId::StaticId(_) => None, - DefWithBodyId::ConstId(f) => Some(f.into()), - DefWithBodyId::VariantId(f) => Some(f.into()), - }; - let generics = g_def.map(|g_def| generics(db.upcast(), g_def)); + let generics = owner.as_generic_def_id().map(|g_def| generics(db.upcast(), g_def)); let filler = &mut Filler { db, subst: &subst, trait_env, generics, owner }; let body = db.mir_body_for_closure(closure)?; let mut body = (*body).clone(); @@ -356,13 +344,7 @@ pub fn monomorphize_mir_body_bad( trait_env: Arc<crate::TraitEnvironment>, ) -> Result<MirBody, MirLowerError> { let owner = body.owner; - let g_def = match owner { - DefWithBodyId::FunctionId(f) => Some(f.into()), - DefWithBodyId::StaticId(_) => None, - DefWithBodyId::ConstId(f) => Some(f.into()), - DefWithBodyId::VariantId(f) => Some(f.into()), - }; - let generics = g_def.map(|g_def| generics(db.upcast(), g_def)); + let generics = owner.as_generic_def_id().map(|g_def| generics(db.upcast(), g_def)); let filler = &mut Filler { db, subst: &subst, trait_env, generics, owner }; filler.fill_body(&mut body)?; Ok(body) |