Skip to content
Merged
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
145 changes: 142 additions & 3 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
Comment on lines +91 to +97
Copy link
Contributor

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)

}

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn is_slice_ty(&self, ty: Ty<'tcx>, span: Span) -> bool {
self.autoderef(span, ty)
Expand Down Expand Up @@ -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) => {
Expand All @@ -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)))
Copy link
Contributor

Choose a reason for hiding this comment

The 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)...

Copy link
Member Author

Choose a reason for hiding this comment

The 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 missing_trait_names), falling back to the full path. But this logic might be complicated as you said, and the later note "the traits ... must be implemented" should help users understand what it is. So I'd go for the current style.
But I'm happy to follow-up later if you have a different thought here.

.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 ",
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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"),)
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/proc-macro/quote/not-repeatable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0599]: the method `quote_into_iter` exists for struct `Ipv4Addr`, but its
--> $DIR/not-repeatable.rs:11:13
|
LL | struct Ipv4Addr;
| --------------- method `quote_into_iter` not found for this struct because it doesn't satisfy `Ipv4Addr: Iterator`, `Ipv4Addr: ToTokens`, `Ipv4Addr: proc_macro::ext::RepIteratorExt` or `Ipv4Addr: proc_macro::ext::RepToTokensExt`
| --------------- method `quote_into_iter` not found for this struct because `Ipv4Addr` doesn't implement `Iterator` or `ToTokens`
...
LL | let _ = quote! { $($ip)* };
| ^^^^^^^^^^^^^^^^^^ method cannot be called on `Ipv4Addr` due to unsatisfied trait bounds
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/traits/associated-item-unsatisfied-trait-bounds.rs
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
}
33 changes: 33 additions & 0 deletions tests/ui/traits/associated-item-unsatisfied-trait-bounds.stderr
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`.
Loading