-
Notifications
You must be signed in to change notification settings - Fork 13.9k
library: core: document layout guarantee of TypeId
#148265
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
base: master
Are you sure you want to change the base?
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use |
|
Cc: @Darksonn |
|
Do you have a reference to a place where there was an agreement to commit to this? Or is this a new libs-api/libs/compiler request? |
This PR is a request for the stdlib to make this commitment. There is some prior discussion at #rust-for-linux > TypeId layout. |
|
This was discussed during this Thursday's compiler triage meeting #t-compiler/meetings > [weekly] 2025-10-30 @ 💬. The consensus of those present at the meeting was:
There was also an alternative approach for this use case that was discussed during the meeting. For FFI purposes, can The TL;DR is that we'd prefer not to guarantee this until we decide on what to do about typeid collisions. @rustbot label: -I-compiler-nominated |
|
I disagree with that, and I have opened a Zulip thread to further discuss it, so don't take the above message as a final compiler decision yet, it may or may not change: |
|
I would emphasize that restricting TypeId to 16 bytes only restricts the inline size, not the total size in the binary. I don't think we'd be closing the door on 256, 512 or 1024 bit hashes or even full v0 name mangling if we wanted to go down that route. |
|
as mentioned on Zulip, I think we can guarantee #[repr(C)]
union TypeIdBytes {
a: u128,
b: [*const (); 2], // not usize since that may not match pointer size on CHERI
}To be clear, I'm not proposing that That is 16 bytes except for platforms where pointers are >8 bytes (e.g. CHERI or rv128i). This properly handles our current implementation of having just a 128-bit hash, as well as larger values by letting us have: #[repr(C)]
pub struct TypeId {
small_hash: u64,
full_data: &'static FullData, // e.g. a ThinCStr or a >=256-bit hash
}
impl PartialEq for TypeId {
fn eq(&self, other: &Self) -> bool {
if self.small_hash != other.small_hash {
false // fast not-equal
} else if ptr::eq(self.full_data, other.full_data) {
true // fast equal, can be made the usual case by e.g. linker deduplication
} else {
self.full_data == other.full_data // rare slow path
}
}
} |
…uarantees, r=scottmcm Make explicit that `TypeId`'s layout and size are unstable Or worded differently, explicitly remark non-stable-guarantee of `TypeId` layout and size. This PR makes no *additional* guarantees or non-guarantees, it only emphasizes that `TypeId`'s size and layout are unstable like any other `#[repr(Rust)]` types. This was discussed during [#t-compiler/meetings > &rust-lang#91;weekly&rust-lang#93; 2025-10-30 @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202025-10-30/near/547949347), where the compiler team discussed a request rust-lang#148265 to have the standard library (and language) commit to `TypeId` guaranteeing a size upper bound of 16 bytes. In the meeting, the consensus was: - We were sympathetic to the use case discussed in the request PR, however we feel like this stability guarantee is premature, given that there are unresolved questions surrounding the intended purpose of `TypeId`, and concerns surrounding its collision-resistance properties rust-lang#10389 and rust-lang#129014. We would prefer not making any of such guarantee until the collision-resistance concerns are resolved. - Committing to a stability guarantee on the size upper bound now would close the door to making `TypeId` larger (even if unlikely for perf reasons). Given that we have previously broken people who asserted the size of `TypeId` is 8 bytes, it was also discussed in the meeting that we should *explicitly* note that the size and layout of `TypeId` is not a stable guarantee, and is subject to changes between Rust releases, and thus cannot be relied upon -- if breakage in people's code is due to that assumption, it will be considered a won't-fix. - So even if `#[repr(Rust)]` types have unstable size and layout, this PR makes it explicit for `TypeId` since this type can feel "special" and users can be lead into thinking its size and layout is something they can rely upon. r? `@scottmcm` (or libs/libs-api/lang)
Rollup merge of #148394 - jieyouxu:remark-typeid-no-layout-guarantees, r=scottmcm Make explicit that `TypeId`'s layout and size are unstable Or worded differently, explicitly remark non-stable-guarantee of `TypeId` layout and size. This PR makes no *additional* guarantees or non-guarantees, it only emphasizes that `TypeId`'s size and layout are unstable like any other `#[repr(Rust)]` types. This was discussed during [#t-compiler/meetings > [weekly] 2025-10-30 @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202025-10-30/near/547949347), where the compiler team discussed a request #148265 to have the standard library (and language) commit to `TypeId` guaranteeing a size upper bound of 16 bytes. In the meeting, the consensus was: - We were sympathetic to the use case discussed in the request PR, however we feel like this stability guarantee is premature, given that there are unresolved questions surrounding the intended purpose of `TypeId`, and concerns surrounding its collision-resistance properties #10389 and #129014. We would prefer not making any of such guarantee until the collision-resistance concerns are resolved. - Committing to a stability guarantee on the size upper bound now would close the door to making `TypeId` larger (even if unlikely for perf reasons). Given that we have previously broken people who asserted the size of `TypeId` is 8 bytes, it was also discussed in the meeting that we should *explicitly* note that the size and layout of `TypeId` is not a stable guarantee, and is subject to changes between Rust releases, and thus cannot be relied upon -- if breakage in people's code is due to that assumption, it will be considered a won't-fix. - So even if `#[repr(Rust)]` types have unstable size and layout, this PR makes it explicit for `TypeId` since this type can feel "special" and users can be lead into thinking its size and layout is something they can rely upon. r? `@scottmcm` (or libs/libs-api/lang)
|
☔ The latest upstream changes (presumably #148425) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Given that #148394 has just been sent and merged, are we further discussing this? |
|
#148394 was explicitly not a decision either way. It just documents the status quo. |
Document the layout guarantee of `TypeId` not to exceed 16 bytes. Additionally, use `repr(C)` to avoid additional padding if layout randomization improved. This is useful for FFI use-cases where a `TypeId` may be stored within a foreign data structure or buffer. An example for such a use-case can be found in [1]. Link: https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/commit/?id=6f61a2637abe4f89877da3280775565baedb60e0 [1] Suggested-by: Alice Ryhl <aliceryhl@google.com> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
For clarity: I merged that as "currently factual", but explicitly expecting that it might change by this PR (or others). If we end up making a confident decision quickly, great. If it ends up taking while, while at least we have the explicit note in the mean time :) |
…, r=scottmcm Make explicit that `TypeId`'s layout and size are unstable Or worded differently, explicitly remark non-stable-guarantee of `TypeId` layout and size. This PR makes no *additional* guarantees or non-guarantees, it only emphasizes that `TypeId`'s size and layout are unstable like any other `#[repr(Rust)]` types. This was discussed during [#t-compiler/meetings > [weekly] 2025-10-30 @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202025-10-30/near/547949347), where the compiler team discussed a request rust-lang/rust#148265 to have the standard library (and language) commit to `TypeId` guaranteeing a size upper bound of 16 bytes. In the meeting, the consensus was: - We were sympathetic to the use case discussed in the request PR, however we feel like this stability guarantee is premature, given that there are unresolved questions surrounding the intended purpose of `TypeId`, and concerns surrounding its collision-resistance properties rust-lang/rust#10389 and rust-lang/rust#129014. We would prefer not making any of such guarantee until the collision-resistance concerns are resolved. - Committing to a stability guarantee on the size upper bound now would close the door to making `TypeId` larger (even if unlikely for perf reasons). Given that we have previously broken people who asserted the size of `TypeId` is 8 bytes, it was also discussed in the meeting that we should *explicitly* note that the size and layout of `TypeId` is not a stable guarantee, and is subject to changes between Rust releases, and thus cannot be relied upon -- if breakage in people's code is due to that assumption, it will be considered a won't-fix. - So even if `#[repr(Rust)]` types have unstable size and layout, this PR makes it explicit for `TypeId` since this type can feel "special" and users can be lead into thinking its size and layout is something they can rely upon. r? `@scottmcm` (or libs/libs-api/lang)
|
We discussed this during today's libs-api meeting. Since TypeId is not really implemented by the library itself and its internals are driven by compiler concerns we're largely ok with deferring to T-compiler on which guarantees they're willing to give / how much they want to constrain their future implementations. One concern we have is the binary size bloat that some of the possible future implementations would cause. Putting the typename behind the pointer could lead to significant binary size increase for crates that make heavy use of type IDs and has long type names. We want TypeId to remain a cheap type so that crates that use it a lot or embedded systems don't run into scaling issues. |
|
should this be nominated again for t-compiler to decide if they want the solution I proposed? |
Document the layout guarantee of
TypeIdnot to exceed 16 bytes.This is useful for FFI use-cases where a
TypeIdmay be stored within a foreign data structure or buffer.An example for such a use-case can be found in [1].
Link: https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/commit/?id=6f61a2637abe4f89877da3280775565baedb60e0 [1]
Suggested-by: Alice Ryhl aliceryhl@google.com