-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Next-Gen transmute
#3844
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?
Next-Gen transmute
#3844
Conversation
3300a1e
to
94cfc1e
Compare
94cfc1e
to
b9e6849
Compare
transmute might not get caught by the linting or post-mono check. | ||
|
||
|
||
# Rationale and alternatives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of exploring an alternative:
Could we have std::mem::transmute
resolve to a different function across editions? Older editions get the typeck-erreors, newer editions get the new union_transmute
behaviour.
This would technically be breaking in that type equality of the function item of std::mem::transmute_e2024
and std::mem::transmute_e2025
would be different. Though I think the only way to actually encounter this in practice would be to have a macro that returns the function item of transmute to a crate on a different edition, which seems unlikely 🤔
I think having a coercion from e2025->e2024 transmute would be sound though. Which may even further make that harder to encounter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(With my types hat on though, I'd prefer we not need to maintain such a coercion forever if we could get away with it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t think you need any type-level wizardry if you have edition-dependent name resolution, and also a way to override the edition default for people who really need to
Change `mem::transmute` from having a magic size check to having an ordinary | ||
`const { … }`-enforced check plus some normal lints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this might cause soundness issues in crates that were assuming that even mentioning std::mem::transmute::<Type1, Type2>
fails to compile if Type1
and Type2
don't have the same size, which seems like would no longer happen under this change (playground link).
Though I think it would be reasonable to declare this as just being an incorrect assumption (after some time for the ecosystem to update and/or yank affected versions etc. Maybe with a FCW, though given that this currently emits an error, a FCW might be of limited use)
Change `mem::transmute` from having a magic size check to having an ordinary | ||
`const { … }`-enforced check plus some normal lints. | ||
|
||
Add a `mem::union_transmute` for an even-less-restricted transmute where size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broadly speaking, having this operation seems reasonable. The name seems like it involves some compiler-tinted glasses. I think the concept of "transmute via a union" doesn't really evoke the notion of a less restricted transmute.
I would suggest, instead, something like transmute_unchecked
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transmute_unchecked
feels pretty silly considering how transmute
itself is already lacking most of the checks you would want for type conversion.
I think that union_transmute
isn't really compiler-tinted, since unions don't need to be used by compilers and are a language feature in Rust. That said, perhaps prefix_transmute
might satisfy your naming requirements: it's not transmuting the whole object, but just the prefix of it in memory. While it is possible for the "prefix" to be larger than the original value, this is always UB and never an intended use of the function, and it's the job of the caller anyway to ensure that the function's preconditions are met.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it is possible for the "prefix" to be larger than the original value, this is always UB
It's not, actually, in this plan. If you union_transmute
a u8
to a #[repr(C, align(4))] struct AlignedByte(u8);
, that's sound 100% of the time, despite the output type being larger than the input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I completely forgot about struct padding; you're right there. Although from this perspective, I guess it's still a prefix, just transmuting onto a prefix instead of from one?
Either way, union_transmute
does feel like a good way to describe this, and as I mentioned, unions are still something that can be used by folks doing weird memory operations and aren't specific to the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added https://github.com/scottmcm/rfcs/blob/newt_level_transfiguration/text/3844-next-gen-transmute.md#why-the-name-union_transmute to discuss this. My thinking was similar to what @clarfonthey describes about union
being a public rust thing, not just a compiler concept. That said, I also put the exact name as a "to figure out in nightly before stabilization" thing because the specific name is not (to me) the critical part of this RFC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps transmute_common_prefix
would be a clearer name? Or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm with @joshtriplett on this one; an appropriate name would be transmute_unchecked
; in fact, that's the name we arrived at in zerocopy's internal polyfill: https://github.com/google/zerocopy/blob/f83ca64f7e217f73698d34335da0a363ba54b3e0/src/util/mod.rs#L266-L301
transmute_unchecked
feels pretty silly considering howtransmute
itself is already lacking most of the checks you would want for type conversion.
With the recent progress on TransmuteFrom
, my expectation is that we'll eventually want to deprecate mem::transmute
(which provides a hodgepodge of checks) and settle on transmute_unchecked
(which performs no checks at all) and TransmuteFrom::transmute
(which performs a configurable set of checks).
4b040a2
to
5a59155
Compare
- Weird crimes using transmute to check sizes without ever actually running the | ||
transmute might not get caught by the linting or post-mono check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Weird crimes using transmute to check sizes without ever actually running the | |
transmute might not get caught by the linting or post-mono check. | |
- The current user experience of PMEs is very poor. | |
- Weird crimes using transmute to check sizes without ever actually running the | |
transmute might not get caught by the linting or post-mono check. |
The UX of PMEs is very poor right now. They don't provide the sort of backtrace mechanism you expect with panics in a non-const context, so it's extraordinarily easy to get into a situation where it's near-impossible to figure out the causal chain that lead to the PME.
So, to work around that, we commit those "weird crimes" for size-checking in zerocopy in order to get good error messages in our abstractions rather than terrible post monomorphization errors; see try_transmute!
. It would not be a soundness regression in zerocopy if the behavior of mem::transmute
changed, but it would be a UX regression.
I don't know how feasible it is to add complete backtraces to PMEs; @lcnr or @compiler-errors might have insights.
Move to a world where the size check for
transmute
is always accurate, preserving the developer experience via linting instead of the special-cased typeck tricks.Rendered