Improve the documentation for ManuallyDrop to resolve conflicting usage of terminology #71625
Conversation
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Diggsey
force-pushed
the
improve-manually-drop-docs
branch
from
24cfd0c
to
904a77c
Compare
Diggsey
force-pushed
the
improve-manually-drop-docs
branch
2 times, most recently
from
72bdab5
to
34c5e0c
Compare
Diggsey
force-pushed
the
improve-manually-drop-docs
branch
from
34c5e0c
to
241ab21
Compare
src/libcore/ptr/mod.rs
Outdated
| @@ -112,11 +112,17 @@ mod mut_ptr; | |||
| /// as the compiler doesn't need to prove that it's sound to elide the | |||
| /// copy. | |||
| /// | |||
| /// * It can be used to drop [pinned] data (pinned data must not be moved | |||
| /// before it is dropped). | |||
There was a problem hiding this comment.
| /// before it is dropped). | |
| /// before it is dropped), except if `T` is `repr(packed). |
I feel currently this reads a bit like a contradiction, where it says "can be used to drop pinned data (because in-place)" but later says "but fields of packed structs are not dropped in-place".
There was a problem hiding this comment.
AIUI, packed structs with destructors cannot be pinned to begin with, and packed structs without destructors are presumably safe to drop using this function anyway?
There was a problem hiding this comment.
They cannot be pinned precisely because of what this comment says. If someone tries anyway, this is the place where they are violating the rules. So this is the place where that should be mentioned.
The compiler won't stop you when you try to pin something packed.
There was a problem hiding this comment.
Fixed. I put the caveat before the part in brackets because I felt it was awkward to read otherwise.
|
r? @RalfJung |
RalfJung
added
S-waiting-on-author
Diggsey
force-pushed
the
improve-manually-drop-docs
branch
from
241ab21
to
d8a3e59
Compare
…ge of terminology.
Diggsey
force-pushed
the
improve-manually-drop-docs
branch
from
d8a3e59
to
7433c4d
Compare
|
This looks great, thanks a lot. :) |
|
📌 Commit 7433c4d has been approved by |
bors
added
S-waiting-on-bors
RalfJung
changed the title
Rollup of 7 pull requests Successful merges: - rust-lang#71625 (Improve the documentation for ManuallyDrop to resolve conflicting usage of terminology) - rust-lang#71919 (Update transitive dependency to work towards removing syn <1.0 dep) - rust-lang#72166 (Simpler slice `Iterator` methods) - rust-lang#72216 (Remove `lang_items\(\).*\.unwrap\(\)`) - rust-lang#72230 (Updated documentation of Prefix::VerbatimDisk) - rust-lang#72234 (Implement Default for proc_macro::TokenStream) - rust-lang#72258 (Fix typo Arbintrary to Arbitrary) Failed merges: r? @ghost
cc @RalfJung
Follow-up from rust-lang/unsafe-code-guidelines#233