-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Make TypeId const comparable #142789
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
Make TypeId const comparable #142789
Conversation
|
Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri Some changes occurred to the CTFE machinery Some changes occurred in compiler/rustc_codegen_gcc |
This comment has been minimized.
This comment has been minimized.
3cddd21 to
1fd7b66
Compare
This comment has been minimized.
This comment has been minimized.
|
It will be a while until I have the capacity to review a PR of this scale. Meanwhile, could you say a bit more about the architecture of the change? It seems you want for the "new kind of allocation" approach, but it's not clear from the PR description how exactly that shows up in Also, I am definitely not comfortable landing this by myself, I can only review the const-eval parts. Changing the representation of |
|
Some changes occurred in compiler/rustc_codegen_ssa |
|
Well, I got private feedback yesterday that instead of encoding a 16 byte value as an 8 byte pointer to the 16 byte value and an 8 byte hash, I should just do the thing where we split up type id internally into pointer sized chunks and codegen will make a hash out of it again. TLDR: no changes to runtime type id anymore in the latest revision of this PR. Only compile-time type id is now a bit funny |
I'm splitting unrelated parts out, so the high level feedback is already useful and I'll look for libs and codegen ppl to review the appropriate parts |
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #142906) made this pull request unmergeable. Please resolve the merge conflicts. |
b8a7a10 to
1c47a64
Compare
This comment has been minimized.
This comment has been minimized.
1c47a64 to
bcb4aa2
Compare
This should unblock stabilizing const
TypeId::ofand allow us to progress into any possible future we want to takeTypeIdto.To achieve that
TypeIdnow contains16 / size_of<usize>()pointers which each are actually justsize_of<usize>()bytes of the stable hash. At compile-time the first of these pointers cannot be dereferenced or otherwise inspected (at present doing so might ICE the compiler). Preventing inspection of this data allows us to refactorTypeIdto any other scheme in the future without breaking anyone who was tempted to transmuteTypeIdto obtain the hash at compile-time.cc @eddyb for their previous work on #95845 (which we still can do in the future if we want to get rid of the hash as the final thing that declares two TypeIds as equal).
const fntype_id#77125r? @RalfJung