Repository: rust-analyzer/rust-analyzer
Pull request #3944 (2020-04-11 12:50:33)

Look up trait impls by self type
This speeds up inference in analysis-stats by ~30% (even more with the recursive solver).

There's a slight difference in inferred types, which I think comes from pre-existing wrong handling of error types in impls, so I think it's fine.


Patch:
diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
index 2d6d44980e..722f207e24 100644
--- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
@@ -40,7 +40,7 @@ enum AddMissingImplMembersMode {
 // }
 //
 // impl Trait<u32> for () {
-//     fn foo(&self) -> u32 { todo!() }
+//     fn foo(&self) -> u32 { unimplemented!() }
 //
 // }
 // ```
@@ -165,7 +165,7 @@ fn add_missing_impl_members_inner(
 
 fn add_body(fn_def: ast::FnDef) -> ast::FnDef {
     if fn_def.body().is_none() {
-        fn_def.with_body(make::block_from_expr(make::expr_todo()))
+        fn_def.with_body(make::block_from_expr(make::expr_unimplemented()))
     } else {
         fn_def
     }
@@ -215,8 +215,8 @@ impl Foo for S {
     fn bar(&self) {}
     <|>type Output;
     const CONST: usize = 42;
-    fn foo(&self) { todo!() }
-    fn baz(&self) { todo!() }
+    fn foo(&self) { unimplemented!() }
+    fn baz(&self) { unimplemented!() }
 
 }",
         );
@@ -250,7 +250,7 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    <|>fn foo(&self) { todo!() }
+    <|>fn foo(&self) { unimplemented!() }
 
 }",
         );
@@ -268,7 +268,7 @@ impl Foo for S { <|> }",
 trait Foo { fn foo(&self); }
 struct S;
 impl Foo for S {
-    <|>fn foo(&self) { todo!() }
+    <|>fn foo(&self) { unimplemented!() }
 }",
         );
     }
@@ -285,7 +285,7 @@ impl Foo<u32> for S { <|> }",
 trait Foo<T> { fn foo(&self, t: T) -> &T; }
 struct S;
 impl Foo<u32> for S {
-    <|>fn foo(&self, t: u32) -> &u32 { todo!() }
+    <|>fn foo(&self, t: u32) -> &u32 { unimplemented!() }
 }",
         );
     }
@@ -302,7 +302,7 @@ impl<U> Foo<U> for S { <|> }",
 trait Foo<T> { fn foo(&self, t: T) -> &T; }
 struct S;
 impl<U> Foo<U> for S {
-    <|>fn foo(&self, t: U) -> &U { todo!() }
+    <|>fn foo(&self, t: U) -> &U { unimplemented!() }
 }",
         );
     }
@@ -319,7 +319,7 @@ impl Foo for S {}<|>",
 trait Foo { fn foo(&self); }
 struct S;
 impl Foo for S {
-    <|>fn foo(&self) { todo!() }
+    <|>fn foo(&self) { unimplemented!() }
 }",
         )
     }
@@ -342,7 +342,7 @@ mod foo {
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar) { todo!() }
+    <|>fn foo(&self, bar: foo::Bar) { unimplemented!() }
 }",
         );
     }
@@ -365,7 +365,7 @@ mod foo {
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar<u32>) { todo!() }
+    <|>fn foo(&self, bar: foo::Bar<u32>) { unimplemented!() }
 }",
         );
     }
@@ -388,7 +388,7 @@ mod foo {
 }
 struct S;
 impl foo::Foo<u32> for S {
-    <|>fn foo(&self, bar: foo::Bar<u32>) { todo!() }
+    <|>fn foo(&self, bar: foo::Bar<u32>) { unimplemented!() }
 }",
         );
     }
@@ -414,7 +414,7 @@ mod foo {
 struct Param;
 struct S;
 impl foo::Foo<Param> for S {
-    <|>fn foo(&self, bar: Param) { todo!() }
+    <|>fn foo(&self, bar: Param) { unimplemented!() }
 }",
         );
     }
@@ -439,7 +439,7 @@ mod foo {
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar<u32>::Assoc) { todo!() }
+    <|>fn foo(&self, bar: foo::Bar<u32>::Assoc) { unimplemented!() }
 }",
         );
     }
@@ -464,7 +464,7 @@ mod foo {
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar<foo::Baz>) { todo!() }
+    <|>fn foo(&self, bar: foo::Bar<foo::Baz>) { unimplemented!() }
 }",
         );
     }
@@ -487,7 +487,7 @@ mod foo {
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: dyn Fn(u32) -> i32) { todo!() }
+    <|>fn foo(&self, bar: dyn Fn(u32) -> i32) { unimplemented!() }
 }",
         );
     }
@@ -544,7 +544,7 @@ trait Foo {
 struct S;
 impl Foo for S {
     <|>type Output;
-    fn foo(&self) { todo!() }
+    fn foo(&self) { unimplemented!() }
 }"#,
         )
     }
diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs
index 7c0d936910..be4b0accb8 100644
--- a/crates/ra_hir_def/src/adt.rs
+++ b/crates/ra_hir_def/src/adt.rs
@@ -4,6 +4,7 @@ use std::sync::Arc;
 
 use either::Either;
 use hir_expand::{
+    hygiene::Hygiene,
     name::{AsName, Name},
     InFile,
 };
@@ -12,7 +13,7 @@ use ra_prof::profile;
 use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner};
 
 use crate::{
-    body::CfgExpander, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace,
+    attr::Attrs, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace,
     type_ref::TypeRef, visibility::RawVisibility, EnumId, HasModule, LocalEnumVariantId,
     LocalStructFieldId, Lookup, ModuleId, StructId, UnionId, VariantId,
 };
@@ -124,9 +125,8 @@ fn lower_enum(
 
 impl VariantData {
     fn new(db: &dyn DefDatabase, flavor: InFile<ast::StructKind>, module_id: ModuleId) -> Self {
-        let mut expander = CfgExpander::new(db, flavor.file_id, module_id.krate);
         let mut trace = Trace::new_for_arena();
-        match lower_struct(db, &mut expander, &mut trace, &flavor) {
+        match lower_struct(db, &mut trace, &flavor, module_id) {
             StructKind::Tuple => VariantData::Tuple(trace.into_arena()),
             StructKind::Record => VariantData::Record(trace.into_arena()),
             StructKind::Unit => VariantData::Unit,
@@ -178,9 +178,8 @@ impl HasChildSource for VariantId {
                 it.lookup(db).container.module(db),
             ),
         };
-        let mut expander = CfgExpander::new(db, src.file_id, module_id.krate);
         let mut trace = Trace::new_for_map();
-        lower_struct(db, &mut expander, &mut trace, &src);
+        lower_struct(db, &mut trace, &src, module_id);
         src.with_value(trace.into_map())
     }
 }
@@ -194,15 +193,16 @@ pub enum StructKind {
 
 fn lower_struct(
     db: &dyn DefDatabase,
-    expander: &mut CfgExpander,
     trace: &mut Trace<StructFieldData, Either<ast::TupleFieldDef, ast::RecordFieldDef>>,
     ast: &InFile<ast::StructKind>,
+    module_id: ModuleId,
 ) -> StructKind {
+    let crate_graph = db.crate_graph();
     match &ast.value {
         ast::StructKind::Tuple(fl) => {
             for (i, fd) in fl.fields().enumerate() {
-                let attrs = expander.parse_attrs(&fd);
-                if !expander.is_cfg_enabled(&attrs) {
+                let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id));
+                if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
                     continue;
                 }
 
@@ -219,8 +219,8 @@ fn lower_struct(
         }
         ast::StructKind::Record(fl) => {
             for fd in fl.fields() {
-                let attrs = expander.parse_attrs(&fd);
-                if !expander.is_cfg_enabled(&attrs) {
+                let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id));
+                if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
                     continue;
                 }
 
diff --git a/crates/ra_hir_def/src/attr.rs b/crates/ra_hir_def/src/attr.rs
index 2f2e3e5ba2..7b0c506b16 100644
--- a/crates/ra_hir_def/src/attr.rs
+++ b/crates/ra_hir_def/src/attr.rs
@@ -93,7 +93,6 @@ impl Attrs {
     }
 
     pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> bool {
-        // FIXME: handle cfg_attr :-)
         self.by_key("cfg").tt_values().all(|tt| cfg_options.is_cfg_enabled(tt) != Some(false))
     }
 }
diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs
index 7fac6ce66e..e09996c6f8 100644
--- a/crates/ra_hir_def/src/body.rs
+++ b/crates/ra_hir_def/src/body.rs
@@ -14,7 +14,6 @@ use ra_syntax::{ast, AstNode, AstPtr};
 use rustc_hash::FxHashMap;
 
 use crate::{
-    attr::Attrs,
     db::DefDatabase,
     expr::{Expr, ExprId, Pat, PatId},
     item_scope::BuiltinShadowMode,
@@ -24,62 +23,26 @@ use crate::{
     src::HasSource,
     AsMacroCall, DefWithBodyId, HasModule, Lookup, ModuleId,
 };
-use ra_cfg::CfgOptions;
-use ra_db::CrateId;
-
-/// A subser of Exander that only deals with cfg attributes. We only need it to
-/// avoid cyclic queries in crate def map during enum processing.
-pub(crate) struct CfgExpander {
-    cfg_options: CfgOptions,
-    hygiene: Hygiene,
-}
 
 pub(crate) struct Expander {
-    cfg_expander: CfgExpander,
     crate_def_map: Arc<CrateDefMap>,
     current_file_id: HirFileId,
+    hygiene: Hygiene,
     ast_id_map: Arc<AstIdMap>,
     module: ModuleId,
     recursive_limit: usize,
 }
 
-impl CfgExpander {
-    pub(crate) fn new(
-        db: &dyn DefDatabase,
-        current_file_id: HirFileId,
-        krate: CrateId,
-    ) -> CfgExpander {
-        let hygiene = Hygiene::new(db.upcast(), current_file_id);
-        let cfg_options = db.crate_graph()[krate].cfg_options.clone();
-        CfgExpander { cfg_options, hygiene }
-    }
-
-    pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs {
-        Attrs::new(owner, &self.hygiene)
-    }
-
-    pub(crate) fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
-        attrs.is_cfg_enabled(&self.cfg_options)
-    }
-}
-
 impl Expander {
     pub(crate) fn new(
         db: &dyn DefDatabase,
         current_file_id: HirFileId,
         module: ModuleId,
     ) -> Expander {
-        let cfg_expander = CfgExpander::new(db, current_file_id, module.krate);
         let crate_def_map = db.crate_def_map(module.krate);
+        let hygiene = Hygiene::new(db.upcast(), current_file_id);
         let ast_id_map = db.ast_id_map(current_file_id);
-        Expander {
-            cfg_expander,
-            crate_def_map,
-            current_file_id,
-            ast_id_map,
-            module,
-            recursive_limit: 0,
-        }
+        Expander { crate_def_map, current_file_id, hygiene, ast_id_map, module, recursive_limit: 0 }
     }
 
     pub(crate) fn enter_expand<T: ast::AstNode>(
@@ -112,7 +75,7 @@ impl Expander {
                         ast_id_map: mem::take(&mut self.ast_id_map),
                         bomb: DropBomb::new("expansion mark dropped"),
                     };
-                    self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id);
+                    self.hygiene = Hygiene::new(db.upcast(), file_id);
                     self.current_file_id = file_id;
                     self.ast_id_map = db.ast_id_map(file_id);
                     self.recursive_limit += 1;
@@ -128,7 +91,7 @@ impl Expander {
     }
 
     pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
-        self.cfg_expander.hygiene = Hygiene::new(db.upcast(), mark.file_id);
+        self.hygiene = Hygiene::new(db.upcast(), mark.file_id);
         self.current_file_id = mark.file_id;
         self.ast_id_map = mem::take(&mut mark.ast_id_map);
         self.recursive_limit -= 1;
@@ -139,16 +102,8 @@ impl Expander {
         InFile { file_id: self.current_file_id, value }
     }
 
-    pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs {
-        self.cfg_expander.parse_attrs(owner)
-    }
-
-    pub(crate) fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
-        self.cfg_expander.is_cfg_enabled(attrs)
-    }
-
     fn parse_path(&mut self, path: ast::Path) -> Option<Path> {
-        Path::from_src(path, &self.cfg_expander.hygiene)
+        Path::from_src(path, &self.hygiene)
     }
 
     fn resolve_path_as_macro(&self, db: &dyn DefDatabase, path: &ModPath) -> Option<MacroDefId> {
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index c1d7eb826a..9d6ee095ef 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -2,7 +2,9 @@
 //! representation.
 
 use either::Either;
+
 use hir_expand::{
+    hygiene::Hygiene,
     name::{name, AsName, Name},
     MacroDefId, MacroDefKind,
 };
@@ -16,8 +18,10 @@ use ra_syntax::{
 };
 use test_utils::tested_by;
 
+use super::{ExprSource, PatSource};
 use crate::{
     adt::StructKind,
+    attr::Attrs,
     body::{Body, BodySourceMap, Expander, PatPtr, SyntheticSyntax},
     builtin_type::{BuiltinFloat, BuiltinInt},
     db::DefDatabase,
@@ -29,12 +33,10 @@ use crate::{
     path::GenericArgs,
     path::Path,
     type_ref::{Mutability, TypeRef},
-    AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId,
-    StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
+    AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, HasModule, Intern,
+    ModuleDefId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
 };
 
-use super::{ExprSource, PatSource};
-
 pub(super) fn lower(
     db: &dyn DefDatabase,
     def: DefWithBodyId,
@@ -298,6 +300,7 @@ impl ExprCollector<'_> {
                 self.alloc_expr(Expr::Return { expr }, syntax_ptr)
             }
             ast::Expr::RecordLit(e) => {
+                let crate_graph = self.db.crate_graph();
                 let path = e.path().and_then(|path| self.expander.parse_path(path));
                 let mut field_ptrs = Vec::new();
                 let record_lit = if let Some(nfl) = e.record_field_list() {
@@ -305,8 +308,13 @@ impl ExprCollector<'_> {
                         .fields()
                         .inspect(|field| field_ptrs.push(AstPtr::new(field)))
                         .filter_map(|field| {
-                            let attrs = self.expander.parse_attrs(&field);
-                            if !self.expander.is_cfg_enabled(&attrs) {
+                            let module_id = ContainerId::DefWithBodyId(self.def).module(self.db);
+                            let attrs = Attrs::new(
+                                &field,
+                                &Hygiene::new(self.db.upcast(), self.expander.current_file_id),
+                            );
+
+                            if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
                                 return None;
                             }
 
diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs
index 56a20c5bd1..b8fbf0ed4b 100644
--- a/crates/ra_hir_def/src/data.rs
+++ b/crates/ra_hir_def/src/data.rs
@@ -20,7 +20,7 @@ use crate::{
     type_ref::{Mutability, TypeBound, TypeRef},
     visibility::RawVisibility,
     AssocContainerId, AssocItemId, ConstId, ConstLoc, Expander, FunctionId, FunctionLoc, HasModule,
-    ImplId, Intern, Lookup, StaticId, TraitId, TypeAliasId, TypeAliasLoc,
+    ImplId, Intern, Lookup, ModuleId, StaticId, TraitId, TypeAliasId, TypeAliasLoc,
 };
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -218,17 +218,10 @@ impl ImplData {
         let mut items = Vec::new();
 
         if let Some(item_list) = src.value.item_list() {
-            let mut expander = Expander::new(db, impl_loc.ast_id.file_id, module_id);
-            items.extend(collect_impl_items(
-                db,
-                &mut expander,
-                item_list.impl_items(),
-                src.file_id,
-                id,
-            ));
+            items.extend(collect_impl_items(db, item_list.impl_items(), src.file_id, id));
             items.extend(collect_impl_items_in_macros(
                 db,
-                &mut expander,
+                module_id,
                 &src.with_value(item_list),
                 id,
             ));
@@ -275,17 +268,18 @@ impl ConstData {
 
 fn collect_impl_items_in_macros(
     db: &dyn DefDatabase,
-    expander: &mut Expander,
+    module_id: ModuleId,
     impl_def: &InFile<ast::ItemList>,
     id: ImplId,
 ) -> Vec<AssocItemId> {
+    let mut expander = Expander::new(db, impl_def.file_id, module_id);
     let mut res = Vec::new();
 
     // We set a limit to protect against infinite recursion
     let limit = 100;
 
     for m in impl_def.value.syntax().children().filter_map(ast::MacroCall::cast) {
-        res.extend(collect_impl_items_in_macro(db, expander, m, id, limit))
+        res.extend(collect_impl_items_in_macro(db, &mut expander, m, id, limit))
     }
 
     res
@@ -306,7 +300,6 @@ fn collect_impl_items_in_macro(
         let items: InFile<ast::MacroItems> = expander.to_source(items);
         let mut res = collect_impl_items(
             db,
-            expander,
             items.value.items().filter_map(|it| ImplItem::cast(it.syntax().clone())),
             items.file_id,
             id,
@@ -326,26 +319,32 @@ fn collect_impl_items_in_macro(
 
 fn collect_impl_items(
     db: &dyn DefDatabase,
-    expander: &mut Expander,
     impl_items: impl Iterator<Item = ImplItem>,
     file_id: crate::HirFileId,
     id: ImplId,
 ) -> Vec<AssocItemId> {
     let items = db.ast_id_map(file_id);
+    let crate_graph = db.crate_graph();
+    let module_id = id.lookup(db).container.module(db);
 
     impl_items
         .filter_map(|item_node| match item_node {
             ast::ImplItem::FnDef(it) => {
-                let attrs = expander.parse_attrs(&it);
-                if !expander.is_cfg_enabled(&attrs) {
-                    return None;
-                }
                 let def = FunctionLoc {
                     container: AssocContainerId::ImplId(id),
                     ast_id: AstId::new(file_id, items.ast_id(&it)),
                 }
                 .intern(db);
-                Some(def.into())
+
+                if !db
+                    .function_data(def)
+                    .attrs
+                    .is_cfg_enabled(&crate_graph[module_id.krate].cfg_options)
+                {
+                    None
+                } else {
+                    Some(def.into())
+                }
             }
             ast::ImplItem::ConstDef(it) => {
                 let def = ConstLoc {
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index 98c74fe257..8fe3f8617f 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -462,14 +462,6 @@ impl DefCollector<'_> {
                 Some(ModuleDefId::AdtId(AdtId::EnumId(e))) => {
                     tested_by!(glob_enum);
                     // glob import from enum => just import all the variants
-
-                    // XXX: urgh, so this works by accident! Here, we look at
-                    // the enum data, and, in theory, this might require us to
-                    // look back at the crate_def_map, creating a cycle. For
-                    // example, `enum E { crate::some_macro!(); }`. Luckely, the
-                    // only kind of macro that is allowed inside enum is a
-                    // `cfg_macro`, and we don't need to run name resolution for
-                    // it, but this is sheer luck!
                     let enum_data = self.db.enum_data(e);
                     let resolutions = enum_data
                         .variants
@@ -985,7 +977,11 @@ impl ModCollector<'_, '_> {
     }
 
     fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
-        attrs.is_cfg_enabled(self.def_collector.cfg_options)
+        // FIXME: handle cfg_attr :-)
+        attrs
+            .by_key("cfg")
+            .tt_values()
+            .all(|tt| self.def_collector.cfg_options.is_cfg_enabled(tt) != Some(false))
     }
 }
 
diff --git a/crates/ra_hir_ty/src/db.rs b/crates/ra_hir_ty/src/db.rs
index 1462b053f0..33da16b487 100644
--- a/crates/ra_hir_ty/src/db.rs
+++ b/crates/ra_hir_ty/src/db.rs
@@ -11,7 +11,7 @@ use ra_db::{impl_intern_key, salsa, CrateId, Upcast};
 use ra_prof::profile;
 
 use crate::{
-    method_resolution::CrateImplDefs,
+    method_resolution::{CrateImplDefs, TyFingerprint},
     traits::{chalk, AssocTyValue, Impl},
     Binders, CallableDef, GenericPredicate, InferenceResult, PolyFnSig, Substs, TraitRef, Ty,
     TyDefId, TypeCtor, ValueTyDefId,
@@ -65,7 +65,12 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
     fn impls_in_crate(&self, krate: CrateId) -> Arc<CrateImplDefs>;
 
     #[salsa::invoke(crate::traits::impls_for_trait_query)]
-    fn impls_for_trait(&self, krate: CrateId, trait_: TraitId) -> Arc<[ImplId]>;
+    fn impls_for_trait(
+        &self,
+        krate: CrateId,
+        trait_: TraitId,
+        self_ty_fp: Option<TyFingerprint>,
+    ) -> Arc<[ImplId]>;
 
     // Interned IDs for Chalk integration
     #[salsa::interned]
diff --git a/crates/ra_hir_ty/src/method_resolution.rs b/crates/ra_hir_ty/src/method_resolution.rs
index 74a0bc7db8..657284fd01 100644
--- a/crates/ra_hir_ty/src/method_resolution.rs
+++ b/crates/ra_hir_ty/src/method_resolution.rs
@@ -34,7 +34,7 @@ impl TyFingerprint {
     /// Creates a TyFingerprint for looking up an impl. Only certain types can
     /// have impls: if we have some `struct S`, we can have an `impl S`, but not
     /// `impl &S`. Hence, this will return `None` for reference types and such.
-    fn for_impl(ty: &Ty) -> Option<TyFingerprint> {
+    pub(crate) fn for_impl(ty: &Ty) -> Option<TyFingerprint> {
         match ty {
             Ty::Apply(a_ty) => Some(TyFingerprint::Apply(a_ty.ctor)),
             _ => None,
@@ -45,7 +45,7 @@ impl TyFingerprint {
 #[derive(Debug, PartialEq, Eq)]
 pub struct CrateImplDefs {
     impls: FxHashMap<TyFingerprint, Vec<ImplId>>,
-    impls_by_trait: FxHashMap<TraitId, Vec<ImplId>>,
+    impls_by_trait: FxHashMap<TraitId, FxHashMap<Option<TyFingerprint>, Vec<ImplId>>>,
 }
 
 impl CrateImplDefs {
@@ -59,7 +59,14 @@ impl CrateImplDefs {
             for impl_id in module_data.scope.impls() {
                 match db.impl_trait(impl_id) {
                     Some(tr) => {
-                        res.impls_by_trait.entry(tr.value.trait_).or_default().push(impl_id);
+                        let self_ty = db.impl_self_ty(impl_id);
+                        let self_ty_fp = TyFingerprint::for_impl(&self_ty.value);
+                        res.impls_by_trait
+                            .entry(tr.value.trait_)
+                            .or_default()
+                            .entry(self_ty_fp)
+                            .or_default()
+                            .push(impl_id);
                     }
                     None => {
                         let self_ty = db.impl_self_ty(impl_id);
@@ -79,11 +86,39 @@ impl CrateImplDefs {
     }
 
     pub fn lookup_impl_defs_for_trait(&self, tr: TraitId) -> impl Iterator<Item = ImplId> + '_ {
-        self.impls_by_trait.get(&tr).into_iter().flatten().copied()
+        self.impls_by_trait
+            .get(&tr)
+            .into_iter()
+            .flat_map(|m| m.values().flat_map(|v| v.iter().copied()))
+    }
+
+    pub fn lookup_impl_defs_for_trait_and_ty(
+        &self,
+        tr: TraitId,
+        fp: TyFingerprint,
+    ) -> impl Iterator<Item = ImplId> + '_ {
+        self.impls_by_trait
+            .get(&tr)
+            .and_then(|m| m.get(&Some(fp)))
+            .into_iter()
+            .flatten()
+            .copied()
+            .chain(
+                self.impls_by_trait
+                    .get(&tr)
+                    .and_then(|m| m.get(&None))
+                    .into_iter()
+                    .flatten()
+                    .copied(),
+            )
     }
 
     pub fn all_impls<'a>(&'a self) -> impl Iterator<Item = ImplId> + 'a {
-        self.impls.values().chain(self.impls_by_trait.values()).flatten().copied()
+        self.impls
+            .values()
+            .chain(self.impls_by_trait.values().flat_map(|m| m.values()))
+            .flatten()
+            .copied()
     }
 }
 
diff --git a/crates/ra_hir_ty/src/traits.rs b/crates/ra_hir_ty/src/traits.rs
index 21e2333799..43d8d1e802 100644
--- a/crates/ra_hir_ty/src/traits.rs
+++ b/crates/ra_hir_ty/src/traits.rs
@@ -7,7 +7,7 @@ use ra_db::{impl_intern_key, salsa, CrateId};
 use ra_prof::profile;
 use rustc_hash::FxHashSet;
 
-use crate::{db::HirDatabase, DebruijnIndex};
+use crate::{db::HirDatabase, method_resolution::TyFingerprint, DebruijnIndex};
 
 use super::{Canonical, GenericPredicate, HirDisplay, ProjectionTy, TraitRef, Ty, TypeWalk};
 
@@ -40,7 +40,12 @@ pub(crate) fn impls_for_trait_query(
     db: &dyn HirDatabase,
     krate: CrateId,
     trait_: TraitId,
+    self_ty_fp: Option<TyFingerprint>,
 ) -> Arc<[ImplId]> {
+    // FIXME: We could be a lot smarter here - because of the orphan rules and
+    // the fact that the trait and the self type need to be in the dependency
+    // tree of a crate somewhere for an impl to exist, we could skip looking in
+    // a lot of crates completely
     let mut impls = FxHashSet::default();
     // We call the query recursively here. On the one hand, this means we can
     // reuse results from queries for different crates; on the other hand, this
@@ -48,10 +53,13 @@ pub(crate) fn impls_for_trait_query(
     // ones the user is editing), so this may actually be a waste of memory. I'm
     // doing it like this mainly for simplicity for now.
     for dep in &db.crate_graph()[krate].dependencies {
-        impls.extend(db.impls_for_trait(dep.crate_id, trait_).iter());
+        impls.extend(db.impls_for_trait(dep.crate_id, trait_, self_ty_fp).iter());
     }
     let crate_impl_defs = db.impls_in_crate(krate);
-    impls.extend(crate_impl_defs.lookup_impl_defs_for_trait(trait_));
+    match self_ty_fp {
+        Some(fp) => impls.extend(crate_impl_defs.lookup_impl_defs_for_trait_and_ty(trait_, fp)),
+        None => impls.extend(crate_impl_defs.lookup_impl_defs_for_trait(trait_)),
+    }
     impls.into_iter().collect()
 }
 
diff --git a/crates/ra_hir_ty/src/traits/chalk.rs b/crates/ra_hir_ty/src/traits/chalk.rs
index c5f1b52324..e05fea8430 100644
--- a/crates/ra_hir_ty/src/traits/chalk.rs
+++ b/crates/ra_hir_ty/src/traits/chalk.rs
@@ -16,8 +16,8 @@ use ra_db::{
 
 use super::{builtin, AssocTyValue, Canonical, ChalkContext, Impl, Obligation};
 use crate::{
-    db::HirDatabase, display::HirDisplay, utils::generics, ApplicationTy, GenericPredicate,
-    ProjectionTy, Substs, TraitRef, Ty, TypeCtor,
+    db::HirDatabase, display::HirDisplay, method_resolution::TyFingerprint, utils::generics,
+    ApplicationTy, GenericPredicate, ProjectionTy, Substs, TraitRef, Ty, TypeCtor,
 };
 
 pub(super) mod tls;
@@ -647,19 +647,22 @@ impl<'a> chalk_solve::RustIrDatabase<Interner> for ChalkContext<'a> {
         debug!("impls_for_trait {:?}", trait_id);
         let trait_: hir_def::TraitId = from_chalk(self.db, trait_id);
 
+        let ty: Ty = from_chalk(self.db, parameters[0].assert_ty_ref(&Interner).clone());
+
+        let self_ty_fp = TyFingerprint::for_impl(&ty);
+
         // Note: Since we're using impls_for_trait, only impls where the trait
         // can be resolved should ever reach Chalk. `impl_datum` relies on that
         // and will panic if the trait can't be resolved.
         let mut result: Vec<_> = self
             .db
-            .impls_for_trait(self.krate, trait_)
+            .impls_for_trait(self.krate, trait_, self_ty_fp)
             .iter()
             .copied()
             .map(Impl::ImplDef)
             .map(|impl_| impl_.to_chalk(self.db))
             .collect();
 
-        let ty: Ty = from_chalk(self.db, parameters[0].assert_ty_ref(&Interner).clone());
         let arg: Option<Ty> =
             parameters.get(1).map(|p| from_chalk(self.db, p.assert_ty_ref(&Interner).clone()));
 
diff --git a/crates/ra_syntax/src/ast/make.rs b/crates/ra_syntax/src/ast/make.rs
index 0f4a50be47..f39559e9ea 100644
--- a/crates/ra_syntax/src/ast/make.rs
+++ b/crates/ra_syntax/src/ast/make.rs
@@ -100,9 +100,6 @@ pub fn expr_empty_block() -> ast::Expr {
 pub fn expr_unimplemented() -> ast::Expr {
     expr_from_text("unimplemented!()")
 }
-pub fn expr_todo() -> ast::Expr {
-    expr_from_text("todo!()")
-}
 pub fn expr_path(path: ast::Path) -> ast::Expr {
     expr_from_text(&path.to_string())
 }
diff --git a/docs/user/assists.md b/docs/user/assists.md
index cc42169d8e..754131f6fe 100644
--- a/docs/user/assists.md
+++ b/docs/user/assists.md
@@ -175,7 +175,7 @@ trait Trait<T> {
 }
 
 impl Trait<u32> for () {
-    fn foo(&self) -> u32 { todo!() }
+    fn foo(&self) -> u32 { unimplemented!() }
 
 }
 ```
diff --git a/editors/code/src/config.ts b/editors/code/src/config.ts
index 35a05131c8..21c1c9f232 100644
--- a/editors/code/src/config.ts
+++ b/editors/code/src/config.ts
@@ -66,44 +66,23 @@ export class Config {
         return vscode.workspace.getConfiguration(this.rootSection);
     }
 
-    /**
-     * Beware that postfix `!` operator erases both `null` and `undefined`.
-     * This is why the following doesn't work as expected:
-     *
-     * ```ts
-     * const nullableNum = vscode
-     *  .workspace
-     *  .getConfiguration
-     *  .getConfiguration("rust-analyer")
-     *  .get<number | null>(path)!;
-     *
-     * // What happens is that type of `nullableNum` is `number` but not `null | number`:
-     * const fullFledgedNum: number = nullableNum;
-     * ```
-     * So this getter handles this quirk by not requiring the caller to use postfix `!`
-     */
-    private get<T>(path: string): T {
-        return this.cfg.get<T>(path)!;
-    }
-
-    get serverPath() { return this.get<null | string>("serverPath"); }
-    get channel() { return this.get<UpdatesChannel>("updates.channel"); }
-    get askBeforeDownload() { return this.get<boolean>("updates.askBeforeDownload"); }
-    get traceExtension() { return this.get<boolean>("trace.extension"); }
-
+    get serverPath() { return this.cfg.get<null | string>("serverPath")!; }
+    get channel() { return this.cfg.get<UpdatesChannel>("updates.channel")!; }
+    get askBeforeDownload() { return this.cfg.get<boolean>("updates.askBeforeDownload")!; }
+    get traceExtension() { return this.cfg.get<boolean>("trace.extension")!; }
 
     get inlayHints() {
         return {
-            typeHints: this.get<boolean>("inlayHints.typeHints"),
-            parameterHints: this.get<boolean>("inlayHints.parameterHints"),
-            chainingHints: this.get<boolean>("inlayHints.chainingHints"),
-            maxLength: this.get<null | number>("inlayHints.maxLength"),
+            typeHints: this.cfg.get<boolean>("inlayHints.typeHints")!,
+            parameterHints: this.cfg.get<boolean>("inlayHints.parameterHints")!,
+            chainingHints: this.cfg.get<boolean>("inlayHints.chainingHints")!,
+            maxLength: this.cfg.get<null | number>("inlayHints.maxLength")!,
         };
     }
 
     get checkOnSave() {
         return {
-            command: this.get<string>("checkOnSave.command"),
+            command: this.cfg.get<string>("checkOnSave.command")!,
         };
     }
 }
diff --git a/editors/code/src/inlay_hints.ts b/editors/code/src/inlay_hints.ts
index da74f03d2d..6a8bd942e1 100644
--- a/editors/code/src/inlay_hints.ts
+++ b/editors/code/src/inlay_hints.ts
@@ -229,13 +229,13 @@ interface InlaysDecorations {
 }
 
 interface RustSourceFile {
-    /**
-     * Source of the token to cancel in-flight inlay hints request if any.
-     */
+    /*
+    * Source of the token to cancel in-flight inlay hints request if any.
+    */
     inlaysRequest: null | vscode.CancellationTokenSource;
     /**
-     * Last applied decorations.
-     */
+    * Last applied decorations.
+    */
     cachedDecorations: null | InlaysDecorations;
 
     document: RustDocument;


Test patch:
diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs
index 24392836c0..64444ee3aa 100644
--- a/crates/ra_assists/src/doc_tests/generated.rs
+++ b/crates/ra_assists/src/doc_tests/generated.rs
@@ -180,7 +180,7 @@ trait Trait<T> {
 }
 
 impl Trait<u32> for () {
-    fn foo(&self) -> u32 { todo!() }
+    fn foo(&self) -> u32 { unimplemented!() }
 
 }
 "#####,
diff --git a/crates/ra_hir_def/src/nameres/tests/incremental.rs b/crates/ra_hir_def/src/nameres/tests/incremental.rs
index 87165ac33a..496fc6b081 100644
--- a/crates/ra_hir_def/src/nameres/tests/incremental.rs
+++ b/crates/ra_hir_def/src/nameres/tests/incremental.rs
@@ -32,9 +32,6 @@ fn typing_inside_a_function_should_not_invalidate_def_map() {
 
         use crate::foo::bar::Baz;
 
-        enum E { A, B }
-        use E::*;
-
         fn foo() -> i32 {
             1 + 1
         }
@@ -49,9 +46,6 @@ fn typing_inside_a_function_should_not_invalidate_def_map() {
 
         use crate::foo::bar::Baz;
 
-        enum E { A, B }
-        use E::*;
-
         fn foo() -> i32 { 92 }
         ",
     );
diff --git a/xtask/tests/tidy-tests/main.rs b/xtask/tests/tidy-tests/main.rs
index 4f525fcd04..80911a68e8 100644
--- a/xtask/tests/tidy-tests/main.rs
+++ b/xtask/tests/tidy-tests/main.rs
@@ -20,16 +20,7 @@ fn rust_files_are_tidy() {
 }
 
 fn check_todo(path: &Path, text: &str) {
-    let whitelist = &[
-        // This file itself is whitelisted since this test itself contains matches.
-        "tests/cli.rs",
-        // Some of our assists generate `todo!()` so those files are whitelisted.
-        "doc_tests/generated.rs",
-        "handlers/add_missing_impl_members.rs",
-        // To support generating `todo!()` in assists, we have `expr_todo()` in ast::make.
-        "ast/make.rs",
-    ];
-    if whitelist.iter().any(|p| path.ends_with(p)) {
+    if path.ends_with("tests/cli.rs") {
         return;
     }
     if text.contains("TODO") || text.contains("TOOD") || text.contains("todo!") {