Skip to content

Commit d665f28

Browse files
Document lack of panic safety guarantees of Clone::clone_from
It isn't panic safe de-facto (e.g. for `Vec`) so I just document current behaviour. Panic safety was mentioned by @scottmcm when discussing [PR with deriving clone_from](#98445 (comment)). Co-authored-by: Jane Losare-Lusby <[email protected]>
1 parent 3d829a0 commit d665f28

File tree

1 file changed

+53
-3
lines changed

1 file changed

+53
-3
lines changed

library/core/src/clone.rs

+53-3
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,59 @@ pub trait Clone: Sized {
124124

125125
/// Performs copy-assignment from `source`.
126126
///
127-
/// `a.clone_from(&b)` is equivalent to `a = b.clone()` in functionality,
128-
/// but can be overridden to reuse the resources of `a` to avoid unnecessary
129-
/// allocations.
127+
/// `a.clone_from(&b)` is equivalent to `a = b.clone()` in functionality except cases
128+
/// when it panics in the middle of operation. It can be overridden to reuse the resources
129+
/// of `a` to avoid unnecessary allocations.
130+
///
131+
/// # Panic behaviour
132+
///
133+
/// Due to it's nature, this method cannot guarantee that it would be transactional.
134+
/// If call panics, `self` can be left in inconsistent state.
135+
/// This is different from `a = b.clone()` because in that case `a` is overwritten
136+
/// only if `clone()` succeedes. Therefore, if you need transactional behaviour,
137+
/// you shouldn't use this method.
138+
///
139+
/// `clone_from` is intended to preserve resources owned by `self`
140+
/// so it cannot preserve old data stored in those resources.
141+
///
142+
/// That example shows one case of such behaviour:
143+
/// ```no_run
144+
/// use std::sync::Mutex;
145+
///
146+
/// #[derive(PartialEq, Eq, Debug)]
147+
/// struct PanicAtTheClone(bool);
148+
///
149+
/// impl Clone for PanicAtTheClone {
150+
/// fn clone(&self) -> Self {
151+
/// if self.0 {
152+
/// panic!()
153+
/// } else {
154+
/// PanicAtTheClone(false)
155+
/// }
156+
/// }
157+
/// }
158+
///
159+
/// // first element will copy just fine, second element will panic if cloned!
160+
/// let src = vec![PanicAtTheClone(false), PanicAtTheClone(true)];
161+
/// // start with an empty vec
162+
/// let dest = Mutex::new(vec![]);
163+
///
164+
/// // clone from src into dest
165+
/// std::panic::catch_unwind(|| {
166+
/// dest.lock().unwrap().clone_from(&src);
167+
/// })
168+
/// .expect_err("Must panic");
169+
///
170+
/// // handle the poisoned mutex (without the mutex even attempting this produces a compiler error!)
171+
/// let guard = dest
172+
/// .lock()
173+
/// .expect_err("mutex should be poisoned by the panic")
174+
/// .into_inner();
175+
///
176+
/// // dest is left in an incomplete state with only half of the clone having
177+
/// // completed successfully
178+
/// assert_eq!(vec![PanicAtTheClone(false)], *guard);
179+
/// ```
130180
#[inline]
131181
#[stable(feature = "rust1", since = "1.0.0")]
132182
#[cfg_attr(bootstrap, default_method_body_is_const)]

0 commit comments

Comments
 (0)