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

Mention implementers of unsatisfied trait #91873

Merged
merged 8 commits into from
Apr 5, 2022

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Dec 13, 2021

When encountering an unsatisfied trait bound, if there are no other
suggestions, mention all the types that do implement that trait:

error[E0277]: the trait bound `f32: Foo` is not satisfied
  --> $DIR/impl_wf.rs:22:6
   |
LL | impl Baz<f32> for f32 { }
   |      ^^^^^^^^ the trait `Foo` is not implemented for `f32`
   |
   = help: the trait `Foo` is implemented for `i32`
note: required by a bound in `Baz`
  --> $DIR/impl_wf.rs:18:31
   |
LL | trait Baz<U: ?Sized> where U: Foo { }
   |                               ^^^ required by this bound in `Baz`
error[E0277]: the trait bound `u32: Foo` is not satisfied
  --> $DIR/associated-types-path-2.rs:29:5
   |
LL |     f1(2u32, 4u32);
   |     ^^ the trait `Foo` is not implemented for `u32`
   |
   = help: the trait `Foo` is implemented for `i32`
note: required by a bound in `f1`
  --> $DIR/associated-types-path-2.rs:13:14
   |
LL | pub fn f1<T: Foo>(a: T, x: T::A) {}
   |              ^^^ required by this bound in `f1`

Suggest dereferencing in more cases.

Fix #87437, fix #90970.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 13, 2021
@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(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 Dec 13, 2021
@camelid camelid added the A-diagnostics Area: Messages for errors, warnings, and lints label Dec 13, 2021
@estebank estebank force-pushed the mention-impls-for-unsatisfied-trait branch from a8bcbd3 to f1eb1c0 Compare December 14, 2021 03:09
@fmease
Copy link
Member

fmease commented Dec 14, 2021

Does this fix/work towards fixing #90970?

@estebank
Copy link
Contributor Author

estebank commented Dec 14, 2021

As is, it does not:

error[E0277]: expected a `FnMut<(char,)>` closure, found `u8`
 --> f23.rs:2:20
  |
2 |     s.strip_suffix(b'\n').unwrap_or(s)
  |       ------------ ^^^^^ expected an `FnMut<(char,)>` closure, found `u8`
  |       |
  |       required by a bound introduced by this call
  |
  = help: the trait `FnMut<(char,)>` is not implemented for `u8`

Looking if there's a straightforward change that can be done.


Edit: With a quick check, I managed to get the following:

error[E0277]: expected a `FnMut<(char,)>` closure, found `u8`
 --> f23.rs:2:20
  |
2 |     s.strip_suffix(b'\n').unwrap_or(s)
  |       ------------ ^^^^^ expected an `FnMut<(char,)>` closure, found `u8`
  |       |
  |       required by a bound introduced by this call
  |
  = help: the trait `FnMut<(char,)>` is not implemented for `u8`
  = note: required because of the requirements on the impl of `Pattern<'_>` for `u8`
  = note: the following other types implement `Pattern<'_>`:
            &'b String
            &'b [char; N]
            &'b [char]
            &'b str
            &'c &'b str
            [char; N]
            char
            pattern::MultiCharEqPattern<C>

I would like to iterate a bit further (and report the error higher in the obligation chain so we only talk about Pattern and not FnMut), but that will be better served by a separate PR.

What do you think?

@estebank
Copy link
Contributor Author

Update: now it does cover it, slightly.

@fmease
Copy link
Member

fmease commented Dec 15, 2021

What do you think?

I think it's already an improvement that it lists those other implementations and thus likely clears up the potential confusion of the user much faster which may arise from the incorrect mention of FnMut (instead of Pattern). So thank you very much! ❤️

I first faced this error message a few years ago when I wasn't that familiar with Rust yet and I remember it took me a quite a bit of time to figure out what was going on. So any steps towards fixing this is great!

I would like to iterate a bit further (and report the error higher in the obligation chain so we only talk about Pattern and not FnMut), but that will be better served by a separate PR.

Sure! I hope you (or someone else of course) can figure out how to suppress, really, references to Fn* obligations in certain situations. From past experiences, I know that sometimes for some reason Fn* impls like to “take over” certain error messages. I don't have a concrete example at hand right now, though (apart from Pattern obviously).

@bors

This comment was marked as resolved.

@estebank estebank force-pushed the mention-impls-for-unsatisfied-trait branch from d2ada99 to 9a5872f Compare December 16, 2021 20:51
@michaelwoerister
Copy link
Member

Sorry for the delay. I think it might be better if someone from wg-diagnostics reviewed this.

r? rust-lang/diagnostics

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

I like the diagnostic change, couple comments on the implementation where we might be able to deduplicate things a bit.

@davidtwco davidtwco added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 5, 2022
@bors

This comment was marked as resolved.

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 12, 2022
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2022
@estebank estebank force-pushed the mention-impls-for-unsatisfied-trait branch 2 times, most recently from 845dc7d to ba3450e Compare March 26, 2022 03:47
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2022
@rust-log-analyzer

This comment was marked as resolved.

@estebank estebank force-pushed the mention-impls-for-unsatisfied-trait branch from 38ef896 to 1bb5b95 Compare March 27, 2022 19:09
@estebank estebank force-pushed the mention-impls-for-unsatisfied-trait branch from c278cd1 to e2f263e Compare March 28, 2022 02:32
Copy link
Member

@davidtwco davidtwco 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 unless you want to squash the commits first

@bors
Copy link
Contributor

bors commented Apr 1, 2022

☔ The latest upstream changes (presumably #95552) made this pull request unmergeable. Please resolve the merge conflicts.

@davidtwco davidtwco added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2022
When encountering an unsatisfied trait bound, if there are no other
suggestions, mention all the types that *do* implement that trait:

```
error[E0277]: the trait bound `f32: Foo` is not satisfied
  --> $DIR/impl_wf.rs:22:6
   |
LL | impl Baz<f32> for f32 { }
   |      ^^^^^^^^ the trait `Foo` is not implemented for `f32`
   |
   = help: the following other types implement trait `Foo`:
             Option<T>
             i32
             str
note: required by a bound in `Baz`
  --> $DIR/impl_wf.rs:18:31
   |
LL | trait Baz<U: ?Sized> where U: Foo { }
   |                               ^^^ required by this bound in `Baz`
```

Mention implementers of traits in `ImplObligation`s.

Do not mention other `impl`s for closures, ranges and `?`.
@estebank estebank force-pushed the mention-impls-for-unsatisfied-trait branch from e2f263e to e1ef833 Compare April 4, 2022 21:14
@estebank
Copy link
Contributor Author

estebank commented Apr 4, 2022

@bors r=davidtwco

@bors
Copy link
Contributor

bors commented Apr 4, 2022

📌 Commit e1ef833 has been approved by davidtwco

@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 Apr 4, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 4, 2022
…ied-trait, r=davidtwco

Mention implementers of unsatisfied trait

When encountering an unsatisfied trait bound, if there are no other
suggestions, mention all the types that *do* implement that trait:

```
error[E0277]: the trait bound `f32: Foo` is not satisfied
  --> $DIR/impl_wf.rs:22:6
   |
LL | impl Baz<f32> for f32 { }
   |      ^^^^^^^^ the trait `Foo` is not implemented for `f32`
   |
   = help: the trait `Foo` is implemented for `i32`
note: required by a bound in `Baz`
  --> $DIR/impl_wf.rs:18:31
   |
LL | trait Baz<U: ?Sized> where U: Foo { }
   |                               ^^^ required by this bound in `Baz`
```
```
error[E0277]: the trait bound `u32: Foo` is not satisfied
  --> $DIR/associated-types-path-2.rs:29:5
   |
LL |     f1(2u32, 4u32);
   |     ^^ the trait `Foo` is not implemented for `u32`
   |
   = help: the trait `Foo` is implemented for `i32`
note: required by a bound in `f1`
  --> $DIR/associated-types-path-2.rs:13:14
   |
LL | pub fn f1<T: Foo>(a: T, x: T::A) {}
   |              ^^^ required by this bound in `f1`
```

Suggest dereferencing in more cases.

Fix rust-lang#87437, fix rust-lang#90970.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#91873 (Mention implementers of unsatisfied trait)
 - rust-lang#95588 (explicitly distinguish pointer::addr and pointer::expose_addr)
 - rust-lang#95603 (Fix late-bound ICE in `dyn` return type suggestion)
 - rust-lang#95620 (interpret: remove MemoryExtra in favor of giving access to the Machine)
 - rust-lang#95630 (Update `NonNull` pointer provenance methods' documentation)
 - rust-lang#95631 (Refactor: remove unnecessary nested blocks)
 - rust-lang#95642 (`CandidateSource::XCandidate` -> `CandidateSource::X`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a5c8169 into rust-lang:master Apr 5, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 5, 2022
@estebank estebank deleted the mention-impls-for-unsatisfied-trait branch November 9, 2023 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
10 participants