Detect unmet bound error caused by lack of perfect derives #143993
Conversation
When encountering an unmet bound E0599 error originating in a derived trait impl, provide additional context:
```
error[E0599]: the method `clone` exists for struct `S<X, X, Y>`, but its trait bounds were not satisfied
--> $DIR/lack-of-perfect-derives.rs:27:16
|
LL | struct S<T, K, V>(Arc<T>, Rc<K>, Arc<V>, ());
| ----------------- method `clone` not found for this struct because it doesn't satisfy `S<X, X, Y>: Clone`
...
LL | struct X;
| -------- doesn't satisfy `X: Clone`
...
LL | let s2 = s.clone();
| ^^^^^ method cannot be called on `S<X, X, Y>` due to unsatisfied trait bounds
|
note: trait bound `X: Clone` was not satisfied
--> $DIR/lack-of-perfect-derives.rs:4:10
|
LL | #[derive(Clone, PartialEq)]
| ^^^^^ unsatisfied trait bound introduced in this `derive` macro
LL | struct S<T, K, V>(Arc<T>, Rc<K>, Arc<V>, ());
| - - implicit unsatisfied bound on this type parameter
| |
| implicit unsatisfied bound on this type parameter
= note: `#[derive(Clone)]` introduces `where`-bounds requiring every type parameter to implement `Clone`, even when not strictly necessary
= help: you can manually implement `Clone` with more targeted bounds: `impl<T, K, V> Clone for S<T, K, V> where Arc<T>: Clone, Rc<K>: Clone, Arc<V>: Clone { /* .. */ }`
help: consider annotating `X` with `#[derive(Clone)]`
|
LL + #[derive(Clone)]
LL | struct X;
|
```
We now add a `note` mentioning that `derive` introduces `where`-bounds on the type parameters.
If the derived bounds would be different under "perfect derives", meaning a relaxation of the impl bounds could make the impl apply, then we mention (an approximate of) the impl that can be manually written.
rustbot
added
S-waiting-on-review
| })) if let ( | ||
| ExpnKind::Macro(MacroKind::Derive, mac), _) | ||
| | (_, Some(ExpnKind::Macro(MacroKind::Derive, mac)) | ||
| ) = ( | ||
| self_ty.span.ctxt().outer_expn_data().kind, | ||
| of_trait.as_ref().map(|t| t.path.span.ctxt().outer_expn_data().kind), | ||
| ) => |
There was a problem hiding this comment.
This is done so that we can get the macro name from the Span of either the implemented trait or the type being implemented. I don't think we strictly need it to be like this, but I didn't try alternatives.
| LL | struct Bar<T: Foo> { | ||
| | - implicit unsatisfied bound on this type parameter | ||
| = note: `#[derive(Clone)]` introduces `where`-bounds requiring every type parameter to implement `Clone`, even when not strictly necessary | ||
| = help: you can manually implement `Clone` with more targeted bounds: `impl<T> Clone for Bar<T> where T::X: Clone { /* .. */ }` |
There was a problem hiding this comment.
The correct version of this would be impl<T: Foo> Clone for Bar<T> where T::X: Clone { /* .. */ }, but that is a where bound on the impl. The naïve approach to dealing with predicates in the suggestion code isn't conductive to properly handling this case :-/
| error[E0369]: binary operation `==` cannot be applied to type `S<X, X, X>` | ||
| --> $DIR/lack-of-perfect-derives.rs:23:15 | ||
| | | ||
| LL | let _ = s == s2; | ||
| | - ^^ -- S<X, X, X> | ||
| | | | ||
| | S<X, X, X> | ||
| | | ||
| note: an implementation of `PartialEq` might be missing for `X` | ||
| --> $DIR/lack-of-perfect-derives.rs:15:1 | ||
| | | ||
| LL | struct X; | ||
| | ^^^^^^^^ must implement `PartialEq` | ||
| help: consider annotating `X` with `#[derive(PartialEq)]` | ||
| | | ||
| LL + #[derive(PartialEq)] | ||
| LL | struct X; | ||
| | |
There was a problem hiding this comment.
This added test is unrelated to the codepath being taken here, but E0369 should eventually use part of the same logic that E0599 does, so that it can provide more suggestions than it does today.
| error[E0599]: the method `clone` exists for struct `S<X, X, Y>`, but its trait bounds were not satisfied | ||
| --> $DIR/lack-of-perfect-derives.rs:27:16 | ||
| | | ||
| LL | struct S<T, K, V>(Arc<T>, Rc<K>, Arc<V>, ()); | ||
| | ----------------- method `clone` not found for this struct because it doesn't satisfy `S<X, X, Y>: Clone` | ||
| ... | ||
| LL | struct X; | ||
| | -------- doesn't satisfy `X: Clone` | ||
| ... | ||
| LL | let s2 = s.clone(); | ||
| | ^^^^^ method cannot be called on `S<X, X, Y>` due to unsatisfied trait bounds | ||
| | | ||
| note: trait bound `X: Clone` was not satisfied | ||
| --> $DIR/lack-of-perfect-derives.rs:4:10 | ||
| | | ||
| LL | #[derive(Clone, PartialEq)] | ||
| | ^^^^^ unsatisfied trait bound introduced in this `derive` macro | ||
| LL | struct S<T, K, V>(Arc<T>, Rc<K>, Arc<V>, ()); | ||
| | - - implicit unsatisfied bound on this type parameter | ||
| | | | ||
| | implicit unsatisfied bound on this type parameter | ||
| = note: `#[derive(Clone)]` introduces `where`-bounds requiring every type parameter to implement `Clone`, even when not strictly necessary | ||
| = help: you can manually implement `Clone` with more targeted bounds: `impl<T, K, V> Clone for S<T, K, V> where V: Clone { /* .. */ }` | ||
| help: consider annotating `X` with `#[derive(Clone)]` | ||
| | | ||
| LL + #[derive(Clone)] | ||
| LL | struct X; | ||
| | |
There was a problem hiding this comment.
This is the case that "perfect derives" would fix, and main impetus behind this change. Note that the help still includes where V: Clone, because that obligation was met (because V ends up being Y, which is Clone. We could change this behavior slightly to not include that bound either, but ran out of time to look into that.
I would like to remove some of the output from this error, like the suggestion to make X cloneable, and maybe the secondary span label pointing at S, as that is already pointed at by the "unsatisfied trait bound X: Clone" note.
| note: trait bound `X: PartialEq` was not satisfied | ||
| --> $DIR/lack-of-perfect-derives.rs:4:17 | ||
| | | ||
| LL | #[derive(Clone, PartialEq)] | ||
| | ^^^^^^^^^ unsatisfied trait bound introduced in this `derive` macro | ||
| LL | struct S<T, K, V>(Arc<T>, Rc<K>, Arc<V>, ()); | ||
| | - - - implicit unsatisfied bound on this type parameter | ||
| | | | | ||
| | | implicit unsatisfied bound on this type parameter | ||
| | implicit unsatisfied bound on this type parameter | ||
| = note: `#[derive(PartialEq)]` introduces `where`-bounds requiring every type parameter to implement `PartialEq`, even when not strictly necessary | ||
| = help: you can manually implement `PartialEq` with more targeted bounds: `impl<T, K, V> PartialEq for S<T, K, V> where Arc<T>: PartialEq, Rc<K>: PartialEq, Arc<V>: PartialEq { /* .. */ }` |
There was a problem hiding this comment.
This is the opposite perfect derive case: instead of the impl having no additional bounds, the bounds added are all not directly on the type parameter but rather on the wrapper types.
| = note: the following trait bounds were not satisfied: | ||
| `S<X, X, X>: Iterator` | ||
| which is required by `&mut S<X, X, X>: Iterator` | ||
| note: the trait `Iterator` must be implemented | ||
| --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL |
There was a problem hiding this comment.
Pre-existing issue. We shouldn't be talking about Iterator at all for this case.
| note: trait bound `X: Clone` was not satisfied | ||
| --> $DIR/lack-of-perfect-derives.rs:8:10 | ||
| | | ||
| LL | #[derive(Clone, PartialEq)] | ||
| | ^^^^^ unsatisfied trait bound introduced in this `derive` macro | ||
| LL | struct N<T, K, V>(Arc<T>, T, Rc<K>, K, Arc<V>, V, ()); | ||
| | - - implicit unsatisfied bound on this type parameter | ||
| | | | ||
| | implicit unsatisfied bound on this type parameter | ||
| = note: `#[derive(Clone)]` introduces `where`-bounds requiring every type parameter to implement `Clone` |
There was a problem hiding this comment.
Note the mixed use of Arc<T> and T. We don't suggest where Arc<T>: Clone, T: Clone, so in the end we suggest no impl.
| @@ -1752,6 +1762,226 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||
| err.emit() | |||
| } | |||
|
|
|||
| fn extract_derive_bounds( | |||
There was a problem hiding this comment.
The new suggestion logic starting here isn't... the cleanest and would love it if anyone comes up with a neater alternative, but this is what I got to.
| bounds | ||
| .entry(self_ty.to_string()) | ||
| .or_default() | ||
| .insert("placeholder".to_string()); |
There was a problem hiding this comment.
"placeholder" is so that we keep the space of types that can implement the trait without obligating the original type parameter to also implement the trait (like for Arc and unlike PartialEq).
When encountering an unmet bound E0599 error originating in a derived trait impl, provide additional context:
We now add a
notementioning thatderiveintroduceswhere-bounds on the type parameters.If the derived bounds would be different under "perfect derives", meaning a relaxation of the impl bounds could make the impl apply, then we mention (an approximate of) the impl that can be manually written.
Fix #143714.