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]>

Author: bors[bot]

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)> {

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

crates/ra_hir/src/ty/tests.rs

@@ -2501,6 +2501,35 @@ fn test() { (&S).foo()<|>; }
     assert_eq!(t, "u128");
 }
 
+#[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
@@ -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()<|>; }
 "#,

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_)