Skip to content

Don't recommend ManuallyDrop to customize drop order - #76150

Merged
bors merged 1 commit into
rust-lang:masterfrom
matklad:droporder
Sep 22, 2020
Merged

Don't recommend ManuallyDrop to customize drop order#76150
bors merged 1 commit into
rust-lang:masterfrom
matklad:droporder

Conversation

@matklad

Copy link
Copy Markdown
Contributor

See
https://internals.rust-lang.org/t/need-for-controlling-drop-order-of-fields/12914/21
for the discussion.

TL;DR: ManuallyDrop is unsafe and footguny, but you can just ask the compiler to do all the work for you by re-ordering declarations.

Specifically, the original example from the docs is much better written as

structPeach;structBanana;structMelon;structFruitBox{melon:Melon,// XXX: mind the relative drop order of the fields belowpeach:Peach,banana:Banana,}

@matkladmatklad added the T-libs-api [DEPRECATED; DO NOT USE] label Aug 31, 2020
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2020
@crlf0710

Copy link
Copy Markdown
Member

r? @dtolnay

@withoutboats

Copy link
Copy Markdown
Contributor

I agree with this change to the guideline. I don't know if these docs date to before we had agreed to define the drop order or not.

I would like the documentation to briefly explain the defined drop order of struct fields, though, and maybe the code sample using that to get a certain drop order. I know its contained in the associated link, but it would make the documentation clearer here.

See
https://internals.rust-lang.org/t/need-for-controlling-drop-order-of-fields/12914/21
for the discussion.
TL;DR: ManuallyDrop is unsafe and footguny, but you can just ask the
compiler to do all the work for you by re-ordering declarations.
@matklad

Copy link
Copy Markdown
ContributorAuthor

Yeah, that would be helpful, included an example!

@RalfJung

Copy link
Copy Markdown
Member

r=me unless you want another review.

@withoutboats

Copy link
Copy Markdown
Contributor

@bors r+ looks good!

@bors

bors commented Sep 21, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 60b102d has been approved by withoutboats

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 21, 2020
@ecstatic-morse

Copy link
Copy Markdown
Contributor

@bors rollup

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 22, 2020
Don't recommend ManuallyDrop to customize drop order
See
https://internals.rust-lang.org/t/need-for-controlling-drop-order-of-fields/12914/21
for the discussion.
TL;DR: ManuallyDrop is unsafe and footguny, but you can just ask the compiler to do all the work for you by re-ordering declarations.
Specifically, the original example from the docs is much better written as
```rust
struct Peach;
struct Banana;
struct Melon;
struct FruitBox {
melon: Melon,
// XXX: mind the relative drop order of the fields below
peach: Peach,
banana: Banana,
}
```
ecstatic-morse added a commit to ecstatic-morse/rust that referenced this pull request Sep 22, 2020
Don't recommend ManuallyDrop to customize drop order
See
https://internals.rust-lang.org/t/need-for-controlling-drop-order-of-fields/12914/21
for the discussion.
TL;DR: ManuallyDrop is unsafe and footguny, but you can just ask the compiler to do all the work for you by re-ordering declarations.
Specifically, the original example from the docs is much better written as
```rust
struct Peach;
struct Banana;
struct Melon;
struct FruitBox {
melon: Melon,
// XXX: mind the relative drop order of the fields below
peach: Peach,
banana: Banana,
}
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 22, 2020
…atic-morse
Rollup of 13 pull requests
Successful merges:
- rust-lang#72734 (Reduce duplicate in liballoc reserve error handling)
- rust-lang#76131 (Don't use `zip` to compare iterators during pretty-print hack)
- rust-lang#76150 (Don't recommend ManuallyDrop to customize drop order)
- rust-lang#76275 (Implementation of Write for some immutable ref structs)
- rust-lang#76489 (Add explanation for E0756)
- rust-lang#76581 (do not ICE on bound variables, return `TooGeneric` instead)
- rust-lang#76655 (Make some methods of `Pin` unstable const)
- rust-lang#76783 (Only get ImplKind::Impl once)
- rust-lang#76807 (Use const-checking to forbid use of unstable features in const-stable functions)
- rust-lang#76888 (use if let instead of single match arm expressions)
- rust-lang#76914 (extend `Ty` and `TyCtxt` lints to self types)
- rust-lang#77022 (Reduce boilerplate for BytePos and CharPos)
- rust-lang#77032 (lint missing docs for extern items)
Failed merges:
r? `@ghost`
@bors
bors merged commit 4f3697b into rust-lang:masterSep 22, 2020
@rustbotrustbot added this to the 1.48.0 milestone Sep 22, 2020
@matklad
matklad deleted the droporder branch September 25, 2020 07:48
@matklad
matklad restored the droporder branch September 25, 2020 07:48
@matklad
matklad deleted the droporder branch September 25, 2020 07:48
@vojtechkral

Copy link
Copy Markdown
Contributor

Reminder: Rustonomicon (still?) advises exactly the other way around 🙂

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-libs-api[DEPRECATED; DO NOT USE]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@matklad@rust-highfive@crlf0710@withoutboats@RalfJung@bors@ecstatic-morse@vojtechkral@dtolnay@rustbot