Merge #4023

4023: Fix another crash from wrong binders r=matklad a=flodiebold

Basically, if we had something like `dyn Trait<T>` (where `T` is a type parameter) in an impl we lowered that to `dyn Trait<^0.0>`, when it should be `dyn Trait<^1.0>` because the `dyn` introduces a new binder. With one type parameter, that's just wrong, with two, it'll lead to crashes.

Co-authored-by: Florian Diebold <flodiebold@gmail.com>

crates/ra_hir_ty/src/lib.rs

@@ -396,12 +396,12 @@ impl Substs {
     }
 
     /// Return Substs that replace each parameter by a bound variable.
-    pub(crate) fn bound_vars(generic_params: &Generics) -> Substs {
+    pub(crate) fn bound_vars(generic_params: &Generics, debruijn: DebruijnIndex) -> Substs {
         Substs(
             generic_params
                 .iter()
                 .enumerate()
-                .map(|(idx, _)| Ty::Bound(BoundVar::new(DebruijnIndex::INNERMOST, idx)))
+                .map(|(idx, _)| Ty::Bound(BoundVar::new(debruijn, idx)))
                 .collect(),
         )
     }

crates/ra_hir_ty/src/lower.rs

@@ -39,6 +39,7 @@ use crate::{
 pub struct TyLoweringContext<'a> {
     pub db: &'a dyn HirDatabase,
     pub resolver: &'a Resolver,
+    in_binders: DebruijnIndex,
     /// Note: Conceptually, it's thinkable that we could be in a location where
     /// some type params should be represented as placeholders, and others
     /// should be converted to variables. I think in practice, this isn't
@@ -53,7 +54,27 @@ impl<'a> TyLoweringContext<'a> {
         let impl_trait_counter = std::cell::Cell::new(0);
         let impl_trait_mode = ImplTraitLoweringMode::Disallowed;
         let type_param_mode = TypeParamLoweringMode::Placeholder;
-        Self { db, resolver, impl_trait_mode, impl_trait_counter, type_param_mode }
+        let in_binders = DebruijnIndex::INNERMOST;
+        Self { db, resolver, in_binders, impl_trait_mode, impl_trait_counter, type_param_mode }
+    }
+
+    pub fn with_shifted_in<T>(
+        &self,
+        debruijn: DebruijnIndex,
+        f: impl FnOnce(&TyLoweringContext) -> T,
+    ) -> T {
+        let new_ctx = Self {
+            in_binders: self.in_binders.shifted_in_from(debruijn),
+            impl_trait_counter: std::cell::Cell::new(self.impl_trait_counter.get()),
+            ..*self
+        };
+        let result = f(&new_ctx);
+        self.impl_trait_counter.set(new_ctx.impl_trait_counter.get());
+        result
+    }
+
+    pub fn shifted_in(self, debruijn: DebruijnIndex) -> Self {
+        Self { in_binders: self.in_binders.shifted_in_from(debruijn), ..self }
     }
 
     pub fn with_impl_trait_mode(self, impl_trait_mode: ImplTraitLoweringMode) -> Self {
@@ -134,22 +155,26 @@ impl Ty {
             }
             TypeRef::DynTrait(bounds) => {
                 let self_ty = Ty::Bound(BoundVar::new(DebruijnIndex::INNERMOST, 0));
-                let predicates = bounds
+                let predicates = ctx.with_shifted_in(DebruijnIndex::ONE, |ctx| {
+                    bounds
                         .iter()
                         .flat_map(|b| GenericPredicate::from_type_bound(ctx, b, self_ty.clone()))
-                    .collect();
+                        .collect()
+                });
                 Ty::Dyn(predicates)
             }
             TypeRef::ImplTrait(bounds) => {
                 match ctx.impl_trait_mode {
                     ImplTraitLoweringMode::Opaque => {
                         let self_ty = Ty::Bound(BoundVar::new(DebruijnIndex::INNERMOST, 0));
-                        let predicates = bounds
+                        let predicates = ctx.with_shifted_in(DebruijnIndex::ONE, |ctx| {
+                            bounds
                                 .iter()
                                 .flat_map(|b| {
                                     GenericPredicate::from_type_bound(ctx, b, self_ty.clone())
                                 })
-                            .collect();
+                                .collect()
+                        });
                         Ty::Opaque(predicates)
                     }
                     ImplTraitLoweringMode::Param => {
@@ -180,7 +205,7 @@ impl Ty {
                                 (0, 0, 0, 0)
                             };
                         Ty::Bound(BoundVar::new(
-                            DebruijnIndex::INNERMOST,
+                            ctx.in_binders,
                             idx as usize + parent_params + self_params + list_params,
                         ))
                     }
@@ -293,7 +318,7 @@ impl Ty {
                     TypeParamLoweringMode::Placeholder => Ty::Placeholder(param_id),
                     TypeParamLoweringMode::Variable => {
                         let idx = generics.param_idx(param_id).expect("matching generics");
-                        Ty::Bound(BoundVar::new(DebruijnIndex::INNERMOST, idx))
+                        Ty::Bound(BoundVar::new(ctx.in_binders, idx))
                     }
                 }
             }
@@ -303,7 +328,9 @@ impl Ty {
                     TypeParamLoweringMode::Placeholder => {
                         Substs::type_params_for_generics(&generics)
                     }
-                    TypeParamLoweringMode::Variable => Substs::bound_vars(&generics),
+                    TypeParamLoweringMode::Variable => {
+                        Substs::bound_vars(&generics, ctx.in_binders)
+                    }
                 };
                 ctx.db.impl_self_ty(impl_id).subst(&substs)
             }
@@ -313,7 +340,9 @@ impl Ty {
                     TypeParamLoweringMode::Placeholder => {
                         Substs::type_params_for_generics(&generics)
                     }
-                    TypeParamLoweringMode::Variable => Substs::bound_vars(&generics),
+                    TypeParamLoweringMode::Variable => {
+                        Substs::bound_vars(&generics, ctx.in_binders)
+                    }
                 };
                 ctx.db.ty(adt.into()).subst(&substs)
             }
@@ -797,7 +826,7 @@ fn fn_sig_for_fn(db: &dyn HirDatabase, def: FunctionId) -> PolyFnSig {
 /// function body.
 fn type_for_fn(db: &dyn HirDatabase, def: FunctionId) -> Binders<Ty> {
     let generics = generics(db.upcast(), def.into());
-    let substs = Substs::bound_vars(&generics);
+    let substs = Substs::bound_vars(&generics, DebruijnIndex::INNERMOST);
     Binders::new(substs.len(), Ty::apply(TypeCtor::FnDef(def.into()), substs))
 }
 
@@ -851,7 +880,7 @@ fn type_for_struct_constructor(db: &dyn HirDatabase, def: StructId) -> Binders<T
         return type_for_adt(db, def.into());
     }
     let generics = generics(db.upcast(), def.into());
-    let substs = Substs::bound_vars(&generics);
+    let substs = Substs::bound_vars(&generics, DebruijnIndex::INNERMOST);
     Binders::new(substs.len(), Ty::apply(TypeCtor::FnDef(def.into()), substs))
 }
 
@@ -876,13 +905,13 @@ fn type_for_enum_variant_constructor(db: &dyn HirDatabase, def: EnumVariantId) -
         return type_for_adt(db, def.parent.into());
     }
     let generics = generics(db.upcast(), def.parent.into());
-    let substs = Substs::bound_vars(&generics);
+    let substs = Substs::bound_vars(&generics, DebruijnIndex::INNERMOST);
     Binders::new(substs.len(), Ty::apply(TypeCtor::FnDef(def.into()), substs))
 }
 
 fn type_for_adt(db: &dyn HirDatabase, adt: AdtId) -> Binders<Ty> {
     let generics = generics(db.upcast(), adt.into());
-    let substs = Substs::bound_vars(&generics);
+    let substs = Substs::bound_vars(&generics, DebruijnIndex::INNERMOST);
     Binders::new(substs.len(), Ty::apply(TypeCtor::Adt(adt), substs))
 }
 
@@ -892,7 +921,7 @@ fn type_for_type_alias(db: &dyn HirDatabase, t: TypeAliasId) -> Binders<Ty> {
     let ctx =
         TyLoweringContext::new(db, &resolver).with_type_param_mode(TypeParamLoweringMode::Variable);
     let type_ref = &db.type_alias_data(t).type_ref;
-    let substs = Substs::bound_vars(&generics);
+    let substs = Substs::bound_vars(&generics, DebruijnIndex::INNERMOST);
     let inner = Ty::from_hir(&ctx, type_ref.as_ref().unwrap_or(&TypeRef::Error));
     Binders::new(substs.len(), inner)
 }

crates/ra_hir_ty/src/tests/traits.rs

@@ -1210,6 +1210,42 @@ fn test(x: dyn Trait<u64>, y: &dyn Trait<u64>) {
     );
 }
 
+#[test]
+fn dyn_trait_in_impl() {
+    assert_snapshot!(
+        infer(r#"
+trait Trait<T, U> {
+    fn foo(&self) -> (T, U);
+}
+struct S<T, U> {}
+impl<T, U> S<T, U> {
+    fn bar(&self) -> &dyn Trait<T, U> { loop {} }
+}
+trait Trait2<T, U> {
+    fn baz(&self) -> (T, U);
+}
+impl<T, U> Trait2<T, U> for dyn Trait<T, U> { }
+
+fn test(s: S<u32, i32>) {
+    s.bar().baz();
+}
+"#),
+        @r###"
+    [33; 37) 'self': &Self
+    [103; 107) 'self': &S<T, U>
+    [129; 140) '{ loop {} }': &dyn Trait<T, U>
+    [131; 138) 'loop {}': !
+    [136; 138) '{}': ()
+    [176; 180) 'self': &Self
+    [252; 253) 's': S<u32, i32>
+    [268; 290) '{     ...z(); }': ()
+    [274; 275) 's': S<u32, i32>
+    [274; 281) 's.bar()': &dyn Trait<u32, i32>
+    [274; 287) 's.bar().baz()': (u32, i32)
+    "###
+    );
+}
+
 #[test]
 fn dyn_trait_bare() {
     assert_snapshot!(

crates/ra_hir_ty/src/traits/chalk.rs

@@ -17,7 +17,7 @@ use ra_db::{
 use super::{builtin, AssocTyValue, Canonical, ChalkContext, Impl, Obligation};
 use crate::{
     db::HirDatabase, display::HirDisplay, method_resolution::TyFingerprint, utils::generics,
-    ApplicationTy, GenericPredicate, ProjectionTy, Substs, TraitRef, Ty, TypeCtor,
+    ApplicationTy, DebruijnIndex, GenericPredicate, ProjectionTy, Substs, TraitRef, Ty, TypeCtor,
 };
 
 pub(super) mod tls;
@@ -815,7 +815,7 @@ pub(crate) fn associated_ty_data_query(
     // Lower bounds -- we could/should maybe move this to a separate query in `lower`
     let type_alias_data = db.type_alias_data(type_alias);
     let generic_params = generics(db.upcast(), type_alias.into());
-    let bound_vars = Substs::bound_vars(&generic_params);
+    let bound_vars = Substs::bound_vars(&generic_params, DebruijnIndex::INNERMOST);
     let resolver = hir_def::resolver::HasResolver::resolver(type_alias, db.upcast());
     let ctx = crate::TyLoweringContext::new(db, &resolver)
         .with_type_param_mode(crate::lower::TypeParamLoweringMode::Variable);
@@ -849,7 +849,7 @@ pub(crate) fn trait_datum_query(
     let trait_data = db.trait_data(trait_);
     debug!("trait {:?} = {:?}", trait_id, trait_data.name);
     let generic_params = generics(db.upcast(), trait_.into());
-    let bound_vars = Substs::bound_vars(&generic_params);
+    let bound_vars = Substs::bound_vars(&generic_params, DebruijnIndex::INNERMOST);
     let flags = chalk_rust_ir::TraitFlags {
         auto: trait_data.auto,
         upstream: trait_.lookup(db.upcast()).container.module(db.upcast()).krate != krate,
@@ -888,7 +888,7 @@ pub(crate) fn struct_datum_query(
         .as_generic_def()
         .map(|generic_def| {
             let generic_params = generics(db.upcast(), generic_def);
-            let bound_vars = Substs::bound_vars(&generic_params);
+            let bound_vars = Substs::bound_vars(&generic_params, DebruijnIndex::INNERMOST);
             convert_where_clauses(db, generic_def, &bound_vars)
         })
         .unwrap_or_else(Vec::new);
@@ -934,7 +934,7 @@ fn impl_def_datum(
     let impl_data = db.impl_data(impl_id);
 
     let generic_params = generics(db.upcast(), impl_id.into());
-    let bound_vars = Substs::bound_vars(&generic_params);
+    let bound_vars = Substs::bound_vars(&generic_params, DebruijnIndex::INNERMOST);
     let trait_ = trait_ref.trait_;
     let impl_type = if impl_id.lookup(db.upcast()).container.module(db.upcast()).krate == krate {
         chalk_rust_ir::ImplType::Local