From 3a1b4c29b395b5d3dd542fda87f3d72a43955618 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 27 Feb 2024 10:00:45 +0100 Subject: [PATCH] internal: Remove dead branches in `method_resolutiopn::is_valid_candidate` --- crates/hir-ty/src/method_resolution.rs | 241 +++++++++++-------------- 1 file changed, 107 insertions(+), 134 deletions(-) diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index ad62aee22e5..e68dbe7b02e 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -1176,11 +1176,12 @@ 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_method_candidate(table, name, receiver_ty, item, self_ty) { - IsValidCandidate::Yes => true, - IsValidCandidate::NotVisible => false, - IsValidCandidate::No => continue, - }; + let visible = + match is_valid_trait_method_candidate(table, t, name, receiver_ty, item, self_ty) { + IsValidCandidate::Yes => true, + IsValidCandidate::NotVisible => false, + IsValidCandidate::No => continue, + }; if !known_implemented { let goal = generic_implements_goal(db, env.clone(), t, &canonical_self_ty); if db.trait_solve(env.krate, env.block, goal.cast(Interner)).is_none() { @@ -1302,12 +1303,18 @@ fn iterate_inherent_trait_methods( let data = db.trait_data(t); for &(_, item) in data.items.iter() { // We don't pass `visible_from_module` as all trait items should be visible. - let visible = - match is_valid_candidate(table, name, receiver_ty, item, self_ty, None) { - IsValidCandidate::Yes => true, - IsValidCandidate::NotVisible => false, - IsValidCandidate::No => continue, - }; + let visible = match is_valid_trait_method_candidate( + table, + t, + name, + receiver_ty, + item, + self_ty, + ) { + IsValidCandidate::Yes => true, + IsValidCandidate::NotVisible => false, + IsValidCandidate::No => continue, + }; callback(receiver_adjustments.clone().unwrap_or_default(), item, visible)?; } } @@ -1325,17 +1332,16 @@ fn impls_for_self_ty( visible_from_module: Option, callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>, ) -> ControlFlow<()> { - let db = table.db; - let impls_for_self_ty = impls.for_self_ty(self_ty); - for &impl_def in impls_for_self_ty { - for &item in &db.impl_data(impl_def).items { - let visible = match is_valid_candidate( + for &impl_id in impls.for_self_ty(self_ty) { + for &item in &table.db.impl_data(impl_id).items { + let visible = match is_valid_impl_method_candidate( table, - name, - receiver_ty, - item, self_ty, + receiver_ty, visible_from_module, + name, + impl_id, + item, ) { IsValidCandidate::Yes => true, IsValidCandidate::NotVisible => false, @@ -1378,21 +1384,34 @@ macro_rules! check_that { }; } +enum IsValidCandidate { + Yes, + No, + NotVisible, +} + #[tracing::instrument(skip_all, fields(name))] -fn is_valid_candidate( +fn is_valid_impl_method_candidate( table: &mut InferenceTable<'_>, - name: Option<&Name>, - receiver_ty: Option<&Ty>, - item: AssocItemId, self_ty: &Ty, + receiver_ty: Option<&Ty>, visible_from_module: Option, + name: Option<&Name>, + impl_id: ImplId, + item: AssocItemId, ) -> IsValidCandidate { - let db = table.db; match item { - AssocItemId::FunctionId(f) => { - is_valid_fn_candidate(table, f, name, receiver_ty, self_ty, visible_from_module) - } + AssocItemId::FunctionId(f) => is_valid_impl_fn_candidate( + table, + impl_id, + f, + name, + receiver_ty, + self_ty, + visible_from_module, + ), AssocItemId::ConstId(c) => { + let db = table.db; check_that!(receiver_ty.is_none()); check_that!(name.map_or(true, |n| db.const_data(c).name.as_ref() == Some(n))); @@ -1402,17 +1421,14 @@ fn is_valid_candidate( return IsValidCandidate::NotVisible; } } - if let ItemContainerId::ImplId(impl_id) = c.lookup(db.upcast()).container { - let self_ty_matches = table.run_in_snapshot(|table| { - let expected_self_ty = TyBuilder::impl_self_ty(db, impl_id) - .fill_with_inference_vars(table) - .build(); - table.unify(&expected_self_ty, self_ty) - }); - if !self_ty_matches { - cov_mark::hit!(const_candidate_self_type_mismatch); - return IsValidCandidate::No; - } + let self_ty_matches = table.run_in_snapshot(|table| { + let expected_self_ty = + TyBuilder::impl_self_ty(db, impl_id).fill_with_inference_vars(table).build(); + table.unify(&expected_self_ty, self_ty) + }); + if !self_ty_matches { + cov_mark::hit!(const_candidate_self_type_mismatch); + return IsValidCandidate::No; } IsValidCandidate::Yes } @@ -1421,12 +1437,10 @@ 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( +fn is_valid_trait_method_candidate( table: &mut InferenceTable<'_>, + trait_id: TraitId, name: Option<&Name>, receiver_ty: Option<&Ty>, item: AssocItemId, @@ -1440,24 +1454,10 @@ fn is_valid_method_candidate( 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!(), - }; + let impl_subst = TyBuilder::subst_for_def(db, trait_id, None) + .fill_with_inference_vars(table) + .build(); + let expect_self_ty = impl_subst.at(Interner, 0).assert_ty_ref(Interner).clone(); check_that!(table.unify(&expect_self_ty, self_ty)); @@ -1488,15 +1488,10 @@ fn is_valid_method_candidate( } } -enum IsValidCandidate { - Yes, - No, - NotVisible, -} - #[tracing::instrument(skip_all, fields(name))] -fn is_valid_fn_candidate( +fn is_valid_impl_fn_candidate( table: &mut InferenceTable<'_>, + impl_id: ImplId, fn_id: FunctionId, name: Option<&Name>, receiver_ty: Option<&Ty>, @@ -1514,24 +1509,10 @@ 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 = - 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!(), - }; + let impl_subst = + TyBuilder::subst_for_def(db, impl_id, None).fill_with_inference_vars(table).build(); + let expect_self_ty = db.impl_self_ty(impl_id).substitute(Interner, &impl_subst); check_that!(table.unify(&expect_self_ty, self_ty)); @@ -1550,63 +1531,55 @@ fn is_valid_fn_candidate( check_that!(table.unify(receiver_ty, &expected_receiver)); } - 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()); - let goals = predicates.iter().map(|p| { - let (p, b) = p - .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); + // 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()); + let goals = predicates.iter().map(|p| { + let (p, b) = p + .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.cast::(Interner) - }); + p.cast::(Interner) + }); - for goal in goals.clone() { - let in_env = InEnvironment::new(&table.trait_env.env, goal); - let canonicalized = table.canonicalize(in_env); - let solution = table.db.trait_solve( - table.trait_env.krate, - table.trait_env.block, - canonicalized.value.clone(), - ); + for goal in goals.clone() { + let in_env = InEnvironment::new(&table.trait_env.env, goal); + let canonicalized = table.canonicalize(in_env); + let solution = table.db.trait_solve( + table.trait_env.krate, + table.trait_env.block, + canonicalized.value.clone(), + ); - match solution { - Some(Solution::Unique(canonical_subst)) => { - canonicalized.apply_solution( - table, - Canonical { - binders: canonical_subst.binders, - value: canonical_subst.value.subst, - }, - ); - } - Some(Solution::Ambig(Guidance::Definite(substs))) => { - canonicalized.apply_solution(table, substs); - } - Some(_) => (), - None => return IsValidCandidate::No, + match solution { + Some(Solution::Unique(canonical_subst)) => { + canonicalized.apply_solution( + table, + Canonical { + binders: canonical_subst.binders, + value: canonical_subst.value.subst, + }, + ); } - } - - for goal in goals { - if table.try_obligation(goal).is_none() { - return IsValidCandidate::No; + Some(Solution::Ambig(Guidance::Definite(substs))) => { + canonicalized.apply_solution(table, substs); } + Some(_) => (), + None => return IsValidCandidate::No, } - - IsValidCandidate::Yes - } 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. - IsValidCandidate::Yes } + + for goal in goals { + if table.try_obligation(goal).is_none() { + return IsValidCandidate::No; + } + } + + IsValidCandidate::Yes }) }