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`](e0c1c8bc50/library/core/src/clone.rs (L286)) to construct the `*mut [T]` to pass to `CopySpec::clone_slice` in `<[T]>::clone_to_uninit`.
This commit is contained in:
commit
17dcadd587
12 changed files with 45 additions and 44 deletions
|
|
@ -1735,7 +1735,7 @@ impl<T: Clone, A: Allocator + Clone> Clone for Box<T, A> {
|
|||
// Pre-allocate memory to allow writing the cloned value directly.
|
||||
let mut boxed = Self::new_uninit_in(self.1.clone());
|
||||
unsafe {
|
||||
(**self).clone_to_uninit(boxed.as_mut_ptr());
|
||||
(**self).clone_to_uninit(boxed.as_mut_ptr().cast());
|
||||
boxed.assume_init()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1876,7 +1876,7 @@ impl<T: ?Sized + CloneToUninit, A: Allocator + Clone> Rc<T, A> {
|
|||
// Initialize with clone of this.
|
||||
let initialized_clone = unsafe {
|
||||
// Clone. If the clone panics, `in_progress` will be dropped and clean up.
|
||||
this_data_ref.clone_to_uninit(in_progress.data_ptr());
|
||||
this_data_ref.clone_to_uninit(in_progress.data_ptr().cast());
|
||||
// Cast type of pointer, now that it is initialized.
|
||||
in_progress.into_rc()
|
||||
};
|
||||
|
|
|
|||
|
|
@ -2272,7 +2272,7 @@ impl<T: ?Sized + CloneToUninit, A: Allocator + Clone> Arc<T, A> {
|
|||
|
||||
let initialized_clone = unsafe {
|
||||
// Clone. If the clone panics, `in_progress` will be dropped and clean up.
|
||||
this_data_ref.clone_to_uninit(in_progress.data_ptr());
|
||||
this_data_ref.clone_to_uninit(in_progress.data_ptr().cast());
|
||||
// Cast type of pointer, now that it is initialized.
|
||||
in_progress.into_arc()
|
||||
};
|
||||
|
|
|
|||
|
|
@ -232,20 +232,20 @@ pub struct AssertParamIsCopy<T: Copy + ?Sized> {
|
|||
pub unsafe trait CloneToUninit {
|
||||
/// Performs copy-assignment from `self` to `dst`.
|
||||
///
|
||||
/// This is analogous to `std::ptr::write(dst, self.clone())`,
|
||||
/// This is analogous to `std::ptr::write(dst.cast(), self.clone())`,
|
||||
/// except that `self` may be a dynamically-sized type ([`!Sized`](Sized)).
|
||||
///
|
||||
/// Before this function is called, `dst` may point to uninitialized memory.
|
||||
/// After this function is called, `dst` will point to initialized memory; it will be
|
||||
/// sound to create a `&Self` reference from the pointer.
|
||||
/// sound to create a `&Self` reference from the pointer with the [pointer metadata]
|
||||
/// from `self`.
|
||||
///
|
||||
/// # Safety
|
||||
///
|
||||
/// Behavior is undefined if any of the following conditions are violated:
|
||||
///
|
||||
/// * `dst` must be [valid] for writes.
|
||||
/// * `dst` must be properly aligned.
|
||||
/// * `dst` must have the same [pointer metadata] (slice length or `dyn` vtable) as `self`.
|
||||
/// * `dst` must be [valid] for writes for `std::mem::size_of_val(self)` bytes.
|
||||
/// * `dst` must be properly aligned to `std::mem::align_of_val(self)`.
|
||||
///
|
||||
/// [valid]: crate::ptr#safety
|
||||
/// [pointer metadata]: crate::ptr::metadata()
|
||||
|
|
@ -266,15 +266,15 @@ pub unsafe trait CloneToUninit {
|
|||
/// that might have already been created. (For example, if a `[Foo]` of length 3 is being
|
||||
/// cloned, and the second of the three calls to `Foo::clone()` unwinds, then the first `Foo`
|
||||
/// cloned should be dropped.)
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut Self);
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut u8);
|
||||
}
|
||||
|
||||
#[unstable(feature = "clone_to_uninit", issue = "126799")]
|
||||
unsafe impl<T: Clone> CloneToUninit for T {
|
||||
#[inline]
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut Self) {
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut u8) {
|
||||
// SAFETY: we're calling a specialization with the same contract
|
||||
unsafe { <T as self::uninit::CopySpec>::clone_one(self, dst) }
|
||||
unsafe { <T as self::uninit::CopySpec>::clone_one(self, dst.cast::<T>()) }
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -282,7 +282,8 @@ unsafe impl<T: Clone> CloneToUninit for T {
|
|||
unsafe impl<T: Clone> CloneToUninit for [T] {
|
||||
#[inline]
|
||||
#[cfg_attr(debug_assertions, track_caller)]
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut Self) {
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut u8) {
|
||||
let dst: *mut [T] = dst.with_metadata_of(self);
|
||||
// SAFETY: we're calling a specialization with the same contract
|
||||
unsafe { <T as self::uninit::CopySpec>::clone_slice(self, dst) }
|
||||
}
|
||||
|
|
@ -292,21 +293,21 @@ unsafe impl<T: Clone> CloneToUninit for [T] {
|
|||
unsafe impl CloneToUninit for str {
|
||||
#[inline]
|
||||
#[cfg_attr(debug_assertions, track_caller)]
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut Self) {
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut u8) {
|
||||
// SAFETY: str is just a [u8] with UTF-8 invariant
|
||||
unsafe { self.as_bytes().clone_to_uninit(dst as *mut [u8]) }
|
||||
unsafe { self.as_bytes().clone_to_uninit(dst) }
|
||||
}
|
||||
}
|
||||
|
||||
#[unstable(feature = "clone_to_uninit", issue = "126799")]
|
||||
unsafe impl CloneToUninit for crate::ffi::CStr {
|
||||
#[cfg_attr(debug_assertions, track_caller)]
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut Self) {
|
||||
unsafe fn clone_to_uninit(&self, dst: *mut u8) {
|
||||
// SAFETY: For now, CStr is just a #[repr(trasnsparent)] [c_char] with some invariants.
|
||||
// And we can cast [c_char] to [u8] on all supported platforms (see: to_bytes_with_nul).
|
||||
// The pointer metadata properly preserves the length (NUL included).
|
||||
// The pointer metadata properly preserves the length (so NUL is also copied).
|
||||
// See: `cstr_metadata_is_length_with_nul` in tests.
|
||||
unsafe { self.to_bytes_with_nul().clone_to_uninit(dst as *mut [u8]) }
|
||||
unsafe { self.to_bytes_with_nul().clone_to_uninit(dst) }
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ fn test_clone_to_uninit_slice_success() {
|
|||
|
||||
let mut storage: MaybeUninit<[String; 3]> = MaybeUninit::uninit();
|
||||
let b: [String; 3] = unsafe {
|
||||
a[..].clone_to_uninit(storage.as_mut_ptr() as *mut [String]);
|
||||
a[..].clone_to_uninit(storage.as_mut_ptr().cast());
|
||||
storage.assume_init()
|
||||
};
|
||||
|
||||
|
|
@ -70,7 +70,7 @@ fn test_clone_to_uninit_slice_drops_on_panic() {
|
|||
let mut storage: MaybeUninit<[CountsDropsAndPanics; 3]> = MaybeUninit::uninit();
|
||||
// This should panic halfway through
|
||||
unsafe {
|
||||
a[..].clone_to_uninit(storage.as_mut_ptr() as *mut [CountsDropsAndPanics]);
|
||||
a[..].clone_to_uninit(storage.as_mut_ptr().cast());
|
||||
}
|
||||
})
|
||||
.unwrap_err();
|
||||
|
|
@ -89,13 +89,13 @@ fn test_clone_to_uninit_str() {
|
|||
let a = "hello";
|
||||
|
||||
let mut storage: MaybeUninit<[u8; 5]> = MaybeUninit::uninit();
|
||||
unsafe { a.clone_to_uninit(storage.as_mut_ptr() as *mut [u8] as *mut str) };
|
||||
unsafe { a.clone_to_uninit(storage.as_mut_ptr().cast()) };
|
||||
assert_eq!(a.as_bytes(), unsafe { storage.assume_init() }.as_slice());
|
||||
|
||||
let mut b: Box<str> = "world".into();
|
||||
assert_eq!(a.len(), b.len());
|
||||
assert_ne!(a, &*b);
|
||||
unsafe { a.clone_to_uninit(ptr::from_mut::<str>(&mut b)) };
|
||||
unsafe { a.clone_to_uninit(ptr::from_mut::<str>(&mut b).cast()) };
|
||||
assert_eq!(a, &*b);
|
||||
}
|
||||
|
||||
|
|
@ -104,13 +104,13 @@ fn test_clone_to_uninit_cstr() {
|
|||
let a = c"hello";
|
||||
|
||||
let mut storage: MaybeUninit<[u8; 6]> = MaybeUninit::uninit();
|
||||
unsafe { a.clone_to_uninit(storage.as_mut_ptr() as *mut [u8] as *mut CStr) };
|
||||
unsafe { a.clone_to_uninit(storage.as_mut_ptr().cast()) };
|
||||
assert_eq!(a.to_bytes_with_nul(), unsafe { storage.assume_init() }.as_slice());
|
||||
|
||||
let mut b: Box<CStr> = c"world".into();
|
||||
assert_eq!(a.count_bytes(), b.count_bytes());
|
||||
assert_ne!(a, &*b);
|
||||
unsafe { a.clone_to_uninit(ptr::from_mut::<CStr>(&mut b)) };
|
||||
unsafe { a.clone_to_uninit(ptr::from_mut::<CStr>(&mut b).cast()) };
|
||||
assert_eq!(a, &*b);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue