Merge #5124

5124: (Partially) fix handling of type params depending on type params r=matklad a=flodiebold

If the first type parameter gets inferred, that's still not handled correctly; it'll require some more refactoring: E.g. if we have `Thing<T, F=fn() -> T>` and then instantiate `Thing<_>`, that gets turned into `Thing<_, fn() -> _>` before the `_` is instantiated into a type variable -- so afterwards, we have two type variables without any connection to each other.

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

crates/ra_hir/src/code_model.rs

@@ -543,7 +543,7 @@ impl_froms!(Adt: Struct, Union, Enum);
 impl Adt {
     pub fn has_non_default_type_params(self, db: &dyn HirDatabase) -> bool {
         let subst = db.generic_defaults(self.into());
-        subst.iter().any(|ty| ty == &Ty::Unknown)
+        subst.iter().any(|ty| &ty.value == &Ty::Unknown)
     }
 
     /// Turns this ADT into a type. Any type parameters of the ADT will be
@@ -775,7 +775,7 @@ pub struct TypeAlias {
 impl TypeAlias {
     pub fn has_non_default_type_params(self, db: &dyn HirDatabase) -> bool {
         let subst = db.generic_defaults(self.id.into());
-        subst.iter().any(|ty| ty == &Ty::Unknown)
+        subst.iter().any(|ty| &ty.value == &Ty::Unknown)
     }
 
     pub fn module(self, db: &dyn HirDatabase) -> Module {
@@ -1035,7 +1035,10 @@ impl TypeParam {
         let local_idx = hir_ty::param_idx(db, self.id)?;
         let resolver = self.id.parent.resolver(db.upcast());
         let environment = TraitEnvironment::lower(db, &resolver);
-        params.get(local_idx).cloned().map(|ty| Type {
+        let ty = params.get(local_idx)?.clone();
+        let subst = Substs::type_params(db, self.id.parent);
+        let ty = ty.subst(&subst.prefix(local_idx));
+        Some(Type {
             krate: self.id.parent.module(db.upcast()).krate,
             ty: InEnvironment { value: ty, environment },
         })

crates/ra_hir_ty/src/db.rs

@@ -14,7 +14,7 @@ use crate::{
     method_resolution::CrateImplDefs,
     traits::{chalk, AssocTyValue, Impl},
     Binders, CallableDef, GenericPredicate, InferenceResult, OpaqueTyId, PolyFnSig,
-    ReturnTypeImplTraits, Substs, TraitRef, Ty, TyDefId, TypeCtor, ValueTyDefId,
+    ReturnTypeImplTraits, TraitRef, Ty, TyDefId, TypeCtor, ValueTyDefId,
 };
 use hir_expand::name::Name;
 
@@ -65,7 +65,7 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
     fn generic_predicates(&self, def: GenericDefId) -> Arc<[Binders<GenericPredicate>]>;
 
     #[salsa::invoke(crate::lower::generic_defaults_query)]
-    fn generic_defaults(&self, def: GenericDefId) -> Substs;
+    fn generic_defaults(&self, def: GenericDefId) -> Arc<[Binders<Ty>]>;
 
     #[salsa::invoke(crate::method_resolution::CrateImplDefs::impls_in_crate_query)]
     fn impls_in_crate(&self, krate: CrateId) -> Arc<CrateImplDefs>;

crates/ra_hir_ty/src/display.rs

@@ -308,7 +308,6 @@ impl HirDisplay for ApplicationTy {
                 }
 
                 if self.parameters.len() > 0 {
-                    let mut non_default_parameters = Vec::with_capacity(self.parameters.len());
                     let parameters_to_write =
                         if f.display_target.is_source_code() || f.omit_verbose_types() {
                             match self
@@ -319,20 +318,23 @@ impl HirDisplay for ApplicationTy {
                             {
                                 None => self.parameters.0.as_ref(),
                                 Some(default_parameters) => {
+                                    let mut default_from = 0;
                                     for (i, parameter) in self.parameters.iter().enumerate() {
                                         match (parameter, default_parameters.get(i)) {
                                             (&Ty::Unknown, _) | (_, None) => {
-                                                non_default_parameters.push(parameter.clone())
+                                                default_from = i + 1;
                                             }
-                                            (_, Some(default_parameter))
+                                            (_, Some(default_parameter)) => {
-                                                if parameter != default_parameter =>
+                                                let actual_default = default_parameter
-                                            {
+                                                    .clone()
-                                                non_default_parameters.push(parameter.clone())
+                                                    .subst(&self.parameters.prefix(i));
+                                                if parameter != &actual_default {
+                                                    default_from = i + 1;
+                                                }
                                             }
-                                            _ => (),
                                         }
                                     }
-                                    &non_default_parameters
+                                    &self.parameters.0[0..default_from]
                                 }
                             }
                         } else {

crates/ra_hir_ty/src/lower.rs

@@ -578,11 +578,13 @@ fn substs_from_path_segment(
     // (i.e. defaults aren't used).
     if !infer_args || had_explicit_args {
         if let Some(def_generic) = def_generic {
-            let default_substs = ctx.db.generic_defaults(def_generic);
+            let defaults = ctx.db.generic_defaults(def_generic);
-            assert_eq!(total_len, default_substs.len());
+            assert_eq!(total_len, defaults.len());
 
-            for default_ty in default_substs.iter().skip(substs.len()) {
+            for default_ty in defaults.iter().skip(substs.len()) {
-                substs.push(default_ty.clone());
+                // each default can depend on the previous parameters
+                let substs_so_far = Substs(substs.clone().into());
+                substs.push(default_ty.clone().subst(&substs_so_far));
             }
         }
     }
@@ -945,17 +947,42 @@ pub(crate) fn generic_predicates_query(
 }
 
 /// Resolve the default type params from generics
-pub(crate) fn generic_defaults_query(db: &dyn HirDatabase, def: GenericDefId) -> Substs {
+pub(crate) fn generic_defaults_query(
+    db: &dyn HirDatabase,
+    def: GenericDefId,
+) -> Arc<[Binders<Ty>]> {
     let resolver = def.resolver(db.upcast());
-    let ctx = TyLoweringContext::new(db, &resolver);
+    let ctx =
+        TyLoweringContext::new(db, &resolver).with_type_param_mode(TypeParamLoweringMode::Variable);
     let generic_params = generics(db.upcast(), def);
 
     let defaults = generic_params
         .iter()
-        .map(|(_idx, p)| p.default.as_ref().map_or(Ty::Unknown, |t| Ty::from_hir(&ctx, t)))
+        .enumerate()
+        .map(|(idx, (_, p))| {
+            let mut ty = p.default.as_ref().map_or(Ty::Unknown, |t| Ty::from_hir(&ctx, t));
+
+            // Each default can only refer to previous parameters.
+            ty.walk_mut_binders(
+                &mut |ty, binders| match ty {
+                    Ty::Bound(BoundVar { debruijn, index }) if *debruijn == binders => {
+                        if *index >= idx {
+                            // type variable default referring to parameter coming
+                            // after it. This is forbidden (FIXME: report
+                            // diagnostic)
+                            *ty = Ty::Unknown;
+                        }
+                    }
+                    _ => {}
+                },
+                DebruijnIndex::INNERMOST,
+            );
+
+            Binders::new(idx, ty)
+        })
         .collect();
 
-    Substs(defaults)
+    defaults
 }
 
 fn fn_sig_for_fn(db: &dyn HirDatabase, def: FunctionId) -> PolyFnSig {

crates/ra_hir_ty/src/tests/simple.rs

@@ -2152,3 +2152,48 @@ fn test() {
     "###
     );
 }
+
+#[test]
+fn generic_default_depending_on_other_type_arg() {
+    assert_snapshot!(
+        infer(r#"
+struct Thing<T = u128, F = fn() -> T> { t: T }
+
+fn test(t1: Thing<u32>, t2: Thing) {
+    t1;
+    t2;
+    Thing::<_> { t: 1u32 };
+}
+"#),
+        // FIXME: the {unknown} is a bug
+        @r###"
+    56..58 't1': Thing<u32, fn() -> u32>
+    72..74 't2': Thing<u128, fn() -> u128>
+    83..130 '{     ...2 }; }': ()
+    89..91 't1': Thing<u32, fn() -> u32>
+    97..99 't2': Thing<u128, fn() -> u128>
+    105..127 'Thing:...1u32 }': Thing<u32, fn() -> {unknown}>
+    121..125 '1u32': u32
+    "###
+    );
+}
+
+#[test]
+fn generic_default_depending_on_other_type_arg_forward() {
+    assert_snapshot!(
+        infer(r#"
+struct Thing<F = fn() -> T, T = u128> { t: T }
+
+fn test(t1: Thing) {
+    t1;
+}
+"#),
+        // the {unknown} here is intentional, as defaults are not allowed to
+        // refer to type parameters coming later
+        @r###"
+    56..58 't1': Thing<fn() -> {unknown}, u128>
+    67..78 '{     t1; }': ()
+    73..75 't1': Thing<fn() -> {unknown}, u128>
+    "###
+    );
+}