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

When suggesting associated fn with type parameters, include in the structured suggestion #68689

Merged
merged 2 commits into from
Feb 9, 2020

Conversation

estebank
Copy link
Contributor

Address #50734.

error[E0046]: not all trait items implemented, missing: `foo`, `bar`, `baz`
  --> file.rs:14:1
   |
14 | impl TraitA<()> for S {
   | ^^^^^^^^^^^^^^^^^^^^^ missing `foo`, `bar`, `baz` in implementation
   |
   = help: implement the missing item: `fn foo<T>(_: T) -> Self where T: TraitB, TraitB::Item = A { unimplemented!() }`
   = help: implement the missing item: `fn bar<T>(_: T) -> Self { unimplemented!() }`
   = help: implement the missing item: `fn baz<T>(_: T) -> Self where T: TraitB, <T as TraitB>::Item: std::marker::Copy { unimplemented!() }`

It doesn't work well for associated types with ty::Predicate::Projections as we need to resugar T: Trait, Trait::Assoc = KT: Trait<Assoc = K>.

@rust-highfive
Copy link
Collaborator

r? @varkor

(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 Jan 31, 2020
LL | impl FromIterator<()> for X {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `from_iter` in implementation
|
= help: implement the missing item: `fn from_iter<T>(_: T) -> Self where T: std::iter::IntoIterator, std::iter::IntoIterator::Item = A { unimplemented!() }`
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 should be

fn from_iter<T>(_: T) -> Self where T: std::iter::IntoIterator<Item = A> { unimplemented!() }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #68982 to track this

@estebank
Copy link
Contributor Author

cc @rust-lang/compiler in order for this suggestion to be accurate we would need a way to resugar ty::Predicate::Trait and ty::Predicate::Projection into a single thing, at least on a best effort basis. Does anyone have any reservations on doing so? Do we already have that anywhere?

@eddyb
Copy link
Member

eddyb commented Feb 1, 2020

@estebank There's some code like that in rustdoc, might be nice to share that between the two usecases (if this is even a good idea).

@estebank
Copy link
Contributor Author

estebank commented Feb 1, 2020

might be nice to share that between the two usecases

Definitely.

if this is even a good idea

It's categorically not a good idea, but we might need to do it anyways.

@estebank
Copy link
Contributor Author

estebank commented Feb 4, 2020

For reference for future me, src/librustdoc/clean/inline.rs seems to have the rustdoc logic to do this in build_external_trait.

Copy link
Member

@varkor varkor left a comment

Choose a reason for hiding this comment

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

r=me after fixing the comment.

src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved
@estebank
Copy link
Contributor Author

estebank commented Feb 9, 2020

@bors r=varkor

@bors
Copy link
Contributor

bors commented Feb 9, 2020

📌 Commit 3cdd7ae has been approved by varkor

@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 Feb 9, 2020
@bors
Copy link
Contributor

bors commented Feb 9, 2020

⌛ Testing commit 3cdd7ae with merge 64ea639...

bors added a commit that referenced this pull request Feb 9, 2020
When suggesting associated fn with type parameters, include in the structured suggestion

Address #50734.

```
error[E0046]: not all trait items implemented, missing: `foo`, `bar`, `baz`
  --> file.rs:14:1
   |
14 | impl TraitA<()> for S {
   | ^^^^^^^^^^^^^^^^^^^^^ missing `foo`, `bar`, `baz` in implementation
   |
   = help: implement the missing item: `fn foo<T>(_: T) -> Self where T: TraitB, TraitB::Item = A { unimplemented!() }`
   = help: implement the missing item: `fn bar<T>(_: T) -> Self { unimplemented!() }`
   = help: implement the missing item: `fn baz<T>(_: T) -> Self where T: TraitB, <T as TraitB>::Item: std::marker::Copy { unimplemented!() }`
```

It doesn't work well for associated types with `ty::Predicate::Projection`s as we need to resugar `T: Trait, Trait::Assoc = K` → `T: Trait<Assoc = K>`.
@bors
Copy link
Contributor

bors commented Feb 9, 2020

☀️ Test successful - checks-azure
Approved by: varkor
Pushing 64ea639 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 9, 2020
@bors bors merged commit 3cdd7ae into rust-lang:master Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

None yet

6 participants