From 9f0172f8fddf94546ad6e919a90fc36d651df629 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Fri, 20 Jan 2017 20:37:00 +0200 Subject: [PATCH 1/8] Manually drop --- text/0000-manually-drop.md | 161 +++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 text/0000-manually-drop.md diff --git a/text/0000-manually-drop.md b/text/0000-manually-drop.md new file mode 100644 index 00000000000..c4a4213f6c5 --- /dev/null +++ b/text/0000-manually-drop.md @@ -0,0 +1,161 @@ +- Feature Name: manually_drop +- Start Date: 2017-01-20 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Include the `ManuallyDrop` wrapper in `core::mem`. + +# Motivation +[motivation]: #motivation + +Currently Rust does not specify the order in which the destructors are run. Furthermore, this order +differs depending on context. RFC issue [#744](https://github.com/rust-lang/rfcs/issues/744) +exposed the fact that the current, but unspecified behaviour is relied onto for code validity and +that there’s at least a few instances of such code in the wild. + +While a move to stabilise and document the order of destructor evaluation would technically fix the +problem describe above, there’s another important aspect to consider here – implicitness. Consider +such code: + +```rust +struct FruitBox { + peach: Peach, + banana: Banana, +} +``` + +Does this structure depend on `Peach`’s destructor being run before `Banana` for correctness? +Perhaps its the other way around and it is `Banana`’s destructor that has to run first? In the +common case structures do not have any such dependencies between fields, and therefore it is easy +to overlook such a dependency while changing the code above to the snippet below (e.g. so the +fields are sorted by name). + +```rust +struct FruitBox { + banana: Banana, + peach: Peach, +} +``` + +For structures with dependencies between fields it is worthwhile to have ability to explicitly +annotate the dependencies somehow. + +# Detailed design +[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 +functions very similar in purpose: `drop` and `forget`. + +```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)] +pub union ManuallyDrop{ value: T } + +impl ManuallyDrop { + /// Wraps a value to be manually dropped. + #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] + pub fn new(value: T) -> ManuallyDrop { + ManuallyDrop { value: value } + } + + /// Extracts the value from the ManuallyDrop container. + #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] + pub fn into_inner(self) -> T { + unsafe { + self.value + } + } + + /// Extracts the value from the ManuallyDrop container. + /// Could also be implemented as Deref. + #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] + pub fn as_ref(&self) -> &T { + unsafe { + &self.value + } + } + + /// Extracts the value from the ManuallyDrop container. + /// Could also be implemented as a DerefMut. + #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] + pub fn as_mut(&mut self) -> &mut T { + unsafe { + &mut self.value + } + } + + /// Manually drops the contained value. + /// + /// # Unsafety + /// + /// This function runs the destructor of the contained value and thus makes any further action + /// with the value within invalid. The fact that this function does not consume the wrapper + /// does not statically prevent further reuse. + #[unstable(feature = "manually_drop", reason = "recently added", issue = "0")] + pub unsafe fn manually_drop(&mut self) { + ptr::drop_in_place(&mut self.value) + } +} +``` + +Let us apply this structure to the example from the motivation: + +```rust +struct FruitBox { + // Immediately clear there’s something non-trivial going on with these fields. + peach: ManuallyDrop, + banana: ManuallyDrop, +} + +impl Drop for FruitBox { + fn drop(&mut self) { + unsafe { + // Explicit ordering in which field destructors are run specified in the intuitive + // 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(); + } + } +} +``` + +It is proposed that such code would become idiomatic for structures where fields must be dropped in +a particular order. + +# How We Teach This +[how-we-teach-this]: #how-we-teach-this + +It is expected that the functions and wrapper added as a result of this RFC would be seldom +necessary. + +In addition to the usual API documentation, this structure should be mentioned in +reference/nomicon/elsewhere where drop ordering guarantees are described as a way to explicitly +control the order in which the structure fields gets dropped. + + + +# Alternatives +[alternatives]: #alternatives + +* Stabilise some sort of drop order and make people to write code that’s hard to figure out at a +glance; +* Bikeshed colour; +* Stabilise union and let people implement this themselves: + * Precludes (or makes it much harder) from recommending this pattern as the idiomatic way to + implement destructors with dependencies. + +# Unresolved questions +[unresolved]: #unresolved-questions + +* Best way to expose value inside the wrapper. From b9674db3e0a7584078d806d6f3b99eedb93f89dc Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Fri, 20 Jan 2017 21:56:25 +0200 Subject: [PATCH 2/8] Nits --- text/0000-manually-drop.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/text/0000-manually-drop.md b/text/0000-manually-drop.md index c4a4213f6c5..18ba2a2c87c 100644 --- a/text/0000-manually-drop.md +++ b/text/0000-manually-drop.md @@ -46,14 +46,14 @@ annotate the dependencies somehow. # Detailed design [design]: #detailed-design -This RFC proposes adding following `struct` to the `core::mem` (and by extension the `std::mem`) +This RFC proposes adding following `union` 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 functions very similar in purpose: `drop` and `forget`. ```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)] +#[allow(unions_with_drop_fields)] pub union ManuallyDrop{ value: T } impl ManuallyDrop { @@ -101,6 +101,8 @@ impl ManuallyDrop { ptr::drop_in_place(&mut self.value) } } + +// Other common impls such as `Debug for T: Debug`. ``` Let us apply this structure to the example from the motivation: From 89270821881e0ea05b2ea94281c716fc7496129a Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Fri, 20 Jan 2017 22:09:50 +0200 Subject: [PATCH 3/8] More nits --- text/0000-manually-drop.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/text/0000-manually-drop.md b/text/0000-manually-drop.md index 18ba2a2c87c..1d7d65deeb8 100644 --- a/text/0000-manually-drop.md +++ b/text/0000-manually-drop.md @@ -105,7 +105,7 @@ impl ManuallyDrop { // Other common impls such as `Debug for T: Debug`. ``` -Let us apply this structure to the example from the motivation: +Let us apply this union to the example from the motivation: ```rust struct FruitBox { @@ -127,8 +127,8 @@ impl Drop for FruitBox { } ``` -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 +in a particular order. # How We Teach This [how-we-teach-this]: #how-we-teach-this @@ -136,9 +136,9 @@ a particular order. It is expected that the functions and wrapper added as a result of this RFC would be seldom necessary. -In addition to the usual API documentation, this structure should be mentioned in -reference/nomicon/elsewhere where drop ordering guarantees are described as a way to explicitly -control the order in which the structure fields gets dropped. +In addition to the usual API documentation, `ManuallyDrop` should be mentioned in +reference/nomicon/elsewhere as the solution to the desire of explicit control of the order in which +the structure fields gets dropped.