Merge #2636

2636: Chalk update and refactoring r=flodiebold a=flodiebold

This updates the Chalk integration to rust-lang/chalk#311, which will presumably get merged soon, and refactors it some more, most notably introducing our own `TypeFamily` instead of reusing `ChalkIr`. It's still mostly the same as `ChalkIr` though, except for using Salsa `InternId`s directly.

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

Author: bors[bot]

Cargo.lock

@@ -21,10 +21,9 @@ ra_prof = { path = "../ra_prof" }
 ra_syntax = { path = "../ra_syntax" }
 test_utils = { path = "../test_utils" }
 
-# https://github.com/rust-lang/chalk/pull/294
-chalk-solve =   { git = "https://github.com/rust-lang/chalk.git", rev = "151949dece8117d180b5d197a7afa968c3ba14bb" }
-chalk-rust-ir = { git = "https://github.com/rust-lang/chalk.git", rev = "151949dece8117d180b5d197a7afa968c3ba14bb" }
-chalk-ir =      { git = "https://github.com/rust-lang/chalk.git", rev = "151949dece8117d180b5d197a7afa968c3ba14bb" }
+chalk-solve =   { git = "https://github.com/rust-lang/chalk.git", rev = "ff65b5ac9860f3c36bd892c865ab23d5ff0bbae5" }
+chalk-rust-ir = { git = "https://github.com/rust-lang/chalk.git", rev = "ff65b5ac9860f3c36bd892c865ab23d5ff0bbae5" }
+chalk-ir =      { git = "https://github.com/rust-lang/chalk.git", rev = "ff65b5ac9860f3c36bd892c865ab23d5ff0bbae5" }
 
 lalrpop-intern = "0.15.1"
 

crates/ra_hir_ty/Cargo.toml

@@ -10,7 +10,7 @@ use ra_db::{salsa, CrateId};
 
 use crate::{
     method_resolution::CrateImplBlocks,
-    traits::{AssocTyValue, Impl},
+    traits::{chalk, AssocTyValue, Impl},
     CallableDef, FnSig, GenericPredicate, InferenceResult, Substs, TraitRef, Ty, TyDefId, TypeCtor,
     ValueTyDefId,
 };
@@ -77,39 +77,24 @@ pub trait HirDatabase: DefDatabase {
     #[salsa::interned]
     fn intern_assoc_ty_value(&self, assoc_ty_value: AssocTyValue) -> crate::traits::AssocTyValueId;
 
-    #[salsa::invoke(crate::traits::chalk::associated_ty_data_query)]
-    fn associated_ty_data(
-        &self,
-        id: chalk_ir::TypeId,
-    ) -> Arc<chalk_rust_ir::AssociatedTyDatum<chalk_ir::family::ChalkIr>>;
+    #[salsa::invoke(chalk::associated_ty_data_query)]
+    fn associated_ty_data(&self, id: chalk::AssocTypeId) -> Arc<chalk::AssociatedTyDatum>;
 
-    #[salsa::invoke(crate::traits::chalk::trait_datum_query)]
-    fn trait_datum(
-        &self,
-        krate: CrateId,
-        trait_id: chalk_ir::TraitId,
-    ) -> Arc<chalk_rust_ir::TraitDatum<chalk_ir::family::ChalkIr>>;
+    #[salsa::invoke(chalk::trait_datum_query)]
+    fn trait_datum(&self, krate: CrateId, trait_id: chalk::TraitId) -> Arc<chalk::TraitDatum>;
 
-    #[salsa::invoke(crate::traits::chalk::struct_datum_query)]
-    fn struct_datum(
-        &self,
-        krate: CrateId,
-        struct_id: chalk_ir::StructId,
-    ) -> Arc<chalk_rust_ir::StructDatum<chalk_ir::family::ChalkIr>>;
+    #[salsa::invoke(chalk::struct_datum_query)]
+    fn struct_datum(&self, krate: CrateId, struct_id: chalk::StructId) -> Arc<chalk::StructDatum>;
 
     #[salsa::invoke(crate::traits::chalk::impl_datum_query)]
-    fn impl_datum(
-        &self,
-        krate: CrateId,
-        impl_id: chalk_ir::ImplId,
-    ) -> Arc<chalk_rust_ir::ImplDatum<chalk_ir::family::ChalkIr>>;
+    fn impl_datum(&self, krate: CrateId, impl_id: chalk::ImplId) -> Arc<chalk::ImplDatum>;
 
     #[salsa::invoke(crate::traits::chalk::associated_ty_value_query)]
     fn associated_ty_value(
         &self,
         krate: CrateId,
-        id: chalk_rust_ir::AssociatedTyValueId,
-    ) -> Arc<chalk_rust_ir::AssociatedTyValue<chalk_ir::family::ChalkIr>>;
+        id: chalk::AssociatedTyValueId,
+    ) -> Arc<chalk::AssociatedTyValue>;
 
     #[salsa::invoke(crate::traits::trait_solve_query)]
     fn trait_solve(

crates/ra_hir_ty/src/db.rs

@@ -865,7 +865,7 @@ mod foo {
 
 #[test]
 fn method_resolution_where_clause_for_unknown_trait() {
-    // The blanket impl shouldn't apply because we can't even resolve UnknownTrait
+    // The blanket impl currently applies because we ignore the unresolved where clause
     let t = type_at(
         r#"
 //- /main.rs
@@ -875,7 +875,7 @@ impl<T> Trait for T where T: UnknownTrait {}
 fn test() { (&S).foo()<|>; }
 "#,
     );
-    assert_eq!(t, "{unknown}");
+    assert_eq!(t, "u128");
 }
 
 #[test]

crates/ra_hir_ty/src/tests/method_resolution.rs

@@ -958,6 +958,23 @@ fn test() {
     );
 }
 
+#[test]
+fn error_bound_chalk() {
+    let t = type_at(
+        r#"
+//- /main.rs
+trait Trait {
+    fn foo(&self) -> u32 {}
+}
+
+fn test(x: (impl Trait + UnknownTrait)) {
+    x.foo()<|>;
+}
+"#,
+    );
+    assert_eq!(t, "u32");
+}
+
 #[test]
 fn assoc_type_bindings() {
     assert_snapshot!(

crates/ra_hir_ty/src/tests/traits.rs

@@ -1,7 +1,7 @@
 //! Trait solving using Chalk.
 use std::sync::{Arc, Mutex};
 
-use chalk_ir::{cast::Cast, family::ChalkIr};
+use chalk_ir::cast::Cast;
 use hir_def::{expr::ExprId, DefWithBodyId, ImplId, TraitId, TypeAliasId};
 use log::debug;
 use ra_db::{impl_intern_key, salsa, CrateId};
@@ -12,15 +12,15 @@ use crate::db::HirDatabase;
 
 use super::{Canonical, GenericPredicate, HirDisplay, ProjectionTy, TraitRef, Ty, TypeWalk};
 
-use self::chalk::{from_chalk, ToChalk};
+use self::chalk::{from_chalk, ToChalk, TypeFamily};
 
 pub(crate) mod chalk;
 mod builtin;
 
 #[derive(Debug, Clone)]
 pub struct TraitSolver {
     krate: CrateId,
-    inner: Arc<Mutex<chalk_solve::Solver<ChalkIr>>>,
+    inner: Arc<Mutex<chalk_solve::Solver<TypeFamily>>>,
 }
 
 /// We need eq for salsa
@@ -36,8 +36,8 @@ impl TraitSolver {
     fn solve(
         &self,
         db: &impl HirDatabase,
-        goal: &chalk_ir::UCanonical<chalk_ir::InEnvironment<chalk_ir::Goal<ChalkIr>>>,
-    ) -> Option<chalk_solve::Solution<ChalkIr>> {
+        goal: &chalk_ir::UCanonical<chalk_ir::InEnvironment<chalk_ir::Goal<TypeFamily>>>,
+    ) -> Option<chalk_solve::Solution<TypeFamily>> {
         let context = ChalkContext { db, krate: self.krate };
         debug!("solve goal: {:?}", goal);
         let mut solver = match self.inner.lock() {
@@ -201,17 +201,17 @@ pub(crate) fn trait_solve_query(
 
 fn solution_from_chalk(
     db: &impl HirDatabase,
-    solution: chalk_solve::Solution<ChalkIr>,
+    solution: chalk_solve::Solution<TypeFamily>,
 ) -> Solution {
-    let convert_subst = |subst: chalk_ir::Canonical<chalk_ir::Substitution<ChalkIr>>| {
+    let convert_subst = |subst: chalk_ir::Canonical<chalk_ir::Substitution<TypeFamily>>| {
         let value = subst
             .value
             .parameters
             .into_iter()
             .map(|p| {
-                let ty = match p {
-                    chalk_ir::Parameter(chalk_ir::ParameterKind::Ty(ty)) => from_chalk(db, ty),
-                    chalk_ir::Parameter(chalk_ir::ParameterKind::Lifetime(_)) => unimplemented!(),
+                let ty = match p.ty() {
+                    Some(ty) => from_chalk(db, ty.clone()),
+                    None => unimplemented!(),
                 };
                 ty
             })
@@ -291,7 +291,7 @@ impl FnTrait {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct ClosureFnTraitImplData {
     def: DefWithBodyId,
     expr: ExprId,
@@ -300,7 +300,7 @@ pub struct ClosureFnTraitImplData {
 
 /// An impl. Usually this comes from an impl block, but some built-in types get
 /// synthetic impls.
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum Impl {
     /// A normal impl from an impl block.
     ImplBlock(ImplId),

crates/ra_hir_ty/src/traits.rs

@@ -28,24 +28,24 @@ pub(super) fn get_builtin_impls(
     trait_: TraitId,
     mut callback: impl FnMut(Impl),
 ) {
+    // Note: since impl_datum needs to be infallible, we need to make sure here
+    // that we have all prerequisites to build the respective impls.
     if let Ty::Apply(ApplicationTy { ctor: TypeCtor::Closure { def, expr }, .. }) = ty {
         for &fn_trait in [super::FnTrait::FnOnce, super::FnTrait::FnMut, super::FnTrait::Fn].iter()
         {
             if let Some(actual_trait) = get_fn_trait(db, krate, fn_trait) {
                 if trait_ == actual_trait {
                     let impl_ = super::ClosureFnTraitImplData { def: *def, expr: *expr, fn_trait };
-                    callback(Impl::ClosureFnTraitImpl(impl_));
+                    if check_closure_fn_trait_impl_prerequisites(db, krate, impl_) {
+                        callback(Impl::ClosureFnTraitImpl(impl_));
+                    }
                 }
             }
         }
     }
 }
 
-pub(super) fn impl_datum(
-    db: &impl HirDatabase,
-    krate: CrateId,
-    impl_: Impl,
-) -> Option<BuiltinImplData> {
+pub(super) fn impl_datum(db: &impl HirDatabase, krate: CrateId, impl_: Impl) -> BuiltinImplData {
     match impl_ {
         Impl::ImplBlock(_) => unreachable!(),
         Impl::ClosureFnTraitImpl(data) => closure_fn_trait_impl_datum(db, krate, data),
@@ -65,21 +65,38 @@ pub(super) fn associated_ty_value(
     }
 }
 
+fn check_closure_fn_trait_impl_prerequisites(
+    db: &impl HirDatabase,
+    krate: CrateId,
+    data: super::ClosureFnTraitImplData,
+) -> bool {
+    // the respective Fn/FnOnce/FnMut trait needs to exist
+    if get_fn_trait(db, krate, data.fn_trait).is_none() {
+        return false;
+    }
+
+    // FIXME: there are more assumptions that we should probably check here:
+    // the traits having no type params, FnOnce being a supertrait
+
+    // the FnOnce trait needs to exist and have an assoc type named Output
+    let fn_once_trait = match get_fn_trait(db, krate, super::FnTrait::FnOnce) {
+        Some(t) => t,
+        None => return false,
+    };
+    db.trait_data(fn_once_trait).associated_type_by_name(&name![Output]).is_some()
+}
+
 fn closure_fn_trait_impl_datum(
     db: &impl HirDatabase,
     krate: CrateId,
     data: super::ClosureFnTraitImplData,
-) -> Option<BuiltinImplData> {
+) -> BuiltinImplData {
     // for some closure |X, Y| -> Z:
     // impl<T, U, V> Fn<(T, U)> for closure<fn(T, U) -> V> { Output = V }
 
-    let trait_ = get_fn_trait(db, krate, data.fn_trait)?; // get corresponding fn trait
-
-    // validate FnOnce trait, since we need it in the assoc ty value definition
-    // and don't want to return a valid value only to find out later that FnOnce
-    // is broken
-    let fn_once_trait = get_fn_trait(db, krate, super::FnTrait::FnOnce)?;
-    let _output = db.trait_data(fn_once_trait).associated_type_by_name(&name![Output])?;
+    let trait_ = get_fn_trait(db, krate, data.fn_trait) // get corresponding fn trait
+        // the existence of the Fn trait has been checked before
+        .expect("fn trait for closure impl missing");
 
     let num_args: u16 = match &db.body(data.def.into())[data.expr] {
         Expr::Lambda { args, .. } => args.len() as u16,
@@ -107,12 +124,12 @@ fn closure_fn_trait_impl_datum(
 
     let output_ty_id = AssocTyValue::ClosureFnTraitImplOutput(data.clone());
 
-    Some(BuiltinImplData {
+    BuiltinImplData {
         num_vars: num_args as usize + 1,
         trait_ref,
         where_clauses: Vec::new(),
         assoc_ty_values: vec![output_ty_id],
-    })
+    }
 }
 
 fn closure_fn_trait_output_assoc_ty_value(