Improve transmute docs with further clarifications #82592
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (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. |
rust-highfive
added
the
S-waiting-on-review
|
cc @RalfJung -- does this sound reasonable to you? |
library/core/src/intrinsics.rs
Outdated
| @@ -841,11 +841,21 @@ extern "rust-intrinsic" { | |||
| /// Both types must have the same size. Neither the original, nor the result, | |||
| /// may be an [invalid value](../../nomicon/what-unsafe-does.html). | |||
| /// | |||
| /// When transmuting containers, make sure to not violate the container invariants too. | |||
| /// Some containers might rely on the size of the type, alignment, or even the `TypeId`, | |||
| /// in which case transmuting wouldn't be possible without violating the container invariants. | |||
There was a problem hiding this comment.
Oh, you moved it all the way up here, that was a misunderstanding... I think this is less important than some of the comments that follow, so this is too prominent of a position IMO.
What I meant was to put this together with "Turning a Vec<&T> into a Vec<Option<&T>>". And I meant the entire paragraph; now the "To transmute the inner type of the contents of a container" still exists down there in the code and duplicates some of the text, which seems redundant.
|
I'm not entirely happy with the wording on 2e6d3fa, please let me know if you have more suggestions. Heh, so much back and forth, and I thought documentation changes were among the easiest possible changes. |
Good documentation for unsafe functions is a very tricky subject. Thanks for seeing this through. :) |
|
I made one final tweak, I think this is ready now. :) |
|
Should be good to go! |
|
Awesome. :D |
|
📌 Commit fbc1741 has been approved by |
bors
added
S-waiting-on-bors
Improve transmute docs with further clarifications Closes rust-lang#82493. Please let me know if any of the new wording sounds off, English is not my mother tongue.
Improve transmute docs with further clarifications Closes rust-lang#82493. Please let me know if any of the new wording sounds off, English is not my mother tongue.
Rollup of 13 pull requests Successful merges: - rust-lang#77916 (Change built-in kernel targets to be os = none throughout) - rust-lang#82130 (Make some Option, Result methods unstably const) - rust-lang#82292 (Prevent specialized ZipImpl from calling `__iterator_get_unchecked` twice with the same index) - rust-lang#82402 (Remove RefCell around `module_trait_cache`) - rust-lang#82592 (Improve transmute docs with further clarifications) - rust-lang#82651 (Cleanup rustdoc warnings) - rust-lang#82720 (Fix diagnostic suggests adding type `[type error]`) - rust-lang#82751 (improve offset_from docs) - rust-lang#82793 (Move some tests to more suitable subdirs) - rust-lang#82803 (rustdoc: Add an unstable option to print all unversioned files) - rust-lang#82808 (Sync rustc_codegen_cranelift) - rust-lang#82822 (Fix typo) - rust-lang#82837 (tweak MaybeUninit docs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Closes #82493.
Please let me know if any of the new wording sounds off, English is not my mother tongue.