Auto merge of #13475 - lowr:fix/lookup-impl-method-trait-ref, r=Veykril

fix: Test all type args for trait when finding matching impl

Addresses #13463 (comment)

When finding matching impl for a trait method, we've been testing the unifiability of self type. However, there can be multiple impl of a trait for the same type with different generic arguments for the trait. This patch takes it into account and tests the unifiability of all type arguments for the trait (the first being the self type) thus enables rust-analyzer to find the correct impl even in such cases.

Author: bors

crates/hir-ty/src/method_resolution.rs

@@ -25,7 +25,7 @@ use crate::{
     static_lifetime,
     utils::all_super_traits,
     AdtId, Canonical, CanonicalVarKinds, DebruijnIndex, ForeignDefId, InEnvironment, Interner,
-    Scalar, TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyExt, TyKind,
+    Scalar, Substitution, TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyExt, TyKind,
 };
 
 /// This is used as a key for indexing impls.
@@ -625,17 +625,22 @@ pub(crate) fn iterate_method_candidates<T>(
 }
 
 pub fn lookup_impl_method(
-    self_ty: &Ty,
     db: &dyn HirDatabase,
     env: Arc<TraitEnvironment>,
     trait_: TraitId,
     name: &Name,
+    fn_subst: Substitution,
 ) -> Option<FunctionId> {
+    let trait_params = db.generic_params(trait_.into()).type_or_consts.len();
+    let fn_params = fn_subst.len(Interner) - trait_params;
+    let trait_subst = Substitution::from_iter(Interner, fn_subst.iter(Interner).skip(fn_params));
+
+    let self_ty = trait_subst.at(Interner, 0).ty(Interner)?;
     let self_ty_fp = TyFingerprint::for_trait_impl(self_ty)?;
     let trait_impls = db.trait_impls_in_deps(env.krate);
     let impls = trait_impls.for_trait_and_self_ty(trait_, self_ty_fp);
-    let mut table = InferenceTable::new(db, env.clone());
-    find_matching_impl(impls, &mut table, &self_ty).and_then(|data| {
+    let mut table = InferenceTable::new(db, env);
+    find_matching_impl(impls, &mut table, trait_subst).and_then(|data| {
         data.items.iter().find_map(|it| match it {
             AssocItemId::FunctionId(f) => (db.function_data(*f).name == *name).then(|| *f),
             _ => None,
@@ -646,30 +651,41 @@ pub fn lookup_impl_method(
 fn find_matching_impl(
     mut impls: impl Iterator<Item = ImplId>,
     table: &mut InferenceTable<'_>,
-    self_ty: &Ty,
+    expected_subst: Substitution,
 ) -> Option<Arc<ImplData>> {
     let db = table.db;
     loop {
         let impl_ = impls.next()?;
         let r = table.run_in_snapshot(|table| {
             let impl_data = db.impl_data(impl_);
-            let substs =
+            let impl_substs =
                 TyBuilder::subst_for_def(db, impl_, None).fill_with_inference_vars(table).build();
-            let impl_ty = db.impl_self_ty(impl_).substitute(Interner, &substs);
-
-            table
-                .unify(self_ty, &impl_ty)
-                .then(|| {
-                    let wh_goals =
-                        crate::chalk_db::convert_where_clauses(db, impl_.into(), &substs)
-                            .into_iter()
-                            .map(|b| b.cast(Interner));
-
-                    let goal = crate::Goal::all(Interner, wh_goals);
+            let trait_ref = db
+                .impl_trait(impl_)
+                .expect("non-trait method in find_matching_impl")
+                .substitute(Interner, &impl_substs);
+            assert_eq!(trait_ref.substitution.len(Interner), expected_subst.len(Interner));
+
+            for (actual, expected) in
+                trait_ref.substitution.iter(Interner).zip(expected_subst.iter(Interner))
+            {
+                // FIXME: test unifiability of const args when supported.
+                if let Some(actual) = actual.ty(Interner) {
+                    if !stdx::always!(expected.ty(Interner).is_some()) {
+                        return None;
+                    }
+                    let expected = expected.assert_ty_ref(Interner);
+                    if !table.unify(actual, expected) {
+                        return None;
+                    }
+                }
+            }
 
-                    table.try_obligation(goal).map(|_| impl_data)
-                })
-                .flatten()
+            let wcs = crate::chalk_db::convert_where_clauses(db, impl_.into(), &impl_substs)
+                .into_iter()
+                .map(|b| b.cast(Interner));
+            let goal = crate::Goal::all(Interner, wcs);
+            table.try_obligation(goal).map(|_| impl_data)
         });
         if r.is_some() {
             break r;

crates/hir/src/source_analyzer.rs

@@ -270,7 +270,7 @@ impl SourceAnalyzer {
         let expr_id = self.expr_id(db, &call.clone().into())?;
         let (f_in_trait, substs) = self.infer.as_ref()?.method_resolution(expr_id)?;
 
-        Some(self.resolve_impl_method_or_trait_def(db, f_in_trait, &substs))
+        Some(self.resolve_impl_method_or_trait_def(db, f_in_trait, substs))
     }
 
     pub(crate) fn resolve_await_to_poll(
@@ -311,7 +311,7 @@ impl SourceAnalyzer {
         // HACK: subst for `poll()` coincides with that for `Future` because `poll()` itself
         // doesn't have any generic parameters, so we skip building another subst for `poll()`.
         let substs = hir_ty::TyBuilder::subst_for_def(db, future_trait, None).push(ty).build();
-        Some(self.resolve_impl_method_or_trait_def(db, poll_fn, &substs))
+        Some(self.resolve_impl_method_or_trait_def(db, poll_fn, substs))
     }
 
     pub(crate) fn resolve_prefix_expr(
@@ -331,7 +331,7 @@ impl SourceAnalyzer {
         // don't have any generic parameters, so we skip building another subst for the methods.
         let substs = hir_ty::TyBuilder::subst_for_def(db, op_trait, None).push(ty.clone()).build();
 
-        Some(self.resolve_impl_method_or_trait_def(db, op_fn, &substs))
+        Some(self.resolve_impl_method_or_trait_def(db, op_fn, substs))
     }
 
     pub(crate) fn resolve_index_expr(
@@ -351,7 +351,7 @@ impl SourceAnalyzer {
             .push(base_ty.clone())
             .push(index_ty.clone())
             .build();
-        Some(self.resolve_impl_method_or_trait_def(db, op_fn, &substs))
+        Some(self.resolve_impl_method_or_trait_def(db, op_fn, substs))
     }
 
     pub(crate) fn resolve_bin_expr(
@@ -372,7 +372,7 @@ impl SourceAnalyzer {
             .push(rhs.clone())
             .build();
 
-        Some(self.resolve_impl_method_or_trait_def(db, op_fn, &substs))
+        Some(self.resolve_impl_method_or_trait_def(db, op_fn, substs))
     }
 
     pub(crate) fn resolve_try_expr(
@@ -392,7 +392,7 @@ impl SourceAnalyzer {
         // doesn't have any generic parameters, so we skip building another subst for `branch()`.
         let substs = hir_ty::TyBuilder::subst_for_def(db, op_trait, None).push(ty.clone()).build();
 
-        Some(self.resolve_impl_method_or_trait_def(db, op_fn, &substs))
+        Some(self.resolve_impl_method_or_trait_def(db, op_fn, substs))
     }
 
     pub(crate) fn resolve_field(
@@ -487,17 +487,17 @@ impl SourceAnalyzer {
 
         let mut prefer_value_ns = false;
         let resolved = (|| {
+            let infer = self.infer.as_deref()?;
             if let Some(path_expr) = parent().and_then(ast::PathExpr::cast) {
                 let expr_id = self.expr_id(db, &path_expr.into())?;
-                let infer = self.infer.as_ref()?;
                 if let Some(assoc) = infer.assoc_resolutions_for_expr(expr_id) {
                     let assoc = match assoc {
                         AssocItemId::FunctionId(f_in_trait) => {
                             match infer.type_of_expr.get(expr_id) {
                                 None => assoc,
                                 Some(func_ty) => {
                                     if let TyKind::FnDef(_fn_def, subs) = func_ty.kind(Interner) {
-                                        self.resolve_impl_method(db, f_in_trait, subs)
+                                        self.resolve_impl_method(db, f_in_trait, subs.clone())
                                             .map(AssocItemId::FunctionId)
                                             .unwrap_or(assoc)
                                     } else {
@@ -520,18 +520,18 @@ impl SourceAnalyzer {
                 prefer_value_ns = true;
             } else if let Some(path_pat) = parent().and_then(ast::PathPat::cast) {
                 let pat_id = self.pat_id(&path_pat.into())?;
-                if let Some(assoc) = self.infer.as_ref()?.assoc_resolutions_for_pat(pat_id) {
+                if let Some(assoc) = infer.assoc_resolutions_for_pat(pat_id) {
                     return Some(PathResolution::Def(AssocItem::from(assoc).into()));
                 }
                 if let Some(VariantId::EnumVariantId(variant)) =
-                    self.infer.as_ref()?.variant_resolution_for_pat(pat_id)
+                    infer.variant_resolution_for_pat(pat_id)
                 {
                     return Some(PathResolution::Def(ModuleDef::Variant(variant.into())));
                 }
             } else if let Some(rec_lit) = parent().and_then(ast::RecordExpr::cast) {
                 let expr_id = self.expr_id(db, &rec_lit.into())?;
                 if let Some(VariantId::EnumVariantId(variant)) =
-                    self.infer.as_ref()?.variant_resolution_for_expr(expr_id)
+                    infer.variant_resolution_for_expr(expr_id)
                 {
                     return Some(PathResolution::Def(ModuleDef::Variant(variant.into())));
                 }
@@ -541,8 +541,7 @@ impl SourceAnalyzer {
                     || parent().and_then(ast::TupleStructPat::cast).map(ast::Pat::from);
                 if let Some(pat) = record_pat.or_else(tuple_struct_pat) {
                     let pat_id = self.pat_id(&pat)?;
-                    let variant_res_for_pat =
-                        self.infer.as_ref()?.variant_resolution_for_pat(pat_id);
+                    let variant_res_for_pat = infer.variant_resolution_for_pat(pat_id);
                     if let Some(VariantId::EnumVariantId(variant)) = variant_res_for_pat {
                         return Some(PathResolution::Def(ModuleDef::Variant(variant.into())));
                     }
@@ -784,31 +783,33 @@ impl SourceAnalyzer {
         &self,
         db: &dyn HirDatabase,
         func: FunctionId,
-        substs: &Substitution,
+        fn_substs: Substitution,
     ) -> Option<FunctionId> {
         let impled_trait = match func.lookup(db.upcast()).container {
             ItemContainerId::TraitId(trait_id) => trait_id,
             _ => return None,
         };
-        if substs.is_empty(Interner) {
-            return None;
-        }
-        let self_ty = substs.at(Interner, 0).ty(Interner)?;
         let krate = self.resolver.krate();
         let trait_env = self.resolver.body_owner()?.as_generic_def_id().map_or_else(
             || Arc::new(hir_ty::TraitEnvironment::empty(krate)),
             |d| db.trait_environment(d),
         );
 
         let fun_data = db.function_data(func);
-        method_resolution::lookup_impl_method(self_ty, db, trait_env, impled_trait, &fun_data.name)
+        method_resolution::lookup_impl_method(
+            db,
+            trait_env,
+            impled_trait,
+            &fun_data.name,
+            fn_substs,
+        )
     }
 
     fn resolve_impl_method_or_trait_def(
         &self,
         db: &dyn HirDatabase,
         func: FunctionId,
-        substs: &Substitution,
+        substs: Substitution,
     ) -> FunctionId {
         self.resolve_impl_method(db, func, substs).unwrap_or(func)
     }

crates/ide/src/goto_definition.rs

@@ -1834,4 +1834,86 @@ fn f() {
 "#,
         );
     }
+
+    #[test]
+    fn goto_bin_op_multiple_impl() {
+        check(
+            r#"
+//- minicore: add
+struct S;
+impl core::ops::Add for S {
+    fn add(
+     //^^^
+    ) {}
+}
+impl core::ops::Add<usize> for S {
+    fn add(
+    ) {}
+}
+
+fn f() {
+    S +$0 S
+}
+"#,
+        );
+
+        check(
+            r#"
+//- minicore: add
+struct S;
+impl core::ops::Add for S {
+    fn add(
+    ) {}
+}
+impl core::ops::Add<usize> for S {
+    fn add(
+     //^^^
+    ) {}
+}
+
+fn f() {
+    S +$0 0usize
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn path_call_multiple_trait_impl() {
+        check(
+            r#"
+trait Trait<T> {
+    fn f(_: T);
+}
+impl Trait<i32> for usize {
+    fn f(_: i32) {}
+     //^
+}
+impl Trait<i64> for usize {
+    fn f(_: i64) {}
+}
+fn main() {
+    usize::f$0(0i32);
+}
+"#,
+        );
+
+        check(
+            r#"
+trait Trait<T> {
+    fn f(_: T);
+}
+impl Trait<i32> for usize {
+    fn f(_: i32) {}
+}
+impl Trait<i64> for usize {
+    fn f(_: i64) {}
+     //^
+}
+fn main() {
+    usize::f$0(0i64);
+}
+"#,
+        )
+    }
 }