Derive Hash for ThreadId + better example

[sagebind]

Derive Hash for ThreadId (see comments in #21507). Useful for making maps based on thread, e.g. HashMap<ThreadId, ?>. Also update example code for thread IDs to be more useful.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[sfackler]

Looks like the doctests are failing:

failures:
[01:02:27] [01:02:27] ---- thread/mod.rs - thread::Thread::id (line 791) stdout ----
[01:02:27] error: no method named `unwrap` found for type `std::thread::JoinHandle<std::thread::ThreadId>` in the current scope
[01:02:27] --> <anon>:10:4
[01:02:27] |
[01:02:27] 10 | }).unwrap();
[01:02:27] | ^^^^^^
[01:02:27] [01:02:27] error: aborting due to previous error(s)
[01:02:27] [01:02:27] thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:203
[01:02:27] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:02:27] [01:02:27] ---- thread/mod.rs - thread::ThreadId (line 660) stdout ----
[01:02:27] error: no method named `unwrap` found for type `std::thread::JoinHandle<std::thread::ThreadId>` in the current scope
[01:02:27] --> <anon>:10:4
[01:02:27] |
[01:02:27] 10 | }).unwrap();
[01:02:27] | ^^^^^^
[01:02:27] [01:02:27] error: aborting due to previous error(s)
[01:02:27] [01:02:27] thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:203
[01:02:27] [01:02:27] [01:02:27] failures:
[01:02:27] thread/mod.rs - thread::Thread::id (line 791)
[01:02:27] thread/mod.rs - thread::ThreadId (line 660)
[01:02:27] [01:02:27] test result: FAILED. 776 passed; 2 failed; 22 ignored; 0 measured
[01:02:27] [01:02:27] error: test failed

Could you add Debug into the derive list while you're in there?

[nrc]

Could you add Debug into the derive list while you're in there?

Note that there is already a Debug impl, it just prints ThreadId { .. } rather than the more useful, obvious solution. If we do want to derive Debug, then we would need to remove the explicit impl.

[sfackler]

Oh, that's weird. I have no problem with a derived implementation that actually shows the value.

[sagebind]

I think we originally did not derive Debug because we did not want to expose the actual value in any way, even for debugging. I see no issue with displaying it though, for debugging purposes anyway. Would probably be really helpful for identifying threads for logs, etc.

[alexcrichton]

Yeah hiding the value from the Debug impl seems like it's trying to hard to me, I'd also expect #[derive(Debug)] here (and would be ok with it)

[nrc]

hiding the value from the Debug impl seems like it's trying to hard to me

it is irritating but also useless - you can always just transmute

[sfackler]

You monster!

[alexcrichton]

@rfcbot fcp merge

[rfcbot]

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:

• @BurntSushi
• @GuillaumeGomez
• @Kimundi
• @alexcrichton
• @aturon
• @brson
• @frewsxcv
• @jonathandturner
• @peschkaj
• @sfackler
• @steveklabnik

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[frewsxcv]

I don't remember why I tagged this T-doc, but here's my checkmark anyways.

[sagebind]

@frewsxcv Perhaps because the example code is updated for the ThreadId type?

[peschkaj]

@rfcbot reviewed

[sophiajt]

@rfcbot reviewed

[GuillaumeGomez]

@rfcbot reviewed

[alexcrichton]

@bors: r+

[bors]

📌 Commit cd14a32 has been approved by alexcrichton

[bors]

⌛ Testing commit cd14a32 with merge d62179c...

[bors]

💔 Test failed - status-appveyor

[TimNN]

@bors retry

• Spurious failures on AppVeyor due to ar.exe errors #40546

[bors]

⌛ Testing commit cd14a32 with merge 4a78fc0...

[bors]

💔 Test failed - status-appveyor

[TimNN]

@bors retry

• Spurious failures on AppVeyor due to ar.exe errors #40546

[bors]

⌛ Testing commit cd14a32 with merge 14481f7...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 14481f7 to master...