Skip to content

Bound errors span label cleanup #120400

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

Merged
merged 4 commits into from
Jan 30, 2024
Merged

Bound errors span label cleanup #120400

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3691ab8
Use only one label for multiple unsatisfied bounds on type (astconv)
estebank Jan 26, 2024
757b726
Use only one label for multiple unsatisfied bounds on type (typeck)
estebank Jan 26, 2024
7df4a09
Use single label for method not found due to unmet bound
estebank Jan 26, 2024
fc964fb
review comments
estebank Jan 29, 2024
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
39 changes: 28 additions & 11 deletions compiler/rustc_hir_analysis/src/astconv/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,22 +461,24 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
return err.emit();
}

let mut bound_spans = Vec::new();
let mut bound_spans: FxHashMap<Span, Vec<String>> = Default::default();

let mut bound_span_label = |self_ty: Ty<'_>, obligation: &str, quiet: &str| {
let msg = format!(
"doesn't satisfy `{}`",
if obligation.len() > 50 { quiet } else { obligation }
);
let msg = format!("`{}`", if obligation.len() > 50 { quiet } else { obligation });
match &self_ty.kind() {
// Point at the type that couldn't satisfy the bound.
ty::Adt(def, _) => bound_spans.push((tcx.def_span(def.did()), msg)),
ty::Adt(def, _) => {
bound_spans.entry(tcx.def_span(def.did())).or_default().push(msg)
}
// Point at the trait object that couldn't satisfy the bound.
ty::Dynamic(preds, _, _) => {
for pred in preds.iter() {
match pred.skip_binder() {
ty::ExistentialPredicate::Trait(tr) => {
bound_spans.push((tcx.def_span(tr.def_id), msg.clone()))
bound_spans
.entry(tcx.def_span(tr.def_id))
.or_default()
.push(msg.clone());
}
ty::ExistentialPredicate::Projection(_)
| ty::ExistentialPredicate::AutoTrait(_) => {}
Expand All @@ -485,7 +487,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
// Point at the closure that couldn't satisfy the bound.
ty::Closure(def_id, _) => {
bound_spans.push((tcx.def_span(*def_id), format!("doesn't satisfy `{quiet}`")))
bound_spans
.entry(tcx.def_span(*def_id))
.or_default()
.push(format!("`{quiet}`"));
}
_ => {}
}
Expand Down Expand Up @@ -554,12 +559,24 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
format!("associated type cannot be referenced on `{self_ty}` due to unsatisfied trait bounds")
);

bound_spans.sort();
bound_spans.dedup();
for (span, msg) in bound_spans {
let mut bound_spans: Vec<(Span, Vec<String>)> = bound_spans
.into_iter()
.map(|(span, mut bounds)| {
bounds.sort();
bounds.dedup();
(span, bounds)
})
.collect();
bound_spans.sort_by_key(|(span, _)| *span);
for (span, bounds) in bound_spans {
if !tcx.sess.source_map().is_span_accessible(span) {
continue;
}
let msg = match &bounds[..] {
[bound] => format!("doesn't satisfy {bound}"),
[bounds @ .., last] => format!("doesn't satisfy {} or {last}", bounds.join(", ")),
[] => unreachable!(),
};
err.span_label(span, msg);
}
add_def_label(&mut err);
Expand Down
48 changes: 35 additions & 13 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::Expectation;
use crate::FnCtxt;
use rustc_ast::ast::Mutability;
use rustc_attr::parse_confusables;
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
use rustc_data_structures::unord::UnordSet;
use rustc_errors::StashKey;
use rustc_errors::{
Expand Down Expand Up @@ -547,7 +547,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}

let mut bound_spans = vec![];
let mut bound_spans: FxHashMap<Span, Vec<String>> = Default::default();
let mut restrict_type_params = false;
let mut unsatisfied_bounds = false;
if item_name.name == sym::count && self.is_slice_ty(rcvr_ty, span) {
Expand Down Expand Up @@ -642,28 +642,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
};
let mut bound_span_label = |self_ty: Ty<'_>, obligation: &str, quiet: &str| {
let msg = format!(
"doesn't satisfy `{}`",
if obligation.len() > 50 { quiet } else { obligation }
);
let msg = format!("`{}`", if obligation.len() > 50 { quiet } else { obligation });
match &self_ty.kind() {
// Point at the type that couldn't satisfy the bound.
ty::Adt(def, _) => bound_spans.push((self.tcx.def_span(def.did()), msg)),
ty::Adt(def, _) => {
bound_spans.entry(tcx.def_span(def.did())).or_default().push(msg)
}
// Point at the trait object that couldn't satisfy the bound.
ty::Dynamic(preds, _, _) => {
for pred in preds.iter() {
match pred.skip_binder() {
ty::ExistentialPredicate::Trait(tr) => {
bound_spans.push((self.tcx.def_span(tr.def_id), msg.clone()))
bound_spans
.entry(tcx.def_span(tr.def_id))
.or_default()
.push(msg.clone());
}
ty::ExistentialPredicate::Projection(_)
| ty::ExistentialPredicate::AutoTrait(_) => {}
}
}
}
// Point at the closure that couldn't satisfy the bound.
ty::Closure(def_id, _) => bound_spans
.push((tcx.def_span(*def_id), format!("doesn't satisfy `{quiet}`"))),
ty::Closure(def_id, _) => {
bound_spans
.entry(tcx.def_span(*def_id))
.or_default()
.push(format!("`{quiet}`"));
}
_ => {}
}
};
Expand Down Expand Up @@ -1170,9 +1176,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

self.suggest_unwrapping_inner_self(&mut err, source, rcvr_ty, item_name);

bound_spans.sort();
bound_spans.dedup();
for (span, msg) in bound_spans.into_iter() {
#[allow(rustc::potential_query_instability)] // We immediately sort the resulting Vec.
let mut bound_spans: Vec<(Span, Vec<String>)> = bound_spans
.into_iter()
.map(|(span, mut bounds)| {
bounds.sort();
bounds.dedup();
(span, bounds)
})
.collect();
bound_spans.sort_by_key(|(span, _)| *span);
for (span, bounds) in bound_spans {
if !tcx.sess.source_map().is_span_accessible(span) {
continue;
}
let msg = match &bounds[..] {
[bound] => format!("doesn't satisfy {bound}"),
[bounds @ .., last] => format!("doesn't satisfy {} or {last}", bounds.join(", ")),
[] => unreachable!(),
};
err.span_label(span, msg);
}

Expand Down
5 changes: 1 addition & 4 deletions tests/ui/associated-inherent-types/not-found-unsatisfied-bounds-in-multiple-impls.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ error: the associated type `X` exists for `S<Featureless, Featureless>`, but its
LL | struct S<A, B>(A, B);
| -------------- associated item `X` not found for this struct
LL | struct Featureless;
| ------------------
| |
| doesn't satisfy `Featureless: One`
| doesn't satisfy `Featureless: Two`
| ------------------ doesn't satisfy `Featureless: One` or `Featureless: Two`
...
LL | let _: S::<Featureless, Featureless>::X;
| ^ associated type cannot be referenced on `S<Featureless, Featureless>` due to unsatisfied trait bounds
Expand Down
15 changes: 4 additions & 11 deletions tests/ui/box/unit/unique-object-noncopyable.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
error[E0599]: the method `clone` exists for struct `Box<dyn Foo>`, but its trait bounds were not satisfied
--> $DIR/unique-object-noncopyable.rs:24:16
|
LL | trait Foo {
| ---------
| |
| doesn't satisfy `dyn Foo: Clone`
| doesn't satisfy `dyn Foo: Sized`
LL | trait Foo {
| --------- doesn't satisfy `dyn Foo: Clone` or `dyn Foo: Sized`
...
LL | let _z = y.clone();
| ^^^^^ method cannot be called on `Box<dyn Foo>` due to unsatisfied trait bounds
--> $SRC_DIR/alloc/src/boxed.rs:LL:COL
::: $SRC_DIR/alloc/src/boxed.rs:LL:COL
|
= note: doesn't satisfy `Box<dyn Foo>: Clone`
LL | let _z = y.clone();
| ^^^^^ method cannot be called on `Box<dyn Foo>` due to unsatisfied trait bounds
|
= note: the following trait bounds were not satisfied:
`dyn Foo: Sized`
Expand Down
12 changes: 4 additions & 8 deletions tests/ui/box/unit/unique-pinned-nocopy.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
error[E0599]: the method `clone` exists for struct `Box<R>`, but its trait bounds were not satisfied
--> $DIR/unique-pinned-nocopy.rs:12:16
|
LL | struct R {
| -------- doesn't satisfy `R: Clone`
LL | struct R {
| -------- doesn't satisfy `R: Clone`
...
LL | let _j = i.clone();
| ^^^^^ method cannot be called on `Box<R>` due to unsatisfied trait bounds
--> $SRC_DIR/alloc/src/boxed.rs:LL:COL
::: $SRC_DIR/alloc/src/boxed.rs:LL:COL
|
= note: doesn't satisfy `Box<R>: Clone`
LL | let _j = i.clone();
| ^^^^^ method cannot be called on `Box<R>` due to unsatisfied trait bounds
|
= note: the following trait bounds were not satisfied:
`R: Clone`
Expand Down
5 changes: 1 addition & 4 deletions tests/ui/derives/deriving-with-repr-packed-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ LL | pub struct Foo<T>(T, T, T);
| doesn't satisfy `Foo<NonCopy>: Clone`
LL |
LL | struct NonCopy;
| --------------
| |
| doesn't satisfy `NonCopy: Clone`
| doesn't satisfy `NonCopy: Copy`
| -------------- doesn't satisfy `NonCopy: Clone` or `NonCopy: Copy`
...
LL | _ = x.clone();
| ^^^^^ method cannot be called on `Foo<NonCopy>` due to unsatisfied trait bounds
Expand Down
25 changes: 4 additions & 21 deletions tests/ui/derives/issue-91550.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ error[E0599]: the method `insert` exists for struct `HashSet<Value>`, but its tr
--> $DIR/issue-91550.rs:8:8
|
LL | struct Value(u32);
| ------------
| |
| doesn't satisfy `Value: Eq`
| doesn't satisfy `Value: Hash`
| doesn't satisfy `Value: PartialEq`
| ------------ doesn't satisfy `Value: Eq`, `Value: Hash` or `Value: PartialEq`
...
LL | hs.insert(Value(0));
| ^^^^^^
Expand All @@ -26,10 +22,7 @@ error[E0599]: the method `use_eq` exists for struct `Object<NoDerives>`, but its
--> $DIR/issue-91550.rs:26:9
|
LL | pub struct NoDerives;
| --------------------
| |
| doesn't satisfy `NoDerives: Eq`
| doesn't satisfy `NoDerives: PartialEq`
| -------------------- doesn't satisfy `NoDerives: Eq` or `NoDerives: PartialEq`
LL |
LL | struct Object<T>(T);
| ---------------- method `use_eq` not found for this struct
Expand Down Expand Up @@ -57,12 +50,7 @@ error[E0599]: the method `use_ord` exists for struct `Object<NoDerives>`, but it
--> $DIR/issue-91550.rs:27:9
|
LL | pub struct NoDerives;
| --------------------
| |
| doesn't satisfy `NoDerives: Eq`
| doesn't satisfy `NoDerives: Ord`
| doesn't satisfy `NoDerives: PartialEq`
| doesn't satisfy `NoDerives: PartialOrd`
| -------------------- doesn't satisfy `NoDerives: Eq`, `NoDerives: Ord`, `NoDerives: PartialEq` or `NoDerives: PartialOrd`
LL |
LL | struct Object<T>(T);
| ---------------- method `use_ord` not found for this struct
Expand Down Expand Up @@ -94,12 +82,7 @@ error[E0599]: the method `use_ord_and_partial_ord` exists for struct `Object<NoD
--> $DIR/issue-91550.rs:28:9
|
LL | pub struct NoDerives;
| --------------------
| |
| doesn't satisfy `NoDerives: Eq`
| doesn't satisfy `NoDerives: Ord`
| doesn't satisfy `NoDerives: PartialEq`
| doesn't satisfy `NoDerives: PartialOrd`
| -------------------- doesn't satisfy `NoDerives: Eq`, `NoDerives: Ord`, `NoDerives: PartialEq` or `NoDerives: PartialOrd`
Copy link
Contributor

@oli-obk oli-obk Jan 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to have a single "NoDerives does not implement Eq, Ord, PartialEq and PartialOrd" for when it's just a bunch of trait bounds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather we did that as part of its own PR to make reviewing easier :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea makes sense

LL |
LL | struct Object<T>(T);
| ---------------- method `use_ord_and_partial_ord` not found for this struct
Expand Down
3 changes: 0 additions & 3 deletions ...ric-associated-types/issue-119942-unsatisified-gat-bound-during-assoc-ty-selection.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ LL | enum Node<T, P: PointerFamily> {
...
LL | let mut list = RcNode::<i32>::new();
| ^^^ doesn't have a size known at compile-time
--> $SRC_DIR/core/src/ops/deref.rs:LL:COL
|
= note: doesn't satisfy `_: Sized`
|
note: trait bound `Node<i32, RcFamily>: Sized` was not satisfied
--> $DIR/issue-119942-unsatisified-gat-bound-during-assoc-ty-selection.rs:4:18
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ impl<T: X<Y<i32> = i32>> M for T {}

struct S;
//~^ NOTE method `f` not found for this
//~| NOTE doesn't satisfy `<S as X>::Y<i32> = i32`
//~| NOTE doesn't satisfy `S: M`
//~| NOTE doesn't satisfy `<S as X>::Y<i32> = i32` or `S: M`

impl X for S {
type Y<T> = bool;
Expand Down
5 changes: 2 additions & 3 deletions tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
error[E0599]: the method `f` exists for struct `S`, but its trait bounds were not satisfied
--> $DIR/method-unsatisfied-assoc-type-predicate.rs:28:7
--> $DIR/method-unsatisfied-assoc-type-predicate.rs:27:7
|
LL | struct S;
| --------
| |
| method `f` not found for this struct
| doesn't satisfy `<S as X>::Y<i32> = i32`
| doesn't satisfy `S: M`
| doesn't satisfy `<S as X>::Y<i32> = i32` or `S: M`
...
LL | a.f();
| ^ method cannot be called on `S` due to unsatisfied trait bounds
Expand Down
3 changes: 0 additions & 3 deletions tests/ui/iterators/vec-on-unimplemented.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ error[E0599]: `Vec<bool>` is not an iterator
|
LL | vec![true, false].map(|v| !v).collect::<Vec<_>>();
| ^^^ `Vec<bool>` is not an iterator; try calling `.into_iter()` or `.iter()`
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
|
= note: doesn't satisfy `Vec<bool>: Iterator`
|
= note: the following trait bounds were not satisfied:
`Vec<bool>: Iterator`
Expand Down
6 changes: 1 addition & 5 deletions tests/ui/mismatched_types/issue-36053-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@ error[E0599]: the method `count` exists for struct `Filter<Fuse<Once<&str>>, {cl
LL | once::<&str>("str").fuse().filter(|a: &str| true).count();
| --------- ^^^^^ method cannot be called due to unsatisfied trait bounds
| |
| doesn't satisfy `<_ as FnOnce<(&&str,)>>::Output = bool`
| doesn't satisfy `_: FnMut<(&&str,)>`
--> $SRC_DIR/core/src/iter/adapters/filter.rs:LL:COL
|
= note: doesn't satisfy `_: Iterator`
| doesn't satisfy `<_ as FnOnce<(&&str,)>>::Output = bool` or `_: FnMut<(&&str,)>`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really an improvement imo. The vertical list is easier to read

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the real issue here is that we need to special case closure trait mismatches to talk in terms that are clearer, instead of just "list of bounds" like we do here.

|
= note: the following trait bounds were not satisfied:
`<{closure@$DIR/issue-36053-2.rs:7:39: 7:48} as FnOnce<(&&str,)>>::Output = bool`
Expand Down
16 changes: 2 additions & 14 deletions tests/ui/suggestions/derive-trait-for-method-call.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ error[E0599]: the method `test` exists for struct `Foo<Enum, CloneEnum>`, but it
--> $DIR/derive-trait-for-method-call.rs:28:15
|
LL | enum Enum {
| ---------
| |
| doesn't satisfy `Enum: Clone`
| doesn't satisfy `Enum: Default`
| --------- doesn't satisfy `Enum: Clone` or `Enum: Default`
...
LL | enum CloneEnum {
| -------------- doesn't satisfy `CloneEnum: Default`
Expand Down Expand Up @@ -40,10 +37,7 @@ error[E0599]: the method `test` exists for struct `Foo<Struct, CloneStruct>`, bu
--> $DIR/derive-trait-for-method-call.rs:34:15
|
LL | struct Struct {
| -------------
| |
| doesn't satisfy `Struct: Clone`
| doesn't satisfy `Struct: Default`
| ------------- doesn't satisfy `Struct: Clone` or `Struct: Default`
...
LL | struct CloneStruct {
| ------------------ doesn't satisfy `CloneStruct: Default`
Expand Down Expand Up @@ -85,12 +79,6 @@ LL | struct Foo<X, Y> (X, Y);
...
LL | let y = x.test();
| ^^^^ method cannot be called on `Foo<Vec<Enum>, Instant>` due to unsatisfied trait bounds
--> $SRC_DIR/std/src/time.rs:LL:COL
|
= note: doesn't satisfy `Instant: Default`
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
|
= note: doesn't satisfy `Vec<Enum>: Clone`
|
note: the following trait bounds were not satisfied:
`Instant: Default`
Expand Down
3 changes: 0 additions & 3 deletions tests/ui/suggestions/mut-borrow-needed-by-trait.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ error[E0599]: the method `write_fmt` exists for struct `BufWriter<&dyn Write>`,
|
LL | writeln!(fp, "hello world").unwrap();
| ---------^^---------------- method cannot be called on `BufWriter<&dyn Write>` due to unsatisfied trait bounds
--> $SRC_DIR/std/src/io/buffered/bufwriter.rs:LL:COL
|
= note: doesn't satisfy `BufWriter<&dyn std::io::Write>: std::io::Write`
|
note: must implement `io::Write`, `fmt::Write`, or have a `write_fmt` method
--> $DIR/mut-borrow-needed-by-trait.rs:21:14
Expand Down
3 changes: 0 additions & 3 deletions tests/ui/suggestions/suggest-change-mut.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ error[E0599]: the method `read_until` exists for struct `BufReader<&T>`, but its
|
LL | stream_reader.read_until(b'\n', &mut buffer).expect("Reading into buffer failed");
| ^^^^^^^^^^ method cannot be called on `BufReader<&T>` due to unsatisfied trait bounds
--> $SRC_DIR/std/src/io/buffered/bufreader.rs:LL:COL
|
= note: doesn't satisfy `BufReader<&T>: BufRead`
|
= note: the following trait bounds were not satisfied:
`&T: std::io::Read`
Expand Down
Loading