-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Tweak std::mem docs (#29362) #36357
Tweak std::mem docs (#29362) #36357
Conversation
Thanks for the PR! However: Linkcheck stage2 (x86_64-unknown-linux-gnu)
core/mem/fn.size_of_val.html:54: broken link - core/primitive.slice.html
core/mem/fn.forget.html:63: broken link - core/rc/struct.Rc.html
core/mem/fn.forget.html:64: broken link - core/process/fn.exit.html
core/mem/fn.forget.html:229: broken link - core/boxed/struct.Box.html
core/mem/fn.forget.html:229: broken link - core/boxed/struct.Box.html
core/mem/fn.uninitialized.html:69: broken link - core/ptr/fn.copy.html
core/mem/fn.uninitialized.html:69: broken link - core/ptr/fn.copy_nonoverlapping.html |
/// [box]: ../boxed/struct.Box.html | ||
/// [into_raw]: ../boxed/struct.Box.html#method.into_raw | ||
/// [ub]: ../../reference.html#behavior-considered-undefined | ||
/// ``` |
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 think this "```" is too much.
I don't know if there are other missing urls but could add urls to all types/traits please? (Like |
/// Returns the ABI-required minimum alignment of a type | ||
/// Returns the [ABI][]-required minimum alignment of a type. | ||
/// | ||
/// Every valid address of a value of the type `T` must be a multiple of this number. |
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.
Nit: while current version is not wrong IMO “Every valid address to a value of the type” reads better.
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.
Really? I think "pointer to" is fine but "address to" sounds weird to me. I chose "address" because it focuses on the numerical value of the pointer, rather than the pointer itself, which is a more abstract type.
Thanks for the review! I've addressed your comments. |
Now please squash your commits. If you don't know how to do, take a look here. |
8f58c23
to
9a4a121
Compare
Squashed and rebased to |
/// | ||
/// This effectively does nothing for | ||
/// [types which implement `Copy`](../../book/ownership.html#copy-types), | ||
/// e.g. integers. Such values are copied and _then_ moved into the function, | ||
/// so the value persists after this function call. | ||
/// | ||
/// This function is not magic; it is literally defined as | ||
/// | ||
/// ```rust,ignore |
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.
Could this be rust,no_run
instead of rust,ignore
?
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.
Just no_run
actually. :p
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.
It's actually runnable; I was thinking the name clash with the prelude would be a problem, but it just shadows I think.
These changes are great! Thanks @kmcallister! |
9a4a121
to
f97bae8
Compare
I made that last |
/// Rust drops it automatically, like at the end of a scope or after a panic. | ||
/// Running the destructor on an uninitialized value would be [undefined behavior][ub]. | ||
/// | ||
/// ```rust |
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.
No need for rust
.
f97bae8
to
72e103f
Compare
Thanks! @bors: r+ rollup |
📌 Commit 72e103f has been approved by |
⌛ Testing commit 72e103f with merge 89752d1... |
💔 Test failed - auto-mac-64-opt-rustbuild |
@bors: retry On Sat, Sep 10, 2016 at 6:00 PM, bors notifications@github.com wrote:
|
…ocs, r=GuillaumeGomez Tweak std::mem docs (rust-lang#29362) r? @steveklabnik
r? @steveklabnik