From 8613d1bd36702f595f8c6b0ffff4b26e9e199858 Mon Sep 17 00:00:00 2001 From: David Barsky Date: Tue, 13 Feb 2024 13:25:03 -0500 Subject: [PATCH] completions: speed up completions by filtering non-applicable traits --- crates/hir-ty/src/infer/unify.rs | 1 + crates/hir-ty/src/method_resolution.rs | 79 +++++++- crates/hir-ty/src/traits.rs | 1 + crates/hir/src/lib.rs | 4 + crates/ide-completion/src/tests/flyimport.rs | 180 +++++++++++++++++++ crates/ide-db/src/imports/import_assets.rs | 33 +++- 6 files changed, 294 insertions(+), 4 deletions(-) diff --git a/crates/hir-ty/src/infer/unify.rs b/crates/hir-ty/src/infer/unify.rs index 709760b64fd3..4a181f4a325c 100644 --- a/crates/hir-ty/src/infer/unify.rs +++ b/crates/hir-ty/src/infer/unify.rs @@ -457,6 +457,7 @@ impl<'a> InferenceTable<'a> { } /// Unify two relatable values (e.g. `Ty`) and register new trait goals that arise from that. + #[tracing::instrument(skip_all)] pub(crate) fn unify>(&mut self, ty1: &T, ty2: &T) -> bool { let result = match self.try_unify(ty1, ty2) { Ok(r) => r, diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index a4baf572d9e3..4bb412d01c28 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -254,6 +254,11 @@ impl TraitImpls { .flat_map(|v| v.iter().copied()) } + /// Queries whether `self_ty` has potentially applicable implementations of `trait_`. + pub fn has_impls_for_trait_and_self_ty(&self, trait_: TraitId, self_ty: TyFingerprint) -> bool { + self.for_trait_and_self_ty(trait_, self_ty).next().is_some() + } + pub fn all_impls(&self) -> impl Iterator + '_ { self.map.values().flat_map(|map| map.values().flat_map(|v| v.iter().copied())) } @@ -1170,7 +1175,7 @@ fn iterate_trait_method_candidates( for &(_, item) in data.items.iter() { // Don't pass a `visible_from_module` down to `is_valid_candidate`, // since only inherent methods should be included into visibility checking. - let visible = match is_valid_candidate(table, name, receiver_ty, item, self_ty, None) { + let visible = match is_valid_method_candidate(table, name, receiver_ty, item, self_ty) { IsValidCandidate::Yes => true, IsValidCandidate::NotVisible => false, IsValidCandidate::No => continue, @@ -1414,6 +1419,74 @@ fn is_valid_candidate( } } +/// Checks whether a given `AssocItemId` is applicable for `receiver_ty`. +/// +/// This method should *only* be called by [`iterate_trait_method_candidates`], +/// as it is responsible for determining applicability in completions. +#[tracing::instrument(skip_all, fields(name))] +fn is_valid_method_candidate( + table: &mut InferenceTable<'_>, + name: Option<&Name>, + receiver_ty: Option<&Ty>, + item: AssocItemId, + self_ty: &Ty, +) -> IsValidCandidate { + let db = table.db; + match item { + AssocItemId::FunctionId(fn_id) => { + let data = db.function_data(fn_id); + + check_that!(name.map_or(true, |n| n == &data.name)); + + table.run_in_snapshot(|table| { + let container = fn_id.lookup(db.upcast()).container; + let (impl_subst, expect_self_ty) = match container { + ItemContainerId::ImplId(it) => { + let subst = TyBuilder::subst_for_def(db, it, None) + .fill_with_inference_vars(table) + .build(); + let self_ty = db.impl_self_ty(it).substitute(Interner, &subst); + (subst, self_ty) + } + ItemContainerId::TraitId(it) => { + let subst = TyBuilder::subst_for_def(db, it, None) + .fill_with_inference_vars(table) + .build(); + let self_ty = subst.at(Interner, 0).assert_ty_ref(Interner).clone(); + (subst, self_ty) + } + _ => unreachable!(), + }; + + check_that!(table.unify(&expect_self_ty, self_ty)); + + if let Some(receiver_ty) = receiver_ty { + check_that!(data.has_self_param()); + + let fn_subst = TyBuilder::subst_for_def(db, fn_id, Some(impl_subst.clone())) + .fill_with_inference_vars(table) + .build(); + + let sig = db.callable_item_signature(fn_id.into()); + let expected_receiver = + sig.map(|s| s.params()[0].clone()).substitute(Interner, &fn_subst); + + check_that!(table.unify(receiver_ty, &expected_receiver)); + } + + IsValidCandidate::Yes + }) + } + AssocItemId::ConstId(c) => { + check_that!(receiver_ty.is_none()); + check_that!(name.map_or(true, |n| db.const_data(c).name.as_ref() == Some(n))); + + IsValidCandidate::Yes + } + _ => IsValidCandidate::No, + } +} + enum IsValidCandidate { Yes, No, @@ -1441,6 +1514,8 @@ fn is_valid_fn_candidate( } table.run_in_snapshot(|table| { let container = fn_id.lookup(db.upcast()).container; + + let _p = tracing::span!(tracing::Level::INFO, "subst_for_def").entered(); let (impl_subst, expect_self_ty) = match container { ItemContainerId::ImplId(it) => { let subst = @@ -1460,6 +1535,7 @@ fn is_valid_fn_candidate( check_that!(table.unify(&expect_self_ty, self_ty)); if let Some(receiver_ty) = receiver_ty { + let _p = tracing::span!(tracing::Level::INFO, "check_receiver_ty").entered(); check_that!(data.has_self_param()); let fn_subst = TyBuilder::subst_for_def(db, fn_id, Some(impl_subst.clone())) @@ -1474,6 +1550,7 @@ fn is_valid_fn_candidate( } if let ItemContainerId::ImplId(impl_id) = container { + let _p = tracing::span!(tracing::Level::INFO, "check_item_container").entered(); // We need to consider the bounds on the impl to distinguish functions of the same name // for a type. let predicates = db.generic_predicates(impl_id.into()); diff --git a/crates/hir-ty/src/traits.rs b/crates/hir-ty/src/traits.rs index b2232b920aa0..71a4ab020b35 100644 --- a/crates/hir-ty/src/traits.rs +++ b/crates/hir-ty/src/traits.rs @@ -139,6 +139,7 @@ fn solve( block: Option, goal: &chalk_ir::UCanonical>>, ) -> Option> { + let _p = tracing::span!(tracing::Level::INFO, "solve", ?krate, ?block).entered(); let context = ChalkContext { db, krate, block }; tracing::debug!("solve goal: {:?}", goal); let mut solver = create_chalk_solver(); diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 2d8811cf5ebe..cdcd44be9915 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -4239,6 +4239,10 @@ impl Type { } } + pub fn fingerprint_for_trait_impl(&self) -> Option { + TyFingerprint::for_trait_impl(&self.ty) + } + pub(crate) fn canonical(&self) -> Canonical { hir_ty::replace_errors_with_variables(&self.ty) } diff --git a/crates/ide-completion/src/tests/flyimport.rs b/crates/ide-completion/src/tests/flyimport.rs index fff193ba4c9b..d399d95ba849 100644 --- a/crates/ide-completion/src/tests/flyimport.rs +++ b/crates/ide-completion/src/tests/flyimport.rs @@ -374,6 +374,186 @@ fn main() { ); } +#[test] +fn trait_method_fuzzy_completion_aware_of_fundamental_boxes() { + let fixture = r#" +//- /fundamental.rs crate:fundamental +#[lang = "owned_box"] +#[fundamental] +pub struct Box(T); +//- /foo.rs crate:foo +pub trait TestTrait { + fn some_method(&self); +} +//- /main.rs crate:main deps:foo,fundamental +struct TestStruct; + +impl foo::TestTrait for fundamental::Box { + fn some_method(&self) {} +} + +fn main() { + let t = fundamental::Box(TestStruct); + t.$0 +} +"#; + + check( + fixture, + expect![[r#" + me some_method() (use foo::TestTrait) fn(&self) + "#]], + ); + + check_edit( + "some_method", + fixture, + r#" +use foo::TestTrait; + +struct TestStruct; + +impl foo::TestTrait for fundamental::Box { + fn some_method(&self) {} +} + +fn main() { + let t = fundamental::Box(TestStruct); + t.some_method()$0 +} +"#, + ); +} + +#[test] +fn trait_method_fuzzy_completion_aware_of_fundamental_recusive_boxes() { + let fixture = r#" +//- /fundamental.rs crate:fundamental +#[lang = "owned_box"] +#[fundamental] +pub struct Box(T); +//- /foo.rs crate:foo +pub trait TestTrait { + fn some_method(&self); +} +//- /main.rs crate:main deps:foo,fundamental +struct TestStruct; + +impl foo::TestTrait for fundamental::Box> { + fn some_method(&self) {} +} + +fn main() { + let t = fundamental::Box(fundamental::Box(TestStruct)); + t.$0 +} +"#; + + check( + fixture, + expect![[r#" + me some_method() (use foo::TestTrait) fn(&self) + "#]], + ); + + check_edit( + "some_method", + fixture, + r#" +use foo::TestTrait; + +struct TestStruct; + +impl foo::TestTrait for fundamental::Box> { + fn some_method(&self) {} +} + +fn main() { + let t = fundamental::Box(fundamental::Box(TestStruct)); + t.some_method()$0 +} +"#, + ); +} + +#[test] +fn trait_method_fuzzy_completion_aware_of_fundamental_references() { + let fixture = r#" +//- /foo.rs crate:foo +pub trait TestTrait { + fn some_method(&self); +} +//- /main.rs crate:main deps:foo +struct TestStruct; + +impl foo::TestTrait for &TestStruct { + fn some_method(&self) {} +} + +fn main() { + let t = &TestStruct; + t.$0 +} +"#; + + check( + fixture, + expect![[r#" + me some_method() (use foo::TestTrait) fn(&self) + "#]], + ); + + check_edit( + "some_method", + fixture, + r#" +use foo::TestTrait; + +struct TestStruct; + +impl foo::TestTrait for &TestStruct { + fn some_method(&self) {} +} + +fn main() { + let t = &TestStruct; + t.some_method()$0 +} +"#, + ); +} + +#[test] +fn trait_method_fuzzy_completion_aware_of_unit_type() { + let fixture = r#" +//- /test_trait.rs crate:test_trait +pub trait TestInto { + fn into(self) -> T; +} + +//- /main.rs crate:main deps:test_trait +struct A; + +impl test_trait::TestInto for () { + fn into(self) -> A { + A + } +} + +fn main() { + let a = (); + a.$0 +} +"#; + + check( + fixture, + expect![[r#" + me into() (use test_trait::TestInto) fn(self) -> T + "#]], + ); +} + #[test] fn trait_method_from_alias() { let fixture = r#" diff --git a/crates/ide-db/src/imports/import_assets.rs b/crates/ide-db/src/imports/import_assets.rs index a71d8e9002d4..c597555a3bf6 100644 --- a/crates/ide-db/src/imports/import_assets.rs +++ b/crates/ide-db/src/imports/import_assets.rs @@ -1,8 +1,9 @@ //! Look up accessible paths for items. use hir::{ - AsAssocItem, AssocItem, AssocItemContainer, Crate, ItemInNs, ModPath, Module, ModuleDef, Name, - PathResolution, PrefixKind, ScopeDef, Semantics, SemanticsScope, Type, + db::HirDatabase, AsAssocItem, AssocItem, AssocItemContainer, Crate, HasCrate, ItemInNs, + ModPath, Module, ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics, + SemanticsScope, Trait, Type, }; use itertools::{EitherOrBoth, Itertools}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -517,7 +518,7 @@ fn trait_applicable_items( let related_traits = inherent_traits.chain(env_traits).collect::>(); let mut required_assoc_items = FxHashSet::default(); - let trait_candidates: FxHashSet<_> = items_locator::items_with_name( + let mut trait_candidates: FxHashSet<_> = items_locator::items_with_name( sema, current_crate, trait_candidate.assoc_item_name.clone(), @@ -538,6 +539,32 @@ fn trait_applicable_items( }) .collect(); + trait_candidates.retain(|&candidate_trait_id| { + // we care about the following cases: + // 1. Trait's definition crate + // 2. Definition crates for all trait's generic arguments + // a. This is recursive for fundamental types: `Into> for ()`` is OK, but + // `Into> for ()`` is *not*. + // 3. Receiver type definition crate + // a. This is recursive for fundamental types + let defining_crate_for_trait = Trait::from(candidate_trait_id).krate(db); + let Some(receiver) = trait_candidate.receiver_ty.fingerprint_for_trait_impl() else { + return false; + }; + let definitions_exist_in_trait_crate = db + .trait_impls_in_crate(defining_crate_for_trait.into()) + .has_impls_for_trait_and_self_ty(candidate_trait_id, receiver); + + // this is a closure for laziness: if `definitions_exist_in_trait_crate` is true, + // we can avoid a second db lookup. + let definitions_exist_in_receiver_crate = || { + db.trait_impls_in_crate(trait_candidate.receiver_ty.krate(db).into()) + .has_impls_for_trait_and_self_ty(candidate_trait_id, receiver) + }; + + definitions_exist_in_trait_crate || definitions_exist_in_receiver_crate() + }); + let mut located_imports = FxHashSet::default(); let mut trait_import_paths = FxHashMap::default();