From 13ffbee1735c7329d6e7b3521712996c914c4ef9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:07:58 +0100 Subject: [PATCH 01/15] Add MaybeUninit::read_uninitialized Also remove a no-longer accurate comments --- src/libcore/mem.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 43afc9a522a30..3955839ac3625 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1185,6 +1185,61 @@ impl MaybeUninit { ManuallyDrop::into_inner(self.value) } + /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject + /// to the usual drop handling. + /// + /// # Unsafety + /// + /// 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. + /// + /// Moreover, this leaves a copy of the same data behind in the `MaybeUninit`. When using + /// multiple copies of the data (by calling `read_initialized` multiple times, or first + /// calling `read_initialized` and then [`into_initialized`]), it is your responsibility + /// to ensure that that data may indeed be duplicated. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::::uninitialized(); + /// x.set(13); + /// let x1 = unsafe { x.read_initialized() }; + /// // `u32` is `Copy`, so we may read multiple times. + /// let x2 = unsafe { x.read_initialized() }; + /// + /// let mut x = MaybeUninit::>>::uninitialized(); + /// x.set(None); + /// let x1 = unsafe { x.read_initialized() }; + /// // Duplicating a `None` value is okay, so we may read multiple times. + /// let x2 = unsafe { x.read_initialized() }; + /// ``` + /// + /// *Incorrect* usafe of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>>::uninitialized(); + /// x.set(Some(vec![0,1,2])); + /// let x1 = unsafe { x.read_initialized() }; + /// let x2 = unsafe { x.read_initialized() }; + /// // We now created two copies of the same vector, leading to a double-free when + /// // they both get dropped! + /// ``` + #[unstable(feature = "maybe_uninit", issue = "53491")] + #[inline(always)] + pub unsafe fn read_initialized(&self) -> T { + intrinsics::panic_if_uninhabited::(); + self.as_ptr().read() + } + /// Gets a reference to the contained value. /// /// # Unsafety From dc570fb3f26b45d101440bb4fe57121ca06884d3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:11:37 +0100 Subject: [PATCH 02/15] also add examples to MaybeUninit::into_initialized --- src/libcore/mem.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 3955839ac3625..42f30c1dd6d72 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1178,6 +1178,31 @@ impl MaybeUninit { /// 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. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::::uninitialized(); + /// x.set(true); + /// let x_init = unsafe { x.into_initialized() }; + /// assert_eq!(x_init, true); + /// ``` + /// + /// *Incorrect* usage of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let x = MaybeUninit::>::uninitialized(); + /// let x_init = unsafe { x.into_initialized() }; + /// // `x` had not been initialized yet, so this last line causes undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub unsafe fn into_initialized(self) -> T { @@ -1212,15 +1237,17 @@ impl MaybeUninit { /// let x1 = unsafe { x.read_initialized() }; /// // `u32` is `Copy`, so we may read multiple times. /// let x2 = unsafe { x.read_initialized() }; + /// assert_eq!(x1, x2); /// /// let mut x = MaybeUninit::>>::uninitialized(); /// x.set(None); /// let x1 = unsafe { x.read_initialized() }; /// // Duplicating a `None` value is okay, so we may read multiple times. /// let x2 = unsafe { x.read_initialized() }; + /// assert_eq!(x1, x2); /// ``` /// - /// *Incorrect* usafe of this method: + /// *Incorrect* usage of this method: /// /// ```rust,no_run /// #![feature(maybe_uninit)] From 10f511daa01d98e4c8f524cbdf38e9dd6c3ea9e3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:17:04 +0100 Subject: [PATCH 03/15] misc tweaks --- src/libcore/mem.rs | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 42f30c1dd6d72..556f8a9e8a2af 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1056,12 +1056,22 @@ impl DerefMut for ManuallyDrop { /// This is exploited by the compiler for various optimizations, such as eliding /// run-time checks and optimizing `enum` layout. /// -/// Not initializing memory at all (instead of zero-initializing it) causes the same -/// issue: after all, the initial value of the variable might just happen to be -/// one that violates the invariant. Moreover, uninitialized memory is special -/// in that the compiler knows that it does not have a fixed value. This makes -/// it undefined behavior to have uninitialized data in a variable even if that -/// variable has otherwise no restrictions about which values are valid: +/// Similarly, entirely uninitialized memory may have any content, while a `bool` must +/// always be `true` or `false`. Hence, creating an uninitialized `bool` is undefined behavior: +/// +/// ```rust,no_run +/// #![feature(maybe_uninit)] +/// use std::mem::{self, MaybeUninit}; +/// +/// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! +/// // equivalent code with `MaybeUninit` +/// let b: bool = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! +/// ``` +/// +/// Moreover, uninitialized memory is special in that the compiler knows that +/// it does not have a fixed value. This makes it undefined behavior to have +/// uninitialized data in a variable even if that variable has integer type, +/// which otherwise can hold any bit pattern: /// /// ```rust,no_run /// #![feature(maybe_uninit)] @@ -1074,8 +1084,8 @@ impl DerefMut for ManuallyDrop { /// (Notice that the rules around uninitialized integers are not finalized yet, but /// until they are, it is advisable to avoid them.) /// -/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data: -/// it is a signal to the compiler indicating that the data here might *not* +/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data. +/// It is a signal to the compiler indicating that the data here might *not* /// be initialized: /// /// ```rust @@ -1092,11 +1102,11 @@ impl DerefMut for ManuallyDrop { /// let x = unsafe { x.into_initialized() }; /// ``` /// -/// The compiler then knows to not optimize this code. +/// The compiler then knows to not make any incorrect assumptions or optimizations on this code. // FIXME before stabilizing, explain how to initialize a struct field-by-field. #[allow(missing_debug_implementations)] #[unstable(feature = "maybe_uninit", issue = "53491")] -// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::{uninitialized,zeroed}` +// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized` pub union MaybeUninit { uninit: (), value: ManuallyDrop, @@ -1154,7 +1164,7 @@ impl MaybeUninit { } /// Gets a pointer to the contained value. Reading from this pointer or turning it - /// into a reference will be undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_ptr(&self) -> *const T { @@ -1162,7 +1172,7 @@ impl MaybeUninit { } /// Gets a mutable pointer to the contained value. Reading from this pointer or turning it - /// into a reference will be undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_mut_ptr(&mut self) -> *mut T { From 48aa59e74d6a2b3fea5162eaed902798dc0f95f8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:23:31 +0100 Subject: [PATCH 04/15] examples for as[_mut]_ptr --- src/libcore/mem.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 556f8a9e8a2af..d586f45534ea0 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1165,6 +1165,32 @@ impl MaybeUninit { /// Gets a pointer to the contained value. Reading from this pointer or turning it /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>::uninitialized(); + /// x.set(vec![0,1,2]); + /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// let x_vec = unsafe { &*x.as_ptr() }; + /// assert_eq!(x_vec.len(), 3); + /// ``` + /// + /// *Incorrect* usage of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let x = MaybeUninit::>::uninitialized(); + /// let x_vec = unsafe { &*x.as_ptr() }; + /// // We have created a reference to an uninitialized vector! This is undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_ptr(&self) -> *const T { @@ -1173,6 +1199,33 @@ impl MaybeUninit { /// Gets a mutable pointer to the contained value. Reading from this pointer or turning it /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>::uninitialized(); + /// x.set(vec![0,1,2]); + /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; + /// x_vec.push(3); + /// assert_eq!(x_vec.len(), 4); + /// ``` + /// + /// *Incorrect* usage of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>::uninitialized(); + /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; + /// // We have created a reference to an uninitialized vector! This is undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_mut_ptr(&mut self) -> *mut T { From 084ee7a875cb02c0f0c0c6d1ffaff81d69cec74e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:35:18 +0100 Subject: [PATCH 05/15] examples for MaybeUninit::zeroed --- src/libcore/mem.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index d586f45534ea0..8f6798e0f6e61 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1141,6 +1141,35 @@ impl MaybeUninit { /// /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. + /// + /// # Example + /// + /// Correct usage of this method: initializing a struct with zero, where all + /// fields of the struct can hold 0 as a valid value. + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let x = MaybeUninit::<(u8, bool)>::zeroed(); + /// let x = unsafe { x.into_initialized() }; + /// assert_eq!(x, (0, false)); + /// ``` + /// + /// *Incorrect* usage of this method: initializing a struct with zero, where some fields + /// cannot hold 0 as a valid value. + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// enum NotZero { One = 1, Two = 2 }; + /// + /// let x = MaybeUninit::<(u8, NotZero)>::zeroed(); + /// let x = unsafe { x.into_initialized() }; + /// // We create a `NotZero` (inside a pair) that does not have a valid discriminant. + /// // This is undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline] pub fn zeroed() -> MaybeUninit { From d10366fe27bf1a1e6ab67076bdc268f486abeb88 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:47:30 +0100 Subject: [PATCH 06/15] avoid unnecessary use of MaybeUninit::get_ref, and expand comment on the others --- src/libcore/fmt/float.rs | 4 ++++ src/libcore/ptr.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcore/fmt/float.rs b/src/libcore/fmt/float.rs index 20c626cef1b16..edeb65afd67b2 100644 --- a/src/libcore/fmt/float.rs +++ b/src/libcore/fmt/float.rs @@ -15,6 +15,7 @@ fn float_to_decimal_common_exact(fmt: &mut Formatter, num: &T, // FIXME(#53491): Technically, this is calling `get_mut` on an uninitialized // `MaybeUninit` (here and elsewhere in this file). Revisit this once // we decided whether that is valid or not. + // Using `freeze` is *not enough*; `flt2dec::Part` is an enum! let formatted = flt2dec::to_exact_fixed_str(flt2dec::strategy::grisu::format_exact, *num, sign, precision, false, buf.get_mut(), parts.get_mut()); @@ -33,6 +34,7 @@ fn float_to_decimal_common_shortest(fmt: &mut Formatter, num: &T, // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninitialized(); let mut parts = MaybeUninit::<[flt2dec::Part; 4]>::uninitialized(); + // FIXME(#53491) let formatted = flt2dec::to_shortest_str(flt2dec::strategy::grisu::format_shortest, *num, sign, precision, false, buf.get_mut(), parts.get_mut()); @@ -71,6 +73,7 @@ fn float_to_exponential_common_exact(fmt: &mut Formatter, num: &T, unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninitialized(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part; 6]>::uninitialized(); + // FIXME(#53491) let formatted = flt2dec::to_exact_exp_str(flt2dec::strategy::grisu::format_exact, *num, sign, precision, upper, buf.get_mut(), parts.get_mut()); @@ -90,6 +93,7 @@ fn float_to_exponential_common_shortest(fmt: &mut Formatter, // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninitialized(); let mut parts = MaybeUninit::<[flt2dec::Part; 6]>::uninitialized(); + // FIXME(#53491) let formatted = flt2dec::to_shortest_exp_str(flt2dec::strategy::grisu::format_shortest, *num, sign, (0, 0), upper, buf.get_mut(), parts.get_mut()); diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 866c8d0896b3c..a2599ae834c69 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -301,7 +301,7 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { // Perform the swap copy_nonoverlapping(x, tmp.as_mut_ptr(), 1); copy(y, x, 1); // `x` and `y` may overlap - copy_nonoverlapping(tmp.get_ref(), y, 1); + copy_nonoverlapping(tmp.as_ptr(), y, 1); } /// Swaps `count * size_of::()` bytes between the two regions of memory From aa4a9b0827f09efa8a06d99df6cae07b21e6729c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:58:53 +0100 Subject: [PATCH 07/15] make MaybeUninit Copy --- src/libcore/mem.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 8f6798e0f6e61..296f15d830302 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1106,12 +1106,22 @@ impl DerefMut for ManuallyDrop { // FIXME before stabilizing, explain how to initialize a struct field-by-field. #[allow(missing_debug_implementations)] #[unstable(feature = "maybe_uninit", issue = "53491")] +#[derive(Copy)] // NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized` pub union MaybeUninit { uninit: (), value: ManuallyDrop, } +#[unstable(feature = "maybe_uninit", issue = "53491")] +impl Clone for MaybeUninit { + #[inline(always)] + fn clone(&self) -> Self { + // Not calling T::clone(), we cannot know if we are initialized enough for that. + *self + } +} + impl MaybeUninit { /// Create a new `MaybeUninit` initialized with the given value. /// From 53c027588263abf69bf0124f15eb6d261cd43316 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 23 Feb 2019 16:17:07 +0100 Subject: [PATCH 08/15] Apply suggestions from code review Co-Authored-By: RalfJung --- src/libcore/mem.rs | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 296f15d830302..46ba523c7722b 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1064,13 +1064,13 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // The equivalent code with `MaybeUninit`: /// let b: bool = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// /// Moreover, uninitialized memory is special in that the compiler knows that /// it does not have a fixed value. This makes it undefined behavior to have -/// uninitialized data in a variable even if that variable has integer type, +/// uninitialized data in a variable even if that variable has an integer type, /// which otherwise can hold any bit pattern: /// /// ```rust,no_run @@ -1084,7 +1084,7 @@ impl DerefMut for ManuallyDrop { /// (Notice that the rules around uninitialized integers are not finalized yet, but /// until they are, it is advisable to avoid them.) /// -/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data. +/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data. /// It is a signal to the compiler indicating that the data here might *not* /// be initialized: /// @@ -1107,7 +1107,7 @@ impl DerefMut for ManuallyDrop { #[allow(missing_debug_implementations)] #[unstable(feature = "maybe_uninit", issue = "53491")] #[derive(Copy)] -// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized` +// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized`. pub union MaybeUninit { uninit: (), value: ManuallyDrop, @@ -1123,7 +1123,7 @@ impl Clone for MaybeUninit { } impl MaybeUninit { - /// Create a new `MaybeUninit` initialized with the given value. + /// Create a new `MaybeUninit` initialized with the given value. /// /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. @@ -1149,13 +1149,13 @@ impl MaybeUninit { /// but `MaybeUninit<&'static i32>::zeroed()` is not because references must not /// be null. /// - /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. + /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. /// /// # Example /// - /// Correct usage of this method: initializing a struct with zero, where all - /// fields of the struct can hold 0 as a valid value. + /// Correct usage of this function: initializing a struct with zero, where all + /// fields of the struct can hold the bit-pattern 0 as a valid value. /// /// ```rust /// #![feature(maybe_uninit)] @@ -1166,7 +1166,7 @@ impl MaybeUninit { /// assert_eq!(x, (0, false)); /// ``` /// - /// *Incorrect* usage of this method: initializing a struct with zero, where some fields + /// *Incorrect* usage of this function: initializing a struct with zero, where some fields /// cannot hold 0 as a valid value. /// /// ```rust,no_run @@ -1177,7 +1177,7 @@ impl MaybeUninit { /// /// let x = MaybeUninit::<(u8, NotZero)>::zeroed(); /// let x = unsafe { x.into_initialized() }; - /// // We create a `NotZero` (inside a pair) that does not have a valid discriminant. + /// // Inside a pair, we create a `NotZero` that does not have a valid discriminant. /// // This is undefined behavior. /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] @@ -1203,7 +1203,7 @@ impl MaybeUninit { } /// Gets a pointer to the contained value. Reading from this pointer or turning it - /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. /// /// # Examples /// @@ -1237,7 +1237,7 @@ impl MaybeUninit { } /// Gets a mutable pointer to the contained value. Reading from this pointer or turning it - /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. /// /// # Examples /// @@ -1249,7 +1249,8 @@ impl MaybeUninit { /// /// let mut x = MaybeUninit::>::uninitialized(); /// x.set(vec![0,1,2]); - /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// // Create a reference into the `MaybeUninit>`. + /// // This is okay because we initialized it. /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; /// x_vec.push(3); /// assert_eq!(x_vec.len(), 4); @@ -1303,7 +1304,7 @@ impl MaybeUninit { /// /// let x = MaybeUninit::>::uninitialized(); /// let x_init = unsafe { x.into_initialized() }; - /// // `x` had not been initialized yet, so this last line causes undefined behavior. + /// // `x` had not been initialized yet, so this last line caused undefined behavior. /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1312,16 +1313,16 @@ impl MaybeUninit { ManuallyDrop::into_inner(self.value) } - /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject + /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject /// to the usual drop handling. /// - /// # Unsafety + /// # Safety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// 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. /// - /// Moreover, this leaves a copy of the same data behind in the `MaybeUninit`. When using + /// Moreover, this leaves a copy of the same data behind in the `MaybeUninit`. When using /// multiple copies of the data (by calling `read_initialized` multiple times, or first /// calling `read_initialized` and then [`into_initialized`]), it is your responsibility /// to ensure that that data may indeed be duplicated. From ac2284b80b5bf50702af2289c2ac4f04efa573ec Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Feb 2019 16:18:50 +0100 Subject: [PATCH 09/15] expand type name --- src/libcore/mem.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 46ba523c7722b..b354815a3a142 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1035,7 +1035,7 @@ impl DerefMut for ManuallyDrop { } } -/// A newtype to construct uninitialized instances of `T`. +/// A wrapper to construct uninitialized instances of `T`. /// /// The compiler, in general, assumes that variables are properly initialized /// at their respective type. For example, a variable of reference type must @@ -1049,7 +1049,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: &i32 = unsafe { mem::zeroed() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // equivalent code with `MaybeUninit<&i32>` /// let x: &i32 = unsafe { MaybeUninit::zeroed().into_initialized() }; // undefined behavior! /// ``` /// @@ -1064,7 +1064,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! -/// // The equivalent code with `MaybeUninit`: +/// // The equivalent code with `MaybeUninit`: /// let b: bool = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// @@ -1078,7 +1078,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: i32 = unsafe { mem::uninitialized() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // equivalent code with `MaybeUninit` /// let x: i32 = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// (Notice that the rules around uninitialized integers are not finalized yet, but @@ -1093,7 +1093,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::MaybeUninit; /// /// // Create an explicitly uninitialized reference. The compiler knows that data inside -/// // a `MaybeUninit` may be invalid, and hence this is not UB: +/// // a `MaybeUninit` may be invalid, and hence this is not UB: /// let mut x = MaybeUninit::<&i32>::uninitialized(); /// // Set it to a valid value. /// x.set(&0); @@ -1125,7 +1125,7 @@ impl Clone for MaybeUninit { impl MaybeUninit { /// Create a new `MaybeUninit` initialized with the given value. /// - /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. + /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1133,9 +1133,9 @@ impl MaybeUninit { MaybeUninit { value: ManuallyDrop::new(val) } } - /// Creates a new `MaybeUninit` in an uninitialized state. + /// Creates a new `MaybeUninit` in an uninitialized state. /// - /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. + /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1143,7 +1143,7 @@ impl MaybeUninit { MaybeUninit { uninit: () } } - /// Creates a new `MaybeUninit` in an uninitialized state, with the memory being + /// Creates a new `MaybeUninit` in an uninitialized state, with the memory being /// filled with `0` bytes. It depends on `T` whether that already makes for /// proper initialization. For example, `MaybeUninit::zeroed()` is initialized, /// but `MaybeUninit<&'static i32>::zeroed()` is not because references must not @@ -1190,9 +1190,9 @@ impl MaybeUninit { u } - /// Sets the value of the `MaybeUninit`. This overwrites any previous value without dropping it. - /// For your convenience, this also returns a mutable reference to the (now safely initialized) - /// contents of `self`. + /// Sets the value of the `MaybeUninit`. This overwrites any previous value + /// without dropping it. For your convenience, this also returns a mutable + /// reference to the (now safely initialized) contents of `self`. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn set(&mut self, val: T) -> &mut T { @@ -1215,7 +1215,7 @@ impl MaybeUninit { /// /// let mut x = MaybeUninit::>::uninitialized(); /// x.set(vec![0,1,2]); - /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. /// let x_vec = unsafe { &*x.as_ptr() }; /// assert_eq!(x_vec.len(), 3); /// ``` @@ -1272,13 +1272,13 @@ impl MaybeUninit { unsafe { &mut *self.value as *mut T } } - /// Extracts the value from the `MaybeUninit` container. This is a great way + /// Extracts the value from the `MaybeUninit` container. This is a great way /// to ensure that the data will get dropped, because the resulting `T` is /// subject to the usual drop handling. /// /// # Unsafety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// 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. /// @@ -1374,7 +1374,7 @@ impl MaybeUninit { /// /// # Unsafety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// 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. #[unstable(feature = "maybe_uninit_ref", issue = "53491")] @@ -1387,7 +1387,7 @@ impl MaybeUninit { /// /// # Unsafety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// 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. // FIXME(#53491): We currently rely on the above being incorrect, i.e., we have references From 8ce9b8667ac831821729f11b1e7c685af1e9e21c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Feb 2019 16:20:00 +0100 Subject: [PATCH 10/15] fix link --- src/libcore/mem.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index b354815a3a142..30fa904101d27 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1327,6 +1327,8 @@ impl MaybeUninit { /// calling `read_initialized` and then [`into_initialized`]), it is your responsibility /// to ensure that that data may indeed be duplicated. /// + /// [`into_initialized`]: #method.into_initialized + /// /// # Examples /// /// Correct usage of this method: From a5e2d0c4e5f4afe1bd52ed0ebe0be03890d3af62 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 24 Feb 2019 12:25:46 +0100 Subject: [PATCH 11/15] remark that the rules are unfinished --- src/libcore/mem.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 30fa904101d27..d4d51f8eeb76e 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1230,6 +1230,8 @@ impl MaybeUninit { /// let x_vec = unsafe { &*x.as_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` + /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_ptr(&self) -> *const T { @@ -1266,6 +1268,8 @@ impl MaybeUninit { /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` + /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_mut_ptr(&mut self) -> *mut T { From be8d728f3cb1cb79a630c6e6aba6df923dd3e999 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 24 Feb 2019 16:58:04 +0100 Subject: [PATCH 12/15] show how to set with ptr::write --- src/libcore/mem.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index d4d51f8eeb76e..967a36f6f1c39 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1191,7 +1191,8 @@ impl MaybeUninit { } /// Sets the value of the `MaybeUninit`. This overwrites any previous value - /// without dropping it. For your convenience, this also returns a mutable + /// without dropping it, so be careful not to use this twice unless you want to + /// skip running the destructor. For your convenience, this also returns a mutable /// reference to the (now safely initialized) contents of `self`. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1214,7 +1215,7 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::>::uninitialized(); - /// x.set(vec![0,1,2]); + /// unsafe { x.as_mut_ptr().write(vec![0,1,2]); } /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. /// let x_vec = unsafe { &*x.as_ptr() }; /// assert_eq!(x_vec.len(), 3); @@ -1250,7 +1251,7 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::>::uninitialized(); - /// x.set(vec![0,1,2]); + /// unsafe { x.as_mut_ptr().write(vec![0,1,2]); } /// // Create a reference into the `MaybeUninit>`. /// // This is okay because we initialized it. /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; @@ -1295,7 +1296,7 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::::uninitialized(); - /// x.set(true); + /// unsafe { x.as_mut_ptr().write(true); } /// let x_init = unsafe { x.into_initialized() }; /// assert_eq!(x_init, true); /// ``` From 6d32e5ae1a7a6cf7bc28f41d4c99ed81ac6113aa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 24 Feb 2019 20:34:24 +0100 Subject: [PATCH 13/15] prefer into_initialized over read_initialited --- src/libcore/mem.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 967a36f6f1c39..4b5056c5adffa 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1321,6 +1321,9 @@ impl MaybeUninit { /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject /// to the usual drop handling. /// + /// Whenever possible, it is preferrable to use [`into_initialized`] instead, which + /// prevents duplicating the content of the `MaybeUninit`. + /// /// # Safety /// /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized From 90bb07eca6c40ab98d31401597ba3f6c50a70c92 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 6 Mar 2019 19:57:00 +0100 Subject: [PATCH 14/15] Apply suggestions from code review Co-Authored-By: RalfJung --- src/libcore/mem.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 4b5056c5adffa..2e82c8c77816c 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1049,7 +1049,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: &i32 = unsafe { mem::zeroed() }; // undefined behavior! -/// // equivalent code with `MaybeUninit<&i32>` +/// // The equivalent code with `MaybeUninit<&i32>`: /// let x: &i32 = unsafe { MaybeUninit::zeroed().into_initialized() }; // undefined behavior! /// ``` /// @@ -1078,7 +1078,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: i32 = unsafe { mem::uninitialized() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // The equivalent code with `MaybeUninit`: /// let x: i32 = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// (Notice that the rules around uninitialized integers are not finalized yet, but @@ -1231,7 +1231,7 @@ impl MaybeUninit { /// let x_vec = unsafe { &*x.as_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` - /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// (Notice that the rules around references to uninitialized data are not finalized yet, but /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] From cefe9b09c120cfd8684fc2310ed2b295aafca01c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Mar 2019 20:02:50 +0100 Subject: [PATCH 15/15] Apply suggestions from code review --- src/libcore/mem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 2e82c8c77816c..2b6e7ab9b9950 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1269,7 +1269,7 @@ impl MaybeUninit { /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` - /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// (Notice that the rules around references to uninitialized data are not finalized yet, but /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)]