Rollup merge of #127733 - GrigorenkoPV:don't-forget, r=Amanieu

Replace some `mem::forget`'s with `ManuallyDrop`

              > but I would like to see a larger effort to replace all uses of `mem::forget`.

_Originally posted by `@saethlin` in https://github.com/rust-lang/rust/issues/127584#issuecomment-2226087767_

So,
r? `@saethlin`

Sorry, I have finished writing all of this before I got your response.

library/alloc/src/rc.rs

@@ -259,7 +259,7 @@ use core::intrinsics::abort;
 #[cfg(not(no_global_oom_handling))]
 use core::iter;
 use core::marker::{PhantomData, Unsize};
-use core::mem::{self, align_of_val_raw, forget, ManuallyDrop};
+use core::mem::{self, align_of_val_raw, ManuallyDrop};
 use core::ops::{CoerceUnsized, Deref, DerefMut, DerefPure, DispatchFromDyn, Receiver};
 use core::panic::{RefUnwindSafe, UnwindSafe};
 #[cfg(not(no_global_oom_handling))]
@@ -908,19 +908,18 @@ impl<T, A: Allocator> Rc<T, A> {
     #[stable(feature = "rc_unique", since = "1.4.0")]
     pub fn try_unwrap(this: Self) -> Result<T, Self> {
         if Rc::strong_count(&this) == 1 {
-            unsafe {
+            let this = ManuallyDrop::new(this);
-                let val = ptr::read(&*this); // copy the contained object
-                let alloc = ptr::read(&this.alloc); // copy the allocator
 
-                // Indicate to Weaks that they can't be promoted by decrementing
+            let val: T = unsafe { ptr::read(&**this) }; // copy the contained object
-                // the strong count, and then remove the implicit "strong weak"
+            let alloc: A = unsafe { ptr::read(&this.alloc) }; // copy the allocator
-                // pointer while also handling drop logic by just crafting a
+
-                // fake Weak.
+            // Indicate to Weaks that they can't be promoted by decrementing
-                this.inner().dec_strong();
+            // the strong count, and then remove the implicit "strong weak"
-                let _weak = Weak { ptr: this.ptr, alloc };
+            // pointer while also handling drop logic by just crafting a
-                forget(this);
+            // fake Weak.
-                Ok(val)
+            this.inner().dec_strong();
-            }
+            let _weak = Weak { ptr: this.ptr, alloc };
+            Ok(val)
         } else {
             Err(this)
         }
@@ -1354,9 +1353,8 @@ impl<T: ?Sized, A: Allocator> Rc<T, A> {
     #[stable(feature = "rc_raw", since = "1.17.0")]
     #[rustc_never_returns_null_ptr]
     pub fn into_raw(this: Self) -> *const T {
-        let ptr = Self::as_ptr(&this);
+        let this = ManuallyDrop::new(this);
-        mem::forget(this);
+        Self::as_ptr(&*this)
-        ptr
     }
 
     /// Consumes the `Rc`, returning the wrapped pointer and allocator.
@@ -2127,7 +2125,7 @@ impl<T> Rc<[T]> {
             }
 
             // All clear. Forget the guard so it doesn't free the new RcBox.
-            forget(guard);
+            mem::forget(guard);
 
             Self::from_ptr(ptr)
         }
@@ -3080,9 +3078,7 @@ impl<T: ?Sized, A: Allocator> Weak<T, A> {
     #[must_use = "losing the pointer will leak memory"]
     #[stable(feature = "weak_into_raw", since = "1.45.0")]
     pub fn into_raw(self) -> *const T {
-        let result = self.as_ptr();
+        mem::ManuallyDrop::new(self).as_ptr()
-        mem::forget(self);
-        result
     }
 
     /// Consumes the `Weak<T>`, returning the wrapped pointer and allocator.
@@ -3762,10 +3758,11 @@ impl<T: ?Sized, A: Allocator> UniqueRcUninit<T, A> {
     /// # Safety
     ///
     /// The data must have been initialized (by writing to [`Self::data_ptr()`]).
-    unsafe fn into_rc(mut self) -> Rc<T, A> {
+    unsafe fn into_rc(self) -> Rc<T, A> {
-        let ptr = self.ptr;
+        let mut this = ManuallyDrop::new(self);
-        let alloc = self.alloc.take().unwrap();
+        let ptr = this.ptr;
-        mem::forget(self);
+        let alloc = this.alloc.take().unwrap();
+
         // SAFETY: The pointer is valid as per `UniqueRcUninit::new`, and the caller is responsible
         // for having initialized the data.
         unsafe { Rc::from_ptr_in(ptr.as_ptr(), alloc) }

library/alloc/src/sync.rs

@@ -20,7 +20,7 @@ use core::intrinsics::abort;
 #[cfg(not(no_global_oom_handling))]
 use core::iter;
 use core::marker::{PhantomData, Unsize};
-use core::mem::{self, align_of_val_raw};
+use core::mem::{self, align_of_val_raw, ManuallyDrop};
 use core::ops::{CoerceUnsized, Deref, DerefPure, DispatchFromDyn, Receiver};
 use core::panic::{RefUnwindSafe, UnwindSafe};
 use core::pin::Pin;
@@ -960,16 +960,14 @@ impl<T, A: Allocator> Arc<T, A> {
 
         acquire!(this.inner().strong);
 
-        unsafe {
+        let this = ManuallyDrop::new(this);
-            let elem = ptr::read(&this.ptr.as_ref().data);
+        let elem: T = unsafe { ptr::read(&this.ptr.as_ref().data) };
-            let alloc = ptr::read(&this.alloc); // copy the allocator
+        let alloc: A = unsafe { ptr::read(&this.alloc) }; // copy the allocator
 
-            // Make a weak pointer to clean up the implicit strong-weak reference
+        // Make a weak pointer to clean up the implicit strong-weak reference
-            let _weak = Weak { ptr: this.ptr, alloc };
+        let _weak = Weak { ptr: this.ptr, alloc };
-            mem::forget(this);
 
-            Ok(elem)
+        Ok(elem)
-        }
     }
 
     /// Returns the inner value, if the `Arc` has exactly one strong reference.
@@ -1493,9 +1491,8 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
     #[stable(feature = "rc_raw", since = "1.17.0")]
     #[rustc_never_returns_null_ptr]
     pub fn into_raw(this: Self) -> *const T {
-        let ptr = Self::as_ptr(&this);
+        let this = ManuallyDrop::new(this);
-        mem::forget(this);
+        Self::as_ptr(&*this)
-        ptr
     }
 
     /// Consumes the `Arc`, returning the wrapped pointer and allocator.
@@ -2801,9 +2798,7 @@ impl<T: ?Sized, A: Allocator> Weak<T, A> {
     #[must_use = "losing the pointer will leak memory"]
     #[stable(feature = "weak_into_raw", since = "1.45.0")]
     pub fn into_raw(self) -> *const T {
-        let result = self.as_ptr();
+        ManuallyDrop::new(self).as_ptr()
-        mem::forget(self);
-        result
     }
 
     /// Consumes the `Weak<T>`, returning the wrapped pointer and allocator.
@@ -3875,13 +3870,14 @@ impl<T: ?Sized, A: Allocator> UniqueArcUninit<T, A> {
     /// # Safety
     ///
     /// The data must have been initialized (by writing to [`Self::data_ptr()`]).
-    unsafe fn into_arc(mut self) -> Arc<T, A> {
+    unsafe fn into_arc(self) -> Arc<T, A> {
-        let ptr = self.ptr;
+        let mut this = ManuallyDrop::new(self);
-        let alloc = self.alloc.take().unwrap();
+        let ptr = this.ptr.as_ptr();
-        mem::forget(self);
+        let alloc = this.alloc.take().unwrap();
+
         // SAFETY: The pointer is valid as per `UniqueArcUninit::new`, and the caller is responsible
         // for having initialized the data.
-        unsafe { Arc::from_ptr_in(ptr.as_ptr(), alloc) }
+        unsafe { Arc::from_ptr_in(ptr, alloc) }
     }
 }
 

library/core/src/task/wake.rs

@@ -1,10 +1,9 @@
 #![stable(feature = "futures_api", since = "1.36.0")]
 
-use crate::mem::transmute;
-
 use crate::any::Any;
 use crate::fmt;
 use crate::marker::PhantomData;
+use crate::mem::{transmute, ManuallyDrop};
 use crate::panic::AssertUnwindSafe;
 use crate::ptr;
 
@@ -465,16 +464,14 @@ impl Waker {
     pub fn wake(self) {
         // The actual wakeup call is delegated through a virtual function call
         // to the implementation which is defined by the executor.
-        let wake = self.waker.vtable.wake;
-        let data = self.waker.data;
 
         // Don't call `drop` -- the waker will be consumed by `wake`.
-        crate::mem::forget(self);
+        let this = ManuallyDrop::new(self);
 
         // SAFETY: This is safe because `Waker::from_raw` is the only way
         // to initialize `wake` and `data` requiring the user to acknowledge
         // that the contract of `RawWaker` is upheld.
-        unsafe { (wake)(data) };
+        unsafe { (this.waker.vtable.wake)(this.waker.data) };
     }
 
     /// Wake up the task associated with this `Waker` without consuming the `Waker`.
@@ -726,16 +723,14 @@ impl LocalWaker {
     pub fn wake(self) {
         // The actual wakeup call is delegated through a virtual function call
         // to the implementation which is defined by the executor.
-        let wake = self.waker.vtable.wake;
-        let data = self.waker.data;
 
         // Don't call `drop` -- the waker will be consumed by `wake`.
-        crate::mem::forget(self);
+        let this = ManuallyDrop::new(self);
 
         // SAFETY: This is safe because `Waker::from_raw` is the only way
         // to initialize `wake` and `data` requiring the user to acknowledge
         // that the contract of `RawWaker` is upheld.
-        unsafe { (wake)(data) };
+        unsafe { (this.waker.vtable.wake)(this.waker.data) };
     }
 
     /// Wake up the task associated with this `LocalWaker` without consuming the `LocalWaker`.

library/proc_macro/src/bridge/buffer.rs

@@ -1,7 +1,7 @@
 //! Buffer management for same-process client<->server communication.
 
 use std::io::{self, Write};
-use std::mem;
+use std::mem::{self, ManuallyDrop};
 use std::ops::{Deref, DerefMut};
 use std::slice;
 
@@ -129,17 +129,16 @@ impl Drop for Buffer {
 }
 
 impl From<Vec<u8>> for Buffer {
-    fn from(mut v: Vec<u8>) -> Self {
+    fn from(v: Vec<u8>) -> Self {
+        let mut v = ManuallyDrop::new(v);
         let (data, len, capacity) = (v.as_mut_ptr(), v.len(), v.capacity());
-        mem::forget(v);
 
         // This utility function is nested in here because it can *only*
         // be safely called on `Buffer`s created by *this* `proc_macro`.
         fn to_vec(b: Buffer) -> Vec<u8> {
             unsafe {
-                let Buffer { data, len, capacity, .. } = b;
+                let b = ManuallyDrop::new(b);
-                mem::forget(b);
+                Vec::from_raw_parts(b.data, b.len, b.capacity)
-                Vec::from_raw_parts(data, len, capacity)
             }
         }
 

library/proc_macro/src/bridge/client.rs

@@ -51,9 +51,7 @@ macro_rules! define_client_handles {
 
             impl<S> Encode<S> for $oty {
                 fn encode(self, w: &mut Writer, s: &mut S) {
-                    let handle = self.handle;
+                    mem::ManuallyDrop::new(self).handle.encode(w, s);
-                    mem::forget(self);
-                    handle.encode(w, s);
                 }
             }
 

library/std/src/os/fd/owned.rs

@@ -8,7 +8,7 @@ use crate::fmt;
 use crate::fs;
 use crate::io;
 use crate::marker::PhantomData;
-use crate::mem::forget;
+use crate::mem::ManuallyDrop;
 #[cfg(not(any(target_arch = "wasm32", target_env = "sgx", target_os = "hermit")))]
 use crate::sys::cvt;
 use crate::sys_common::{AsInner, FromInner, IntoInner};
@@ -148,9 +148,7 @@ impl AsRawFd for OwnedFd {
 impl IntoRawFd for OwnedFd {
     #[inline]
     fn into_raw_fd(self) -> RawFd {
-        let fd = self.fd;
+        ManuallyDrop::new(self).fd
-        forget(self);
-        fd
     }
 }
 

library/std/src/os/solid/io.rs

@@ -48,7 +48,7 @@
 
 use crate::fmt;
 use crate::marker::PhantomData;
-use crate::mem::forget;
+use crate::mem::ManuallyDrop;
 use crate::net;
 use crate::sys;
 use crate::sys_common::{self, AsInner, FromInner, IntoInner};
@@ -148,9 +148,7 @@ impl AsRawFd for OwnedFd {
 impl IntoRawFd for OwnedFd {
     #[inline]
     fn into_raw_fd(self) -> RawFd {
-        let fd = self.fd;
+        ManuallyDrop::new(self).fd
-        forget(self);
-        fd
     }
 }
 

library/std/src/os/windows/io/handle.rs

@@ -7,7 +7,7 @@ use crate::fmt;
 use crate::fs;
 use crate::io;
 use crate::marker::PhantomData;
-use crate::mem::{forget, ManuallyDrop};
+use crate::mem::ManuallyDrop;
 use crate::ptr;
 use crate::sys;
 use crate::sys::cvt;
@@ -319,9 +319,7 @@ impl AsRawHandle for OwnedHandle {
 impl IntoRawHandle for OwnedHandle {
     #[inline]
     fn into_raw_handle(self) -> RawHandle {
-        let handle = self.handle;
+        ManuallyDrop::new(self).handle
-        forget(self);
-        handle
     }
 }
 

library/std/src/os/windows/io/socket.rs

@@ -6,8 +6,7 @@ use super::raw::{AsRawSocket, FromRawSocket, IntoRawSocket, RawSocket};
 use crate::fmt;
 use crate::io;
 use crate::marker::PhantomData;
-use crate::mem;
+use crate::mem::{self, ManuallyDrop};
-use crate::mem::forget;
 use crate::sys;
 #[cfg(not(target_vendor = "uwp"))]
 use crate::sys::cvt;
@@ -191,9 +190,7 @@ impl AsRawSocket for OwnedSocket {
 impl IntoRawSocket for OwnedSocket {
     #[inline]
     fn into_raw_socket(self) -> RawSocket {
-        let socket = self.socket;
+        ManuallyDrop::new(self).socket
-        forget(self);
-        socket
     }
 }
 

library/std/src/sys/pal/hermit/thread.rs

@@ -3,7 +3,7 @@
 use super::hermit_abi;
 use crate::ffi::CStr;
 use crate::io;
-use crate::mem;
+use crate::mem::ManuallyDrop;
 use crate::num::NonZero;
 use crate::ptr;
 use crate::time::Duration;
@@ -90,9 +90,7 @@ impl Thread {
 
     #[inline]
     pub fn into_id(self) -> Tid {
-        let id = self.tid;
+        ManuallyDrop::new(self).tid
-        mem::forget(self);
-        id
     }
 }
 

library/std/src/sys/pal/sgx/abi/tls/mod.rs

@@ -95,8 +95,8 @@ impl Tls {
     #[allow(unused)]
     pub unsafe fn activate_persistent(self: Box<Self>) {
         // FIXME: Needs safety information. See entry.S for `set_tls_ptr` definition.
-        unsafe { set_tls_ptr(core::ptr::addr_of!(*self) as _) };
+        let ptr = Box::into_raw(self).cast_const().cast::<u8>();
-        mem::forget(self);
+        unsafe { set_tls_ptr(ptr) };
     }
 
     unsafe fn current<'a>() -> &'a Tls {

library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs

@@ -5,7 +5,7 @@ use crate::cell::UnsafeCell;
 use crate::cmp;
 use crate::convert::TryInto;
 use crate::intrinsics;
-use crate::mem;
+use crate::mem::{self, ManuallyDrop};
 use crate::ops::{CoerceUnsized, Deref, DerefMut, Index, IndexMut};
 use crate::ptr::{self, NonNull};
 use crate::slice;
@@ -176,6 +176,7 @@ unsafe impl<T: UserSafeSized> UserSafe for [T] {
 /// are used solely to indicate intent: a mutable reference is for writing to
 /// user memory, an immutable reference for reading from user memory.
 #[unstable(feature = "sgx_platform", issue = "56975")]
+#[repr(transparent)]
 pub struct UserRef<T: ?Sized>(UnsafeCell<T>);
 /// An owned type in userspace memory. `User<T>` is equivalent to `Box<T>` in
 /// enclave memory. Access to the memory is only allowed by copying to avoid
@@ -266,9 +267,7 @@ where
     /// Converts this value into a raw pointer. The value will no longer be
     /// automatically freed.
     pub fn into_raw(self) -> *mut T {
-        let ret = self.0;
+        ManuallyDrop::new(self).0.as_ptr() as _
-        mem::forget(self);
-        ret.as_ptr() as _
     }
 }
 

library/std/src/sys/pal/sgx/fd.rs

@@ -2,7 +2,7 @@ use fortanix_sgx_abi::Fd;
 
 use super::abi::usercalls;
 use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut};
-use crate::mem;
+use crate::mem::ManuallyDrop;
 use crate::sys::{AsInner, FromInner, IntoInner};
 
 #[derive(Debug)]
@@ -21,9 +21,7 @@ impl FileDesc {
 
     /// Extracts the actual file descriptor without closing it.
     pub fn into_raw(self) -> Fd {
-        let fd = self.fd;
+        ManuallyDrop::new(self).fd
-        mem::forget(self);
-        fd
     }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
@@ -70,9 +68,7 @@ impl AsInner<Fd> for FileDesc {
 
 impl IntoInner<Fd> for FileDesc {
     fn into_inner(self) -> Fd {
-        let fd = self.fd;
+        ManuallyDrop::new(self).fd
-        mem::forget(self);
-        fd
     }
 }
 

library/std/src/sys/pal/teeos/thread.rs

@@ -1,9 +1,7 @@
-use core::convert::TryInto;
-
 use crate::cmp;
 use crate::ffi::CStr;
 use crate::io;
-use crate::mem;
+use crate::mem::{self, ManuallyDrop};
 use crate::num::NonZero;
 use crate::ptr;
 use crate::sys::os;
@@ -115,11 +113,9 @@ impl Thread {
 
     /// must join, because no pthread_detach supported
     pub fn join(self) {
-        unsafe {
+        let id = self.into_id();
-            let ret = libc::pthread_join(self.id, ptr::null_mut());
+        let ret = unsafe { libc::pthread_join(id, ptr::null_mut()) };
-            mem::forget(self);
+        assert!(ret == 0, "failed to join thread: {}", io::Error::from_raw_os_error(ret));
-            assert!(ret == 0, "failed to join thread: {}", io::Error::from_raw_os_error(ret));
-        }
     }
 
     pub fn id(&self) -> libc::pthread_t {
@@ -127,9 +123,7 @@ impl Thread {
     }
 
     pub fn into_id(self) -> libc::pthread_t {
-        let id = self.id;
+        ManuallyDrop::new(self).id
-        mem::forget(self);
-        id
     }
 }
 

library/std/src/sys/pal/unix/thread.rs

@@ -1,7 +1,7 @@
 use crate::cmp;
 use crate::ffi::CStr;
 use crate::io;
-use crate::mem;
+use crate::mem::{self, ManuallyDrop};
 use crate::num::NonZero;
 use crate::ptr;
 use crate::sys::{os, stack_overflow};
@@ -268,11 +268,9 @@ impl Thread {
     }
 
     pub fn join(self) {
-        unsafe {
+        let id = self.into_id();
-            let ret = libc::pthread_join(self.id, ptr::null_mut());
+        let ret = unsafe { libc::pthread_join(id, ptr::null_mut()) };
-            mem::forget(self);
+        assert!(ret == 0, "failed to join thread: {}", io::Error::from_raw_os_error(ret));
-            assert!(ret == 0, "failed to join thread: {}", io::Error::from_raw_os_error(ret));
-        }
     }
 
     pub fn id(&self) -> libc::pthread_t {
@@ -280,9 +278,7 @@ impl Thread {
     }
 
     pub fn into_id(self) -> libc::pthread_t {
-        let id = self.id;
+        ManuallyDrop::new(self).id
-        mem::forget(self);
-        id
     }
 }
 

library/std/src/sys/pal/wasi/thread.rs

@@ -172,12 +172,10 @@ impl Thread {
     pub fn join(self) {
         cfg_if::cfg_if! {
             if #[cfg(target_feature = "atomics")] {
-                unsafe {
+                let id = mem::ManuallyDrop::new(self).id;
-                    let ret = libc::pthread_join(self.id, ptr::null_mut());
+                let ret = unsafe { libc::pthread_join(id, ptr::null_mut()) };
-                    mem::forget(self);
+                if ret != 0 {
-                    if ret != 0 {
+                    rtabort!("failed to join thread: {}", io::Error::from_raw_os_error(ret));
-                        rtabort!("failed to join thread: {}", io::Error::from_raw_os_error(ret));
-                    }
                 }
             } else {
                 self.0

library/std/src/thread/mod.rs

@@ -165,7 +165,7 @@ use crate::ffi::CStr;
 use crate::fmt;
 use crate::io;
 use crate::marker::PhantomData;
-use crate::mem::{self, forget};
+use crate::mem::{self, forget, ManuallyDrop};
 use crate::num::NonZero;
 use crate::panic;
 use crate::panicking;
@@ -510,11 +510,10 @@ impl Builder {
                 MaybeDangling(mem::MaybeUninit::new(x))
             }
             fn into_inner(self) -> T {
-                // SAFETY: we are always initialized.
-                let ret = unsafe { self.0.assume_init_read() };
                 // Make sure we don't drop.
-                mem::forget(self);
+                let this = ManuallyDrop::new(self);
-                ret
+                // SAFETY: we are always initialized.
+                unsafe { this.0.assume_init_read() }
             }
         }
         impl<T> Drop for MaybeDangling<T> {

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_deadlock.stderr

@@ -10,7 +10,7 @@ LL |             assert_eq!(libc::pthread_mutex_lock(lock_copy.0.get() as *mut _
 error: deadlock: the evaluated program deadlocked
   --> RUSTLIB/std/src/sys/pal/PLATFORM/thread.rs:LL:CC
    |
-LL |             let ret = libc::pthread_join(self.id, ptr::null_mut());
+LL |         let ret = unsafe { libc::pthread_join(id, ptr::null_mut()) };
    |                                                                  ^ the evaluated program deadlocked
    |
    = note: BACKTRACE:

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_rwlock_write_read_deadlock.stderr

@@ -10,7 +10,7 @@ LL |             assert_eq!(libc::pthread_rwlock_wrlock(lock_copy.0.get() as *mu
 error: deadlock: the evaluated program deadlocked
   --> RUSTLIB/std/src/sys/pal/PLATFORM/thread.rs:LL:CC
    |
-LL |             let ret = libc::pthread_join(self.id, ptr::null_mut());
+LL |         let ret = unsafe { libc::pthread_join(id, ptr::null_mut()) };
    |                                                                  ^ the evaluated program deadlocked
    |
    = note: BACKTRACE:

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_rwlock_write_write_deadlock.stderr

@@ -10,7 +10,7 @@ LL |             assert_eq!(libc::pthread_rwlock_wrlock(lock_copy.0.get() as *mu
 error: deadlock: the evaluated program deadlocked
   --> RUSTLIB/std/src/sys/pal/PLATFORM/thread.rs:LL:CC
    |
-LL |             let ret = libc::pthread_join(self.id, ptr::null_mut());
+LL |         let ret = unsafe { libc::pthread_join(id, ptr::null_mut()) };
    |                                                                  ^ the evaluated program deadlocked
    |
    = note: BACKTRACE: