Auto merge of rust-lang#13074 - lowr:fix/method-resolution-with-impl-predicate, r=lowr

Consider bounds on inherent impl in method resolution

There are three type-related things we should consider in method resolution: `Self` type, receiver type, and impl bounds. While we check the first two and impl bounds on trait impls, we've been ignoring the impl bounds on inherent impls. With this patch rust-analyzer now takes them into account and is able to select the appropriate inherent method.

Resolves rust-lang#5441
Resolves rust-lang#12308

Author: bors

crates/hir-ty/src/method_resolution.rs

@@ -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>,
@@ -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);
@@ -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,

crates/hir-ty/src/tests/method_resolution.rs

@@ -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
+}
+        "#,
+    );
+}