-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Provide suggestions for type parameters missing bounds for associated types #70908
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
LL | impl<T> Baz for T where T: Foo { | ||
| ^^^ expected type parameter `T`, found associated type | ||
| - ^^^ expected type parameter `T`, found associated type | ||
| | | ||
| this type parameter | ||
| | ||
= note: expected associated type `<T as Foo>::Bar<'_, 'static>` | ||
found associated type `<<T as Baz>::Quux<'_> as Foo>::Bar<'_, 'static>` | ||
= note: you might be missing a type parameter or trait bound | ||
help: consider further restricting this bound | ||
| | ||
LL | impl<T> Baz for T where T: Foo + Baz<Quux = T> { | ||
| ^^^^^^^^^^^^^^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This suggestion will cause another error:
error[E0275]: overflow evaluating the requirement `<T as Baz>::Quux`
--> src/test/ui/generic-associated-types/construct_with_other_type.rs:19:9
|
19 | impl<T> Baz for T where T: Foo + Baz<Quux = T> {
| ^^^
|
= note: required because of the requirements on the impl of `Baz` for `T`
= note: required because of the requirements on the impl of `Baz` for `T`
I feel this is "fine" since the output was already confusing and the code itself is a bit contrived and requires #![feature(generic_associated_types)]
to be enabled to even be triggered.
For tracking purposes, the original impetus to make this change came from seeing this thread https://users.rust-lang.org/t/you-might-be-missing-a-type-parameter-or-trait-bound/38333. This PR should help
These three cases are handled by different code (in different commits), but all should be handled in concert to catch people at any point of their "incorrect mental model" trip. The last one is particularly insidious because the associated type binding equality constraint syntax can be extrapolated from the rest of the syntax and displayed errors and is being worked on, but is not yet available. Pointing people in the right direction instead of just saying "sorry, computer says no" is important to the learnability of the language. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't get all the way through review this yet, just the first commit. Left some comments.
This comment has been minimized.
This comment has been minimized.
4539157
to
654b851
Compare
d4fa067
to
c0892a1
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few nits but they don't matter really
@@ -67,3 +72,180 @@ impl<'tcx> TyS<'tcx> { | |||
} | |||
} | |||
} | |||
|
|||
/// Suggest restricting a type param with a new bound. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice to add a brief example.
if generics.where_clause.predicates.is_empty() | ||
// Given `trait Base<T = String>: Super<T>` where `T: Copy`, suggest restricting in the | ||
// `where` clause instead of `trait Base<T: Copy = String>: Super<T>`. | ||
&& !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!matches!
what have we wrought
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clearly this should be ¡Mátches!
.
|
||
if generics.where_clause.predicates.is_empty() | ||
// Given `trait Base<T = String>: Super<T>` where `T: Copy`, suggest restricting in the | ||
// `where` clause instead of `trait Base<T: Copy = String>: Super<T>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment accurate? It seems like this covers the case where we don't suggest a where clause, at least some of the time
@bors r+ |
📌 Commit c0a9d52 has been approved by |
Provide suggestions for type parameters missing bounds for associated types When implementing the binary operator traits it is easy to forget to restrict the `Output` associated type. `rustc` now accounts for different cases to lead users in the right direction to add the necessary restrictions. The structured suggestions in the following output are new: ``` error: equality constraints are not yet supported in `where` clauses --> $DIR/missing-bounds.rs:37:33 | LL | impl<B: Add> Add for E<B> where <B as Add>::Output = B { | ^^^^^^^^^^^^^^^^^^^^^^ not supported | = note: see issue rust-lang#20041 <rust-lang#20041> for more information help: if `Output` is an associated type you're trying to set, use the associated type binding syntax | LL | impl<B: Add> Add for E<B> where B: Add<Output = B> { | ^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types --> $DIR/missing-bounds.rs:11:11 | 7 | impl<B> Add for A<B> where B: Add { | - this type parameter ... 11 | A(self.0 + rhs.0) | ^^^^^^^^^^^^^^ expected type parameter `B`, found associated type | = note: expected type parameter `B` found associated type `<B as std::ops::Add>::Output` help: consider further restricting this bound | 7 | impl<B> Add for A<B> where B: Add + std::ops::Add<Output = B> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0369]: cannot add `B` to `B` --> $DIR/missing-bounds.rs:31:21 | 31 | Self(self.0 + rhs.0) | ------ ^ ----- B | | | B | help: consider restricting type parameter `B` | 27 | impl<B: std::ops::Add<Output = B>> Add for D<B> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` That output is given for the following cases: ```rust struct A<B>(B); impl<B> Add for A<B> where B: Add { type Output = Self; fn add(self, rhs: Self) -> Self { A(self.0 + rhs.0) //~ ERROR mismatched types } } struct D<B>(B); impl<B> Add for D<B> { type Output = Self; fn add(self, rhs: Self) -> Self { Self(self.0 + rhs.0) //~ ERROR cannot add `B` to `B` } } struct E<B>(B); impl<B: Add> Add for E<B> where <B as Add>::Output = B { type Output = Self; fn add(self, rhs: Self) -> Self { Self(self.0 + rhs.0) } } ```
failed in rollup @bors r- |
…onstraint, r=oli-obk On type mismatch involving associated type, suggest constraint When an associated type is found when a specific type was expected, if possible provide a structured suggestion constraining the associated type in a bound. ``` error[E0271]: type mismatch resolving `<T as Foo>::Y == i32` --> $DIR/associated-types-multiple-types-one-trait.rs:13:5 | LL | want_y(t); | ^^^^^^ expected `i32`, found associated type ... LL | fn want_y<T:Foo<Y=i32>>(t: &T) { } | ----- required by this bound in `want_y` | = note: expected type `i32` found associated type `<T as Foo>::Y` help: consider constraining the associated type `<T as Foo>::Y` to `i32` | LL | fn have_x_want_y<T:Foo<X=u32, Y = i32>>(t: &T) | ^^^^^^^^^ ``` ``` error[E0308]: mismatched types --> $DIR/trait-with-missing-associated-type-restriction.rs:12:9 | LL | qux(x.func()) | ^^^^^^^^ expected `usize`, found associated type | = note: expected type `usize` found associated type `<impl Trait as Trait>::A` help: consider constraining the associated type `<impl Trait as Trait>::A` to `usize` | LL | fn foo(x: impl Trait<A = usize>) { | ^^^^^^^^^^ ``` Fix rust-lang#71035. Related to rust-lang#70908.
This comment has been minimized.
This comment has been minimized.
When encountering a binary operation involving a type parameter that has no bindings, suggest adding the appropriate bound.
When encountering a projection that isn't satisfied by a type parameter, suggest constraining the type parameter.
…ropriate syntax When encountering `where <A as Foo>::Bar = B`, it is possible that `Bar` is an associated type. If so, suggest `where A: Foo<Bar = B>`. CC rust-lang#20041.
@bors r=nikomatsakis |
📌 Commit b17b20c has been approved by |
⌛ Testing commit b17b20c with merge 7ab8af3dd0318629a0a3d835468d9e3eb121a948... |
@bors retry yied |
Provide suggestions for type parameters missing bounds for associated types When implementing the binary operator traits it is easy to forget to restrict the `Output` associated type. `rustc` now accounts for different cases to lead users in the right direction to add the necessary restrictions. The structured suggestions in the following output are new: ``` error: equality constraints are not yet supported in `where` clauses --> $DIR/missing-bounds.rs:37:33 | LL | impl<B: Add> Add for E<B> where <B as Add>::Output = B { | ^^^^^^^^^^^^^^^^^^^^^^ not supported | = note: see issue rust-lang#20041 <rust-lang#20041> for more information help: if `Output` is an associated type you're trying to set, use the associated type binding syntax | LL | impl<B: Add> Add for E<B> where B: Add<Output = B> { | ^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types --> $DIR/missing-bounds.rs:11:11 | 7 | impl<B> Add for A<B> where B: Add { | - this type parameter ... 11 | A(self.0 + rhs.0) | ^^^^^^^^^^^^^^ expected type parameter `B`, found associated type | = note: expected type parameter `B` found associated type `<B as std::ops::Add>::Output` help: consider further restricting this bound | 7 | impl<B> Add for A<B> where B: Add + std::ops::Add<Output = B> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0369]: cannot add `B` to `B` --> $DIR/missing-bounds.rs:31:21 | 31 | Self(self.0 + rhs.0) | ------ ^ ----- B | | | B | help: consider restricting type parameter `B` | 27 | impl<B: std::ops::Add<Output = B>> Add for D<B> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` That output is given for the following cases: ```rust struct A<B>(B); impl<B> Add for A<B> where B: Add { type Output = Self; fn add(self, rhs: Self) -> Self { A(self.0 + rhs.0) //~ ERROR mismatched types } } struct D<B>(B); impl<B> Add for D<B> { type Output = Self; fn add(self, rhs: Self) -> Self { Self(self.0 + rhs.0) //~ ERROR cannot add `B` to `B` } } struct E<B>(B); impl<B: Add> Add for E<B> where <B as Add>::Output = B { type Output = Self; fn add(self, rhs: Self) -> Self { Self(self.0 + rhs.0) } } ```
Rollup of 5 pull requests Successful merges: - rust-lang#70908 (Provide suggestions for type parameters missing bounds for associated types) - rust-lang#71731 (Turn off rustc-dev-guide toolstate for now) - rust-lang#71888 (refactor suggest_traits_to_import) - rust-lang#71918 (Rename methods section) - rust-lang#71950 (Miri validation error handling cleanup) Failed merges: r? @ghost
When implementing the binary operator traits it is easy to forget to restrict the
Output
associated type.rustc
now accounts for different cases to lead users in the right direction to add the necessary restrictions. The structured suggestions in the following output are new:That output is given for the following cases: