-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
Tweak E0599 to consolidate unsatisfied trait bound messages #151488
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,9 @@ use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; | |
| use rustc_data_structures::sorted_map::SortedMap; | ||
| use rustc_data_structures::unord::UnordSet; | ||
| use rustc_errors::codes::*; | ||
| use rustc_errors::{Applicability, Diag, MultiSpan, StashKey, pluralize, struct_span_code_err}; | ||
| use rustc_errors::{ | ||
| Applicability, Diag, MultiSpan, StashKey, listify, pluralize, struct_span_code_err, | ||
| }; | ||
| use rustc_hir::attrs::AttributeKind; | ||
| use rustc_hir::def::{CtorKind, DefKind, Res}; | ||
| use rustc_hir::def_id::DefId; | ||
|
|
@@ -50,6 +52,51 @@ use crate::errors::{self, CandidateTraitNote, NoAssociatedItem}; | |
| use crate::method::probe::UnsatisfiedPredicates; | ||
| use crate::{Expectation, FnCtxt}; | ||
|
|
||
| /// Tracks trait bounds and detects duplicates between ref and non-ref versions of self types. | ||
| /// This is used to condense error messages when the same trait bound appears for both | ||
| /// `T` and `&T` (or `&mut T`). | ||
| struct TraitBoundDuplicateTracker { | ||
| trait_def_ids: FxIndexSet<DefId>, | ||
| seen_ref: FxIndexSet<DefId>, | ||
| seen_non_ref: FxIndexSet<DefId>, | ||
| has_ref_dupes: bool, | ||
| } | ||
|
|
||
| impl TraitBoundDuplicateTracker { | ||
| fn new() -> Self { | ||
| Self { | ||
| trait_def_ids: FxIndexSet::default(), | ||
| seen_ref: FxIndexSet::default(), | ||
| seen_non_ref: FxIndexSet::default(), | ||
| has_ref_dupes: false, | ||
| } | ||
| } | ||
|
|
||
| /// Track a trait bound. `is_ref` indicates whether the self type is a reference. | ||
| fn track(&mut self, def_id: DefId, is_ref: bool) { | ||
| self.trait_def_ids.insert(def_id); | ||
| if is_ref { | ||
| if self.seen_non_ref.contains(&def_id) { | ||
| self.has_ref_dupes = true; | ||
| } | ||
| self.seen_ref.insert(def_id); | ||
| } else { | ||
| if self.seen_ref.contains(&def_id) { | ||
| self.has_ref_dupes = true; | ||
| } | ||
| self.seen_non_ref.insert(def_id); | ||
| } | ||
| } | ||
|
|
||
| fn has_ref_dupes(&self) -> bool { | ||
| self.has_ref_dupes | ||
| } | ||
|
|
||
| fn into_trait_def_ids(self) -> FxIndexSet<DefId> { | ||
| self.trait_def_ids | ||
| } | ||
| } | ||
|
|
||
| impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | ||
| fn is_slice_ty(&self, ty: Ty<'tcx>, span: Span) -> bool { | ||
| self.autoderef(span, ty) | ||
|
|
@@ -1004,6 +1051,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| item_ident: Ident, | ||
| item_kind: &str, | ||
| bound_spans: SortedMap<Span, Vec<String>>, | ||
| unsatisfied_predicates: &UnsatisfiedPredicates<'tcx>, | ||
| ) { | ||
| let mut ty_span = match rcvr_ty.kind() { | ||
| ty::Param(param_type) => { | ||
|
|
@@ -1012,13 +1060,61 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| ty::Adt(def, _) if def.did().is_local() => Some(self.tcx.def_span(def.did())), | ||
| _ => None, | ||
| }; | ||
| let rcvr_ty_str = self.tcx.short_string(rcvr_ty, err.long_ty_path()); | ||
| let mut tracker = TraitBoundDuplicateTracker::new(); | ||
| for (predicate, _parent_pred, _cause) in unsatisfied_predicates { | ||
| if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) = | ||
| predicate.kind().skip_binder() | ||
| && let self_ty = pred.trait_ref.self_ty() | ||
| && self_ty.peel_refs() == rcvr_ty | ||
| { | ||
| let is_ref = matches!(self_ty.kind(), ty::Ref(..)); | ||
| tracker.track(pred.trait_ref.def_id, is_ref); | ||
| } | ||
| } | ||
| let has_ref_dupes = tracker.has_ref_dupes(); | ||
| let mut missing_trait_names = tracker | ||
| .into_trait_def_ids() | ||
| .into_iter() | ||
| .map(|def_id| format!("`{}`", self.tcx.def_path_str(def_id))) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has the only issue of not being shortened type paths. Dealing with them might make the logic harder (because the shortened types might actually look the same)...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough! I've given it some thoughts and one of the solutions would be 1) emitting shortened paths for most cases, 2) if conflicted (i.e. we have the same name in |
||
| .collect::<Vec<_>>(); | ||
| missing_trait_names.sort(); | ||
| let should_condense = | ||
| has_ref_dupes && missing_trait_names.len() > 1 && matches!(rcvr_ty.kind(), ty::Adt(..)); | ||
| let missing_trait_list = if should_condense { | ||
| Some(match missing_trait_names.as_slice() { | ||
| [only] => only.clone(), | ||
| [first, second] => format!("{first} or {second}"), | ||
| [rest @ .., last] => format!("{} or {last}", rest.join(", ")), | ||
| [] => String::new(), | ||
| }) | ||
| } else { | ||
| None | ||
| }; | ||
| for (span, mut bounds) in bound_spans { | ||
| if !self.tcx.sess.source_map().is_span_accessible(span) { | ||
| continue; | ||
| } | ||
| bounds.sort(); | ||
| bounds.dedup(); | ||
| let pre = if Some(span) == ty_span { | ||
| let is_ty_span = Some(span) == ty_span; | ||
| if is_ty_span && should_condense { | ||
| ty_span.take(); | ||
| let label = if let Some(missing_trait_list) = &missing_trait_list { | ||
| format!( | ||
| "{item_kind} `{item_ident}` not found for this {} because `{rcvr_ty_str}` doesn't implement {missing_trait_list}", | ||
| rcvr_ty.prefix_string(self.tcx) | ||
| ) | ||
| } else { | ||
| format!( | ||
| "{item_kind} `{item_ident}` not found for this {}", | ||
| rcvr_ty.prefix_string(self.tcx) | ||
| ) | ||
| }; | ||
| err.span_label(span, label); | ||
| continue; | ||
| } | ||
| let pre = if is_ty_span { | ||
| ty_span.take(); | ||
| format!( | ||
| "{item_kind} `{item_ident}` not found for this {} because it ", | ||
|
|
@@ -1248,6 +1344,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| item_ident, | ||
| item_kind, | ||
| bound_spans, | ||
| unsatisfied_predicates, | ||
| ); | ||
|
|
||
| self.note_derefed_ty_has_method(&mut err, source, rcvr_ty, item_ident, expected); | ||
|
|
@@ -1507,6 +1604,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| bound_spans: &mut SortedMap<Span, Vec<String>>, | ||
| ) { | ||
| let tcx = self.tcx; | ||
| let rcvr_ty_str = self.tcx.short_string(rcvr_ty, err.long_ty_path()); | ||
| let mut type_params = FxIndexMap::default(); | ||
|
|
||
| // Pick out the list of unimplemented traits on the receiver. | ||
|
|
@@ -1798,14 +1896,55 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| let mut spanned_predicates: Vec<_> = spanned_predicates.into_iter().collect(); | ||
| spanned_predicates.sort_by_key(|(span, _)| *span); | ||
| for (_, (primary_spans, span_labels, predicates)) in spanned_predicates { | ||
| let mut tracker = TraitBoundDuplicateTracker::new(); | ||
| let mut all_trait_bounds_for_rcvr = true; | ||
| for pred in &predicates { | ||
| match pred.kind().skip_binder() { | ||
| ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => { | ||
| let self_ty = pred.trait_ref.self_ty(); | ||
| if self_ty.peel_refs() != rcvr_ty { | ||
| all_trait_bounds_for_rcvr = false; | ||
| break; | ||
| } | ||
| let is_ref = matches!(self_ty.kind(), ty::Ref(..)); | ||
| tracker.track(pred.trait_ref.def_id, is_ref); | ||
| } | ||
| _ => { | ||
| all_trait_bounds_for_rcvr = false; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| let has_ref_dupes = tracker.has_ref_dupes(); | ||
| let trait_def_ids = tracker.into_trait_def_ids(); | ||
| let mut preds: Vec<_> = predicates | ||
| .iter() | ||
| .filter_map(|pred| format_pred(**pred)) | ||
| .map(|(p, _)| format!("`{p}`")) | ||
| .collect(); | ||
| preds.sort(); | ||
| preds.dedup(); | ||
| let msg = if let [pred] = &preds[..] { | ||
| let availability_note = if all_trait_bounds_for_rcvr | ||
| && has_ref_dupes | ||
| && trait_def_ids.len() > 1 | ||
| && matches!(rcvr_ty.kind(), ty::Adt(..)) | ||
| { | ||
| let mut trait_names = trait_def_ids | ||
| .into_iter() | ||
| .map(|def_id| format!("`{}`", tcx.def_path_str(def_id))) | ||
| .collect::<Vec<_>>(); | ||
| trait_names.sort(); | ||
| listify(&trait_names, |name| name.to_string()).map(|traits| { | ||
| format!( | ||
| "for `{item_ident}` to be available, `{rcvr_ty_str}` must implement {traits}" | ||
| ) | ||
| }) | ||
| } else { | ||
| None | ||
| }; | ||
| let msg = if let Some(availability_note) = availability_note { | ||
| availability_note | ||
| } else if let [pred] = &preds[..] { | ||
| format!("trait bound {pred} was not satisfied") | ||
| } else { | ||
| format!("the following trait bounds were not satisfied:\n{}", preds.join("\n"),) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| struct Foo; | ||
| trait Bar {} | ||
| trait Baz {} | ||
| trait Bat { fn bat(&self); } | ||
| impl<T> Bat for T where T: 'static + Bar + Baz { fn bat(&self) { println!("generic bat"); } } | ||
|
|
||
| pub fn main() { | ||
| Foo::bat(()); //~ ERROR E0599 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| error[E0599]: the function or associated item `bat` exists for struct `Foo`, but its trait bounds were not satisfied | ||
| --> $DIR/associated-item-unsatisfied-trait-bounds.rs:8:10 | ||
| | | ||
| LL | struct Foo; | ||
| | ---------- function or associated item `bat` not found for this struct because `Foo` doesn't implement `Bar` or `Baz` | ||
| ... | ||
| LL | Foo::bat(()); | ||
| | ^^^ function or associated item cannot be called on `Foo` due to unsatisfied trait bounds | ||
| | | ||
| note: for `bat` to be available, `Foo` must implement `Bar` and `Baz` | ||
| --> $DIR/associated-item-unsatisfied-trait-bounds.rs:5:38 | ||
| | | ||
| LL | impl<T> Bat for T where T: 'static + Bar + Baz { fn bat(&self) { println!("generic bat"); } } | ||
| | --- - ^^^ ^^^ unsatisfied trait bound introduced here | ||
| | | | ||
| | unsatisfied trait bound introduced here | ||
| note: the traits `Bar` and `Baz` must be implemented | ||
| --> $DIR/associated-item-unsatisfied-trait-bounds.rs:2:1 | ||
| | | ||
| LL | trait Bar {} | ||
| | ^^^^^^^^^ | ||
| LL | trait Baz {} | ||
| | ^^^^^^^^^ | ||
| = help: items from traits can only be used if the trait is implemented and in scope | ||
| note: `Bat` defines an item `bat`, perhaps you need to implement it | ||
| --> $DIR/associated-item-unsatisfied-trait-bounds.rs:4:1 | ||
| | | ||
| LL | trait Bat { fn bat(&self); } | ||
| | ^^^^^^^^^ | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0599`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I haven't found these kind of accessor methods to be needed, but they are not wrong. :)
(Don't change)