Skip to content
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.

Sign up for GitHub

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

Jump to bottom

mem::forget docs: mention ManuallyDrop #66411

Merged
merged 1 commit into from
Nov 24, 2019
Merged

mem::forget docs: mention ManuallyDrop #66411

merged 1 commit into from
Nov 24, 2019
+33 −3

Conversation

RalfJung
Copy link
Member

@rust-highfive
Copy link
Contributor

r? @sfackler

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 14, 2019
@Centril
Copy link
Contributor

Centril commented Nov 14, 2019

Maybe we should deprecate mem::forget? :D

@RalfJung
Copy link
Member Author

I was wondering the same thing but I am very far from sure that ManuallyDrop is always preferable.

@SimonSapin
Copy link
Contributor

Deprecation warnings cause churn, so we should consider what we hope to gain in return.

If most maintainers who see this warning only want to make it go away and literally replace forget(x) with ManuallyDrop::new(x); (note the semicolon) which is exactly the implementation of forget these days, there’s no point.

If we want them to restructure their code to warp a value in ManuallyDrop “before transferring ownership” (which is a concept not easy to explain precisely IMO) we’d have to be very careful about how the deprecation message is phrased.

@JohnCSimon
Copy link
Member

Ping from triage
@sfackler - can you please review this PR?
Thank you!

@sfackler
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 24, 2019

📌 Commit 7009e6d has been approved by sfackler

@bors
Copy link
Collaborator

bors commented Nov 24, 2019

🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened

@bors bors 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 Nov 24, 2019
Centril added a commit to Centril/rust that referenced this pull request Nov 24, 2019
@Centril
Rollup merge of rust-lang#66411 - RalfJung:forget, r=sfackler
9b0427f 
mem::forget docs: mention ManuallyDrop

Cc @SimonSapin @Centril
@Centril Centril mentioned this pull request Nov 24, 2019
Merged
bors added a commit that referenced this pull request Nov 24, 2019
@bors
Auto merge of #66686 - Centril:rollup-07slyoo, r=Centril
e41ced3 
Rollup of 5 pull requests

Successful merges:

 - #64856 (Scope format! temporaries)
 - #66411 (mem::forget docs: mention ManuallyDrop)
 - #66594 (Fix cycle when debug-printing opaque types)
 - #66641 (parser: recover on nested ADTs as enum variants)
 - #66659 (make `./x.py bench` again)

Failed merges:

r? @ghost
@bors bors merged commit 7009e6d into rust-lang:master Nov 24, 2019
@RalfJung RalfJung deleted the forget branch November 27, 2019 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sfackler sfackler

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@RalfJung @rust-highfive @Centril @SimonSapin @JohnCSimon @sfackler @bors