Auto merge of rust-lang#14971 - lowr:fix/captured-item-ty-outer-binder, r=HKalbasi

fix: consider outer binders when folding captured items' type

Fixes rust-lang#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.

Author: bors

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

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),
-        }
     }
 }
 

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(&());
+}
+"#,
+    );
+}

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)