From bcea7a9b5c69224c5e8f5fe614d90136725c93a9 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 22 Feb 2024 11:10:33 +0100 Subject: [PATCH 1/4] Fix wrong implementation of make_mut() for IArray::Single --- src/array.rs | 61 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 15 deletions(-) diff --git a/src/array.rs b/src/array.rs index adc582f..180aaf4 100644 --- a/src/array.rs +++ b/src/array.rs @@ -263,20 +263,57 @@ impl IArray { /// 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 element is cloned into a new `Single` array and a + /// mutable slice into that new array is returned. + /// + /// # Examples /// /// ``` /// # use implicit_clone::unsync::*; - /// # use std::rc::Rc; /// // This will reuse the Rc storage - /// let mut v1 = IArray::::Rc(Rc::new([1,2,3])); - /// v1.make_mut()[1] = 123; - /// assert_eq!(&[1,123,3], v1.as_slice()); + /// let mut data = IArray::::Rc([1,2,3].into()); + /// data.make_mut()[1] = 123; + /// assert_eq!(&[1,123,3], data.as_slice()); + /// assert!(matches!(data, IArray::::Rc(_))); + /// ``` /// + /// ``` + /// # use implicit_clone::unsync::*; /// // This will create a new copy - /// let mut v2 = IArray::::Static(&[1,2,3]); - /// v2.make_mut()[1] = 123; - /// assert_eq!(&[1,123,3], v2.as_slice()); + /// let mut data = IArray::::Rc([1,2,3].into()); + /// 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::::Rc(_))); + /// assert!(matches!(other_data, IArray::::Rc(_))); + /// ``` + /// + /// ``` + /// # use implicit_clone::unsync::*; + /// // This will create a new copy + /// let mut data = IArray::::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::::Rc(_))); + /// assert!(matches!(other_data, IArray::::Static(_))); + /// ``` + /// + /// ``` + /// # use implicit_clone::unsync::*; + /// // This will use the inner array directly + /// let mut data = IArray::::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::::Single(_))); + /// assert!(matches!(other_data, IArray::::Single(_))); /// ``` #[inline] pub fn make_mut(&mut self) -> &mut [T] { @@ -297,13 +334,7 @@ impl IArray { _ => 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, } } } From 9710b83b7fdd21b8c6a4aa3bc5b880fdccb17578 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 22 Feb 2024 11:14:13 +0100 Subject: [PATCH 2/4] Fix doc --- src/array.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/array.rs b/src/array.rs index 180aaf4..279fa18 100644 --- a/src/array.rs +++ b/src/array.rs @@ -266,8 +266,7 @@ impl IArray { /// 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 element is cloned into a new `Single` array and a - /// mutable slice into that new array is returned. + /// If this array is a `Single` element, the inner array is returned directly. /// /// # Examples /// @@ -317,11 +316,12 @@ impl IArray { /// ``` #[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::>(); } From 2f792b3ea8741a167eec77dae3f939630261af04 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 22 Feb 2024 11:57:42 +0100 Subject: [PATCH 3/4] WIP --- src/array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/array.rs b/src/array.rs index 279fa18..58f2d2a 100644 --- a/src/array.rs +++ b/src/array.rs @@ -273,7 +273,7 @@ impl IArray { /// ``` /// # use implicit_clone::unsync::*; /// // This will reuse the Rc storage - /// let mut data = IArray::::Rc([1,2,3].into()); + /// let mut data = IArray::::from(vec![1,2,3]); /// data.make_mut()[1] = 123; /// assert_eq!(&[1,123,3], data.as_slice()); /// assert!(matches!(data, IArray::::Rc(_))); From a19760ff0d1579faefcbeb53325888cba30a8511 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 22 Feb 2024 14:01:25 +0100 Subject: [PATCH 4/4] WIP --- src/array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/array.rs b/src/array.rs index 58f2d2a..b34431f 100644 --- a/src/array.rs +++ b/src/array.rs @@ -282,7 +282,7 @@ impl IArray { /// ``` /// # use implicit_clone::unsync::*; /// // This will create a new copy - /// let mut data = IArray::::Rc([1,2,3].into()); + /// let mut data = IArray::::from(vec![1,2,3]); /// let other_data = data.clone(); /// data.make_mut()[1] = 123; /// assert_eq!(&[1,123,3], data.as_slice());