Merge #3050

3050: Refactor type parameters, implement argument position impl trait r=matklad a=flodiebold

I wanted to implement APIT by lowering to type parameters because we need to do that anyway for correctness and don't need Chalk support for it; this grew into some more wide-ranging refactoring of how type parameters are handled :sweat_smile: 

 - use Ty::Bound instead of Ty::Param to represent polymorphism, and explicitly
   count binders. This gets us closer to Chalk's way of doing things, and means
   that we now only use Param as a placeholder for an unknown type, e.g. within
   a generic function. I.e. we're never using Param in a situation where we want
   to substitute it, and the method to do that is gone; `subst` now always works
   on bound variables. (This changes how the types of generic functions print; 
   previously, you'd get something like `fn identity<i32>(T) -> T`, but now we
   display the substituted signature `fn identity<i32>(i32) -> i32`, which I think 
   makes more sense.)
 - once we do this, it's more natural to represent `Param` by a globally unique
   ID; the use of indices was mostly to make substituting easier. This also
   means we fix the bug where `Param` loses its name when going through Chalk.
 - I would actually like to rename `Param` to `Placeholder` to better reflect its use and
   get closer to Chalk, but I'll leave that to a follow-up.
 - introduce a context for type lowering, to allow lowering `impl Trait` to
   different things depending on where we are. And since we have that, we can
   also lower type parameters directly to variables instead of placeholders.
   Also, we'll be able to use this later to collect diagnostics.
 - implement argument position impl trait by lowering it to type parameters.
   I've realized that this is necessary to correctly implement it; e.g. consider
   `fn foo(impl Display) -> impl Something`. It's observable that the return
   type of e.g. `foo(1u32)` unifies with itself, but doesn't unify with e.g.
   `foo(1i32)`; so the return type needs to be parameterized by the argument
   type.

   
This fixes a few bugs as well:
 - type parameters 'losing' their name when they go through Chalk, as mentioned
   above (i.e. getting `[missing name]` somewhere)
 - impl trait not being considered as implementing the super traits (very
   noticeable for the `db` in RA)
 - the fact that argument impl trait was only turned into variables when the
   function got called caused type mismatches when the function was used as a
   value (fixes a few type mismatches in RA)

The one thing I'm not so happy with here is how we're lowering `impl Trait` types to variables; since `TypeRef`s don't have an identity currently, we just count how many of them we have seen while going through the function signature. That's quite fragile though, since we have to do it while desugaring generics and while lowering the type signature, and in the exact same order in both cases. We could consider either giving only `TypeRef::ImplTrait` a local id, or maybe just giving all `TypeRef`s an identity after all (we talked about this before)...

Follow-up tasks:
 - handle return position impl trait; we basically need to create a variable and some trait obligations for that variable
 - rename `Param` to `Placeholder`

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

1 files changed, 28 insertions, 38 deletions

diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs
index e2eda3134..a9d958c8b 100644
--- a/crates/ra_hir_ty/src/infer.rs
+++ b/crates/ra_hir_ty/src/infer.rs
@@ -34,7 +34,6 @@ use hir_expand::{diagnostics::DiagnosticSink, name::name};
 use ra_arena::map::ArenaMap;
 use ra_prof::profile;
 use ra_syntax::SmolStr;
-use test_utils::tested_by;
 
 use super::{
     primitive::{FloatTy, IntTy},
@@ -42,7 +41,9 @@ use super::{
     ApplicationTy, GenericPredicate, InEnvironment, ProjectionTy, Substs, TraitEnvironment,
     TraitRef, Ty, TypeCtor, TypeWalk, Uncertain,
 };
-use crate::{db::HirDatabase, infer::diagnostics::InferenceDiagnostic};
+use crate::{
+    db::HirDatabase, infer::diagnostics::InferenceDiagnostic, lower::ImplTraitLoweringMode,
+};
 
 pub(crate) use unify::unify;
 
@@ -271,38 +272,21 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
         self.result.diagnostics.push(diagnostic);
     }
 
-    fn make_ty(&mut self, type_ref: &TypeRef) -> Ty {
-        let ty = Ty::from_hir(
-            self.db,
-            // FIXME use right resolver for block
-            &self.resolver,
-            type_ref,
-        );
+    fn make_ty_with_mode(
+        &mut self,
+        type_ref: &TypeRef,
+        impl_trait_mode: ImplTraitLoweringMode,
+    ) -> Ty {
+        // FIXME use right resolver for block
+        let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver)
+            .with_impl_trait_mode(impl_trait_mode);
+        let ty = Ty::from_hir(&ctx, type_ref);
         let ty = self.insert_type_vars(ty);
         self.normalize_associated_types_in(ty)
     }
 
-    /// Replaces `impl Trait` in `ty` by type variables and obligations for
-    /// those variables. This is done for function arguments when calling a
-    /// function, and for return types when inside the function body, i.e. in
-    /// the cases where the `impl Trait` is 'transparent'. In other cases, `impl
-    /// Trait` is represented by `Ty::Opaque`.
-    fn insert_vars_for_impl_trait(&mut self, ty: Ty) -> Ty {
-        ty.fold(&mut |ty| match ty {
-            Ty::Opaque(preds) => {
-                tested_by!(insert_vars_for_impl_trait);
-                let var = self.table.new_type_var();
-                let var_subst = Substs::builder(1).push(var.clone()).build();
-                self.obligations.extend(
-                    preds
-                        .iter()
-                        .map(|pred| pred.clone().subst_bound_vars(&var_subst))
-                        .filter_map(Obligation::from_predicate),
-                );
-                var
-            }
-            _ => ty,
-        })
+    fn make_ty(&mut self, type_ref: &TypeRef) -> Ty {
+        self.make_ty_with_mode(type_ref, ImplTraitLoweringMode::Disallowed)
     }
 
     /// Replaces Ty::Unknown by a new type var, so we can maybe still infer it.
@@ -446,19 +430,20 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             None => return (Ty::Unknown, None),
         };
         let resolver = &self.resolver;
+        let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver);
         // FIXME: this should resolve assoc items as well, see this example:
         // https://play.rust-lang.org/?gist=087992e9e22495446c01c0d4e2d69521
         match resolver.resolve_path_in_type_ns_fully(self.db, path.mod_path()) {
             Some(TypeNs::AdtId(AdtId::StructId(strukt))) => {
-                let substs = Ty::substs_from_path(self.db, resolver, path, strukt.into());
+                let substs = Ty::substs_from_path(&ctx, path, strukt.into());
                 let ty = self.db.ty(strukt.into());
-                let ty = self.insert_type_vars(ty.apply_substs(substs));
+                let ty = self.insert_type_vars(ty.subst(&substs));
                 (ty, Some(strukt.into()))
             }
             Some(TypeNs::EnumVariantId(var)) => {
-                let substs = Ty::substs_from_path(self.db, resolver, path, var.into());
+                let substs = Ty::substs_from_path(&ctx, path, var.into());
                 let ty = self.db.ty(var.parent.into());
-                let ty = self.insert_type_vars(ty.apply_substs(substs));
+                let ty = self.insert_type_vars(ty.subst(&substs));
                 (ty, Some(var.into()))
             }
             Some(_) | None => (Ty::Unknown, None),
@@ -471,13 +456,18 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
 
     fn collect_fn(&mut self, data: &FunctionData) {
         let body = Arc::clone(&self.body); // avoid borrow checker problem
-        for (type_ref, pat) in data.params.iter().zip(body.params.iter()) {
-            let ty = self.make_ty(type_ref);
+        let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver)
+            .with_impl_trait_mode(ImplTraitLoweringMode::Param);
+        let param_tys =
+            data.params.iter().map(|type_ref| Ty::from_hir(&ctx, type_ref)).collect::<Vec<_>>();
+        for (ty, pat) in param_tys.into_iter().zip(body.params.iter()) {
+            let ty = self.insert_type_vars(ty);
+            let ty = self.normalize_associated_types_in(ty);
 
             self.infer_pat(*pat, &ty, BindingMode::default());
         }
-        let return_ty = self.make_ty(&data.ret_type);
-        self.return_ty = self.insert_vars_for_impl_trait(return_ty);
+        let return_ty = self.make_ty_with_mode(&data.ret_type, ImplTraitLoweringMode::Disallowed); // FIXME implement RPIT
+        self.return_ty = return_ty;
     }
 
     fn infer_body(&mut self) {