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
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
28 changes: 23 additions & 5 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
));
}
}
}
}
}
Expand Down
63 changes: 49 additions & 14 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,19 +505,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => None,
};
if let Some(hir::Node::Item(hir::Item { kind, .. })) = node {
if let Some(g) = kind.generics() {
let key = (
g.tail_span_for_predicate_suggestion(),
g.add_where_or_trailing_comma(),
);
type_params
.entry(key)
.or_insert_with(FxHashSet::default)
.insert(obligation.to_owned());
}
if let Some(hir::Node::Item(hir::Item { kind, .. })) = node
&& let Some(g) = kind.generics()
{
let key = (
g.tail_span_for_predicate_suggestion(),
g.add_where_or_trailing_comma(),
);
type_params
.entry(key)
.or_insert_with(FxHashSet::default)
.insert(obligation.to_owned());
return true;
}
}
false
};
let mut bound_span_label = |self_ty: Ty<'_>, obligation: &str, quiet: &str| {
let msg = format!(
Expand Down Expand Up @@ -692,7 +694,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"auto trait is invoked with no method error, but no error reported?",
);
}
Some(_) => unreachable!(),
Some(Node::Item(hir::Item {
ident, kind: hir::ItemKind::Trait(..), ..
})) => {
skip_list.insert(p);
let entry = spanned_predicates.entry(ident.span);
let entry = entry.or_insert_with(|| {
(FxHashSet::default(), FxHashSet::default(), Vec::new())
});
entry.0.insert(cause.span);
entry.1.insert((ident.span, ""));
entry.1.insert((cause.span, "unsatisfied trait bound introduced here"));
entry.2.push(p);
}
Some(node) => unreachable!("encountered `{node:?}`"),
None => (),
}
}
Expand All @@ -719,19 +734,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
unsatisfied_bounds = true;
}

let mut suggested_bounds = FxHashSet::default();
// The requirements that didn't have an `impl` span to show.
let mut bound_list = unsatisfied_predicates
.iter()
.filter_map(|(pred, parent_pred, _cause)| {
let mut suggested = false;
format_pred(*pred).map(|(p, self_ty)| {
collect_type_param_suggestions(self_ty, *pred, &p);
if let Some(parent) = parent_pred && suggested_bounds.contains(parent) {
// We don't suggest `PartialEq` when we already suggest `Eq`.
} else if !suggested_bounds.contains(pred) {
if collect_type_param_suggestions(self_ty, *pred, &p) {
suggested = true;
suggested_bounds.insert(pred);
}
}
(
match parent_pred {
None => format!("`{}`", &p),
Some(parent_pred) => match format_pred(*parent_pred) {
None => format!("`{}`", &p),
Some((parent_p, _)) => {
collect_type_param_suggestions(self_ty, *parent_pred, &p);
if !suggested
&& !suggested_bounds.contains(pred)
&& !suggested_bounds.contains(parent_pred)
{
if collect_type_param_suggestions(
self_ty,
*parent_pred,
&p,
) {
suggested_bounds.insert(pred);
}
}
format!("`{}`\nwhich is required by `{}`", p, parent_p)
}
},
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_infer/src/traits/util.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use smallvec::smallvec;

use crate::infer::outlives::components::{push_outlives_components, Component};
use crate::traits::{Obligation, ObligationCause, PredicateObligation};
use crate::traits::{self, Obligation, ObligationCause, PredicateObligation};
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_middle::ty::{self, ToPredicate, TyCtxt};
use rustc_span::symbol::Ident;
Expand Down Expand Up @@ -145,16 +145,28 @@ impl<'tcx> Elaborator<'tcx> {
// Get predicates declared on the trait.
let predicates = tcx.super_predicates_of(data.def_id());

let obligations = predicates.predicates.iter().map(|&(mut pred, _)| {
let obligations = predicates.predicates.iter().map(|&(mut pred, span)| {
// when parent predicate is non-const, elaborate it to non-const predicates.
if data.constness == ty::BoundConstness::NotConst {
pred = pred.without_const(tcx);
}

let cause = obligation.cause.clone().derived_cause(
bound_predicate.rebind(data),
|derived| {
traits::ImplDerivedObligation(Box::new(
traits::ImplDerivedObligationCause {
derived,
impl_def_id: data.def_id(),
span,
},
))
},
);
predicate_obligation(
pred.subst_supertrait(tcx, &bound_predicate.rebind(data.trait_ref)),
obligation.param_env,
obligation.cause.clone(),
cause,
)
});
debug!(?data, ?obligations, "super_predicates");
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/associated-types/issue-43784-associated-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ error[E0277]: the trait bound `T: Copy` is not satisfied
LL | type Assoc = T;
| ^ the trait `Copy` is not implemented for `T`
|
note: required for `<T as Complete>::Assoc` to implement `Partial<T>`
--> $DIR/issue-43784-associated-type.rs:1:11
|
LL | pub trait Partial<X: ?Sized>: Copy {
| ^^^^^^^
note: required by a bound in `Complete::Assoc`
--> $DIR/issue-43784-associated-type.rs:5:17
|
Expand Down
34 changes: 32 additions & 2 deletions tests/ui/derives/issue-91550.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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)]`
|
Expand All @@ -22,7 +25,10 @@ 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
Expand All @@ -37,6 +43,9 @@ LL | impl<T: Eq> Object<T> {
| ^^ ---------
| |
| unsatisfied trait bound introduced here
= note: the following trait bounds were not satisfied:
`NoDerives: PartialEq`
which is required by `NoDerives: Eq`
help: consider annotating `NoDerives` with `#[derive(Eq, PartialEq)]`
|
LL | #[derive(Eq, PartialEq)]
Expand All @@ -46,7 +55,12 @@ 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
Expand All @@ -61,6 +75,13 @@ LL | impl<T: Ord> Object<T> {
| ^^^ ---------
| |
| unsatisfied trait bound introduced here
= note: the following trait bounds were not satisfied:
`NoDerives: PartialOrd`
which is required by `NoDerives: Ord`
`NoDerives: PartialEq`
which is required by `NoDerives: Ord`
`NoDerives: Eq`
which is required by `NoDerives: Ord`
help: consider annotating `NoDerives` with `#[derive(Eq, Ord, PartialEq, PartialOrd)]`
|
LL | #[derive(Eq, Ord, PartialEq, PartialOrd)]
Expand All @@ -72,7 +93,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);
Expand All @@ -91,6 +114,13 @@ LL | impl<T: Ord + PartialOrd> Object<T> {
| | |
| | unsatisfied trait bound introduced here
| unsatisfied trait bound introduced here
= note: the following trait bounds were not satisfied:
`NoDerives: PartialEq`
which is required by `NoDerives: Ord`
`NoDerives: Eq`
which is required by `NoDerives: Ord`
`NoDerives: PartialEq`
which is required by `NoDerives: PartialOrd`
help: consider annotating `NoDerives` with `#[derive(Eq, Ord, PartialEq, PartialOrd)]`
|
LL | #[derive(Eq, Ord, PartialEq, PartialOrd)]
Expand Down
1 change: 1 addition & 0 deletions tests/ui/generic-associated-types/issue-74824.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ LL | type Copy<T>: Copy = Box<T>;
| ^^^^^^ the trait `Clone` is not implemented for `T`
|
= note: required for `Box<T>` to implement `Clone`
= note: required for `<Self as UnsafeCopy>::Copy<T>` to implement `Copy`
note: required by a bound in `UnsafeCopy::Copy`
--> $DIR/issue-74824.rs:6:19
|
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/missing-trait-bounds/issue-35677.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ 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
|
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/traits/issue-43784-supertrait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ error[E0277]: the trait bound `T: Copy` is not satisfied
LL | impl<T> Complete for T {}
| ^ the trait `Copy` is not implemented for `T`
|
note: required for `T` to implement `Partial`
--> $DIR/issue-43784-supertrait.rs:1:11
|
LL | pub trait Partial: Copy {
| ^^^^^^^
note: required by a bound in `Complete`
--> $DIR/issue-43784-supertrait.rs:4:21
|
Expand Down
88 changes: 88 additions & 0 deletions tests/ui/traits/track-obligations.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// These are simplifications of the tower traits by the same name:

pub trait Service<Request> {
type Response;
}

pub trait Layer<C> {
type Service;
}

// Any type will do here:

pub struct Req;
pub struct Res;

// This is encoding a trait alias.

pub trait ParticularService:
Service<Req, Response = Res> {
}

impl<T> ParticularService for T
where
T: Service<Req, Response = Res>,
{
}

// This is also a trait alias.
// The weird = <Self as ...> bound is there so that users of the trait do not
// need to repeat the bounds. See https://github.com/rust-lang/rust/issues/20671
// for context, and in particular the workaround in:
// https://github.com/rust-lang/rust/issues/20671#issuecomment-529752828

pub trait ParticularServiceLayer<C>:
Layer<C, Service = <Self as ParticularServiceLayer<C>>::Service>
{
type Service: ParticularService;
}

impl<T, C> ParticularServiceLayer<C> for T
where
T: Layer<C>,
T::Service: ParticularService,
{
type Service = T::Service;
}

// These are types that implement the traits that the trait aliases refer to.
// They should also implement the alias traits due to the blanket impls.

struct ALayer<C>(C);
impl<C> Layer<C> for ALayer<C> {
type Service = AService;
}

struct AService;
impl Service<Req> for AService {
// However, AService does _not_ meet the blanket implementation,
// since its Response type is bool, not Res as it should be.
type Response = bool;
}

// This is a wrapper type around ALayer that uses the trait alias
// as a way to communicate the requirements of the provided types.
struct Client<C>(C);

// The method and the free-standing function below both have the same bounds.

impl<C> Client<C>
where
ALayer<C>: ParticularServiceLayer<C>,
{
fn check(&self) {}
}

fn check<C>(_: C) where ALayer<C>: ParticularServiceLayer<C> {}

// But, they give very different error messages.

fn main() {
// This gives a very poor error message that does nothing to point the user
// at the underlying cause of why the types involved do not meet the bounds.
Client(()).check(); //~ ERROR E0599

// This gives a good(ish) error message that points the user at _why_ the
// bound isn't met, and thus how they might fix it.
check(()); //~ ERROR E0271
}
Loading