From 275afd6e79f4e23d870a8017c805f786e400af72 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 4 Jun 2023 19:38:47 +0900 Subject: [PATCH 1/3] fix: consider outer binders when folding captured items' type --- crates/hir-ty/src/infer/closure.rs | 17 +++++---------- crates/hir-ty/src/mir/eval/tests.rs | 34 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/crates/hir-ty/src/infer/closure.rs b/crates/hir-ty/src/infer/closure.rs index 754ac88bb50..e98905f4eee 100644 --- a/crates/hir-ty/src/infer/closure.rs +++ b/crates/hir-ty/src/infer/closure.rs @@ -5,7 +5,7 @@ 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 @@ 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}; @@ -266,24 +266,19 @@ fn try_fold_free_placeholder_const( 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 { 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 { diff --git a/crates/hir-ty/src/mir/eval/tests.rs b/crates/hir-ty/src/mir/eval/tests.rs index dabc76ba15b..ca4268b8fb0 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 { + fn a(&self) {} +} +impl A<()> for () {} + +struct B; +impl B { + pub fn b(s: &dyn A) -> Self { + B + } +} +struct C; +impl C { + fn c(a: &dyn A) -> 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(&()); +} +"#, + ); +} From a3789eabc9281d29dce3c4199c971f0490074dfe Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 4 Jun 2023 19:39:49 +0900 Subject: [PATCH 2/3] Minor refactorings - use `DefWithBodyId::as_generic_def_id()` - add comments on `InferenceResult` invariant - move local helper function to bottom to comply with style guide --- crates/hir-ty/src/infer.rs | 6 +++ crates/hir-ty/src/infer/closure.rs | 45 ++++++++++------------- crates/hir-ty/src/mir/monomorphization.rs | 24 ++---------- 3 files changed, 29 insertions(+), 46 deletions(-) diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index fa81fe39aa1..ccfa626b5fd 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. @@ -575,6 +579,8 @@ fn new( // used this function for another workaround, mention it here. If you really need this function and believe that // there is no problem in it being `pub(crate)`, remove this comment. pub(crate) fn resolve_all(self) -> InferenceResult { + // NOTE: `InferenceResult::closure_info` is `resolve_completely()`'d during + // `InferenceContext::infer_closures()` (in `HirPlace::ty()` specifically). let InferenceContext { mut table, mut result, .. } = self; table.fallback_if_possible(); diff --git a/crates/hir-ty/src/infer/closure.rs b/crates/hir-ty/src/infer/closure.rs index e98905f4eee..23189f383e0 100644 --- a/crates/hir-ty/src/infer/closure.rs +++ b/crates/hir-ty/src/infer/closure.rs @@ -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, @@ -281,36 +299,13 @@ fn try_fold_free_placeholder_ty( 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/monomorphization.rs b/crates/hir-ty/src/mir/monomorphization.rs index b17ac583656..ce3f7a8e510 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, ) -> Result, 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, ) -> Result, 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, ) -> Result { 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) From f549cacc1d3fb09fd682fa4e0c29d45e8bd179a3 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 4 Jun 2023 20:32:46 +0900 Subject: [PATCH 3/3] Destructure `InferenceResult` in `resolve_all()` so that whenever new fields are added we don't forget to handle them. --- crates/hir-ty/src/infer.rs | 48 +++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index ccfa626b5fd..80f32e96ee6 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -579,9 +579,31 @@ fn new( // used this function for another workaround, mention it here. If you really need this function and believe that // there is no problem in it being `pub(crate)`, remove this comment. pub(crate) fn resolve_all(self) -> InferenceResult { - // NOTE: `InferenceResult::closure_info` is `resolve_completely()`'d during - // `InferenceContext::infer_closures()` (in `HirPlace::ty()` specifically). 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(); @@ -590,26 +612,26 @@ pub(crate) fn resolve_all(self) -> InferenceResult { // 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, .. } @@ -637,16 +659,16 @@ pub(crate) fn resolve_all(self) -> InferenceResult { } 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