Skip to content

Make find_similar_impl_candidates a little fuzzier. #92223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use rustc_middle::mir::interpret;
use rustc_middle::thir;
use rustc_middle::traits::specialization_graph;
use rustc_middle::ty::codec::TyEncoder;
use rustc_middle::ty::fast_reject::{self, SimplifiedType, SimplifyParams, StripReferences};
use rustc_middle::ty::fast_reject::{self, SimplifiedType, SimplifyParams};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, SymbolName, Ty, TyCtxt};
use rustc_serialize::{opaque, Encodable, Encoder};
Expand Down Expand Up @@ -2066,7 +2066,6 @@ impl<'tcx, 'v> ItemLikeVisitor<'v> for ImplsVisitor<'tcx> {
self.tcx,
trait_ref.self_ty(),
SimplifyParams::No,
StripReferences::No,
);

self.impls
Expand Down
20 changes: 1 addition & 19 deletions compiler/rustc_middle/src/ty/fast_reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ pub enum SimplifyParams {
No,
}

#[derive(PartialEq, Eq, Debug, Clone, Copy)]
pub enum StripReferences {
Yes,
No,
}

/// Tries to simplify a type by only returning the outermost injective¹ layer, if one exists.
///
/// The idea is to get something simple that we can use to quickly decide if two types could unify,
Expand All @@ -73,8 +67,6 @@ pub enum StripReferences {
/// When using `SimplifyParams::Yes`, we still return a simplified type for params and projections²,
/// the reasoning for this can be seen at the places doing this.
///
/// For diagnostics we strip references with `StripReferences::Yes`. This is currently the best
/// way to skip some unhelpful suggestions.
///
/// ¹ meaning that if two outermost layers are different, then the whole types are also different.
/// ² FIXME(@lcnr): this seems like it can actually end up being unsound with the way it's used during
Expand All @@ -87,7 +79,6 @@ pub fn simplify_type(
tcx: TyCtxt<'_>,
ty: Ty<'_>,
can_simplify_params: SimplifyParams,
strip_references: StripReferences,
) -> Option<SimplifiedType> {
match *ty.kind() {
ty::Bool => Some(BoolSimplifiedType),
Expand All @@ -106,16 +97,7 @@ pub fn simplify_type(
}
_ => Some(MarkerTraitObjectSimplifiedType),
},
ty::Ref(_, ty, mutbl) => {
if strip_references == StripReferences::Yes {
// For diagnostics, when recommending similar impls we want to
// recommend impls even when there is a reference mismatch,
// so we treat &T and T equivalently in that case.
simplify_type(tcx, ty, can_simplify_params, strip_references)
} else {
Some(RefSimplifiedType(mutbl))
}
}
ty::Ref(_, _, mutbl) => Some(RefSimplifiedType(mutbl)),
ty::FnDef(def_id, _) | ty::Closure(def_id, _) => Some(ClosureSimplifiedType(def_id)),
ty::Generator(def_id, _, _) => Some(GeneratorSimplifiedType(def_id)),
ty::GeneratorWitness(ref tys) => {
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::traits::specialization_graph;
use crate::ty::fast_reject::{self, SimplifiedType, SimplifyParams, StripReferences};
use crate::ty::fast_reject::{self, SimplifiedType, SimplifyParams};
use crate::ty::fold::TypeFoldable;
use crate::ty::{Ident, Ty, TyCtxt};
use rustc_hir as hir;
Expand Down Expand Up @@ -172,9 +172,7 @@ impl<'tcx> TyCtxt<'tcx> {
// whose outer level is not a parameter or projection. Especially for things like
// `T: Clone` this is incredibly useful as we would otherwise look at all the impls
// of `Clone` for `Option<T>`, `Vec<T>`, `ConcreteType` and so on.
if let Some(simp) =
fast_reject::simplify_type(self, self_ty, SimplifyParams::Yes, StripReferences::No)
{
if let Some(simp) = fast_reject::simplify_type(self, self_ty, SimplifyParams::Yes) {
if let Some(impls) = impls.non_blanket_impls.get(&simp) {
for &impl_def_id in impls {
if let result @ Some(_) = f(impl_def_id) {
Expand Down Expand Up @@ -234,7 +232,7 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> Trait
}

if let Some(simplified_self_ty) =
fast_reject::simplify_type(tcx, impl_self_ty, SimplifyParams::No, StripReferences::No)
fast_reject::simplify_type(tcx, impl_self_ty, SimplifyParams::No)
{
impls.non_blanket_impls.entry(simplified_self_ty).or_default().push(impl_def_id);
} else {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::traits::{
PredicateObligations, SelectionContext,
};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::fast_reject::{self, SimplifyParams};
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, Ty, TyCtxt};
Expand Down Expand Up @@ -80,8 +80,8 @@ where
impl2_ref.iter().flat_map(|tref| tref.substs.types()),
)
.any(|(ty1, ty2)| {
let t1 = fast_reject::simplify_type(tcx, ty1, SimplifyParams::No, StripReferences::No);
let t2 = fast_reject::simplify_type(tcx, ty2, SimplifyParams::No, StripReferences::No);
let t1 = fast_reject::simplify_type(tcx, ty1, SimplifyParams::No);
let t2 = fast_reject::simplify_type(tcx, ty2, SimplifyParams::No);

if let (Some(t1), Some(t2)) = (t1, t2) {
// Simplified successfully
Expand Down
155 changes: 87 additions & 68 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ use rustc_hir::Item;
use rustc_hir::Node;
use rustc_middle::thir::abstract_const::NotConstEvaluatable;
use rustc_middle::ty::error::ExpectedFound;
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::fold::TypeFolder;
use rustc_middle::ty::{
self, AdtKind, SubtypePredicate, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, TypeFoldable,
self, SubtypePredicate, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, TypeFoldable,
};
use rustc_session::DiagnosticMessageId;
use rustc_span::symbol::{kw, sym};
Expand All @@ -40,6 +39,19 @@ use suggestions::InferCtxtExt as _;

pub use rustc_infer::traits::error_reporting::*;

// When outputting impl candidates, prefer showing those that are more similar.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum CandidateSimilarity {
Exact,
Fuzzy,
}

#[derive(Debug, Clone, Copy)]
pub struct ImplCandidate<'tcx> {
pub trait_ref: ty::TraitRef<'tcx>,
pub similarity: CandidateSimilarity,
}

pub trait InferCtxtExt<'tcx> {
fn report_fulfillment_errors(
&self,
Expand Down Expand Up @@ -1091,11 +1103,11 @@ trait InferCtxtPrivExt<'hir, 'tcx> {
fn find_similar_impl_candidates(
&self,
trait_ref: ty::PolyTraitRef<'tcx>,
) -> Vec<ty::TraitRef<'tcx>>;
) -> Vec<ImplCandidate<'tcx>>;

fn report_similar_impl_candidates(
&self,
impl_candidates: Vec<ty::TraitRef<'tcx>>,
impl_candidates: Vec<ImplCandidate<'tcx>>,
err: &mut DiagnosticBuilder<'_>,
);

Expand Down Expand Up @@ -1409,22 +1421,33 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
ty::Param(..) => Some(12),
ty::Opaque(..) => Some(13),
ty::Never => Some(14),
ty::Adt(adt, ..) => match adt.adt_kind() {
AdtKind::Struct => Some(15),
AdtKind::Union => Some(16),
AdtKind::Enum => Some(17),
},
ty::Generator(..) => Some(18),
ty::Foreign(..) => Some(19),
ty::GeneratorWitness(..) => Some(20),
ty::Adt(..) => Some(15),
ty::Generator(..) => Some(16),
ty::Foreign(..) => Some(17),
ty::GeneratorWitness(..) => Some(18),
ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) | ty::Error(_) => None,
}
}

let strip_references = |mut t: Ty<'tcx>| -> Ty<'tcx> {
loop {
match t.kind() {
ty::Ref(_, inner, _) | ty::RawPtr(ty::TypeAndMut { ty: inner, .. }) => {
t = inner
}
_ => break t,
}
}
};

match (type_category(a), type_category(b)) {
(Some(cat_a), Some(cat_b)) => match (a.kind(), b.kind()) {
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => def_a == def_b,
_ => cat_a == cat_b,
(ty::Adt(def_a, _), ty::Adt(def_b, _)) => def_a == def_b,
_ if cat_a == cat_b => true,
(ty::Ref(..), _) | (_, ty::Ref(..)) => {
self.fuzzy_match_tys(strip_references(a), strip_references(b))
}
_ => false,
},
// infer and error can be equated to all types
_ => true,
Expand All @@ -1443,58 +1466,39 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
fn find_similar_impl_candidates(
&self,
trait_ref: ty::PolyTraitRef<'tcx>,
) -> Vec<ty::TraitRef<'tcx>> {
// We simplify params and strip references here.
//
// This both removes a lot of unhelpful suggestions, e.g.
// when searching for `&Foo: Trait` it doesn't suggestion `impl Trait for &Bar`,
// while also suggesting impls for `&Foo` when we're looking for `Foo: Trait`.
//
// The second thing isn't necessarily always a good thing, but
// any other simple setup results in a far worse output, so 🤷
let simp = fast_reject::simplify_type(
self.tcx,
trait_ref.skip_binder().self_ty(),
SimplifyParams::Yes,
StripReferences::Yes,
);
let all_impls = self.tcx.all_impls(trait_ref.def_id());

match simp {
Some(simp) => all_impls
.filter_map(|def_id| {
let imp = self.tcx.impl_trait_ref(def_id).unwrap();
let imp_simp = fast_reject::simplify_type(
self.tcx,
imp.self_ty(),
SimplifyParams::Yes,
StripReferences::Yes,
);
if let Some(imp_simp) = imp_simp {
if simp != imp_simp {
return None;
}
}
if self.tcx.impl_polarity(def_id) == ty::ImplPolarity::Negative {
return None;
}
Some(imp)
})
.collect(),
None => all_impls
.filter_map(|def_id| {
if self.tcx.impl_polarity(def_id) == ty::ImplPolarity::Negative {
return None;
}
self.tcx.impl_trait_ref(def_id)
})
.collect(),
}
) -> Vec<ImplCandidate<'tcx>> {
self.tcx
.all_impls(trait_ref.def_id())
.filter_map(|def_id| {
if self.tcx.impl_polarity(def_id) == ty::ImplPolarity::Negative {
return None;
}

let imp = self.tcx.impl_trait_ref(def_id).unwrap();

// Check for exact match.
if trait_ref.skip_binder().self_ty() == imp.self_ty() {
return Some(ImplCandidate {
trait_ref: imp,
similarity: CandidateSimilarity::Exact,
});
}

if self.fuzzy_match_tys(trait_ref.skip_binder().self_ty(), imp.self_ty()) {
return Some(ImplCandidate {
trait_ref: imp,
similarity: CandidateSimilarity::Fuzzy,
});
}

None
})
.collect()
}

fn report_similar_impl_candidates(
&self,
impl_candidates: Vec<ty::TraitRef<'tcx>>,
impl_candidates: Vec<ImplCandidate<'tcx>>,
err: &mut DiagnosticBuilder<'_>,
) {
if impl_candidates.is_empty() {
Expand All @@ -1518,13 +1522,24 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
};

// Sort impl candidates so that ordering is consistent for UI tests.
let mut normalized_impl_candidates =
impl_candidates.iter().copied().map(normalize).collect::<Vec<String>>();

// Sort before taking the `..end` range,
// because the ordering of `impl_candidates` may not be deterministic:
// https://github.com/rust-lang/rust/pull/57475#issuecomment-455519507
normalized_impl_candidates.sort();
//
// Prefer more similar candidates first, then sort lexicographically
// by their normalized string representation.
let mut normalized_impl_candidates_and_similarities = impl_candidates
.into_iter()
.map(|ImplCandidate { trait_ref, similarity }| {
let normalized = normalize(trait_ref);
(similarity, normalized)
})
.collect::<Vec<_>>();
normalized_impl_candidates_and_similarities.sort();

let normalized_impl_candidates = normalized_impl_candidates_and_similarities
.into_iter()
.map(|(_, normalized)| normalized)
.collect::<Vec<_>>();

err.help(&format!(
"the following implementations were found:{}{}",
Expand Down Expand Up @@ -1688,7 +1703,11 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
return;
}

let impl_candidates = self.find_similar_impl_candidates(trait_ref);
let impl_candidates = self
.find_similar_impl_candidates(trait_ref)
.into_iter()
.map(|candidate| candidate.trait_ref)
.collect();
let mut err = self.emit_inference_failure_err(
body_id,
span,
Expand Down
11 changes: 3 additions & 8 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use rustc_infer::infer::LateBoundRegionConversionTime;
use rustc_middle::dep_graph::{DepKind, DepNodeIndex};
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::thir::abstract_const::NotConstEvaluatable;
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::fast_reject::{self, SimplifyParams};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::relate::TypeRelation;
use rustc_middle::ty::subst::{GenericArgKind, Subst, SubstsRef};
Expand Down Expand Up @@ -2174,14 +2174,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.tcx(),
obligation_ty,
SimplifyParams::Yes,
StripReferences::No,
);
let simplified_impl_ty = fast_reject::simplify_type(
self.tcx(),
impl_ty,
SimplifyParams::No,
StripReferences::No,
);
let simplified_impl_ty =
fast_reject::simplify_type(self.tcx(), impl_ty, SimplifyParams::No);

simplified_obligation_ty.is_some()
&& simplified_impl_ty.is_some()
Expand Down
Loading