Skip to content

Consider bounds on inherent impl in method resolution #13074

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

Merged
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
142 changes: 97 additions & 45 deletions crates/hir-ty/src/method_resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,14 @@ pub fn resolve_indexing_op(
None
}

macro_rules! check_that {
($cond:expr) => {
if !$cond {
return false;
}
};
}

fn is_valid_candidate(
table: &mut InferenceTable<'_>,
name: Option<&Name>,
Expand All @@ -1072,54 +1080,10 @@ fn is_valid_candidate(
self_ty: &Ty,
visible_from_module: Option<ModuleId>,
) -> bool {
macro_rules! check_that {
($cond:expr) => {
if !$cond {
return false;
}
};
}

let db = table.db;
match item {
AssocItemId::FunctionId(m) => {
let data = db.function_data(m);

check_that!(name.map_or(true, |n| n == &data.name));
check_that!(visible_from_module.map_or(true, |from_module| {
let v = db.function_visibility(m).is_visible_from(db.upcast(), from_module);
if !v {
cov_mark::hit!(autoderef_candidate_not_visible);
}
v
}));

table.run_in_snapshot(|table| {
let subst = TyBuilder::subst_for_def(db, m).fill_with_inference_vars(table).build();
let expect_self_ty = match m.lookup(db.upcast()).container {
ItemContainerId::TraitId(_) => {
subst.at(Interner, 0).assert_ty_ref(Interner).clone()
}
ItemContainerId::ImplId(impl_id) => {
subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner)
}
// We should only get called for associated items (impl/trait)
ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => {
unreachable!()
}
};
check_that!(table.unify(&expect_self_ty, self_ty));
if let Some(receiver_ty) = receiver_ty {
check_that!(data.has_self_param());

let sig = db.callable_item_signature(m.into());
let expected_receiver =
sig.map(|s| s.params()[0].clone()).substitute(Interner, &subst);

check_that!(table.unify(&receiver_ty, &expected_receiver));
}
true
})
is_valid_fn_candidate(table, m, name, receiver_ty, self_ty, visible_from_module)
}
AssocItemId::ConstId(c) => {
let data = db.const_data(c);
Expand Down Expand Up @@ -1152,6 +1116,94 @@ fn is_valid_candidate(
}
}

fn is_valid_fn_candidate(
table: &mut InferenceTable<'_>,
fn_id: FunctionId,
name: Option<&Name>,
receiver_ty: Option<&Ty>,
self_ty: &Ty,
visible_from_module: Option<ModuleId>,
) -> bool {
let db = table.db;
let data = db.function_data(fn_id);

check_that!(name.map_or(true, |n| n == &data.name));
check_that!(visible_from_module.map_or(true, |from_module| {
let v = db.function_visibility(fn_id).is_visible_from(db.upcast(), from_module);
if !v {
cov_mark::hit!(autoderef_candidate_not_visible);
}
v
}));

table.run_in_snapshot(|table| {
let container = fn_id.lookup(db.upcast()).container;
let impl_subst = match container {
ItemContainerId::ImplId(it) => {
TyBuilder::subst_for_def(db, it).fill_with_inference_vars(table).build()
}
ItemContainerId::TraitId(it) => {
TyBuilder::subst_for_def(db, it).fill_with_inference_vars(table).build()
}
_ => unreachable!(),
};

let fn_subst = TyBuilder::subst_for_def(db, fn_id)
.use_parent_substs(&impl_subst)
.fill_with_inference_vars(table)
.build();

let expect_self_ty = match container {
ItemContainerId::TraitId(_) => fn_subst.at(Interner, 0).assert_ty_ref(Interner).clone(),
ItemContainerId::ImplId(impl_id) => {
fn_subst.apply(db.impl_self_ty(impl_id).skip_binders().clone(), Interner)
}
// We should only get called for associated items (impl/trait)
ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => {
unreachable!()
}
};
check_that!(table.unify(&expect_self_ty, self_ty));

if let Some(receiver_ty) = receiver_ty {
check_that!(data.has_self_param());

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));
}

if let ItemContainerId::ImplId(impl_id) = container {
// 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());
predicates
.iter()
.map(|predicate| {
let (p, b) = predicate
.clone()
.substitute(Interner, &impl_subst)
// Skipping the inner binders is ok, as we don't handle quantified where
// clauses yet.
.into_value_and_skipped_binders();
stdx::always!(b.len(Interner) == 0);
p
})
// It's ok to get ambiguity here, as we may not have enough information to prove
// obligations. We'll check if the user is calling the selected method properly
// later anyway.
.all(|p| table.try_obligation(p.cast(Interner)).is_some())
} else {
// For `ItemContainerId::TraitId`, we check if `self_ty` implements the trait in
// `iterate_trait_method_candidates()`.
// For others, this function shouldn't be called.
true
}
})
}

pub fn implements_trait(
ty: &Canonical<Ty>,
db: &dyn HirDatabase,
Expand Down
43 changes: 43 additions & 0 deletions crates/hir-ty/src/tests/method_resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1790,3 +1790,46 @@ impl u16 {
"#,
)
}

#[test]
fn with_impl_bounds() {
check_types(
r#"
trait Trait {}
struct Foo<T>(T);
impl Trait for isize {}

impl<T: Trait> Foo<T> {
fn foo() -> isize { 0 }
fn bar(&self) -> isize { 0 }
}

impl Foo<()> {
fn foo() {}
fn bar(&self) {}
}

fn f() {
let _ = Foo::<isize>::foo();
//^isize
let _ = Foo(0isize).bar();
//^isize
let _ = Foo::<()>::foo();
//^()
let _ = Foo(()).bar();
//^()
let _ = Foo::<usize>::foo();
//^{unknown}
let _ = Foo(0usize).bar();
//^{unknown}
}

fn g<T: Trait>(a: T) {
let _ = Foo::<T>::foo();
//^isize
let _ = Foo(a).bar();
//^isize
}
"#,
);
}