Merge #1266

1266: Chalk integration / method resolution fixes r=matklad a=flodiebold

 - fix impl blocks with unresolved target trait being treated as inherent impls
 - add traits from prelude for method resolution, and deduplicate them
 - blacklist some traits from being considered in where clauses, namely `Send`, `Sync`, `Sized`, and the `Fn` traits. We don't handle these correctly yet for several reasons, and this makes us much less likely to run into cases where Chalk gets very slow (because these usually only happen if there is no solution, and that's more likely to happen for these traits).
 - when there's an errored where clause, return just that one (since it will be always false anyway). This also makes things easier on Chalk ;)

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

4 files changed, 83 insertions, 26 deletions

diff --git a/crates/ra_hir/src/resolve.rs b/crates/ra_hir/src/resolve.rs
index 707556ef8..2fb219908 100644
--- a/crates/ra_hir/src/resolve.rs
+++ b/crates/ra_hir/src/resolve.rs
@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use ra_syntax::ast;
 
-use rustc_hash::FxHashMap;
+use rustc_hash::{FxHashMap, FxHashSet};
 
 use crate::{
     ModuleDef, Trait,
@@ -193,19 +193,18 @@ impl Resolver {
         names
     }
 
-    pub(crate) fn traits_in_scope<'a>(&'a self) -> impl Iterator<Item = Trait> + 'a {
-        // FIXME prelude
-        self.scopes
-            .iter()
-            .rev()
-            .flat_map(|scope| {
-                match scope {
-                    Scope::ModuleScope(m) => Some(m.crate_def_map[m.module_id].scope.traits()),
-                    _ => None,
+    pub(crate) fn traits_in_scope(&self, db: &impl HirDatabase) -> FxHashSet<Trait> {
+        let mut traits = FxHashSet::default();
+        for scope in &self.scopes {
+            if let Scope::ModuleScope(m) = scope {
+                if let Some(prelude) = m.crate_def_map.prelude() {
+                    let prelude_def_map = db.crate_def_map(prelude.krate);
+                    traits.extend(prelude_def_map[prelude.module_id].scope.traits());
                 }
-                .into_iter()
-            })
-            .flatten()
+                traits.extend(m.crate_def_map[m.module_id].scope.traits());
+            }
+        }
+        traits
     }
 
     fn module(&self) -> Option<(&CrateDefMap, CrateModuleId)> {
diff --git a/crates/ra_hir/src/ty/method_resolution.rs b/crates/ra_hir/src/ty/method_resolution.rs
index d8b8c836c..34817a5ec 100644
--- a/crates/ra_hir/src/ty/method_resolution.rs
+++ b/crates/ra_hir/src/ty/method_resolution.rs
@@ -75,11 +75,13 @@ impl CrateImplBlocks {
 
             let target_ty = impl_block.target_ty(db);
 
-            if let Some(tr) = impl_block.target_trait_ref(db) {
-                self.impls_by_trait
-                    .entry(tr.trait_)
-                    .or_insert_with(Vec::new)
-                    .push((module.module_id, impl_id));
+            if impl_block.target_trait(db).is_some() {
+                if let Some(tr) = impl_block.target_trait_ref(db) {
+                    self.impls_by_trait
+                        .entry(tr.trait_)
+                        .or_insert_with(Vec::new)
+                        .push((module.module_id, impl_id));
+                }
             } else {
                 if let Some(target_ty_fp) = TyFingerprint::for_impl(&target_ty) {
                     self.impls
@@ -183,7 +185,7 @@ fn iterate_trait_method_candidates<T>(
     mut callback: impl FnMut(&Ty, Function) -> Option<T>,
 ) -> Option<T> {
     let krate = resolver.krate()?;
-    'traits: for t in resolver.traits_in_scope() {
+    'traits: for t in resolver.traits_in_scope(db) {
         let data = t.trait_data(db);
         // we'll be lazy about checking whether the type implements the
         // trait, but if we find out it doesn't, we'll skip the rest of the
diff --git a/crates/ra_hir/src/ty/tests.rs b/crates/ra_hir/src/ty/tests.rs
index 59c85daed..978cc2587 100644
--- a/crates/ra_hir/src/ty/tests.rs
+++ b/crates/ra_hir/src/ty/tests.rs
@@ -2502,6 +2502,35 @@ fn test() { (&S).foo()<|>; }
 }
 
 #[test]
+fn method_resolution_trait_from_prelude() {
+    let (mut db, pos) = MockDatabase::with_position(
+        r#"
+//- /main.rs
+struct S;
+impl Clone for S {}
+
+fn test() {
+    S.clone()<|>;
+}
+
+//- /lib.rs
+#[prelude_import] use foo::*;
+
+mod foo {
+    trait Clone {
+        fn clone(&self) -> Self;
+    }
+}
+"#,
+    );
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "main": ("/main.rs", ["other_crate"]),
+        "other_crate": ("/lib.rs", []),
+    });
+    assert_eq!("S", type_at_pos(&db, pos));
+}
+
+#[test]
 fn method_resolution_where_clause_for_unknown_trait() {
     // The blanket impl shouldn't apply because we can't even resolve UnknownTrait
     let t = type_at(
@@ -2620,22 +2649,22 @@ fn method_resolution_slow() {
     let t = type_at(
         r#"
 //- /main.rs
-trait Send {}
+trait SendX {}
 
-struct S1; impl Send for S1;
-struct S2; impl Send for S2;
+struct S1; impl SendX for S1;
+struct S2; impl SendX for S2;
 struct U1;
 
 trait Trait { fn method(self); }
 
 struct X1<A, B> {}
-impl<A, B> Send for X1<A, B> where A: Send, B: Send {}
+impl<A, B> SendX for X1<A, B> where A: SendX, B: SendX {}
 
 struct S<B, C> {}
 
-trait Fn {}
+trait FnX {}
 
-impl<B, C> Trait for S<B, C> where C: Fn, B: Send {}
+impl<B, C> Trait for S<B, C> where C: FnX, B: SendX {}
 
 fn test() { (S {}).method()<|>; }
 "#,
diff --git a/crates/ra_hir/src/ty/traits/chalk.rs b/crates/ra_hir/src/ty/traits/chalk.rs
index 027c5ec4c..78440b258 100644
--- a/crates/ra_hir/src/ty/traits/chalk.rs
+++ b/crates/ra_hir/src/ty/traits/chalk.rs
@@ -190,6 +190,14 @@ fn make_binders<T>(value: T, num_vars: usize) -> chalk_ir::Binders<T> {
     }
 }
 
+fn blacklisted_trait(db: &impl HirDatabase, trait_: Trait) -> bool {
+    let name = trait_.name(db).unwrap_or_else(crate::Name::missing).to_string();
+    match &*name {
+        "Send" | "Sync" | "Sized" | "Fn" | "FnMut" | "FnOnce" => true,
+        _ => false,
+    }
+}
+
 fn convert_where_clauses(
     db: &impl HirDatabase,
     def: GenericDef,
@@ -198,6 +206,19 @@ fn convert_where_clauses(
     let generic_predicates = db.generic_predicates(def);
     let mut result = Vec::with_capacity(generic_predicates.len());
     for pred in generic_predicates.iter() {
+        if pred.is_error() {
+            // HACK: Return just the single predicate (which is always false
+            // anyway), otherwise Chalk can easily get into slow situations
+            return vec![pred.clone().subst(substs).to_chalk(db)];
+        }
+        match pred {
+            GenericPredicate::Implemented(trait_ref) => {
+                if blacklisted_trait(db, trait_ref.trait_) {
+                    continue;
+                }
+            }
+            _ => {}
+        }
         result.push(pred.clone().subst(substs).to_chalk(db));
     }
     result
@@ -230,6 +251,7 @@ where
             return Arc::new(TraitDatum { binders: make_binders(trait_datum_bound, 1) });
         }
         let trait_: Trait = from_chalk(self.db, trait_id);
+        debug!("trait {:?} = {:?}", trait_id, trait_.name(self.db));
         let generic_params = trait_.generic_params(self.db);
         let bound_vars = Substs::bound_vars(&generic_params);
         let trait_ref = trait_.trait_ref(self.db).subst(&bound_vars).to_chalk(self.db);
@@ -250,6 +272,7 @@ where
     fn struct_datum(&self, struct_id: chalk_ir::StructId) -> Arc<StructDatum> {
         debug!("struct_datum {:?}", struct_id);
         let type_ctor = from_chalk(self.db, struct_id);
+        debug!("struct {:?} = {:?}", struct_id, type_ctor);
         // FIXME might be nicer if we can create a fake GenericParams for the TypeCtor
         // FIXME extract this to a method on Ty
         let (num_params, where_clauses, upstream) = match type_ctor {
@@ -358,7 +381,11 @@ where
         if trait_id == UNKNOWN_TRAIT {
             return Vec::new();
         }
-        let trait_ = from_chalk(self.db, trait_id);
+        let trait_: Trait = from_chalk(self.db, trait_id);
+        let blacklisted = blacklisted_trait(self.db, trait_);
+        if blacklisted {
+            return Vec::new();
+        }
         let result: Vec<_> = self
             .db
             .impls_for_trait(self.krate, trait_)