From 75e471ade9ea6095eef6cd4ef35cfc0b8bfc9410 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 8 Sep 2020 19:01:05 +0200 Subject: [PATCH 1/6] Add MaybeUninit::drop. ManuallyDrop's documentation tells the user to use MaybeUninit instead when handling uninitialized data. However, the main functionality of ManuallyDrop (drop) was not available directly on MaybeUninit. Adding it makes it easier to switch from one to the other. --- library/core/src/mem/maybe_uninit.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index b64abf68c5e4a..e826ec0a5e3ee 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -2,6 +2,7 @@ use crate::any::type_name; use crate::fmt; use crate::intrinsics; use crate::mem::ManuallyDrop; +use crate::ptr; /// A wrapper type to construct uninitialized instances of `T`. /// @@ -573,6 +574,28 @@ impl MaybeUninit { } } + /// Drops the contained value in place. + /// + /// If you have ownership of the `MaybeUninit`, it is preferable to use + /// [`assume_init`] instead, which prevents duplicating the content. + /// + /// # Safety + /// + /// Calling this when the content is not yet fully initialized causes undefined + /// behavior: it is up to the caller to guarantee that the `MaybeUninit` really + /// is in an initialized state. + /// + /// This function runs the destructor of the contained value in place. + /// Afterwards, the memory is considered uninitialized again, but remains unmodified. + /// + /// [`assume_init`]: MaybeUninit::assume_init + #[unstable(feature = "maybe_uninit_extra", issue = "63567")] + pub unsafe fn drop(&mut self) { + // SAFETY: the caller must guarantee that `self` is initialized. + // Dropping the value in place is safe if that is the case. + unsafe { ptr::drop_in_place(self.as_mut_ptr()) } + } + /// Gets a shared reference to the contained value. /// /// This can be useful when we want to access a `MaybeUninit` that has been From caef83282bfede657faf38d43967f577841b3be4 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 8 Sep 2020 19:34:23 +0200 Subject: [PATCH 2/6] Fix doc comment on MaybeUninit::drop. --- library/core/src/mem/maybe_uninit.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index e826ec0a5e3ee..001ee0898d45e 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -576,8 +576,7 @@ impl MaybeUninit { /// Drops the contained value in place. /// - /// If you have ownership of the `MaybeUninit`, it is preferable to use - /// [`assume_init`] instead, which prevents duplicating the content. + /// If you have ownership of the `MaybeUninit`, you can use [`assume_init`] instead. /// /// # Safety /// From 656a17b44d2e7ee8096f479ed8a04baab2200cae Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 9 Sep 2020 11:27:09 +0200 Subject: [PATCH 3/6] Rename MaybeUninit::drop to assume_init_drop. --- library/core/src/mem/maybe_uninit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 001ee0898d45e..8d19412e4c414 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -589,7 +589,7 @@ impl MaybeUninit { /// /// [`assume_init`]: MaybeUninit::assume_init #[unstable(feature = "maybe_uninit_extra", issue = "63567")] - pub unsafe fn drop(&mut self) { + pub unsafe fn assume_init_drop(&mut self) { // SAFETY: the caller must guarantee that `self` is initialized. // Dropping the value in place is safe if that is the case. unsafe { ptr::drop_in_place(self.as_mut_ptr()) } From a14efd1d0a2f0fa112e4359b9db1e9857589c796 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 9 Sep 2020 11:27:42 +0200 Subject: [PATCH 4/6] Rename MaybeUninit::read to assume_init_read. --- library/core/src/array/iter.rs | 4 ++-- library/core/src/mem/maybe_uninit.rs | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/library/core/src/array/iter.rs b/library/core/src/array/iter.rs index 2e8b6419eea1e..cafb002c01a11 100644 --- a/library/core/src/array/iter.rs +++ b/library/core/src/array/iter.rs @@ -103,7 +103,7 @@ impl Iterator for IntoIter { // dead now (i.e. do not touch). As `idx` was the start of the // alive-zone, the alive zone is now `data[alive]` again, restoring // all invariants. - unsafe { self.data.get_unchecked(idx).read() } + unsafe { self.data.get_unchecked(idx).assume_init_read() } }) } @@ -136,7 +136,7 @@ impl DoubleEndedIterator for IntoIter { // dead now (i.e. do not touch). As `idx` was the end of the // alive-zone, the alive zone is now `data[alive]` again, restoring // all invariants. - unsafe { self.data.get_unchecked(idx).read() } + unsafe { self.data.get_unchecked(idx).assume_init_read() } }) } } diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 8d19412e4c414..38a006ce74ce0 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -520,8 +520,8 @@ impl MaybeUninit { /// this initialization invariant. /// /// Moreover, this leaves a copy of the same data behind in the `MaybeUninit`. When using - /// multiple copies of the data (by calling `read` multiple times, or first - /// calling `read` and then [`assume_init`]), it is your responsibility + /// multiple copies of the data (by calling `assume_init_read` multiple times, or first + /// calling `assume_init_read` and then [`assume_init`]), it is your responsibility /// to ensure that that data may indeed be duplicated. /// /// [inv]: #initialization-invariant @@ -537,16 +537,16 @@ impl MaybeUninit { /// /// let mut x = MaybeUninit::::uninit(); /// x.write(13); - /// let x1 = unsafe { x.read() }; + /// let x1 = unsafe { x.assume_init_read() }; /// // `u32` is `Copy`, so we may read multiple times. - /// let x2 = unsafe { x.read() }; + /// let x2 = unsafe { x.assume_init_read() }; /// assert_eq!(x1, x2); /// /// let mut x = MaybeUninit::>>::uninit(); /// x.write(None); - /// let x1 = unsafe { x.read() }; + /// let x1 = unsafe { x.assume_init_read() }; /// // Duplicating a `None` value is okay, so we may read multiple times. - /// let x2 = unsafe { x.read() }; + /// let x2 = unsafe { x.assume_init_read() }; /// assert_eq!(x1, x2); /// ``` /// @@ -558,14 +558,14 @@ impl MaybeUninit { /// /// let mut x = MaybeUninit::>>::uninit(); /// x.write(Some(vec![0,1,2])); - /// let x1 = unsafe { x.read() }; - /// let x2 = unsafe { x.read() }; + /// let x1 = unsafe { x.assume_init_read() }; + /// let x2 = unsafe { x.assume_init_read() }; /// // We now created two copies of the same vector, leading to a double-free ⚠️ when /// // they both get dropped! /// ``` #[unstable(feature = "maybe_uninit_extra", issue = "63567")] #[inline(always)] - pub unsafe fn read(&self) -> T { + pub unsafe fn assume_init_read(&self) -> T { // SAFETY: the caller must guarantee that `self` is initialized. // Reading from `self.as_ptr()` is safe since `self` should be initialized. unsafe { From a94b2cb034c2521d52e54632b775e181eb7e0bc7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 9 Sep 2020 18:54:17 +0200 Subject: [PATCH 5/6] Add safety docs about T's invariants in MaybeUninit::assume_init_drop. --- library/core/src/mem/maybe_uninit.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 38a006ce74ce0..0d1d563b5ceec 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -580,17 +580,23 @@ impl MaybeUninit { /// /// # Safety /// - /// Calling this when the content is not yet fully initialized causes undefined - /// behavior: it is up to the caller to guarantee that the `MaybeUninit` really - /// is in an initialized state. - /// - /// This function runs the destructor of the contained value in place. - /// Afterwards, the memory is considered uninitialized again, but remains unmodified. + /// It is up to the caller to guarantee that the `MaybeUninit` really is + /// in an initialized state. Calling this when the content is not yet fully + /// initialized causes undefined behavior. + /// + /// On top of that, all additional invariants of the type `T` must be + /// satisfied, as the `Drop` implementation of `T` (or its members) may + /// rely on this. For example, a `1`-initialized [`Vec`] is considered + /// initialized (under the current implementation; this does not constitute + /// a stable guarantee) because the only requirement the compiler knows + /// about it is that the data pointer must be non-null. Dropping such a + /// `Vec` however will cause undefined behaviour. /// /// [`assume_init`]: MaybeUninit::assume_init #[unstable(feature = "maybe_uninit_extra", issue = "63567")] pub unsafe fn assume_init_drop(&mut self) { - // SAFETY: the caller must guarantee that `self` is initialized. + // SAFETY: the caller must guarantee that `self` is initialized and + // satisfies all invariants of `T`. // Dropping the value in place is safe if that is the case. unsafe { ptr::drop_in_place(self.as_mut_ptr()) } } From 43c7a9b72b284bee6ce8517550cb6ee7903c639e Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 9 Sep 2020 18:56:16 +0200 Subject: [PATCH 6/6] Fix broken doc links in MaybeUninit. --- library/core/src/mem/maybe_uninit.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 0d1d563b5ceec..f68e2b8208982 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -472,6 +472,8 @@ impl MaybeUninit { /// *immediate* undefined behavior, but will cause undefined behavior with most /// safe operations (including dropping it). /// + /// [`Vec`]: ../../std/vec/struct.Vec.html + /// /// # Examples /// /// Correct usage of this method: @@ -593,6 +595,7 @@ impl MaybeUninit { /// `Vec` however will cause undefined behaviour. /// /// [`assume_init`]: MaybeUninit::assume_init + /// [`Vec`]: ../../std/vec/struct.Vec.html #[unstable(feature = "maybe_uninit_extra", issue = "63567")] pub unsafe fn assume_init_drop(&mut self) { // SAFETY: the caller must guarantee that `self` is initialized and