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.

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

Fix wrong implementation of make_mut() for IArray::Single #50

Merged
Merged
Changes from all commits
Commits
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
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
Loading