Merge #307

307: Implement `PackedArray::as_slice()`, `as_slice_mut()` r=Bromeon a=Bromeon

Resumes abandoned work of #137.
Closes #306.

Open points:
1. Test CoW across GDScript/Rust boundaries?
2. We now have `slice`, `as_slice` and `as_mut_slice`, where the first returns another array.
    I wonder if it would be clearer to name it `sub_array` or something instead of `slice`?

bors try

Co-authored-by: Jan Haller <bromeon@gmail.com>

godot-core/src/builtin/array.rs

@@ -263,52 +263,52 @@ impl<T: VariantMetadata> Array<T> {
         unsafe { duplicate.assume_type() }
     }
 
-    /// Returns a slice of the `Array`, from `begin` (inclusive) to `end` (exclusive), as a
-    /// new `Array`.
+    /// Returns a sub-range `begin..end`, as a new array.
     ///
-    /// The values of `begin` and `end` will be clamped to the array size.
+    /// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
     ///
     /// If specified, `step` is the relative index between source elements. It can be negative,
     /// in which case `begin` must be higher than `end`. For example,
     /// `TypedArray::from(&[0, 1, 2, 3, 4, 5]).slice(5, 1, -2)` returns `[5, 3]`.
     ///
     /// Array elements are copied to the slice, but any reference types (such as `Array`,
     /// `Dictionary` and `Object`) will still refer to the same value. To create a deep copy, use
-    /// [`slice_deep()`] instead.
-    pub fn slice_shallow(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
-        self.slice_impl(begin, end, step, false)
+    /// [`subarray_deep()`] instead.
+    #[doc(alias = "slice")]
+    pub fn subarray_shallow(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
+        self.subarray_impl(begin, end, step, false)
     }
 
-    /// Returns a slice of the `Array`, from `begin` (inclusive) to `end` (exclusive), as a
-    /// new `Array`.
+    /// Returns a sub-range `begin..end`, as a new `Array`.
     ///
-    /// The values of `begin` and `end` will be clamped to the array size.
+    /// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
     ///
     /// If specified, `step` is the relative index between source elements. It can be negative,
     /// in which case `begin` must be higher than `end`. For example,
     /// `TypedArray::from(&[0, 1, 2, 3, 4, 5]).slice(5, 1, -2)` returns `[5, 3]`.
     ///
     /// All nested arrays and dictionaries are duplicated and will not be shared with the original
     /// array. Note that any `Object`-derived elements will still be shallow copied. To create a
-    /// shallow copy, use [`slice_shallow()`] instead.
-    pub fn slice_deep(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
-        self.slice_impl(begin, end, step, true)
+    /// shallow copy, use [`subarray_shallow()`] instead.
+    #[doc(alias = "slice")]
+    pub fn subarray_deep(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
+        self.subarray_impl(begin, end, step, true)
     }
 
-    fn slice_impl(&self, begin: usize, end: usize, step: Option<isize>, deep: bool) -> Self {
-        assert_ne!(step, Some(0), "slice: step cannot be zero");
+    fn subarray_impl(&self, begin: usize, end: usize, step: Option<isize>, deep: bool) -> Self {
+        assert_ne!(step, Some(0), "subarray: step cannot be zero");
 
         let len = self.len();
         let begin = begin.min(len);
         let end = end.min(len);
         let step = step.unwrap_or(1);
 
-        let slice: VariantArray =
+        let subarray: VariantArray =
             self.as_inner()
                 .slice(to_i64(begin), to_i64(end), step.try_into().unwrap(), deep);
 
         // SAFETY: slice() returns a typed array with the same type as Self
-        unsafe { slice.assume_type() }
+        unsafe { subarray.assume_type() }
     }
 
     /// Appends another array at the end of this array. Equivalent of `append_array` in GDScript.

godot-core/src/builtin/packed_array.rs

@@ -115,17 +115,59 @@ macro_rules! impl_packed_array {
                 self.as_inner().resize(to_i64(size));
             }
 
-            /// Returns a slice of the array, from `begin` (inclusive) to `end` (exclusive), as a
-            /// new array.
+            /// Returns a sub-range `begin..end`, as a new packed array.
             ///
-            /// The values of `begin` and `end` will be clamped to the array size.
-            pub fn slice(&self, begin: usize, end: usize) -> Self {
+            /// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
+            ///
+            /// To obtain Rust slices, see [`as_slice`][Self::as_slice] and [`as_mut_slice`][Self::as_mut_slice].
+            #[doc(alias = "slice")]
+            pub fn subarray(&self, begin: usize, end: usize) -> Self {
                 let len = self.len();
                 let begin = begin.min(len);
                 let end = end.min(len);
                 self.as_inner().slice(to_i64(begin), to_i64(end))
             }
 
+            /// Returns a shared Rust slice of the array.
+            ///
+            /// The resulting slice can be further subdivided or converted into raw pointers.
+            ///
+            /// See also [`as_mut_slice`][Self::as_mut_slice] to get exclusive slices, and
+            /// [`subarray`][Self::subarray] to get a sub-array as a copy.
+            pub fn as_slice(&self) -> &[$Element] {
+                if self.is_empty() {
+                    &[]
+                } else {
+                    let data = self.ptr(0);
+
+                    // SAFETY: PackedArray holds `len` elements in contiguous storage, all of which are initialized.
+                    // The array uses copy-on-write semantics, so the slice may be aliased, but copies will use a new allocation.
+                    unsafe {
+                        std::slice::from_raw_parts(data, self.len())
+                    }
+                }
+            }
+
+            /// Returns an exclusive Rust slice of the array.
+            ///
+            /// The resulting slice can be further subdivided or converted into raw pointers.
+            ///
+            /// See also [`as_slice`][Self::as_slice] to get shared slices, and
+            /// [`subarray`][Self::subarray] to get a sub-array as a copy.
+            pub fn as_mut_slice(&mut self) -> &mut [$Element] {
+                if self.is_empty() {
+                    &mut []
+                } else {
+                    let data = self.ptr_mut(0);
+
+                    // SAFETY: PackedArray holds `len` elements in contiguous storage, all of which are initialized.
+                    // The array uses copy-on-write semantics. ptr_mut() triggers a copy if non-unique, after which the slice is never aliased.
+                    unsafe {
+                        std::slice::from_raw_parts_mut(data, self.len())
+                    }
+                }
+            }
+
             /// Returns a copy of the value at the specified index.
             ///
             /// # Panics
@@ -193,6 +235,7 @@ macro_rules! impl_packed_array {
             /// If `index` is out of bounds.
             pub fn set(&mut self, index: usize, value: $Element) {
                 let ptr_mut = self.ptr_mut(index);
+
                 // SAFETY: `ptr_mut` just checked that the index is not out of bounds.
                 unsafe {
                     *ptr_mut = value;
@@ -302,6 +345,7 @@ macro_rules! impl_packed_array {
             /// If `index` is out of bounds.
             fn ptr_mut(&self, index: usize) -> *mut $Element {
                 self.check_bounds(index);
+
                 // SAFETY: We just checked that the index is not out of bounds.
                 let ptr = unsafe {
                     let item_ptr: *mut $IndexRetType =

itest/rust/src/array_test.rs

@@ -126,29 +126,29 @@ fn array_duplicate_deep() {
 }
 
 #[itest]
-fn array_slice_shallow() {
+fn array_subarray_shallow() {
     let array = array![0, 1, 2, 3, 4, 5];
-    let slice = array.slice_shallow(5, 1, Some(-2));
+    let slice = array.subarray_shallow(5, 1, Some(-2));
     assert_eq!(slice, array![5, 3]);
 
     let subarray = array![2, 3];
     let array = varray![1, subarray];
-    let slice = array.slice_shallow(1, 2, None);
+    let slice = array.subarray_shallow(1, 2, None);
     Array::<i64>::try_from_variant(&slice.get(0))
         .unwrap()
         .set(0, 4);
     assert_eq!(subarray.get(0), 4);
 }
 
 #[itest]
-fn array_slice_deep() {
+fn array_subarray_deep() {
     let array = array![0, 1, 2, 3, 4, 5];
-    let slice = array.slice_deep(5, 1, Some(-2));
+    let slice = array.subarray_deep(5, 1, Some(-2));
     assert_eq!(slice, array![5, 3]);
 
     let subarray = array![2, 3];
     let array = varray![1, subarray];
-    let slice = array.slice_deep(1, 2, None);
+    let slice = array.subarray_deep(1, 2, None);
     Array::<i64>::try_from_variant(&slice.get(0))
         .unwrap()
         .set(0, 4);

itest/rust/src/packed_array_test.rs

@@ -5,7 +5,7 @@
  */
 
 use crate::{expect_panic, itest};
-use godot::builtin::{PackedByteArray, PackedFloat32Array};
+use godot::builtin::{PackedByteArray, PackedFloat32Array, PackedStringArray};
 
 #[itest]
 fn packed_array_default() {
@@ -76,14 +76,57 @@ fn packed_array_clone() {
     #[allow(clippy::redundant_clone)]
     let clone = array.clone();
     array.set(0, 3);
+
     assert_eq!(clone.get(0), 1);
 }
 
 #[itest]
-fn packed_array_slice() {
+fn packed_array_subarray() {
     let array = PackedByteArray::from(&[1, 2, 3]);
-    let slice = array.slice(1, 2);
-    assert_eq!(slice.to_vec(), vec![2]);
+    let subarray = array.subarray(1, 2);
+
+    assert_eq!(subarray.to_vec(), vec![2]);
+}
+
+#[itest]
+fn packed_array_as_slice() {
+    let a = PackedByteArray::from(&[1, 2, 3]);
+    #[allow(clippy::redundant_clone)]
+    let b = a.clone();
+
+    let slice_a = a.as_slice();
+    let slice_b = b.as_slice();
+
+    assert_eq!(slice_a, &[1, 2, 3]);
+    assert_eq!(slice_a, slice_b);
+    assert_eq!(
+        slice_a.as_ptr(),
+        slice_b.as_ptr(),
+        "copy-on-write without modification returns aliased slice"
+    );
+
+    let empty = PackedStringArray::new();
+    assert_eq!(empty.as_slice(), &[]);
+}
+
+#[itest]
+fn packed_array_as_mut_slice() {
+    let a = PackedByteArray::from(&[1, 2, 3]);
+    let mut b = a.clone();
+
+    let slice_a = a.as_slice();
+    let slice_b = b.as_mut_slice(); // triggers CoW
+
+    assert_eq!(slice_a, &mut [1, 2, 3]);
+    assert_eq!(slice_a, slice_b);
+    assert_ne!(
+        slice_a.as_ptr(),
+        slice_b.as_ptr(),
+        "copy-on-write with modification must return independent slice"
+    );
+
+    let mut empty = PackedStringArray::new();
+    assert_eq!(empty.as_mut_slice(), &mut []);
 }
 
 #[itest]