Skip to content

Commit

Permalink
Fix wrong implementation of make_mut() for IArray::Single (#50)
Browse files Browse the repository at this point in the history
  • Loading branch information
cecton authored Feb 23, 2024
1 parent 4f02849 commit 967ccac
Showing 1 changed file with 49 additions and 18 deletions.
67 changes: 49 additions & 18 deletions src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,28 +263,65 @@ impl<T: ImplicitClone + 'static> IArray<T> {
/// a mutable slice of the contained data without any cloning. Otherwise, it clones the
/// data into a new array and returns a mutable slice into that.
///
/// # Example
/// If this array is a `Static`, it clones its elements into a new `Rc` array and returns a
/// mutable slice into that new array.
///
/// If this array is a `Single` element, the inner array is returned directly.
///
/// # Examples
///
/// ```
/// # use implicit_clone::unsync::*;
/// # use std::rc::Rc;
/// // This will reuse the Rc storage
/// let mut v1 = IArray::<u8>::Rc(Rc::new([1,2,3]));
/// v1.make_mut()[1] = 123;
/// assert_eq!(&[1,123,3], v1.as_slice());
/// let mut data = IArray::<u8>::from(vec![1,2,3]);
/// data.make_mut()[1] = 123;
/// assert_eq!(&[1,123,3], data.as_slice());
/// assert!(matches!(data, IArray::<u8>::Rc(_)));
/// ```
///
/// ```
/// # use implicit_clone::unsync::*;
/// // This will create a new copy
/// let mut v2 = IArray::<u8>::Static(&[1,2,3]);
/// v2.make_mut()[1] = 123;
/// assert_eq!(&[1,123,3], v2.as_slice());
/// let mut data = IArray::<u8>::from(vec![1,2,3]);
/// let other_data = data.clone();
/// data.make_mut()[1] = 123;
/// assert_eq!(&[1,123,3], data.as_slice());
/// assert_eq!(&[1,2,3], other_data.as_slice());
/// assert!(matches!(data, IArray::<u8>::Rc(_)));
/// assert!(matches!(other_data, IArray::<u8>::Rc(_)));
/// ```
///
/// ```
/// # use implicit_clone::unsync::*;
/// // This will create a new copy
/// let mut data = IArray::<u8>::Static(&[1,2,3]);
/// let other_data = data.clone();
/// data.make_mut()[1] = 123;
/// assert_eq!(&[1,123,3], data.as_slice());
/// assert_eq!(&[1,2,3], other_data.as_slice());
/// assert!(matches!(data, IArray::<u8>::Rc(_)));
/// assert!(matches!(other_data, IArray::<u8>::Static(_)));
/// ```
///
/// ```
/// # use implicit_clone::unsync::*;
/// // This will use the inner array directly
/// let mut data = IArray::<u8>::Single([1]);
/// let other_data = data.clone();
/// data.make_mut()[0] = 123;
/// assert_eq!(&[123], data.as_slice());
/// assert_eq!(&[1], other_data.as_slice());
/// assert!(matches!(data, IArray::<u8>::Single(_)));
/// assert!(matches!(other_data, IArray::<u8>::Single(_)));
/// ```
#[inline]
pub fn make_mut(&mut self) -> &mut [T] {
// This code is somewhat weirdly written to work around https://github.com/rust-lang/rust/issues/54663 -
// we can't just check if this is an Rc with one reference with get_mut in an if branch and copy otherwise,
// since returning the mutable slice extends its lifetime for the rest of the function.
match self {
Self::Rc(ref mut rc) => {
// This code is somewhat weirdly written to work around
// https://github.com/rust-lang/rust/issues/54663 - we can't just check if this is
// an Rc with one reference with get_mut in an if branch and copy otherwise, since
// returning the mutable slice extends its lifetime for the rest of the function.
if Rc::get_mut(rc).is_none() {
*rc = rc.iter().cloned().collect::<Rc<[T]>>();
}
Expand All @@ -297,13 +334,7 @@ impl<T: ImplicitClone + 'static> IArray<T> {
_ => unreachable!(),
}
}
Self::Single(slice) => {
*self = Self::Rc(slice.iter().cloned().collect());
match self {
Self::Rc(rc) => Rc::get_mut(rc).unwrap(),
_ => unreachable!(),
}
}
Self::Single(array) => array,
}
}
}
Expand Down

0 comments on commit 967ccac

Please sign in to comment.