Conversation
|
@nagisa, |
|
@nagisa, |
| /// Inhibits compiler from automatically calling `T`’s destructor. | ||
| #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] | ||
| #[allow(missing_debug_implementations, unions_with_drop_fields)] | ||
| pub union ManuallyDrop<T>{ value: T } |
There was a problem hiding this comment.
You write adding following struct, but it is a union.
text/0000-manually-drop.md
Outdated
| ```rust | ||
| /// Inhibits compiler from automatically calling `T`’s destructor. | ||
| #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] | ||
| #[allow(missing_debug_implementations, unions_with_drop_fields)] |
There was a problem hiding this comment.
I think it should have impl Debug if T: Debug.
text/0000-manually-drop.md
Outdated
| // Other common impls such as `Debug for T: Debug`. | ||
| ``` | ||
|
|
||
| Let us apply this structure to the example from the motivation: |
There was a problem hiding this comment.
Replace this structure with this union
What do you mean? I’d expect
That is, I believe that large majority of code does not care about the drop order. |
The one who needs to suppress the destructor can add
struct FruitBox {
peach: ManuallyDrop<Peach>,
banana: ManuallyDrop<Banana>,
other_fruit: OtherFruit,
} |
text/0000-manually-drop.md
Outdated
| [design]: #detailed-design | ||
|
|
||
| This RFC proposes adding following `struct` to the `core::mem` (and by extension the `std::mem`) | ||
| module. `mem` module is a most suitable place for such `struct`, as the module already a place for |
There was a problem hiding this comment.
It seems odd to call this a struct. Perhaps "type" instead?
|
I feel like |
|
@Ericson2314, |
|
@KalitaAlexey Sorry, it's in a bunch of old RFCs scattered around. |
|
At random, here's newer #1646 and older https://internals.rust-lang.org/t/pre-rfc-allow-by-value-drop/1845 |
text/0000-manually-drop.md
Outdated
|
|
||
| It is proposed that such code would become idiomatic for structures where fields must be dropped in | ||
| a particular order. | ||
| It is proposed that such pattern would become idiomatic for structures where fields must be dropped |
There was a problem hiding this comment.
Perhaps: "this pattern" instead of "such pattern"? Alternates would be "such patterns" and "such a pattern", but "this" seems more natural here.
|
|
Based on discussion in the lang team meeting: this is a libs RFC, not a lang RFC. Labeling accordingly. |
|
I feel that its at least to some extent related to |
|
I wonder, was perhaps a impl<T> ManuallyDrop<T> {
pub fn new(T) -> Self;
pub fn into_inner(self) -> T;
pub unsafe fn manually_drop(slot: &mut ManuallyDrop<T>); // not inherent to avoid conflicts
}
impl<T> Deref for ManuallyDrop<T> {
type Target = T;
}
impl<T> DerefMut for ManuallyDrop<T> {} |
pub fn into_inner(self) -> T;
pub unsafe fn manually_drop(slot: &mut ManuallyDrop<T>); // not inherent to avoid conflicts
|
|
@SimonSapin yeah I was initially going to recommend that, but I hesitated because almost all Although with that in mind it may be too clever, we have |
These were never problems. It's easy to write a blanket impl from Old Drop to new Drop. Because new Drop grants the implementation a more powerful reference, plain old
I think the reason we don't have Taking a step back, I'm fine with this as a stop-gap, especially if it remains unstable until further thinking about |
|
ping @BurntSushi (checkbox) |
|
also ping @aturon |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
text/0000-manually-drop.md
Outdated
| // location – the destructor of the structure containing the fields. | ||
| // Moreover, one can now reorder fields within the struct however much they want. | ||
| peach.manually_drop(); | ||
| banana.manually_drop(); |
There was a problem hiding this comment.
This should be updated to ManuallyDrop::drop(...).
|
Is it valid to ptr::read from the deref of a ManualDrop to obtain an owned value in a Drop impl? |
|
I do not immediately see any reason you wouldn't be able to. Operation
leaves wrapper in a undefined state so you should take care to not drop or
act on the wrapper in any other way after the ptr::read
…On Mar 16, 2017 12:59 AM, "Oliver Schneider" ***@***.***> wrote:
Is it valid to ptr::read from the deref of a ManualDrop to obtain an owned
value in a Drop impl?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1860 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AApc0vYJcDm8RUWtRj6Yc0TikpHXnKU1ks5rmG1jgaJpZM4Lpopg>
.
|
|
OK. This is a use case I'd like to see mentioned in the docs and tests. Or even explicitly added as an unsafe |
|
Cross linking rust-lang/rust#40559 |
|
|
||
| /// Extracts the value from the ManuallyDrop container. | ||
| #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] | ||
| pub fn into_inner(self) -> T { |
There was a problem hiding this comment.
This should be a static method like drop.
|
I was unfortunately not tuned in to the conversation at that point, oops :( I feel somewhat strongly that it should be static - since we have an otherwise universal convention that smart pointer methods are static, if I see a |
|
That's true yeah, I'd personally be fine either way |
|
The final comment period is now complete. |
|
Looks like the main issue to arise during FCP is whether Thanks for the RFC @nagisa! |
Workaround pulldown-cmark/pulldown-cmark#124 which has broken #1860 and #1990
Rendered