From f6e566185eaa4675cf2791ee69e63eb20ea01edb Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 22 Mar 2017 23:35:09 +0200 Subject: [PATCH 1/5] Implement Manually Drop --- src/libcore/mem.rs | 96 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index f5cf3724d0711..f4a19af02a661 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -736,3 +736,99 @@ pub fn discriminant(v: &T) -> Discriminant { } } + +/// A wrapper to inhibit compiler from automatically calling `T`’s destructor. +/// +/// This wrapper is 0-cost. +/// +/// # Examples +/// +/// This wrapper helps with explicitly documenting the drop order dependencies between fields of +/// the type: +/// +/// ```rust +/// # #![feature(manually_drop)] +/// use std::mem::ManuallyDrop; +/// struct Peach; +/// struct Banana; +/// struct Melon; +/// struct FruitBox { +/// // Immediately clear there’s something non-trivial going on with these fields. +/// peach: ManuallyDrop, +/// melon: Melon, // Field that’s independent of the other two. +/// 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. +/// ManuallyDrop::drop(&mut self.peach); +/// ManuallyDrop::drop(&mut self.banana); +/// } +/// // After destructor for `FruitBox` runs (this function), the destructor for Melon gets +/// // invoked in the usual manner, as it is not wrapped in `ManuallyDrop`. +/// } +/// } +/// ``` +#[unstable(feature = "manually_drop", issue = "40673")] +#[allow(unions_with_drop_fields)] +pub union ManuallyDrop{ value: T } + +impl ManuallyDrop { + /// Wrap a value to be manually dropped. + #[unstable(feature = "manually_drop", issue = "40673")] + pub fn new(value: T) -> ManuallyDrop { + ManuallyDrop { value: value } + } + + /// Extract the value from the ManuallyDrop container. + #[unstable(feature = "manually_drop", issue = "40673")] + pub fn into_inner(self) -> T { + unsafe { + self.value + } + } + + /// Manually drops the contained value. + /// + /// # Unsafety + /// + /// This function runs the destructor of the contained value and thus the wrapped value + /// now represents uninitialized data. It is up to the user of this method to ensure the + /// uninitialized data is not actually used. + #[unstable(feature = "manually_drop", issue = "40673")] + pub unsafe fn drop(slot: &mut ManuallyDrop) { + ptr::drop_in_place(&mut slot.value) + } +} + +#[unstable(feature = "manually_drop", issue = "40673")] +impl ::ops::Deref for ManuallyDrop { + type Target = T; + fn deref(&self) -> &Self::Target { + unsafe { + &self.value + } + } +} + +#[unstable(feature = "manually_drop", issue = "40673")] +impl ::ops::DerefMut for ManuallyDrop { + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { + &mut self.value + } + } +} + +#[unstable(feature = "manually_drop", issue = "40673")] +impl ::fmt::Debug for ManuallyDrop { + fn fmt(&self, fmt: &mut ::fmt::Formatter) -> ::fmt::Result { + unsafe { + fmt.debug_tuple("ManuallyDrop").field(&self.value).finish() + } + } +} From 38713126dd8502e283aa8ec7c6b678a4c43f8c3b Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 22 Mar 2017 19:11:51 +0200 Subject: [PATCH 2/5] Move away from the ad-hoc NoDrop unions --- src/libcollections/lib.rs | 1 + src/libcollections/slice.rs | 12 ++------ src/libcore/slice/sort.rs | 30 ++++++++---------- src/librustc_data_structures/array_vec.rs | 37 +++++------------------ src/librustc_data_structures/lib.rs | 1 + 5 files changed, 25 insertions(+), 56 deletions(-) diff --git a/src/libcollections/lib.rs b/src/libcollections/lib.rs index 248c15e96f8f6..b474bd5d91af1 100644 --- a/src/libcollections/lib.rs +++ b/src/libcollections/lib.rs @@ -44,6 +44,7 @@ #![feature(heap_api)] #![feature(inclusive_range)] #![feature(lang_items)] +#![feature(manually_drop)] #![feature(nonzero)] #![feature(pattern)] #![feature(placement_in)] diff --git a/src/libcollections/slice.rs b/src/libcollections/slice.rs index 6cff315a6ccd9..3069adb12e92c 100644 --- a/src/libcollections/slice.rs +++ b/src/libcollections/slice.rs @@ -1558,7 +1558,7 @@ fn insert_head(v: &mut [T], is_less: &mut F) // performance than with the 2nd method. // // All methods were benchmarked, and the 3rd showed best results. So we chose that one. - let mut tmp = NoDrop { value: ptr::read(&v[0]) }; + let mut tmp = mem::ManuallyDrop::new(ptr::read(&v[0])); // Intermediate state of the insertion process is always tracked by `hole`, which // serves two purposes: @@ -1571,13 +1571,13 @@ fn insert_head(v: &mut [T], is_less: &mut F) // fill the hole in `v` with `tmp`, thus ensuring that `v` still holds every object it // initially held exactly once. let mut hole = InsertionHole { - src: &mut tmp.value, + src: &mut *tmp, dest: &mut v[1], }; ptr::copy_nonoverlapping(&v[1], &mut v[0], 1); for i in 2..v.len() { - if !is_less(&v[i], &tmp.value) { + if !is_less(&v[i], &*tmp) { break; } ptr::copy_nonoverlapping(&v[i], &mut v[i - 1], 1); @@ -1587,12 +1587,6 @@ fn insert_head(v: &mut [T], is_less: &mut F) } } - // Holds a value, but never drops it. - #[allow(unions_with_drop_fields)] - union NoDrop { - value: T - } - // When dropped, copies from `src` into `dest`. struct InsertionHole { src: *mut T, diff --git a/src/libcore/slice/sort.rs b/src/libcore/slice/sort.rs index 7065fdb79fc40..6f9f2915dfe10 100644 --- a/src/libcore/slice/sort.rs +++ b/src/libcore/slice/sort.rs @@ -20,12 +20,6 @@ use cmp; use mem; use ptr; -/// Holds a value, but never drops it. -#[allow(unions_with_drop_fields)] -union NoDrop { - value: T -} - /// When dropped, copies from `src` into `dest`. struct CopyOnDrop { src: *mut T, @@ -49,15 +43,15 @@ fn shift_head(v: &mut [T], is_less: &mut F) // Read the first element into a stack-allocated variable. If a following comparison // operation panics, `hole` will get dropped and automatically write the element back // into the slice. - let mut tmp = NoDrop { value: ptr::read(v.get_unchecked(0)) }; + let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(0))); let mut hole = CopyOnDrop { - src: &mut tmp.value, + src: &mut *tmp, dest: v.get_unchecked_mut(1), }; ptr::copy_nonoverlapping(v.get_unchecked(1), v.get_unchecked_mut(0), 1); for i in 2..len { - if !is_less(v.get_unchecked(i), &tmp.value) { + if !is_less(v.get_unchecked(i), &*tmp) { break; } @@ -81,15 +75,15 @@ fn shift_tail(v: &mut [T], is_less: &mut F) // Read the last element into a stack-allocated variable. If a following comparison // operation panics, `hole` will get dropped and automatically write the element back // into the slice. - let mut tmp = NoDrop { value: ptr::read(v.get_unchecked(len - 1)) }; + let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(len - 1))); let mut hole = CopyOnDrop { - src: &mut tmp.value, + src: &mut *tmp, dest: v.get_unchecked_mut(len - 2), }; ptr::copy_nonoverlapping(v.get_unchecked(len - 2), v.get_unchecked_mut(len - 1), 1); for i in (0..len-2).rev() { - if !is_less(&tmp.value, v.get_unchecked(i)) { + if !is_less(&*tmp, v.get_unchecked(i)) { break; } @@ -403,12 +397,12 @@ fn partition(v: &mut [T], pivot: usize, is_less: &mut F) -> (usize, bool) // Read the pivot into a stack-allocated variable for efficiency. If a following comparison // operation panics, the pivot will be automatically written back into the slice. - let mut tmp = NoDrop { value: unsafe { ptr::read(pivot) } }; + let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) }); let _pivot_guard = CopyOnDrop { - src: unsafe { &mut tmp.value }, + src: &mut *tmp, dest: pivot, }; - let pivot = unsafe { &tmp.value }; + let pivot = &*tmp; // Find the first pair of out-of-order elements. let mut l = 0; @@ -452,12 +446,12 @@ fn partition_equal(v: &mut [T], pivot: usize, is_less: &mut F) -> usize // Read the pivot into a stack-allocated variable for efficiency. If a following comparison // operation panics, the pivot will be automatically written back into the slice. - let mut tmp = NoDrop { value: unsafe { ptr::read(pivot) } }; + let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) }); let _pivot_guard = CopyOnDrop { - src: unsafe { &mut tmp.value }, + src: &mut *tmp, dest: pivot, }; - let pivot = unsafe { &tmp.value }; + let pivot = &*tmp; // Now partition the slice. let mut l = 0; diff --git a/src/librustc_data_structures/array_vec.rs b/src/librustc_data_structures/array_vec.rs index 29fbcb70756ba..adb2219722602 100644 --- a/src/librustc_data_structures/array_vec.rs +++ b/src/librustc_data_structures/array_vec.rs @@ -20,10 +20,11 @@ use std::fmt; use std::mem; use std::collections::range::RangeArgument; use std::collections::Bound::{Excluded, Included, Unbounded}; +use std::mem::ManuallyDrop; pub unsafe trait Array { type Element; - type PartialStorage: Default + Unsize<[ManuallyDrop]>; + type PartialStorage: Unsize<[ManuallyDrop]>; const LEN: usize; } @@ -66,7 +67,7 @@ impl ArrayVec { pub fn new() -> Self { ArrayVec { count: 0, - values: Default::default(), + values: unsafe { ::std::mem::uninitialized() }, } } @@ -81,7 +82,7 @@ impl ArrayVec { /// Panics when the stack vector is full. pub fn push(&mut self, el: A::Element) { let arr = &mut self.values as &mut [ManuallyDrop<_>]; - arr[self.count] = ManuallyDrop { value: el }; + arr[self.count] = ManuallyDrop::new(el); self.count += 1; } @@ -90,8 +91,8 @@ impl ArrayVec { let arr = &mut self.values as &mut [ManuallyDrop<_>]; self.count -= 1; unsafe { - let value = ptr::read(&arr[self.count]); - Some(value.value) + let value = ptr::read(&*arr[self.count]); + Some(value) } } else { None @@ -210,7 +211,7 @@ impl Iterator for Iter { fn next(&mut self) -> Option { let arr = &self.store as &[ManuallyDrop<_>]; unsafe { - self.indices.next().map(|i| ptr::read(&arr[i]).value) + self.indices.next().map(|i| ptr::read(&*arr[i])) } } @@ -233,7 +234,7 @@ impl<'a, A: Array> Iterator for Drain<'a, A> { #[inline] fn next(&mut self) -> Option { - self.iter.next().map(|elt| unsafe { ptr::read(elt as *const ManuallyDrop<_>).value }) + self.iter.next().map(|elt| unsafe { ptr::read(&**elt) }) } fn size_hint(&self) -> (usize, Option) { @@ -295,25 +296,3 @@ impl<'a, A: Array> IntoIterator for &'a mut ArrayVec { self.iter_mut() } } - -// FIXME: This should use repr(transparent) from rust-lang/rfcs#1758. -#[allow(unions_with_drop_fields)] -pub union ManuallyDrop { - value: T, - #[allow(dead_code)] - empty: (), -} - -impl ManuallyDrop { - fn new() -> ManuallyDrop { - ManuallyDrop { - empty: () - } - } -} - -impl Default for ManuallyDrop { - fn default() -> Self { - ManuallyDrop::new() - } -} diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index c1735b4a4ec9a..72c533a74618b 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -39,6 +39,7 @@ #![feature(conservative_impl_trait)] #![feature(discriminant_value)] #![feature(specialization)] +#![feature(manually_drop)] #![cfg_attr(unix, feature(libc))] #![cfg_attr(test, feature(test))] From c94b3f1266779c595e28111e39f47e5e14a34ef2 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sun, 2 Apr 2017 11:13:31 +0300 Subject: [PATCH 3/5] Replace the `forget` intrinsic with ManuallyDrop less intrinsics = better life --- src/libcore/intrinsics.rs | 3 --- src/libcore/mem.rs | 7 ++++++- src/librustc_trans/intrinsic.rs | 2 +- src/librustc_typeck/check/intrinsic.rs | 1 - 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index e0a4707ff665f..b028763158512 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -691,9 +691,6 @@ extern "rust-intrinsic" { /// initialize memory previous set to the result of `uninit`. pub fn uninit() -> T; - /// Moves a value out of scope without running drop glue. - pub fn forget(_: T) -> (); - /// Reinterprets the bits of a value of one type as another type. /// /// Both types must have the same size. Neither the original, nor the result, diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index f4a19af02a661..52ccaa417bc5f 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -171,7 +171,7 @@ pub use intrinsics::transmute; #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn forget(t: T) { - unsafe { intrinsics::forget(t) } + ManuallyDrop::new(t); } /// Returns the size of a type in bytes. @@ -780,12 +780,14 @@ pub union ManuallyDrop{ value: T } impl ManuallyDrop { /// Wrap a value to be manually dropped. #[unstable(feature = "manually_drop", issue = "40673")] + #[inline] pub fn new(value: T) -> ManuallyDrop { ManuallyDrop { value: value } } /// Extract the value from the ManuallyDrop container. #[unstable(feature = "manually_drop", issue = "40673")] + #[inline] pub fn into_inner(self) -> T { unsafe { self.value @@ -800,6 +802,7 @@ impl ManuallyDrop { /// now represents uninitialized data. It is up to the user of this method to ensure the /// uninitialized data is not actually used. #[unstable(feature = "manually_drop", issue = "40673")] + #[inline] pub unsafe fn drop(slot: &mut ManuallyDrop) { ptr::drop_in_place(&mut slot.value) } @@ -808,6 +811,7 @@ impl ManuallyDrop { #[unstable(feature = "manually_drop", issue = "40673")] impl ::ops::Deref for ManuallyDrop { type Target = T; + #[inline] fn deref(&self) -> &Self::Target { unsafe { &self.value @@ -817,6 +821,7 @@ impl ::ops::Deref for ManuallyDrop { #[unstable(feature = "manually_drop", issue = "40673")] impl ::ops::DerefMut for ManuallyDrop { + #[inline] fn deref_mut(&mut self) -> &mut Self::Target { unsafe { &mut self.value diff --git a/src/librustc_trans/intrinsic.rs b/src/librustc_trans/intrinsic.rs index 5e7d612d17f82..0cbc103994ad9 100644 --- a/src/librustc_trans/intrinsic.rs +++ b/src/librustc_trans/intrinsic.rs @@ -188,7 +188,7 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>, C_nil(ccx) } // Effectively no-ops - "uninit" | "forget" => { + "uninit" => { C_nil(ccx) } "needs_drop" => { diff --git a/src/librustc_typeck/check/intrinsic.rs b/src/librustc_typeck/check/intrinsic.rs index 2861fd288326b..cd58fcd4806da 100644 --- a/src/librustc_typeck/check/intrinsic.rs +++ b/src/librustc_typeck/check/intrinsic.rs @@ -124,7 +124,6 @@ pub fn check_intrinsic_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, "rustc_peek" => (1, vec![param(0)], param(0)), "init" => (1, Vec::new(), param(0)), "uninit" => (1, Vec::new(), param(0)), - "forget" => (1, vec![ param(0) ], tcx.mk_nil()), "transmute" => (2, vec![ param(0) ], param(1)), "move_val_init" => { (1, From 486345551c09612aa87155304a20e6b8de492939 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sun, 2 Apr 2017 11:15:59 +0300 Subject: [PATCH 4/5] into_inner to associated function --- src/libcore/mem.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 52ccaa417bc5f..7be927b28ed7e 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -779,6 +779,14 @@ pub union ManuallyDrop{ value: T } impl ManuallyDrop { /// Wrap a value to be manually dropped. + /// + /// # Examples + /// + /// ```rust + /// # #![feature(manually_drop)] + /// use std::mem::ManuallyDrop; + /// ManuallyDrop::new(Box::new(())); + /// ``` #[unstable(feature = "manually_drop", issue = "40673")] #[inline] pub fn new(value: T) -> ManuallyDrop { @@ -786,11 +794,20 @@ impl ManuallyDrop { } /// Extract the value from the ManuallyDrop container. + /// + /// # Examples + /// + /// ```rust + /// # #![feature(manually_drop)] + /// use std::mem::ManuallyDrop; + /// let x = ManuallyDrop::new(Box::new(())); + /// let _: Box<()> = ManuallyDrop::into_inner(x); + /// ``` #[unstable(feature = "manually_drop", issue = "40673")] #[inline] - pub fn into_inner(self) -> T { + pub fn into_inner(slot: ManuallyDrop) -> T { unsafe { - self.value + slot.value } } From c337b99f4cb0968481a03195d25358b484d7db22 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sun, 2 Apr 2017 11:16:17 +0300 Subject: [PATCH 5/5] Fix test failures --- src/doc/unstable-book/src/SUMMARY.md | 1 + src/doc/unstable-book/src/manually-drop.md | 0 src/test/compile-fail/forget-init-unsafe.rs | 3 +-- 3 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 src/doc/unstable-book/src/manually-drop.md diff --git a/src/doc/unstable-book/src/SUMMARY.md b/src/doc/unstable-book/src/SUMMARY.md index d86766bac02a2..90499f3ce8590 100644 --- a/src/doc/unstable-book/src/SUMMARY.md +++ b/src/doc/unstable-book/src/SUMMARY.md @@ -113,6 +113,7 @@ - [loop_break_value](loop-break-value.md) - [macro_reexport](macro-reexport.md) - [main](main.md) +- [manually_drop](manually-drop.md) - [map_entry_recover_keys](map-entry-recover-keys.md) - [mpsc_select](mpsc-select.md) - [n16](n16.md) diff --git a/src/doc/unstable-book/src/manually-drop.md b/src/doc/unstable-book/src/manually-drop.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/src/test/compile-fail/forget-init-unsafe.rs b/src/test/compile-fail/forget-init-unsafe.rs index 521f122f8af0b..48c9fda31e8c8 100644 --- a/src/test/compile-fail/forget-init-unsafe.rs +++ b/src/test/compile-fail/forget-init-unsafe.rs @@ -10,10 +10,9 @@ #![feature(core_intrinsics)] -use std::intrinsics::{init, forget}; +use std::intrinsics::{init}; // Test that the `forget` and `init` intrinsics are really unsafe pub fn main() { let stuff = init::(); //~ ERROR call to unsafe function requires unsafe - forget(stuff); //~ ERROR call to unsafe function requires unsafe }