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.

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

Tweak E0599 and elaborate_predicates #106788

Merged
merged 4 commits into from
Jan 14, 2023
Merged
Changes from 1 commit
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
Next Next commit
Elaborate unmet obligations in E0599 for more context
estebank committed Jan 13, 2023
commit f6e6d2a035d9e86e7053847aa60a99940f41064c
28 changes: 23 additions & 5 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
@@ -1587,11 +1587,29 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
let o = self.resolve_vars_if_possible(o);
if !self.predicate_may_hold(&o) {
result = ProbeResult::NoMatch;
possibly_unsatisfied_predicates.push((
o.predicate,
None,
Some(o.cause),
));
let parent_o = o.clone();
let implied_obligations =
traits::elaborate_obligations(self.tcx, vec![o]);
for o in implied_obligations {
let parent = if o == parent_o {
None
} else {
if o.predicate.to_opt_poly_trait_pred().map(|p| p.def_id())
== self.tcx.lang_items().sized_trait()
Comment on lines +1597 to +1598
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if o.predicate.to_opt_poly_trait_pred().map(|p| p.def_id())
== self.tcx.lang_items().sized_trait()
if let Some(trait_pred) = o.predicate.to_opt_poly_trait_pred()
&& self.tcx.lang_items().sized_trait() == Some(trait_pred.def_id())

This will never happen in practice, but it's better to deal with the None == None case when lang-items are not defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the None == None case you envision? Because trait_pred might have been None (so we don't get into this logic) and sized_trait() might be None which might cause a build where the sized trait lang item isn't present to be too verbose (with extra labels), no other behavioral change.

{
// We don't care to talk about implicit `Sized` bounds.
continue;
}
Some(parent_o.predicate)
};
if !self.predicate_may_hold(&o) {
possibly_unsatisfied_predicates.push((
o.predicate,
parent,
Some(o.cause),
));
}
}
}
}
}
29 changes: 25 additions & 4 deletions tests/ui/derives/issue-91550.stderr
Original file line number Diff line number Diff line change
@@ -6,12 +6,15 @@ LL | struct Value(u32);
| |
| doesn't satisfy `Value: Eq`
| doesn't satisfy `Value: Hash`
| doesn't satisfy `Value: PartialEq`
...
LL | hs.insert(Value(0));
| ^^^^^^
|
= note: the following trait bounds were not satisfied:
`Value: Eq`
`Value: PartialEq`
which is required by `Value: Eq`
`Value: Hash`
help: consider annotating `Value` with `#[derive(Eq, Hash, PartialEq)]`
|
@@ -22,15 +25,20 @@ 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: Eq`
| doesn't satisfy `NoDerives: PartialEq`
LL |
LL | struct Object<T>(T);
| ---------------- method `use_eq` not found for this struct
...
LL | foo.use_eq();
| ^^^^^^ method cannot be called on `Object<NoDerives>` due to unsatisfied trait bounds
|
note: trait bound `NoDerives: Eq` was not satisfied
note: the following trait bounds were not satisfied:
`NoDerives: Eq`
`NoDerives: PartialEq`
--> $DIR/issue-91550.rs:15:9
|
LL | impl<T: Eq> Object<T> {
@@ -46,15 +54,24 @@ 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: Ord`
| --------------------
| |
| doesn't satisfy `NoDerives: Eq`
| doesn't satisfy `NoDerives: Ord`
| doesn't satisfy `NoDerives: PartialEq`
| doesn't satisfy `NoDerives: PartialOrd`
LL |
LL | struct Object<T>(T);
| ---------------- method `use_ord` not found for this struct
...
LL | foo.use_ord();
| ^^^^^^^ method cannot be called on `Object<NoDerives>` due to unsatisfied trait bounds
|
note: trait bound `NoDerives: Ord` was not satisfied
note: the following trait bounds were not satisfied:
`NoDerives: Eq`
`NoDerives: Ord`
`NoDerives: PartialEq`
`NoDerives: PartialOrd`
--> $DIR/issue-91550.rs:18:9
|
LL | impl<T: Ord> Object<T> {
@@ -72,7 +89,9 @@ error[E0599]: the method `use_ord_and_partial_ord` exists for struct `Object<NoD
LL | pub struct NoDerives;
| --------------------
| |
| doesn't satisfy `NoDerives: Eq`
| doesn't satisfy `NoDerives: Ord`
| doesn't satisfy `NoDerives: PartialEq`
| doesn't satisfy `NoDerives: PartialOrd`
LL |
LL | struct Object<T>(T);
@@ -82,7 +101,9 @@ LL | foo.use_ord_and_partial_ord();
| ^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called on `Object<NoDerives>` due to unsatisfied trait bounds
|
note: the following trait bounds were not satisfied:
`NoDerives: Eq`
`NoDerives: Ord`
`NoDerives: PartialEq`
`NoDerives: PartialOrd`
--> $DIR/issue-91550.rs:21:9
|
2 changes: 1 addition & 1 deletion tests/ui/missing-trait-bounds/issue-35677.fixed
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@
use std::collections::HashSet;
use std::hash::Hash;

fn is_subset<T>(this: &HashSet<T>, other: &HashSet<T>) -> bool where T: Eq, T: Hash {
fn is_subset<T>(this: &HashSet<T>, other: &HashSet<T>) -> bool where T: Eq, T: Hash, T: PartialEq {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I don't consider this an improvement -- T: Eq implies T: PartialEq by elaboration.

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 agree, so I tested hiding the suggestions when a parent obligation was present, but that broke some existing tests with applicable suggestions. I'll see if we can inspect the obligation cause code instead to avoid these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

this.is_subset(other)
//~^ ERROR the method
}
6 changes: 4 additions & 2 deletions tests/ui/missing-trait-bounds/issue-35677.stderr
Original file line number Diff line number Diff line change
@@ -6,11 +6,13 @@ LL | this.is_subset(other)
|
= note: the following trait bounds were not satisfied:
`T: Eq`
`T: PartialEq`
which is required by `T: Eq`
`T: Hash`
help: consider restricting the type parameters to satisfy the trait bounds
|
LL | fn is_subset<T>(this: &HashSet<T>, other: &HashSet<T>) -> bool where T: Eq, T: Hash {
| ++++++++++++++++++++
LL | fn is_subset<T>(this: &HashSet<T>, other: &HashSet<T>) -> bool where T: Eq, T: Hash, T: PartialEq {
| ++++++++++++++++++++++++++++++++++

error: aborting due to previous error