about summary refs log tree commit diff
path: root/library/std
diff options
context:
space:
mode:
authorJubilee <workingjubilee@gmail.com>2024-11-13 22:43:37 -0800
committerGitHub <noreply@github.com>2024-11-13 22:43:37 -0800
commit17dcadd587c52780e29b05d8e232dc384d9fad85 (patch)
tree55fb1222924c4fb01ae8b85dfdba08d8ddbcb5f4 /library/std
parentaa189460b8cf37829fb2f80fef592a13ba6eb0b4 (diff)
parent6166b0cda562dee46a5656d8c4641ee688c23421 (diff)
downloadrust-17dcadd587c52780e29b05d8e232dc384d9fad85.tar.gz
rust-17dcadd587c52780e29b05d8e232dc384d9fad85.zip
Rollup merge of #133003 - zachs18:clonetouninit-dyn-compat-u8, r=dtolnay
Make `CloneToUninit` dyn-compatible

Make `CloneToUninit` dyn-compatible, by making `clone_to_uninit`'s `dst` parameter `*mut u8` instead of `*mut Self`, so the method does not reference `Self` except in the `self` parameter and is thus dispatchable from a trait object.

This allows, among other things, adding `CloneToUninit` as a supertrait bound for `trait Foo` to allow cloning `dyn Foo` in some containers. Currently, this means that `Rc::make_mut` and `Arc::make_mut` can work with `dyn Foo` where `trait Foo: CloneToUninit`.

<details><summary>Example</summary>

```rs
#![feature(clone_to_uninit)]
use std::clone::CloneToUninit;
use std::rc::Rc;
use std::fmt::Debug;
use std::borrow::BorrowMut;

trait Foo: BorrowMut<u32> + CloneToUninit + Debug {}

impl<T: BorrowMut<u32> + CloneToUninit + Debug> Foo for T {}

fn main() {
    let foo: Rc<dyn Foo> = Rc::new(42_u32);
    let mut bar = foo.clone();
    *Rc::make_mut(&mut bar).borrow_mut() = 37;
    dbg!(foo, bar); // 42, 37
}
```

</details>

Eventually, `Box::<T>::clone` is planned to be converted to use `T::clone_to_uninit`, which when combined with this change, will allow cloning `Box<dyn Foo>` where `trait Foo: CloneToUninit` without any additional `unsafe` code for the author of `trait Foo`.[^1]

This PR should have no stable side-effects, as `CloneToUninit` is unstable so cannot be mentioned on stable, and `CloneToUninit` is not used as a supertrait anywhere in the stdlib.

This change removes some length checks that could only fail if library UB was already hit (e.g. calling `<[T]>::clone_to_uninit` with a too-small-length `dst` is library UB and was previously detected[^2]; since `dst` does not have a length anymore, this now cannot be detected[^3]).

r? libs-api

-----

I chose to make the parameter `*mut u8` instead of `*mut ()` because that might make it simpler to pass the result of `alloc` to `clone_to_uninit`, but `*mut ()` would also make sense, and any `*mut ConcreteType` would *work*. The original motivation for [using specifically `*mut ()`](https://github.com/rust-lang/rust/pull/116113#discussion_r1335303908) appears to be `std::ptr::from_raw_parts_mut`, but that now [takes `*mut impl Thin`](https://doc.rust-lang.org/nightly/std/ptr/fn.from_raw_parts.html) instead of `*mut ()`. I have another branch where the parameter is `*mut ()`, if that is preferred.

It *could* also take something like `&mut [MaybeUninit<u8>]` to be dyn-compatible but still allow size-checking and in some cases safe writing, but this is already an `unsafe` API where misuse is UB, so I'm not sure how many guardrails it's worth adding here, and `&mut [MaybeUninit<u8>]` might be overly cumbersome to construct for callers compared to `*mut u8`

[^1]:  Note that  `impl<T: CloneToUninit + ?Sized> Clone for Box` must be added before or at the same time as when `CloneToUninit` becomes stable, due to `Box` being `#[fundamental]`, as if there is any stable gap between the stabilization of `CloneToUninit` and `impl<T: CloneToUninit + ?Sized> Clone for Box`, then users could implement both `CloneToUninit for dyn LocalTrait` and separately `Clone for Box<dyn LocalTrait>` during that gap, and be broken by the introduction of  `impl<T: CloneToUninit + ?Sized> Clone for Box`.

[^2]: Using a `debug_assert_eq` in [`core::clone::uninit::CopySpec::clone_slice`](https://doc.rust-lang.org/nightly/src/core/clone/uninit.rs.html#28).

[^3]: This PR just uses [the metadata (length) from `self`](https://github.com/zachs18/rust/blob/e0c1c8bc5058cd3f8831b235c5963ab89840b33b/library/core/src/clone.rs#L286) to construct the `*mut [T]` to pass to `CopySpec::clone_slice` in `<[T]>::clone_to_uninit`.
Diffstat (limited to 'library/std')
-rw-r--r--library/std/src/ffi/os_str.rs8
-rw-r--r--library/std/src/ffi/os_str/tests.rs4
-rw-r--r--library/std/src/path.rs8
-rw-r--r--library/std/src/path/tests.rs4
-rw-r--r--library/std/src/sys/os_str/bytes.rs6
-rw-r--r--library/std/src/sys/os_str/wtf8.rs6
-rw-r--r--library/std/src/sys_common/wtf8.rs6
7 files changed, 21 insertions, 21 deletions
diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs
index b19d482feaa..79dfb47d0c4 100644
--- a/library/std/src/ffi/os_str.rs
+++ b/library/std/src/ffi/os_str.rs
@@ -112,7 +112,7 @@ impl crate::sealed::Sealed for OsString {}
 /// [conversions]: super#conversions
 #[cfg_attr(not(test), rustc_diagnostic_item = "OsStr")]
 #[stable(feature = "rust1", since = "1.0.0")]
-// `OsStr::from_inner` current implementation relies
+// `OsStr::from_inner` and `impl CloneToUninit for OsStr` current implementation relies
 // on `OsStr` being layout-compatible with `Slice`.
 // However, `OsStr` layout is considered an implementation detail and must not be relied upon.
 #[repr(transparent)]
@@ -1278,9 +1278,9 @@ impl Clone for Box<OsStr> {
 unsafe impl CloneToUninit for OsStr {
     #[inline]
     #[cfg_attr(debug_assertions, track_caller)]
-    unsafe fn clone_to_uninit(&self, dst: *mut Self) {
-        // SAFETY: we're just a wrapper around a platform-specific Slice
-        unsafe { self.inner.clone_to_uninit(&raw mut (*dst).inner) }
+    unsafe fn clone_to_uninit(&self, dst: *mut u8) {
+        // SAFETY: we're just a transparent wrapper around a platform-specific Slice
+        unsafe { self.inner.clone_to_uninit(dst) }
     }
 }
 
diff --git a/library/std/src/ffi/os_str/tests.rs b/library/std/src/ffi/os_str/tests.rs
index 67147934b4d..cbec44c8626 100644
--- a/library/std/src/ffi/os_str/tests.rs
+++ b/library/std/src/ffi/os_str/tests.rs
@@ -294,12 +294,12 @@ fn clone_to_uninit() {
     let a = OsStr::new("hello.txt");
 
     let mut storage = vec![MaybeUninit::<u8>::uninit(); size_of_val::<OsStr>(a)];
-    unsafe { a.clone_to_uninit(ptr::from_mut::<[_]>(storage.as_mut_slice()) as *mut OsStr) };
+    unsafe { a.clone_to_uninit(ptr::from_mut::<[_]>(storage.as_mut_slice()).cast()) };
     assert_eq!(a.as_encoded_bytes(), unsafe { MaybeUninit::slice_assume_init_ref(&storage) });
 
     let mut b: Box<OsStr> = OsStr::new("world.exe").into();
     assert_eq!(size_of_val::<OsStr>(a), size_of_val::<OsStr>(&b));
     assert_ne!(a, &*b);
-    unsafe { a.clone_to_uninit(ptr::from_mut::<OsStr>(&mut b)) };
+    unsafe { a.clone_to_uninit(ptr::from_mut::<OsStr>(&mut b).cast()) };
     assert_eq!(a, &*b);
 }
diff --git a/library/std/src/path.rs b/library/std/src/path.rs
index 5662a44d832..b0291e3aa19 100644
--- a/library/std/src/path.rs
+++ b/library/std/src/path.rs
@@ -2128,7 +2128,7 @@ impl AsRef<OsStr> for PathBuf {
 /// ```
 #[cfg_attr(not(test), rustc_diagnostic_item = "Path")]
 #[stable(feature = "rust1", since = "1.0.0")]
-// `Path::new` current implementation relies
+// `Path::new` and `impl CloneToUninit for Path` current implementation relies
 // on `Path` being layout-compatible with `OsStr`.
 // However, `Path` layout is considered an implementation detail and must not be relied upon.
 #[repr(transparent)]
@@ -3170,9 +3170,9 @@ impl Path {
 unsafe impl CloneToUninit for Path {
     #[inline]
     #[cfg_attr(debug_assertions, track_caller)]
-    unsafe fn clone_to_uninit(&self, dst: *mut Self) {
-        // SAFETY: Path is just a wrapper around OsStr
-        unsafe { self.inner.clone_to_uninit(&raw mut (*dst).inner) }
+    unsafe fn clone_to_uninit(&self, dst: *mut u8) {
+        // SAFETY: Path is just a transparent wrapper around OsStr
+        unsafe { self.inner.clone_to_uninit(dst) }
     }
 }
 
diff --git a/library/std/src/path/tests.rs b/library/std/src/path/tests.rs
index b75793d2bc9..ff3f7151bb8 100644
--- a/library/std/src/path/tests.rs
+++ b/library/std/src/path/tests.rs
@@ -2068,7 +2068,7 @@ fn clone_to_uninit() {
     let a = Path::new("hello.txt");
 
     let mut storage = vec![MaybeUninit::<u8>::uninit(); size_of_val::<Path>(a)];
-    unsafe { a.clone_to_uninit(ptr::from_mut::<[_]>(storage.as_mut_slice()) as *mut Path) };
+    unsafe { a.clone_to_uninit(ptr::from_mut::<[_]>(storage.as_mut_slice()).cast()) };
     assert_eq!(a.as_os_str().as_encoded_bytes(), unsafe {
         MaybeUninit::slice_assume_init_ref(&storage)
     });
@@ -2076,6 +2076,6 @@ fn clone_to_uninit() {
     let mut b: Box<Path> = Path::new("world.exe").into();
     assert_eq!(size_of_val::<Path>(a), size_of_val::<Path>(&b));
     assert_ne!(a, &*b);
-    unsafe { a.clone_to_uninit(ptr::from_mut::<Path>(&mut b)) };
+    unsafe { a.clone_to_uninit(ptr::from_mut::<Path>(&mut b).cast()) };
     assert_eq!(a, &*b);
 }
diff --git a/library/std/src/sys/os_str/bytes.rs b/library/std/src/sys/os_str/bytes.rs
index 8e0609fe48c..5b65d862be1 100644
--- a/library/std/src/sys/os_str/bytes.rs
+++ b/library/std/src/sys/os_str/bytes.rs
@@ -352,8 +352,8 @@ impl Slice {
 unsafe impl CloneToUninit for Slice {
     #[inline]
     #[cfg_attr(debug_assertions, track_caller)]
-    unsafe fn clone_to_uninit(&self, dst: *mut Self) {
-        // SAFETY: we're just a wrapper around [u8]
-        unsafe { self.inner.clone_to_uninit(&raw mut (*dst).inner) }
+    unsafe fn clone_to_uninit(&self, dst: *mut u8) {
+        // SAFETY: we're just a transparent wrapper around [u8]
+        unsafe { self.inner.clone_to_uninit(dst) }
     }
 }
diff --git a/library/std/src/sys/os_str/wtf8.rs b/library/std/src/sys/os_str/wtf8.rs
index b3834388df6..a4ad5966afe 100644
--- a/library/std/src/sys/os_str/wtf8.rs
+++ b/library/std/src/sys/os_str/wtf8.rs
@@ -275,8 +275,8 @@ impl Slice {
 unsafe impl CloneToUninit for Slice {
     #[inline]
     #[cfg_attr(debug_assertions, track_caller)]
-    unsafe fn clone_to_uninit(&self, dst: *mut Self) {
-        // SAFETY: we're just a wrapper around Wtf8
-        unsafe { self.inner.clone_to_uninit(&raw mut (*dst).inner) }
+    unsafe fn clone_to_uninit(&self, dst: *mut u8) {
+        // SAFETY: we're just a transparent wrapper around Wtf8
+        unsafe { self.inner.clone_to_uninit(dst) }
     }
 }
diff --git a/library/std/src/sys_common/wtf8.rs b/library/std/src/sys_common/wtf8.rs
index 19d4c94f450..666942bb8a1 100644
--- a/library/std/src/sys_common/wtf8.rs
+++ b/library/std/src/sys_common/wtf8.rs
@@ -1052,8 +1052,8 @@ impl Hash for Wtf8 {
 unsafe impl CloneToUninit for Wtf8 {
     #[inline]
     #[cfg_attr(debug_assertions, track_caller)]
-    unsafe fn clone_to_uninit(&self, dst: *mut Self) {
-        // SAFETY: we're just a wrapper around [u8]
-        unsafe { self.bytes.clone_to_uninit(&raw mut (*dst).bytes) }
+    unsafe fn clone_to_uninit(&self, dst: *mut u8) {
+        // SAFETY: we're just a transparent wrapper around [u8]
+        unsafe { self.bytes.clone_to_uninit(dst) }
     }
 }