Skip to content

remove find_map_relevant_impl #108895

Closed
@lcnr

Description

@lcnr

pub fn find_map_relevant_impl<T, F: FnMut(DefId) -> Option<T>>(
self,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
mut f: F,
) -> Option<T> {

This function iterates over all impls which may apply to self_ty and returns the first Some. Note that just because an impl is given to f it may not actually apply to the whole trait_ref (or deeply matches self_ty). Because of this it ends up being very easy to misuse. Let's go through a few incorrect uses of this function:

tcx.find_map_relevant_impl(trait_, ty, |impl_| {
let trait_ref = tcx.impl_trait_ref(impl_).expect("this is not an inherent impl");
// Check if these are the same type.
let impl_type = trait_ref.skip_binder().self_ty();
trace!(
"comparing type {} with kind {:?} against type {:?}",
impl_type,
impl_type.kind(),
ty
);
// Fast path: if this is a primitive simple `==` will work
// NOTE: the `match` is necessary; see #92662.
// this allows us to ignore generics because the user input
// may not include the generic placeholders
// e.g. this allows us to match Foo (user comment) with Foo<T> (actual type)
let saw_impl = impl_type == ty
|| match (impl_type.kind(), ty.kind()) {
(ty::Adt(impl_def, _), ty::Adt(ty_def, _)) => {
debug!("impl def_id: {:?}, ty def_id: {:?}", impl_def.did(), ty_def.did());
impl_def.did() == ty_def.did()
}
_ => false,
};
if saw_impl { Some((impl_, trait_)) } else { None }
})

only looks at the first maybe applicable impl even if there are multiple, idk how exactly that function is used, but it should be wrong 😅

let get_trait_impl = |trait_def_id| {
self.tcx.find_map_relevant_impl(trait_def_id, trait_ref.skip_binder().self_ty(), Some)
};

Checks whether another trait with the same path has an impl for self_ty. Note that this emits the error message if 2 versions of a crate are used, but the substs implement neither of the traits.

self.tcx.find_map_relevant_impl(id, proj.projection_ty.self_ty(), |did| {

whatever this is doing 😅 but this should be usable to point to an impl which doesn't actually apply.

I would like to change all the uses of find_map_relevant_impl to instead use for_each_relevant_impl.

Metadata

Metadata

Labels

C-cleanupCategory: PRs that clean code up or issues documenting cleanup.E-hardCall for participation: Hard difficulty. Experience needed to fix: A lot.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions