Skip to content

Diagnostics for trait methods with additional unmet bounds can be confusing to read #100433

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

Open
semicoleon opened this issue Aug 12, 2022 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@semicoleon
Copy link
Contributor

semicoleon commented Aug 12, 2022

A URLO thread about confusion over why Iterator's rev method didn't work for HashMap's iterators kicked off some discussion about the way the error message was phrased.

It has all of the information about the root cause of the error (a missing impl), as well as what caused that requirement to be introduced (calling rev). I think the ordering of the information is a little confusing though, especially for people new to the language.

Given the following code:

Playground

use std::collections::HashMap;

fn map() {
    let map = HashMap::<u32, u32>::new();

    for _ in map.iter().rev() {}
}

The current output is:

error[E0277]: the trait bound `std::collections::hash_map::Iter<'_, u32, u32>: DoubleEndedIterator` is not satisfied
    --> src/main.rs:6:25
     |
6    |     for _ in map.iter().rev() {}
     |                         ^^^ the trait `DoubleEndedIterator` is not implemented for `std::collections::hash_map::Iter<'_, u32, u32>`
     |
note: required by a bound in `rev`
    --> /<redacted>/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/iter/traits/iterator.rs:3111:23
     |
3111 |         Self: Sized + DoubleEndedIterator,
     |                       ^^^^^^^^^^^^^^^^^^^ required by this bound in `rev`

error[E0277]: the trait bound `std::collections::hash_map::Iter<'_, u32, u32>: DoubleEndedIterator` is not satisfied
 --> src/main.rs:6:14
  |
6 |     for _ in map.iter().rev() {}
  |              ^^^^^^^^^^^^^^^^ the trait `DoubleEndedIterator` is not implemented for `std::collections::hash_map::Iter<'_, u32, u32>`
  |
  = note: required because of the requirements on the impl of `Iterator` for `Rev<std::collections::hash_map::Iter<'_, u32, u32>>`
  = note: required because of the requirements on the impl of `IntoIterator` for `Rev<std::collections::hash_map::Iter<'_, u32, u32>>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `sample` due to 2 previous errors

I think it might be worth starting with a message about what the user did to cause the error rather than the root cause.

Mostly unrelated As I was writing this up I also noticed that the last error "required because of the requirements on the impl of `IntoIterator`" doesn't explain why `IntoIterator` is involved. It might be worth tacking on a note that for loops call into_iter automatically to clarify why that's there.

Ideally the output should look like:

error[?????]: The method `rev` has additional trait bounds that were not met
    --> src/main.rs:6:25
     |
6    |     for _ in map.iter().rev() {}
     |                         ^^^ the trait `DoubleEndedIterator` is not implemented for `std::collections::hash_map::Iter<'_, u32, u32>`
note: unmet bound in `rev`
    --> /<redacted>/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/iter/traits/iterator.rs:3111:23
     |
3111 |         Self: Sized + DoubleEndedIterator,
     |                       ^^^^^^^^^^^^^^^^^^^ required by this bound in `rev`
... (existing diagnostic messages)
@semicoleon semicoleon added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 12, 2022
@chenyukang
Copy link
Member

This is a known issue,
#100176

I remember @compiler-errors have finished the PR, seems the doc haven't been updated.

@compiler-errors
Copy link
Member

@chenyukang this issue is somewhat orthogonal. #100176 is about displaying the rustdocs for Iterator::rev for HashMap's Keys iterator.

This is about the error message that comes from calling Iterator::rev for a type that does not implement DoubleEndedIterator, not documentation I think. It's the same "root cause" -- this bound -- but different places: rustdoc and rustc.

@compiler-errors
Copy link
Member

compiler-errors commented Aug 12, 2022

@semicoleon I'm not exactly sure if I understand the changes you want to see here.

The for loop (for _ in map.iter().rev() {}) isn't the source of the error here, per se, but instead rev() call. If we change the code to:

use std::collections::HashMap;

fn map() {
    let map = HashMap::<u32, u32>::new();
    let _ = map.iter().rev();
}

Then we get the very similar error message:

error[[E0277]](https://doc.rust-lang.org/stable/error-index.html#E0277): the trait bound `std::collections::hash_map::Iter<'_, u32, u32>: DoubleEndedIterator` is not satisfied
 --> src/lib.rs:5:24
  |
5 |     let _ = map.iter().rev();
  |                        ^^^ the trait `DoubleEndedIterator` is not implemented for `std::collections::hash_map::Iter<'_, u32, u32>`
  |
note: required by a bound in `rev`

For more information about this error, try `rustc --explain E0277`.

I can investigate why this error message is (almost) duplicated though.

@compiler-errors
Copy link
Member

Oh I guess this part is what you're asking for:

The method rev has additional trait bounds that were not met

Yeah, let me see if I can specialize the error message specifically for Self: ... bounds too.

@compiler-errors compiler-errors self-assigned this Aug 12, 2022
@estebank
Copy link
Contributor

estebank commented Aug 12, 2022

I can investigate why this error message is (almost) duplicated though.

That is likely due to the for-loop desugaring expanding to different spans, causing the error to be duplicated. Edit: ah, close but far, it is likely the implicit expr.into_iterator() call that the loop expands to. We might have some mechanism to deduplicate here, including by having a new ObligationKindCode just for this.

Yeah, let me see if I can specialize the error message specifically for Self: ... bounds too.

That would be good.

For the case in particular of DoubleEndedIterator bounds we might want to provide a rustc_on_unimplemented annotation saying something along the lines of "this Iterator can't be reversed" and ideally customize for different containers, like HashMap, stating that it can't because it has no specified ordering.

@semicoleon
Copy link
Contributor Author

I updated the original issue to hopefully be more clear about what I was talking about, sorry for the confusion!

@compiler-errors compiler-errors removed their assignment Aug 11, 2023
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 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants