Skip to content
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.

Sign up for GitHub

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

Reverts Mention implementers of unsatisfied trait #95703

Closed
wants to merge 7 commits into from
Closed

Reverts Mention implementers of unsatisfied trait #95703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9381d9c
Revert "Remove hack, fix fmt and tests"
Dylan-DPC Apr 5, 2022
8562752
Revert "Highlight `is` in the message for emphasis"
Dylan-DPC Apr 5, 2022
1fdedfc
Revert "Refer to the TraitRef::identity in the message to be clearer"
Dylan-DPC Apr 5, 2022
5a93f38
Revert "Dedup logic and improve output for other types that impl trait"
Dylan-DPC Apr 5, 2022
746c9b4
Revert "Fix list length"
Dylan-DPC Apr 5, 2022
3e15b92
Revert "Suggest dereferncing when possible in E0277, fix #87437"
Dylan-DPC Apr 5, 2022
ecb0a11
Revert "Fix #90970, doesn't address #87437"
Dylan-DPC Apr 5, 2022
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
196 changes: 67 additions & 129 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ pub mod on_unimplemented;
pub mod suggestions;

use super::{
DerivedObligationCause, EvaluationResult, FulfillmentContext, FulfillmentError,
FulfillmentErrorCode, ImplDerivedObligationCause, MismatchedProjectionTypes, Obligation,
ObligationCause, ObligationCauseCode, OnUnimplementedDirective, OnUnimplementedNote,
OutputTypeParameterMismatch, Overflow, PredicateObligation, SelectionContext, SelectionError,
TraitNotObjectSafe,
EvaluationResult, FulfillmentContext, FulfillmentError, FulfillmentErrorCode,
MismatchedProjectionTypes, Obligation, ObligationCause, ObligationCauseCode,
OnUnimplementedDirective, OnUnimplementedNote, OutputTypeParameterMismatch, Overflow,
PredicateObligation, SelectionContext, SelectionError, TraitNotObjectSafe,
};

use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode};
Expand All @@ -30,7 +29,7 @@ use rustc_middle::traits::select::OverflowError;
use rustc_middle::ty::error::ExpectedFound;
use rustc_middle::ty::fold::TypeFolder;
use rustc_middle::ty::{
self, SubtypePredicate, ToPolyTraitRef, ToPredicate, TraitRef, Ty, TyCtxt, TypeFoldable,
self, SubtypePredicate, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, TypeFoldable,
};
use rustc_span::symbol::{kw, sym};
use rustc_span::{ExpnKind, MultiSpan, Span, DUMMY_SP};
Expand Down Expand Up @@ -661,77 +660,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
} else if !suggested {
// Can't show anything else useful, try to find similar impls.
let impl_candidates = self.find_similar_impl_candidates(trait_ref);
if !self.report_similar_impl_candidates(
self.report_similar_impl_candidates(
impl_candidates,
trait_ref,
&mut err,
) {
// This is *almost* equivalent to
// `obligation.cause.code().peel_derives()`, but it gives us the
// trait predicate for that corresponding root obligation. This
// lets us get a derived obligation from a type parameter, like
// when calling `string.strip_suffix(p)` where `p` is *not* an
// implementer of `Pattern<'_>`.
let mut code = obligation.cause.code();
let mut trait_pred = trait_predicate;
let mut peeled = false;
loop {
match &*code {
ObligationCauseCode::FunctionArgumentObligation {
parent_code,
..
} => {
code = &parent_code;
}
ObligationCauseCode::ImplDerivedObligation(
box ImplDerivedObligationCause {
derived:
DerivedObligationCause {
parent_code,
parent_trait_pred,
},
..
},
)
| ObligationCauseCode::BuiltinDerivedObligation(
DerivedObligationCause {
parent_code,
parent_trait_pred,
},
)
| ObligationCauseCode::DerivedObligation(
DerivedObligationCause {
parent_code,
parent_trait_pred,
},
) => {
peeled = true;
code = &parent_code;
trait_pred = *parent_trait_pred;
}
_ => break,
};
}
let def_id = trait_pred.def_id();
// Mention *all* the `impl`s for the *top most* obligation, the
// user might have meant to use one of them, if any found. We skip
// auto-traits or fundamental traits that might not be exactly what
// the user might expect to be presented with. Instead this is
// useful for less general traits.
if peeled
&& !self.tcx.trait_is_auto(def_id)
&& !self.tcx.lang_items().items().contains(&Some(def_id))
{
let trait_ref = trait_pred.to_poly_trait_ref();
let impl_candidates =
self.find_similar_impl_candidates(trait_ref);
self.report_similar_impl_candidates(
impl_candidates,
trait_ref,
&mut err,
);
}
}
);
}

// Changing mutability doesn't make a difference to whether we have
Expand Down Expand Up @@ -1762,56 +1695,6 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
trait_ref: ty::PolyTraitRef<'tcx>,
err: &mut Diagnostic,
) -> bool {
let report = |mut candidates: Vec<TraitRef<'tcx>>, err: &mut Diagnostic| {
candidates.sort();
candidates.dedup();
let len = candidates.len();
if candidates.len() == 0 {
return false;
}
if candidates.len() == 1 {
err.highlighted_help(vec![
(
format!("the trait `{}` ", candidates[0].print_only_trait_path()),
Style::NoStyle,
),
("is".to_string(), Style::Highlight),
(" implemented for `".to_string(), Style::NoStyle),
(candidates[0].self_ty().to_string(), Style::Highlight),
("`".to_string(), Style::NoStyle),
]);
return true;
}
let trait_ref = TraitRef::identity(self.tcx, candidates[0].def_id);
// Check if the trait is the same in all cases. If so, we'll only show the type.
let mut traits: Vec<_> =
candidates.iter().map(|c| c.print_only_trait_path().to_string()).collect();
traits.sort();
traits.dedup();

let mut candidates: Vec<String> = candidates
.into_iter()
.map(|c| {
if traits.len() == 1 {
format!("\n {}", c.self_ty())
} else {
format!("\n {}", c)
}
})
.collect();

candidates.sort();
candidates.dedup();
let end = if candidates.len() <= 9 { candidates.len() } else { 8 };
err.help(&format!(
"the following other types implement trait `{}`:{}{}",
trait_ref.print_only_trait_path(),
candidates[..end].join(""),
if len > 9 { format!("\nand {} others", len - 8) } else { String::new() }
));
true
};

let def_id = trait_ref.def_id();
if impl_candidates.is_empty() {
if self.tcx.trait_is_auto(def_id)
Expand All @@ -1821,7 +1704,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
// Mentioning implementers of `Copy`, `Debug` and friends is not useful.
return false;
}
let normalized_impl_candidates: Vec<_> = self
let mut normalized_impl_candidates: Vec<_> = self
.tcx
.all_impls(def_id)
// Ignore automatically derived impls and `!Trait` impls.
Expand All @@ -1832,19 +1715,54 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
.filter_map(|def_id| self.tcx.impl_trait_ref(def_id))
// Avoid mentioning type parameters.
.filter(|trait_ref| !matches!(trait_ref.self_ty().kind(), ty::Param(_)))
.map(|trait_ref| format!("\n {}", trait_ref.self_ty()))
.collect();
return report(normalized_impl_candidates, err);
normalized_impl_candidates.sort();
normalized_impl_candidates.dedup();
let len = normalized_impl_candidates.len();
if len == 0 {
return false;
}
if len == 1 {
err.highlighted_help(vec![
(
format!(
"the trait `{}` is implemented for `",
trait_ref.print_only_trait_path()
),
Style::NoStyle,
),
(normalized_impl_candidates[0].trim().to_string(), Style::Highlight),
("`".to_string(), Style::NoStyle),
]);
return true;
}
let end = if normalized_impl_candidates.len() <= 9 {
normalized_impl_candidates.len()
} else {
8
};
err.help(&format!(
"the following other types implement trait `{}`:{}{}",
trait_ref.print_only_trait_path(),
normalized_impl_candidates[..end].join(""),
if len > 9 { format!("\nand {} others", len - 8) } else { String::new() }
));
return true;
}

let len = impl_candidates.len();
let end = if impl_candidates.len() <= 5 { impl_candidates.len() } else { 4 };

let normalize = |candidate| {
self.tcx.infer_ctxt().enter(|ref infcx| {
let normalized = infcx
.at(&ObligationCause::dummy(), ty::ParamEnv::empty())
.normalize(candidate)
.ok();
match normalized {
Some(normalized) => normalized.value,
None => candidate,
Some(normalized) => format!("\n {}", normalized.value),
None => format!("\n {}", candidate),
}
})
};
Expand All @@ -1855,6 +1773,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
//
// Prefer more similar candidates first, then sort lexicographically
// by their normalized string representation.
let first_candidate = impl_candidates.get(0).map(|candidate| candidate.trait_ref);
let mut normalized_impl_candidates_and_similarities = impl_candidates
.into_iter()
.map(|ImplCandidate { trait_ref, similarity }| {
Expand All @@ -1870,7 +1789,26 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
.map(|(_, normalized)| normalized)
.collect::<Vec<_>>();

report(normalized_impl_candidates, err)
if normalized_impl_candidates.len() == 1 {
err.highlighted_help(vec![
(
format!(
"the trait `{}` is implemented for `",
first_candidate.unwrap().print_only_trait_path()
),
Style::NoStyle,
),
(first_candidate.unwrap().self_ty().to_string(), Style::Highlight),
("`".to_string(), Style::NoStyle),
]);
} else {
err.help(&format!(
"the following implementations were found:{}{}",
normalized_impl_candidates[..end].join(""),
if len > 9 { format!("\nand {} others", len - 8) } else { String::new() }
));
}
true
}

/// Gets the parent trait chain start
Expand Down
107 changes: 38 additions & 69 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
DerivedObligationCause, EvaluationResult, ImplDerivedObligationCause, Obligation,
ObligationCause, ObligationCauseCode, PredicateObligation, SelectionContext,
EvaluationResult, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation,
SelectionContext,
};

use crate::autoderef::Autoderef;
Expand Down Expand Up @@ -504,81 +504,50 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
trait_pred: ty::PolyTraitPredicate<'tcx>,
) -> bool {
// It only make sense when suggesting dereferences for arguments
let ObligationCauseCode::FunctionArgumentObligation { .. } = obligation.cause.code() else {
let code = if let ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } =
obligation.cause.code()
{
parent_code.clone()
} else {
return false;
};
let param_env = obligation.param_env;
let body_id = obligation.cause.body_id;
let span = obligation.cause.span;
let mut real_trait_pred = trait_pred;
let mut code = obligation.cause.code();
loop {
match &code {
ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
code = &parent_code;
}
ObligationCauseCode::ImplDerivedObligation(box ImplDerivedObligationCause {
derived: DerivedObligationCause { parent_code, parent_trait_pred },
..
})
| ObligationCauseCode::BuiltinDerivedObligation(DerivedObligationCause {
parent_code,
parent_trait_pred,
})
| ObligationCauseCode::DerivedObligation(DerivedObligationCause {
parent_code,
parent_trait_pred,
}) => {
code = &parent_code;
real_trait_pred = *parent_trait_pred;
}
_ => break,
};
let Some(real_ty) = real_trait_pred.self_ty().no_bound_vars() else {
continue;
};
let real_trait_pred = match &*code {
ObligationCauseCode::ImplDerivedObligation(cause) => cause.derived.parent_trait_pred,
ObligationCauseCode::DerivedObligation(cause)
| ObligationCauseCode::BuiltinDerivedObligation(cause) => cause.parent_trait_pred,
_ => trait_pred,
};
let Some(real_ty) = real_trait_pred.self_ty().no_bound_vars() else {
return false;
};

if let ty::Ref(region, base_ty, mutbl) = *real_ty.kind() {
let mut autoderef = Autoderef::new(self, param_env, body_id, span, base_ty, span);
if let Some(steps) = autoderef.find_map(|(ty, steps)| {
// Re-add the `&`
let ty = self.tcx.mk_ref(region, TypeAndMut { ty, mutbl });
let obligation =
self.mk_trait_obligation_with_new_self_ty(param_env, real_trait_pred, ty);
Some(steps).filter(|_| self.predicate_may_hold(&obligation))
}) {
if steps > 0 {
if let Ok(src) = self.tcx.sess.source_map().span_to_snippet(span) {
// Don't care about `&mut` because `DerefMut` is used less
// often and user will not expect autoderef happens.
if src.starts_with('&') && !src.starts_with("&mut ") {
let derefs = "*".repeat(steps);
err.span_suggestion(
span,
"consider dereferencing here",
format!("&{}{}", derefs, &src[1..]),
Applicability::MachineApplicable,
);
return true;
}
if let ty::Ref(region, base_ty, mutbl) = *real_ty.kind() {
let mut autoderef = Autoderef::new(self, param_env, body_id, span, base_ty, span);
if let Some(steps) = autoderef.find_map(|(ty, steps)| {
// Re-add the `&`
let ty = self.tcx.mk_ref(region, TypeAndMut { ty, mutbl });
let obligation =
self.mk_trait_obligation_with_new_self_ty(param_env, real_trait_pred, ty);
Some(steps).filter(|_| self.predicate_may_hold(&obligation))
}) {
if steps > 0 {
if let Ok(src) = self.tcx.sess.source_map().span_to_snippet(span) {
// Don't care about `&mut` because `DerefMut` is used less
// often and user will not expect autoderef happens.
if src.starts_with('&') && !src.starts_with("&mut ") {
let derefs = "*".repeat(steps);
err.span_suggestion(
span,
"consider adding dereference here",
format!("&{}{}", derefs, &src[1..]),
Applicability::MachineApplicable,
);
return true;
}
}
} else if real_trait_pred != trait_pred {
// This branch addresses #87437.
let obligation = self.mk_trait_obligation_with_new_self_ty(
param_env,
real_trait_pred,
base_ty,
);
if self.predicate_may_hold(&obligation) {
err.span_suggestion_verbose(
span.shrink_to_lo(),
"consider dereferencing here",
"*".to_string(),
Applicability::MachineApplicable,
);
return true;
}
}
}
}
Expand Down
Loading