-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 more docs - mostly warnings - to std::mem::transmute #34609
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
Perhaps I should also add an example for casting to |
|
@arielb1 Ah, good for the alternatives section! |
@@ -278,17 +278,157 @@ extern "rust-intrinsic" { | |||
/// Moves a value out of scope without running drop glue. | |||
pub fn forget<T>(_: T) -> (); | |||
|
|||
/// Unsafely transforms a value of one type into a value of another type. | |||
/// Reinterprets the bits of a value of one type as another type. Both types |
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.
The first paragraph needs to be a single sentence as it's used as a short summary on the module page and search results.
A lot of these examples aren't compiling, check the travis output and try them locally. I suggest you put each example in it's own code block so they run individually and some examples like the |
@ollie27 Yeah, I've obviously never written documentation like this before xP |
@ollie27 Okay, I can't actually try it locally due to LLDB failures. I'm... not willing to continue to try this. If someone else wants to pick this up, they can, but goddamnit, I've spent a fucking day trying to get rustc to compile. This is ridiculous. Edit: Alright, it's working now. The rustc new-dev experience is... not great :P |
You shouldn't need to compile |
Yes please to both of these! |
/// are as follows: | ||
/// | ||
/// Turning a pointer into a `usize`: | ||
/// ``` |
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.
can you put a blank line between the triple backticks and the descriptions? This applies throughout the file.
/// `transmute` is semantically equivalent to a bitwise move of one type | ||
/// into another. It copies the bits from the destination type into the | ||
/// source type, then forgets the original. If you know C or C++, it's like | ||
/// `memcpy` under the hood. |
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.
We usually don't refer to other languages in the docs. I would just say
then forgets the original, like
memcpy
.
@ubsan thanks for all the work on this, it's improved greatly 😄 I have some extremely small nits and then this is good to go, I think. |
@bors: r+ rollup |
📌 Commit 24f8589 has been approved by |
⌛ Testing commit 24f8589 with merge 3850b22... |
💔 Test failed - auto-win-gnu-32-opt |
@bors: retry On Thu, Jul 21, 2016 at 1:04 PM, bors notifications@github.com wrote:
|
@bors: retry |
@bors: retry (maybe third time's the charm) |
Add more docs - mostly warnings - to std::mem::transmute
⌛ Testing commit 24f8589 with merge 2094bc3... |
💔 Test failed - auto-win-msvc-64-opt |
@bors: retry On Tue, Jul 26, 2016 at 4:03 AM, bors notifications@github.com wrote:
|
Add more docs - mostly warnings - to std::mem::transmute
No description provided.