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

Add an unstable conversion from thread ID to u64 #67566

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

Mark-Simulacrum
Copy link
Member

We see multiple cases inside rustc and ecosystem code where ThreadId is
transmuted to u64, exploiting the underlying detail. This is suboptimal
(can break unexpectedly if we change things in std).

It is unlikely that ThreadId will ever need to be larger than u64 --
creating even 2^32 threads over the course of a program is quite hard,
2^64 is even harder. As such, we do not choose to return a larger sized
type (e.g. u128). If we choose to shrink ThreadId in the future, or
otherwise change its internals, it is likely that a mapping to u64 will
still be applicable (though may become more complex).

I will file a tracking issue as soon as this is loosely approved.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(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 23, 2019
@Mark-Simulacrum
Copy link
Member Author

It is intended for this to close #52780, though it is not mutually exclusive with it. I am against adding a direct Display impl for ThreadId (there is not "one true way" that I would expect it to be formatted); this allows an easy way to get the "just a number" formatting for ThreadId that seems to be commonly wanted.

@pitdicker
Copy link
Contributor

I have wished for a method like this, 👍

@Mark-Simulacrum Mark-Simulacrum added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 24, 2019
@cramertj
Copy link
Member

r? @alexcrichton

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looks good to me! Want to open a tracking issue, set it up here, and then r=me?

We see multiple cases inside rustc and ecosystem code where ThreadId is
transmuted to u64, exploiting the underlying detail. This is suboptimal
(can break unexpectedly if we change things in std).

It is unlikely that ThreadId will ever need to be larger than u64 --
creating even 2^32 threads over the course of a program is quite hard,
2^64 is even harder. As such, we do not choose to return a larger sized
type (e.g. u128). If we choose to shrink ThreadId in the future, or
otherwise change its internals, it is likely that a mapping to u64 will
still be applicable (though may become more complex).
@Mark-Simulacrum
Copy link
Member Author

Updated with a tracking issue and also added a bit of text even further warning against using the returned value for non-equality (specifically, ordering not being defined). @bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jan 6, 2020

📌 Commit d9a7db9 has been approved by alexcrichton

@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 Jan 6, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 6, 2020
…excrichton

Add an unstable conversion from thread ID to u64

We see multiple cases inside rustc and ecosystem code where ThreadId is
transmuted to u64, exploiting the underlying detail. This is suboptimal
(can break unexpectedly if we change things in std).

It is unlikely that ThreadId will ever need to be larger than u64 --
creating even 2^32 threads over the course of a program is quite hard,
2^64 is even harder. As such, we do not choose to return a larger sized
type (e.g. u128). If we choose to shrink ThreadId in the future, or
otherwise change its internals, it is likely that a mapping to u64 will
still be applicable (though may become more complex).

I will file a tracking issue as soon as this is loosely approved.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 7, 2020
…excrichton

Add an unstable conversion from thread ID to u64

We see multiple cases inside rustc and ecosystem code where ThreadId is
transmuted to u64, exploiting the underlying detail. This is suboptimal
(can break unexpectedly if we change things in std).

It is unlikely that ThreadId will ever need to be larger than u64 --
creating even 2^32 threads over the course of a program is quite hard,
2^64 is even harder. As such, we do not choose to return a larger sized
type (e.g. u128). If we choose to shrink ThreadId in the future, or
otherwise change its internals, it is likely that a mapping to u64 will
still be applicable (though may become more complex).

I will file a tracking issue as soon as this is loosely approved.
bors added a commit that referenced this pull request Jan 7, 2020
Rollup of 13 pull requests

Successful merges:

 - #67566 (Add an unstable conversion from thread ID to u64)
 - #67671 (Account for `type X = impl Trait;` in lifetime suggestion)
 - #67727 (Stabilise vec::remove_item)
 - #67877 (Omit underscore constants from rustdoc)
 - #67880 (Handle multiple error fix suggestions carefuly)
 - #67898 (Improve hygiene of `newtype_index`)
 - #67908 (rustdoc: HTML escape const values)
 - #67909 (Fix ICE in const pretty printing and resolve FIXME)
 - #67929 (Formatting an example for method Vec.retain)
 - #67934 (Clean up E0178 explanation)
 - #67936 (fire "non_camel_case_types" for associated types)
 - #67943 (Missing module std in example.)
 - #67962 (Update books)

Failed merges:

r? @ghost
@bors bors merged commit d9a7db9 into rust-lang:master Jan 7, 2020
@Mark-Simulacrum Mark-Simulacrum deleted the thread-id-u64 branch December 15, 2021 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants