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

Provide suggestions for type parameters missing bounds for associated types #70908

Merged
merged 8 commits into from
May 7, 2020

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Apr 7, 2020

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 #20041 <https://github.com/rust-lang/rust/issues/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:

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)
    }
}

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 7, 2020
src/librustc_hir/hir.rs Outdated Show resolved Hide resolved
Comment on lines 4 to +14
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> {
| ^^^^^^^^^^^^^^^
Copy link
Contributor Author

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.

@estebank
Copy link
Contributor Author

estebank commented Apr 8, 2020

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

  • newcomers that want to apply a binary operation to two type arguments
  • people trying to preempt the missing bound error without knowing the whole trait definition
  • people who know that they want to specify the associated type but are unsure about the syntax and follow the code that unmet predicate errors show

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.

@nikomatsakis
Copy link
Contributor

r? @nikomatsakis

Copy link
Contributor

@nikomatsakis nikomatsakis left a 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.

src/librustc_typeck/check/op.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/op.rs Outdated Show resolved Hide resolved
src/librustc_middle/ty/diagnostics.rs Show resolved Hide resolved
@bors

This comment has been minimized.

@bors

This comment has been minimized.

@joelpalmer joelpalmer added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 30, 2020
Copy link
Contributor

@nikomatsakis nikomatsakis left a 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

src/librustc_ast_passes/ast_validation.rs Show resolved Hide resolved
@@ -67,3 +72,180 @@ impl<'tcx> TyS<'tcx> {
}
}
}

/// Suggest restricting a type param with a new bound.
Copy link
Contributor

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(_), .. })
Copy link
Contributor

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

Copy link
Contributor Author

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>`.
Copy link
Contributor

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

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 2, 2020

📌 Commit c0a9d52 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 3, 2020
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)
    }
}
```
@Dylan-DPC-zz
Copy link

failed in rollup

@bors r-

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 3, 2020
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 3, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2020
…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.
@bors

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.
@estebank
Copy link
Contributor Author

estebank commented May 4, 2020

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented May 4, 2020

📌 Commit b17b20c has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2020
@bors
Copy link
Contributor

bors commented May 5, 2020

⌛ Testing commit b17b20c with merge 7ab8af3dd0318629a0a3d835468d9e3eb121a948...

@Dylan-DPC-zz
Copy link

@bors retry yied

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 5, 2020
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)
    }
}
```
bors added a commit to rust-lang-ci/rust that referenced this pull request May 6, 2020
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
@bors bors merged commit ce14d6d into rust-lang:master May 7, 2020
@estebank estebank deleted the suggest-add branch November 9, 2023 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants