Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

performance: Speed up Method Completions By Taking Advantage of Orphan Rules #16555

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/hir-ty/src/infer/unify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: ?Sized + Zip<Interner>>(&mut self, ty1: &T, ty2: &T) -> bool {
let result = match self.try_unify(ty1, ty2) {
Ok(r) => r,
Expand Down
79 changes: 78 additions & 1 deletion crates/hir-ty/src/method_resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = ImplId> + '_ {
self.map.values().flat_map(|map| map.values().flat_map(|v| v.iter().copied()))
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 =
Expand All @@ -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()))
Expand All @@ -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());
Expand Down
1 change: 1 addition & 0 deletions crates/hir-ty/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ fn solve(
block: Option<BlockId>,
goal: &chalk_ir::UCanonical<chalk_ir::InEnvironment<chalk_ir::Goal<Interner>>>,
) -> Option<chalk_solve::Solution<Interner>> {
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();
Expand Down
4 changes: 4 additions & 0 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4239,6 +4239,10 @@ impl Type {
}
}

pub fn fingerprint_for_trait_impl(&self) -> Option<TyFingerprint> {
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
TyFingerprint::for_trait_impl(&self.ty)
}

pub(crate) fn canonical(&self) -> Canonical<Ty> {
hir_ty::replace_errors_with_variables(&self.ty)
}
Expand Down
129 changes: 129 additions & 0 deletions crates/ide-completion/src/tests/flyimport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,135 @@ 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>(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<TestStruct> {
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<TestStruct> {
fn some_method(&self) {}
}

fn main() {
let t = 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;
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
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<T> {
fn into(self) -> T;
}

//- /main.rs crate:main deps:test_trait
struct A;

impl test_trait::TestInto<A> 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#"
Expand Down
33 changes: 30 additions & 3 deletions crates/ide-db/src/imports/import_assets.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -517,7 +518,7 @@ fn trait_applicable_items(
let related_traits = inherent_traits.chain(env_traits).collect::<FxHashSet<_>>();

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(),
Expand All @@ -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<Box<A>> for ()`` is OK, but
// `Into<Vec<A>> for ()`` is *not*.
// 3. Receiver type definition crate
// a. This is recursive for fundamental types
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
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();

Expand Down
Loading