Skip to content

Commit

Permalink
Rollup merge of rust-lang#122201 - coolreader18:doc-clone_from, r=dto…
Browse files Browse the repository at this point in the history
…lnay

Document overrides of `clone_from()` in core/std

As mentioned in rust-lang#96979 (comment)

Specifically, when an override doesn't just forward to an inner type, document the behavior and that it's preferred over simply assigning a clone of source. Also, change instances where the second parameter is "other" to "source".

I reused some of the wording over and over for similar impls, but I'm not sure that the wording is actually *good*. Would appreciate feedback about that.

Also, now some of these seem to provide pretty specific guarantees about behavior (e.g. will reuse the exact same allocation iff the len is the same), but I was basing it off of the docs for [`Box::clone_from`](https://doc.rust-lang.org/1.75.0/std/boxed/struct.Box.html#method.clone_from-1) - I'm not sure if providing those strong guarantees is actually good or not.
  • Loading branch information
matthiaskrgr authored Apr 17, 2024
2 parents d7d1070 + 87db7c3 commit 21deaed
Show file tree
Hide file tree
Showing 14 changed files with 133 additions and 25 deletions.
26 changes: 22 additions & 4 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2088,11 +2088,29 @@ impl<T: Clone, A: Allocator + Clone> Clone for Box<[T], A> {
self.to_vec_in(alloc).into_boxed_slice()
}

fn clone_from(&mut self, other: &Self) {
if self.len() == other.len() {
self.clone_from_slice(&other);
/// Copies `source`'s contents into `self` without creating a new allocation,
/// so long as the two are of the same length.
///
/// # Examples
///
/// ```
/// let x = Box::new([5, 6, 7]);
/// let mut y = Box::new([8, 9, 10]);
/// let yp: *const [i32] = &*y;
///
/// y.clone_from(&x);
///
/// // The value is the same
/// assert_eq!(x, y);
///
/// // And no allocation occurred
/// assert_eq!(yp, &*y);
/// ```
fn clone_from(&mut self, source: &Self) {
if self.len() == source.len() {
self.clone_from_slice(&source);
} else {
*self = other.clone();
*self = source.clone();
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions library/alloc/src/collections/binary_heap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,12 @@ impl<T: Clone, A: Allocator + Clone> Clone for BinaryHeap<T, A> {
BinaryHeap { data: self.data.clone() }
}

/// Overwrites the contents of `self` with a clone of the contents of `source`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible.
///
/// See [`Vec::clone_from()`] for more details.
fn clone_from(&mut self, source: &Self) {
self.data.clone_from(&source.data);
}
Expand Down
4 changes: 2 additions & 2 deletions library/alloc/src/collections/btree/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ impl<T: Clone, A: Allocator + Clone> Clone for BTreeSet<T, A> {
BTreeSet { map: self.map.clone() }
}

fn clone_from(&mut self, other: &Self) {
self.map.clone_from(&other.map);
fn clone_from(&mut self, source: &Self) {
self.map.clone_from(&source.map);
}
}

Expand Down
22 changes: 14 additions & 8 deletions library/alloc/src/collections/linked_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2126,16 +2126,22 @@ impl<T: Clone, A: Allocator + Clone> Clone for LinkedList<T, A> {
list
}

fn clone_from(&mut self, other: &Self) {
let mut iter_other = other.iter();
if self.len() > other.len() {
self.split_off(other.len());
/// Overwrites the contents of `self` with a clone of the contents of `source`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation of the nodes of the linked list. Additionally,
/// if the element type `T` overrides `clone_from()`, this will reuse the
/// resources of `self`'s elements as well.
fn clone_from(&mut self, source: &Self) {
let mut source_iter = source.iter();
if self.len() > source.len() {
self.split_off(source.len());
}
for (elem, elem_other) in self.iter_mut().zip(&mut iter_other) {
elem.clone_from(elem_other);
for (elem, source_elem) in self.iter_mut().zip(&mut source_iter) {
elem.clone_from(source_elem);
}
if !iter_other.is_empty() {
self.extend(iter_other.cloned());
if !source_iter.is_empty() {
self.extend(source_iter.cloned());
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions library/alloc/src/collections/vec_deque/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,13 @@ impl<T: Clone, A: Allocator + Clone> Clone for VecDeque<T, A> {
deq
}

fn clone_from(&mut self, other: &Self) {
/// Overwrites the contents of `self` with a clone of the contents of `source`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible.
fn clone_from(&mut self, source: &Self) {
self.clear();
self.extend(other.iter().cloned());
self.extend(source.iter().cloned());
}
}

Expand Down
4 changes: 4 additions & 0 deletions library/alloc/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2097,6 +2097,10 @@ impl Clone for String {
String { vec: self.vec.clone() }
}

/// Clones the contents of `source` into `self`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible.
fn clone_from(&mut self, source: &Self) {
self.vec.clone_from(&source.vec);
}
Expand Down
26 changes: 24 additions & 2 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2846,8 +2846,30 @@ impl<T: Clone, A: Allocator + Clone> Clone for Vec<T, A> {
crate::slice::to_vec(&**self, alloc)
}

fn clone_from(&mut self, other: &Self) {
crate::slice::SpecCloneIntoVec::clone_into(other.as_slice(), self);
/// Overwrites the contents of `self` with a clone of the contents of `source`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible. Additionally, if the element type
/// `T` overrides `clone_from()`, this will reuse the resources of `self`'s
/// elements as well.
///
/// # Examples
///
/// ```
/// let x = vec![5, 6, 7];
/// let mut y = vec![8, 9, 10];
/// let yp: *const i32 = y.as_ptr();
///
/// y.clone_from(&x);
///
/// // The value is the same
/// assert_eq!(x, y);
///
/// // And no reallocation occurred
/// assert_eq!(yp, y.as_ptr());
/// ```
fn clone_from(&mut self, source: &Self) {
crate::slice::SpecCloneIntoVec::clone_into(source.as_slice(), self);
}
}

Expand Down
6 changes: 3 additions & 3 deletions library/core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1277,11 +1277,11 @@ impl<T: Clone> Clone for RefCell<T> {

/// # Panics
///
/// Panics if `other` is currently mutably borrowed.
/// Panics if `source` is currently mutably borrowed.
#[inline]
#[track_caller]
fn clone_from(&mut self, other: &Self) {
self.get_mut().clone_from(&other.borrow())
fn clone_from(&mut self, source: &Self) {
self.get_mut().clone_from(&source.borrow())
}
}

Expand Down
4 changes: 2 additions & 2 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,8 +688,8 @@ impl<T: Clone> Clone for Reverse<T> {
}

#[inline]
fn clone_from(&mut self, other: &Self) {
self.0.clone_from(&other.0)
fn clone_from(&mut self, source: &Self) {
self.0.clone_from(&source.0)
}
}

Expand Down
36 changes: 36 additions & 0 deletions library/core/src/task/wake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,42 @@ impl Clone for Waker {
}
}

/// Assigns a clone of `source` to `self`, unless [`self.will_wake(source)`][Waker::will_wake] anyway.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids cloning the waker if `self` is already the same waker.
///
/// # Examples
///
/// ```
/// use std::future::Future;
/// use std::pin::Pin;
/// use std::sync::{Arc, Mutex};
/// use std::task::{Context, Poll, Waker};
///
/// struct Waiter {
/// shared: Arc<Mutex<Shared>>,
/// }
///
/// struct Shared {
/// waker: Waker,
/// // ...
/// }
///
/// impl Future for Waiter {
/// type Output = ();
/// fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<()> {
/// let mut shared = self.shared.lock().unwrap();
///
/// // update the waker
/// shared.waker.clone_from(cx.waker());
///
/// // readiness logic ...
/// # Poll::Ready(())
/// }
/// }
///
/// ```
#[inline]
fn clone_from(&mut self, source: &Self) {
if !self.will_wake(source) {
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/collections/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1271,8 +1271,8 @@ where
}

#[inline]
fn clone_from(&mut self, other: &Self) {
self.base.clone_from(&other.base);
fn clone_from(&mut self, source: &Self) {
self.base.clone_from(&source.base);
}
}

Expand Down
4 changes: 4 additions & 0 deletions library/std/src/collections/hash/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,10 @@ where
Self { base: self.base.clone() }
}

/// Overwrites the contents of `self` with a clone of the contents of `source`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible.
#[inline]
fn clone_from(&mut self, other: &Self) {
self.base.clone_from(&other.base);
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/ffi/os_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,10 @@ impl Clone for OsString {
OsString { inner: self.inner.clone() }
}

/// Clones the contents of `source` into `self`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible.
#[inline]
fn clone_from(&mut self, source: &Self) {
self.inner.clone_from(&source.inner)
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1628,6 +1628,10 @@ impl Clone for PathBuf {
PathBuf { inner: self.inner.clone() }
}

/// Clones the contents of `source` into `self`.
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible.
#[inline]
fn clone_from(&mut self, source: &Self) {
self.inner.clone_from(&source.inner)
Expand Down

0 comments on commit 21deaed

Please sign in to comment.