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

Tracking issue for ManuallyDrop #40673

Closed
alexcrichton opened this issue Mar 20, 2017 · 17 comments
Closed

Tracking issue for ManuallyDrop #40673

alexcrichton opened this issue Mar 20, 2017 · 17 comments
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

Tracking issue for rust-lang/rfcs#1860

I believe initially implemented at #40559
@alexcrichton alexcrichton added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 20, 2017
@Ericson2314
Copy link
Contributor

Ericson2314 commented Mar 20, 2017 •
edited
Loading

I left a comment rust-lang/rfcs#1860 (comment) which slipped by FCP. The tl;dr is: this is fine for now in my book, but we should consider &move and improved Drop again prior to stabilization. I'd hope the popularity of this feature would be considered a +1 &move---a wholly nicer, if harder-to-implement, solution---rather than a -1 (with the reasoning that this trick is good enough so &move isn't needed).

@chriskrycho chriskrycho mentioned this issue Apr 1, 2017
Closed
48 tasks
@SimonSapin
Copy link
Contributor

I’d like this to be stabilized. What’s the next step?

The only alternative that I know of is https://crates.io/crates/nodrop, which on stable Rust requires a combination of hacks that look fragile to me.

@crumblingstatue
Copy link
Contributor

crumblingstatue commented May 24, 2017 •
edited
Loading

I am personally using this in a project, so I'm in favor of stabilizing a solution for specifying drop order, but I also don't want @Ericson2314's comment ignored before stabilizing this.

@sfackler
Copy link
Member

I don't believe &move or a new Drop are anywhere near the horizon. It seems unreasonable to hold this API up indefinitely.

@rfcbot fcp merge

@sfackler sfackler added B-unstable Blocker: Implemented in the nightly compiler and unstable. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. and removed B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. labels May 31, 2017
@rfcbot
Copy link

rfcbot commented May 31, 2017 •
edited by brson
Loading

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@dtolnay
Copy link
Member

dtolnay commented Jun 7, 2017

I filed rust-lang/api-guidelines#93 to follow up with a justification of why we prefer into_inner(ManuallyDrop<T>) over into_inner(self).

@rfcbot reviewed

@Ericson2314
Copy link
Contributor

Fair enough, this can always be deprecated.

@SimonSapin
Copy link
Contributor

@brson I think this is waiting on you :)

@rfcbot
Copy link

rfcbot commented Jul 8, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jul 8, 2017
@rfcbot
Copy link

rfcbot commented Jul 18, 2017

The final comment period is now complete.

@Gankra
Copy link
Contributor

Gankra commented Jul 20, 2017

Hey nice I was about to propose stabilizing this. 🎉

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
@bors bors closed this as completed in daeb607 Jul 27, 2017
@bluss
Copy link
Member

bluss commented Aug 5, 2017

Sorry if I have missed this somewhere — do we have a decision for how layout optimization handles things like Option<ManuallyDrop<&i32>>? I can see that right now, the inner value is not used for the discriminant.

@SimonSapin
Copy link
Contributor

In the case of struct ArrayVec<A: Array> { length: usize, data: ManuallyDrop<A> } impl<T, N: usize> Array for [T; N] {…} (assuming integer generics, or with a macro to generate impls for a number of specific N values), it is important that e.g. Option<ArrayVec<[&i32; 16]>> does not use the nonzeroness of &i32 to represent the discriminant of Option because the &i32 items might be uninitialized.

Or, in other words, ArrayVec needs some way to inhibit propagation of enum layout optimizations, and ManuallyDrop could be it.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 12, 2017
@alexcrichton
std: Stabilize manually_drop feature
7e22e24 
Stabilizes

* `core::mem::ManuallyDrop`
* `std::mem::ManuallyDrop`
* `ManuallyDrop::new`
* `ManuallyDrop::into_inner`
* `ManuallyDrop::drop`
* `Deref for ManuallyDrop`
* `DerefMut for ManuallyDrop`

Closes rust-lang#40673
@ldr709
Copy link
Contributor

ldr709 commented Aug 27, 2017

This is probably neither the right time nor place for this, but any plans to impl Copy? It may seem odd to do so, since Copy types can be dropped trivially, but this would be useful in situations where you don't know whether or not a type impls Copy.

@Gankra
Copy link
Contributor

Gankra commented Aug 28, 2017

You can submit a PR to derive(Copy) and it will likely be accepted.

@kjetilkjeka
Copy link
Contributor

When testing it seems like impl<T: Clone> for ManuallyDrop<T> works on nightly/beta, but not on stable. I can't seem to find the PR/issue tracking this, does anyone have a link? Do anyone know if this will this land for 1.21?

this is the test I'm referring to.

@ldr709
Copy link
Contributor

ldr709 commented Nov 16, 2017

@kjetilkjeka Here is the PR. Rust 1.21 has already been released and it doesn't have it. According to this if it is in beta now it should be in 1.22 (maybe a few weeks?), but I don't know much about it.

@dtolnay dtolnay mentioned this issue Nov 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests