From c0e913fdd7f35b197c6da198c25569fc63244b85 Mon Sep 17 00:00:00 2001 From: Noa Date: Fri, 8 Mar 2024 12:27:24 -0600 Subject: Document overrides of `clone_from()` 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". --- library/alloc/src/boxed.rs | 26 ++++++++++++++++++++---- library/alloc/src/collections/binary_heap/mod.rs | 6 ++++++ library/alloc/src/collections/btree/set.rs | 4 ++-- library/alloc/src/collections/linked_list.rs | 22 ++++++++++++-------- library/alloc/src/collections/vec_deque/mod.rs | 10 +++++++-- library/alloc/src/string.rs | 4 ++++ library/alloc/src/vec/mod.rs | 26 ++++++++++++++++++++++-- 7 files changed, 80 insertions(+), 18 deletions(-) (limited to 'library/alloc/src') diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 2736e5ee6c5..91247e7ad4c 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -2084,11 +2084,29 @@ impl 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(); } } } diff --git a/library/alloc/src/collections/binary_heap/mod.rs b/library/alloc/src/collections/binary_heap/mod.rs index 83b2678f7f5..c54f0062824 100644 --- a/library/alloc/src/collections/binary_heap/mod.rs +++ b/library/alloc/src/collections/binary_heap/mod.rs @@ -385,6 +385,12 @@ impl Clone for BinaryHeap { 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); } diff --git a/library/alloc/src/collections/btree/set.rs b/library/alloc/src/collections/btree/set.rs index ed91ae1a66e..2d42de4747c 100644 --- a/library/alloc/src/collections/btree/set.rs +++ b/library/alloc/src/collections/btree/set.rs @@ -116,8 +116,8 @@ impl Clone for BTreeSet { 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); } } diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs index 6dfb82ac807..1c90c171a15 100644 --- a/library/alloc/src/collections/linked_list.rs +++ b/library/alloc/src/collections/linked_list.rs @@ -2126,16 +2126,22 @@ impl Clone for LinkedList { 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()); } } } diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs index c35bab5ef66..8df7ca3411d 100644 --- a/library/alloc/src/collections/vec_deque/mod.rs +++ b/library/alloc/src/collections/vec_deque/mod.rs @@ -113,9 +113,15 @@ impl Clone for VecDeque { 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. 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) { self.clear(); - self.extend(other.iter().cloned()); + self.extend(source.iter().cloned()); } } diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index 98ded7f6cdf..fd36583ec8e 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -2084,6 +2084,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); } diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index c0e934b3b1f..b8ed9710839 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -2758,8 +2758,30 @@ impl Clone for Vec { 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); } } -- cgit 1.4.1-3-g733a5 From 87db7c32a70238785083b6de772cfd00e0adbc10 Mon Sep 17 00:00:00 2001 From: Noa Date: Wed, 17 Apr 2024 00:18:04 -0500 Subject: Address comments --- library/alloc/src/collections/vec_deque/mod.rs | 4 +--- library/core/src/cell.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'library/alloc/src') diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs index 8df7ca3411d..fb098952393 100644 --- a/library/alloc/src/collections/vec_deque/mod.rs +++ b/library/alloc/src/collections/vec_deque/mod.rs @@ -116,9 +116,7 @@ impl Clone for VecDeque { /// 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. + /// as it avoids reallocation if possible. fn clone_from(&mut self, source: &Self) { self.clear(); self.extend(source.iter().cloned()); diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 835edc4424d..5f3daca5008 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -1273,7 +1273,7 @@ impl Clone for RefCell { /// # Panics /// - /// Panics if `other` is currently mutably borrowed. + /// Panics if `source` is currently mutably borrowed. #[inline] #[track_caller] fn clone_from(&mut self, source: &Self) { -- cgit 1.4.1-3-g733a5