remove find_map_relevant_impl

[lcnr]

rust/compiler/rustc_middle/src/ty/trait_def.rs

Lines 153 to 158 in 9b60e6c

	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:

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Lines 738 to 763 in 9b60e6c

	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 😅

rust/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

Lines 2195 to 2197 in 9b60e6c

	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.

rust/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

Line 1818 in 9b60e6c

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.

[jyn514]

Oh hi, I wrote that rustdoc code 😆

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

What does it mean for multiple impls to be applicable? Like different generic arguments for the same generic type? That should be ok - rustdoc is just checking if these functions exist at all for the unsubstituted ty, not whether they can actually be called. e.g. it's ok for it to have a false positive on impl Clone for Vec<Mutex> as long as there's some type T for which Vec<T>: Clone.

[jyn514]

(hate mobile GitHub 😠 )

[vincenzopalazzo]

@rustbot claim

This looks fun!

[compiler-errors]

@vincenzopalazzo are you working on this? I think I'd like to finish this unless you've already put a lot of time into this.

[vincenzopalazzo]

@compiler-errors Sorry I did not have time, so take it. Can you cc me on the PR when it is out I would like to see the code 😄