Improve error messages for HashSet with non-Hash type

[nirvdrum]

I ran into a situation with confusing error messages constructing a HashSet with a type that doesn't inherit Hash.

I tried this code:

let mut live_in = HashSet::<Temp>::new();
live_in.extend(value.uses.iter())`

I expected to see this happen:

The values from value.uses to be inserted into the set live_in. Or, an error message letting me know what's wrong.

Instead, this happened:

I saw the following error:

error[E0599]: no method named `extend` found for struct `std::collections::HashSet<temp::Temp>` in the current scope
  --> src/liveness.rs:88:17
   |
88 |         live_in.extend(value.uses.iter());
   |                 ^^^^^^ method not found in `std::collections::HashSet<temp::Temp>`

This error message isn't terribly helpful. It was especially confusing because I had called extend on other sets without incident. After searching for a while to figure out what was going wrong, I decided to just try manually inserting the values:

for temp in &value.uses {
    live_in.insert(*temp);
}

This time, I received a considerably more helpful error message:

error[E0599]: the method `insert` exists for struct `std::collections::HashSet<temp::Temp>`, but its trait bounds were not satisfied
  --> src/liveness.rs:89:21
   |
89 |             live_in.insert(*temp);
   |                     ^^^^^^ method cannot be called on `std::collections::HashSet<temp::Temp>` due to unsatisfied trait bounds
   | 
  ::: src/temp.rs:14:1
   |
14 | pub struct Temp {
   | --------------- doesn't satisfy `temp::Temp: Hash`
   |
   = note: the following trait bounds were not satisfied:
           `temp::Temp: Hash`

Ultimately, the problem was I neglected to implement Hash for my Temp type. That's obviously a straightforward user error, but also one that I think is reasonably common. The reason I'm filing an issue is I think the error messages could be improved here to help out users better. I suppose what's most surprising to me is that the following line didn't error:

let mut live_in = HashSet::<Temp>::new();

I would have expected the bounds check to happen as the type was being constructed, rather than delaying it until an operation on the value. There's likely some reason to allow construction of HashSet with a non-Hash type, but the interaction leads to errors that I think are a bit removed from the source.

Meta

rustc --version --verbose:

rustc 1.53.0 (53cb7b09b 2021-06-17)
binary: rustc
commit-hash: 53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b
commit-date: 2021-06-17
host: x86_64-unknown-linux-gnu
release: 1.53.0
LLVM version: 12.0.1

[leonardo-m]

There's likely some reason to allow construction of HashSet with a non-Hash type,

What's the reason?

[nirvdrum]

There's likely some reason to allow construction of HashSet with a non-Hash type,

What's the reason?

I have no idea. My experience has been when an established code pattern seems to have a problem with an obvious solution, there's usually a historical reason for it. Changing struct HashSet<T, S = RandomState> to struct HashSet<T : Hash, S = RandomState> would have led to the most helpful error message, but such a change is apt to break someone's code. HashMap has the same omission on its key type, too, which really makes me think this wasn't a simple oversight.

[ibraheemdev]

It's considered a best practice to delay bounds until they are actually needed, and it does lead to confusing error messages sometimes.

[piegamesde]

I just ran into this too. Took me quite a while to figure out the root of the problem. Btw this also applies if the Eq bound is missing. Minimal example (playground link):

use std::collections::HashSet;

#[derive(Default, Eq, PartialEq, Clone)] // no Hash
struct Data;

fn main() {
    let mut set = HashSet::<Data>::new();
    set.extend(std::iter::once(Data));
}

   Compiling playground v0.0.1 (/playground)
error[E0599]: no method named `extend` found for struct `HashSet` in the current scope
 --> src/main.rs:8:9
  |
8 |     set.extend(std::iter::once(Data));
  |         ^^^^^^ method not found in `HashSet<Data>`

error: aborting due to previous error

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

Maybe we could add a few words about this into E0599?

[ibraheemdev]

Hm, that error is significantly worse.