Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand CloneToUninit documentation. #133055

Merged
merged 3 commits into from
Mar 16, 2025
Merged

Expand CloneToUninit documentation. #133055

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
769425a
Expand `CloneToUninit` documentation.
kpreid Nov 14, 2024
2cc999d
Rewrite comments about dropping and leaking.
kpreid Mar 10, 2025
96814ae
Rewrite example to not deal with `Copy` at all.
kpreid Mar 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 131 additions & 28 deletions library/core/src/clone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,43 +262,146 @@ pub struct AssertParamIsCopy<T: Copy + ?Sized> {
_field: crate::marker::PhantomData<T>,
}

/// A generalization of [`Clone`] to dynamically-sized types stored in arbitrary containers.
/// A generalization of [`Clone`] to [dynamically-sized types][DST] stored in arbitrary containers.
///
/// This trait is implemented for all types implementing [`Clone`], and also [slices](slice) of all
/// such types. You may also implement this trait to enable cloning trait objects and custom DSTs
/// (structures containing dynamically-sized fields).
/// This trait is implemented for all types implementing [`Clone`], [slices](slice) of all
/// such types, and other dynamically-sized types in the standard library.
/// You may also implement this trait to enable cloning custom DSTs
/// (structures containing dynamically-sized fields), or use it as a supertrait to enable
/// cloning a [trait object].
///
/// This trait is normally used via operations on container types which support DSTs,
/// so you should not typically need to call `.clone_to_uninit()` explicitly except when
/// implementing such a container or otherwise performing explicit management of an allocation,
/// or when implementing `CloneToUninit` itself.
///
/// # Safety
///
/// Implementations must ensure that when `.clone_to_uninit(dst)` returns normally rather than
/// panicking, it always leaves `*dst` initialized as a valid value of type `Self`.
/// Implementations must ensure that when `.clone_to_uninit(dest)` returns normally rather than
/// panicking, it always leaves `*dest` initialized as a valid value of type `Self`.
///
/// # Examples
///
// FIXME(#126799): when `Box::clone` allows use of `CloneToUninit`, rewrite these examples with it
// since `Rc` is a distraction.
///
/// If you are defining a trait, you can add `CloneToUninit` as a supertrait to enable cloning of
/// `dyn` values of your trait:
///
/// ```
/// #![feature(clone_to_uninit)]
/// use std::rc::Rc;
///
/// trait Foo: std::fmt::Debug + std::clone::CloneToUninit {
/// fn modify(&mut self);
/// fn value(&self) -> i32;
/// }
///
/// impl Foo for i32 {
/// fn modify(&mut self) {
/// *self *= 10;
/// }
/// fn value(&self) -> i32 {
/// *self
/// }
/// }
///
/// let first: Rc<dyn Foo> = Rc::new(1234);
///
/// let mut second = first.clone();
/// Rc::make_mut(&mut second).modify(); // make_mut() will call clone_to_uninit()
///
/// assert_eq!(first.value(), 1234);
/// assert_eq!(second.value(), 12340);
/// ```
///
/// The following is an example of implementing `CloneToUninit` for a custom DST.
/// (It is essentially a limited form of what `derive(CloneToUninit)` would do,
/// if such a derive macro existed.)
///
/// # See also
/// ```
/// #![feature(clone_to_uninit)]
/// use std::clone::CloneToUninit;
/// use std::mem::offset_of;
/// use std::rc::Rc;
///
/// #[derive(PartialEq)]
/// struct MyDst<T: ?Sized> {
/// flag: bool,
/// contents: T,
/// }
///
/// * [`Clone::clone_from`] is a safe function which may be used instead when `Self` is a [`Sized`]
/// unsafe impl<T: ?Sized + CloneToUninit> CloneToUninit for MyDst<T> {
/// unsafe fn clone_to_uninit(&self, dest: *mut u8) {
/// // The offset of `self.contents` is dynamic because it depends on the alignment of T
/// // which can be dynamic (if `T = dyn SomeTrait`). Therefore, we have to obtain it
/// // dynamically by examining `self`, rather than using `offset_of!`.
/// let offset_of_contents =
/// (&raw const self.contents).byte_offset_from_unsigned(&raw const *self);
///
/// // Since `flag` implements `Copy`, we can just copy it.
/// // We use `pointer::write()` instead of assignment because the destination must be
/// // assumed to be uninitialized, whereas an assignment assumes it is initialized.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think assignment assumes initialized memory for Copy types, and Miri is fine with it: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=2d5d4affa1262eff4a7c5d1a2a7fd14b

The unsound operation is dropping the memory behind the pointer, but for Copy types that doesn't happen.

I'd probably still write it this way, but I don't think we should write something misleading in the comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to write a good comment, and was unhappy with all the versions I came up with without also changing the code. I ended up changing the code to .clone_to_uninit() the flag too, because it's easier to justify as “most general”. Not sure that makes sense.

— actually, it doesn't make sense, because the easiest way to ensure the desirable no-memory-leaks property would be to strictly use .clone() on all the sized fields first (so the values are held in drop-tracked locals), then clone the uninit field. And then I don't have to hedge anything about leaking. Please wait while I rewrite again…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now rewritten the example so that

  • the sized field is not Copy, and
  • neither field is leaked on panic,

and there is one straightforward correct way to do that, so we no longer have any reason to talk about the other options. What do you think?

/// dest.add(offset_of!(Self, flag)).cast::<bool>().write(self.flag);
///
/// // Note: if `flag` owned any resources (i.e. had a `Drop` implementation), then we
/// // must prepare to drop it in case `self.contents.clone_to_uninit()` panics.
/// // In this simple case, where we have exactly one field for which `mem::needs_drop()`
/// // might be true (`contents`), we don’t need to care about cleanup or ordering.
/// self.contents.clone_to_uninit(dest.add(offset_of_contents));
///
/// // All fields of the struct have been initialized, therefore the struct is initialized,
/// // and we have satisfied our `unsafe impl CloneToUninit` obligations.
/// }
/// }
///
/// fn main() {
/// // Construct MyDst<[u8; 4]>, then coerce to MyDst<[u8]>.
/// let first: Rc<MyDst<[u8]>> = Rc::new(MyDst {
/// flag: true,
/// contents: [1, 2, 3, 4],
/// });
///
/// let mut second = first.clone();
/// // make_mut() will call clone_to_uninit().
/// for elem in Rc::make_mut(&mut second).contents.iter_mut() {
/// *elem *= 10;
/// }
///
/// assert_eq!(first.contents, [1, 2, 3, 4]);
/// assert_eq!(second.contents, [10, 20, 30, 40]);
/// }
/// ```
///
/// # See Also
///
/// * [`Clone::clone_from`] is a safe function which may be used instead when [`Self: Sized`](Sized)
/// and the destination is already initialized; it may be able to reuse allocations owned by
/// the destination.
/// the destination, whereas `clone_to_uninit` cannot, since its destination is assumed to be
/// uninitialized.
/// * [`ToOwned`], which allocates a new destination container.
///
/// [`ToOwned`]: ../../std/borrow/trait.ToOwned.html
/// [DST]: https://doc.rust-lang.org/reference/dynamically-sized-types.html
/// [trait object]: https://doc.rust-lang.org/reference/types/trait-object.html
#[unstable(feature = "clone_to_uninit", issue = "126799")]
pub unsafe trait CloneToUninit {
/// Performs copy-assignment from `self` to `dst`.
/// Performs copy-assignment from `self` to `dest`.
///
/// This is analogous to `std::ptr::write(dst.cast(), self.clone())`,
/// except that `self` may be a dynamically-sized type ([`!Sized`](Sized)).
/// This is analogous to `std::ptr::write(dest.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
/// Before this function is called, `dest` may point to uninitialized memory.
/// After this function is called, `dest` will point to initialized memory; it will be
/// 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 for `size_of_val(self)` bytes.
/// * `dst` must be properly aligned to `align_of_val(self)`.
/// * `dest` must be [valid] for writes for `size_of_val(self)` bytes.
/// * `dest` must be properly aligned to `align_of_val(self)`.
///
/// [valid]: crate::ptr#safety
/// [pointer metadata]: crate::ptr::metadata()
Expand All @@ -307,60 +410,60 @@ pub unsafe trait CloneToUninit {
///
/// This function may panic. (For example, it might panic if memory allocation for a clone
/// of a value owned by `self` fails.)
/// If the call panics, then `*dst` should be treated as uninitialized memory; it must not be
/// If the call panics, then `*dest` should be treated as uninitialized memory; it must not be
/// read or dropped, because even if it was previously valid, it may have been partially
/// overwritten.
///
/// The caller may also need to take care to deallocate the allocation pointed to by `dst`,
/// The caller may also need to take care to deallocate the allocation pointed to by `dest`,
/// if applicable, to avoid a memory leak, and may need to take other precautions to ensure
/// soundness in the presence of unwinding.
///
/// Implementors should avoid leaking values by, upon unwinding, dropping all component values
/// 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 u8);
unsafe fn clone_to_uninit(&self, dest: *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 u8) {
unsafe fn clone_to_uninit(&self, dest: *mut u8) {
// SAFETY: we're calling a specialization with the same contract
unsafe { <T as self::uninit::CopySpec>::clone_one(self, dst.cast::<T>()) }
unsafe { <T as self::uninit::CopySpec>::clone_one(self, dest.cast::<T>()) }
}
}

#[unstable(feature = "clone_to_uninit", issue = "126799")]
unsafe impl<T: Clone> CloneToUninit for [T] {
#[inline]
#[cfg_attr(debug_assertions, track_caller)]
unsafe fn clone_to_uninit(&self, dst: *mut u8) {
let dst: *mut [T] = dst.with_metadata_of(self);
unsafe fn clone_to_uninit(&self, dest: *mut u8) {
let dest: *mut [T] = dest.with_metadata_of(self);
// SAFETY: we're calling a specialization with the same contract
unsafe { <T as self::uninit::CopySpec>::clone_slice(self, dst) }
unsafe { <T as self::uninit::CopySpec>::clone_slice(self, dest) }
}
}

#[unstable(feature = "clone_to_uninit", issue = "126799")]
unsafe impl CloneToUninit for str {
#[inline]
#[cfg_attr(debug_assertions, track_caller)]
unsafe fn clone_to_uninit(&self, dst: *mut u8) {
unsafe fn clone_to_uninit(&self, dest: *mut u8) {
// SAFETY: str is just a [u8] with UTF-8 invariant
unsafe { self.as_bytes().clone_to_uninit(dst) }
unsafe { self.as_bytes().clone_to_uninit(dest) }
}
}

#[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 u8) {
unsafe fn clone_to_uninit(&self, dest: *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 (so NUL is also copied).
// See: `cstr_metadata_is_length_with_nul` in tests.
unsafe { self.to_bytes_with_nul().clone_to_uninit(dst) }
unsafe { self.to_bytes_with_nul().clone_to_uninit(dest) }
}
}

Expand Down
Loading