Skip to content

Commit

Permalink
address some review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
davidbarsky committed Feb 20, 2024
1 parent eea2a5b commit b67bbd1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 25 deletions.
10 changes: 3 additions & 7 deletions crates/hir-ty/src/method_resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1167,10 +1167,7 @@ fn iterate_trait_method_candidates(
// trait, but if we find out it doesn't, we'll skip the rest of the
// iteration
let mut known_implemented = false;
'items: for &(_, item) in data.items.iter() {
if let AssocItemId::TypeAliasId(_) = item {
continue 'items;
}
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) {
Expand Down Expand Up @@ -1432,7 +1429,6 @@ fn is_valid_method_candidate(
let db = table.db;
match item {
AssocItemId::FunctionId(fn_id) => {
let db = table.db;
let data = db.function_data(fn_id);

check_that!(name.map_or(true, |n| n == &data.name));
Expand Down Expand Up @@ -1533,8 +1529,8 @@ fn is_valid_fn_candidate(

check_that!(table.unify(&expect_self_ty, self_ty));

let _p = tracing::span!(tracing::Level::INFO, "check_receiver_ty").entered();
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 @@ -1548,8 +1544,8 @@ fn is_valid_fn_candidate(
check_that!(table.unify(receiver_ty, &expected_receiver));
}

let _p = tracing::span!(tracing::Level::INFO, "check_item_container").entered();
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
22 changes: 16 additions & 6 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub use {
cfg::{CfgAtom, CfgExpr, CfgOptions},
hir_def::{
attr::{builtin::AttributeTemplate, AttrSourceMap, Attrs, AttrsWithOwner},
data::adt::StructKind,
data::adt::{StructFlags, StructKind},
find_path::PrefixKind,
import_map,
lang_item::LangItem,
Expand Down Expand Up @@ -266,10 +266,6 @@ impl Crate {
let data = &db.crate_graph()[self.id];
data.potential_cfg_options.clone().unwrap_or_else(|| data.cfg_options.clone())
}

pub fn id(&self) -> CrateId {
self.id
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -3783,7 +3779,7 @@ pub enum CaptureKind {
#[derive(Clone, PartialEq, Eq, Debug, Hash)]
pub struct Type {
env: Arc<TraitEnvironment>,
pub ty: Ty,
ty: Ty,
}

impl Type {
Expand Down Expand Up @@ -4243,6 +4239,20 @@ impl Type {
}
}

pub fn is_fundamental(&self, db: &dyn HirDatabase) -> bool {
match self.ty.kind(Interner) {
TyKind::Ref(_, _, _) => true,
&TyKind::Adt(hir_ty::AdtId(AdtId::StructId(s)), ref _subs) => {
db.struct_data(s).flags.contains(StructFlags::IS_FUNDAMENTAL)
}
_ => false,
}
}

pub fn fingerprint_for_trait_impl(&self) -> Option<TyFingerprint> {
TyFingerprint::for_trait_impl(&self.ty)
}

pub(crate) fn canonical(&self) -> Canonical<Ty> {
hir_ty::replace_errors_with_variables(&self.ty)
}
Expand Down
23 changes: 11 additions & 12 deletions crates/ide-db/src/imports/import_assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use hir::{
db::HirDatabase, AsAssocItem, AssocItem, AssocItemContainer, Crate, HasCrate, ItemInNs,
ModPath, Module, ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics,
SemanticsScope, TraitId, TyFingerprint, Type,
SemanticsScope, Trait, TraitId, Type,
};
use itertools::{EitherOrBoth, Itertools};
use rustc_hash::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -538,29 +538,28 @@ fn trait_applicable_items(
Some(assoc_item_trait.into())
})
.filter(|candidate_trait_id| {
// blanket `Trait` implementations for type `A` can only exist in the crate defining
// the trait. However, an implementation for `A`` can only exist in `A`'s or `Trait`'s crate,
// which allows us to reduce the search space substantially.
// 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
let candidate_trait_id: TraitId = *candidate_trait_id;
let definining_crate_for_trait = db.trait_environment(candidate_trait_id.into()).krate;

let Some(receiver_fingerprint) =
TyFingerprint::for_trait_impl(&trait_candidate.receiver_ty.ty)
let definining_crate_for_trait = Trait::from(candidate_trait_id).krate(db);
let Some(receiver_fingerprint) = trait_candidate.receiver_ty.fingerprint_for_trait_impl()
else {
return false;
};
let definitions_exist_in_trait_crate = db
.trait_impls_in_crate(definining_crate_for_trait)
.trait_impls_in_crate(definining_crate_for_trait.into())
.for_trait_and_self_ty(candidate_trait_id, receiver_fingerprint)
.next()
.is_some();

let definitions_exist_in_receiver_crate = db
.trait_impls_in_crate(trait_candidate.receiver_ty.krate(db).id())
.trait_impls_in_crate(trait_candidate.receiver_ty.krate(db).into())
.for_trait_and_self_ty(candidate_trait_id, receiver_fingerprint)
.next()
.is_some();

definitions_exist_in_trait_crate || definitions_exist_in_receiver_crate
})
.collect();
Expand Down

0 comments on commit b67bbd1

Please sign in to comment.