Merge #8360

8360: Fix shifting of binders in FnPointer r=flodiebold a=flodiebold

 - don't shift in/out for Chalk mapping (we want to have the same binders now)
 - do shift in when creating the signature for a closure (though it shouldn't matter much)
 - do shift in when lowering a `fn()` type
 - correctly deal with the implied binder in TypeWalk

Tested with the binders validator on various repos, so I'm pretty sure this doesn't mess things up :grimacing: 

Co-authored-by: Florian Diebold <[email protected]>

4 files changed, 23 insertions, 10 deletions

diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs
index a2a7236a8..ff564106b 100644
--- a/crates/hir_ty/src/infer/expr.rs
+++ b/crates/hir_ty/src/infer/expr.rs
@@ -23,7 +23,7 @@ use crate::{
     traits::{chalk::from_chalk, FnTrait},
     utils::{generics, variant_data, Generics},
     AdtId, Binders, CallableDefId, FnPointer, FnSig, FnSubst, InEnvironment, Interner,
-    ProjectionTyExt, Rawness, Scalar, Substitution, TraitRef, Ty, TyBuilder, TyKind,
+    ProjectionTyExt, Rawness, Scalar, Substitution, TraitRef, Ty, TyBuilder, TyKind, TypeWalk,
 };
 
 use super::{
@@ -262,7 +262,9 @@ impl<'a> InferenceContext<'a> {
                 let sig_ty = TyKind::Function(FnPointer {
                     num_binders: 0,
                     sig: FnSig { abi: (), safety: chalk_ir::Safety::Safe, variadic: false },
-                    substitution: FnSubst(Substitution::from_iter(&Interner, sig_tys.clone())),
+                    substitution: FnSubst(
+                        Substitution::from_iter(&Interner, sig_tys.clone()).shifted_in(&Interner),
+                    ),
                 })
                 .intern(&Interner);
                 let closure_id = self.db.intern_closure((self.owner, tgt_expr)).into();
diff --git a/crates/hir_ty/src/lower.rs b/crates/hir_ty/src/lower.rs
index 20bb7dd59..3cbb6ad54 100644
--- a/crates/hir_ty/src/lower.rs
+++ b/crates/hir_ty/src/lower.rs
@@ -178,8 +178,9 @@ impl<'a> TyLoweringContext<'a> {
             }
             TypeRef::Placeholder => TyKind::Error.intern(&Interner),
             TypeRef::Fn(params, is_varargs) => {
-                let substs =
-                    Substitution::from_iter(&Interner, params.iter().map(|tr| self.lower_ty(tr)));
+                let substs = self.with_shifted_in(DebruijnIndex::ONE, |ctx| {
+                    Substitution::from_iter(&Interner, params.iter().map(|tr| ctx.lower_ty(tr)))
+                });
                 TyKind::Function(FnPointer {
                     num_binders: 0, // FIXME lower `for<'a> fn()` correctly
                     sig: FnSig { abi: (), safety: Safety::Safe, variadic: *is_varargs },
diff --git a/crates/hir_ty/src/traits/chalk/mapping.rs b/crates/hir_ty/src/traits/chalk/mapping.rs
index 13d8d1111..3047fbacb 100644
--- a/crates/hir_ty/src/traits/chalk/mapping.rs
+++ b/crates/hir_ty/src/traits/chalk/mapping.rs
@@ -3,7 +3,7 @@
 //! Chalk (in both directions); plus some helper functions for more specialized
 //! conversions.
 
-use chalk_ir::{cast::Cast, fold::shift::Shift, interner::HasInterner, LifetimeData};
+use chalk_ir::{cast::Cast, interner::HasInterner, LifetimeData};
 use chalk_solve::rust_ir;
 
 use base_db::salsa::InternKey;
@@ -25,7 +25,7 @@ impl ToChalk for Ty {
             TyKind::Ref(m, ty) => ref_to_chalk(db, m, ty),
             TyKind::Array(ty) => array_to_chalk(db, ty),
             TyKind::Function(FnPointer { sig, substitution: substs, .. }) => {
-                let substitution = chalk_ir::FnSubst(substs.0.to_chalk(db).shifted_in(&Interner));
+                let substitution = chalk_ir::FnSubst(substs.0.to_chalk(db));
                 chalk_ir::TyKind::Function(chalk_ir::FnPointer {
                     num_binders: 0,
                     sig,
@@ -132,10 +132,7 @@ impl ToChalk for Ty {
                 ..
             }) => {
                 assert_eq!(num_binders, 0);
-                let substs = crate::FnSubst(from_chalk(
-                    db,
-                    substitution.0.shifted_out(&Interner).expect("fn ptr should have no binders"),
-                ));
+                let substs = crate::FnSubst(from_chalk(db, substitution.0));
                 TyKind::Function(FnPointer { num_binders, sig, substitution: substs })
             }
             chalk_ir::TyKind::BoundVar(idx) => TyKind::BoundVar(idx),
diff --git a/crates/hir_ty/src/walk.rs b/crates/hir_ty/src/walk.rs
index 963eb12c8..e1e77ba37 100644
--- a/crates/hir_ty/src/walk.rs
+++ b/crates/hir_ty/src/walk.rs
@@ -92,6 +92,13 @@ pub trait TypeWalk {
         self
     }
 
+    fn shifted_in(self, _interner: &Interner) -> Self
+    where
+        Self: Sized,
+    {
+        self.shifted_in_from(DebruijnIndex::ONE)
+    }
+
     /// Shifts up debruijn indices of `TyKind::Bound` vars by `n`.
     fn shifted_in_from(self, n: DebruijnIndex) -> Self
     where
@@ -149,6 +156,9 @@ impl TypeWalk for Ty {
             TyKind::Slice(ty) | TyKind::Array(ty) | TyKind::Ref(_, ty) | TyKind::Raw(_, ty) => {
                 ty.walk(f);
             }
+            TyKind::Function(fn_pointer) => {
+                fn_pointer.substitution.0.walk(f);
+            }
             _ => {
                 if let Some(substs) = self.substs() {
                     for t in substs.iter(&Interner) {
@@ -180,6 +190,9 @@ impl TypeWalk for Ty {
             TyKind::Slice(ty) | TyKind::Array(ty) | TyKind::Ref(_, ty) | TyKind::Raw(_, ty) => {
                 ty.walk_mut_binders(f, binders);
             }
+            TyKind::Function(fn_pointer) => {
+                fn_pointer.substitution.0.walk_mut_binders(f, binders.shifted_in());
+            }
             _ => {
                 if let Some(substs) = self.substs_mut() {
                     substs.walk_mut_binders(f, binders);