From ce02852dd4e88b38f5f0927c9abc9a47743e2cc5 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Mon, 12 Jun 2023 17:00:12 +0200 Subject: [PATCH 1/2] Implement PackedArray::as_slice(), as_slice_mut() --- godot-core/src/builtin/packed_array.rs | 44 +++++++++++++++++++++++++ itest/rust/src/packed_array_test.rs | 45 +++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/godot-core/src/builtin/packed_array.rs b/godot-core/src/builtin/packed_array.rs index 708092153..eb173aa77 100644 --- a/godot-core/src/builtin/packed_array.rs +++ b/godot-core/src/builtin/packed_array.rs @@ -119,6 +119,8 @@ macro_rules! impl_packed_array { /// new array. /// /// The values of `begin` and `end` 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]. pub fn slice(&self, begin: usize, end: usize) -> Self { let len = self.len(); let begin = begin.min(len); @@ -126,6 +128,46 @@ macro_rules! impl_packed_array { 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 + /// [`slice`][Self::slice] 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 + /// [`slice`][Self::slice] 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 = diff --git a/itest/rust/src/packed_array_test.rs b/itest/rust/src/packed_array_test.rs index 48f073b77..1c2a13b29 100644 --- a/itest/rust/src/packed_array_test.rs +++ b/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,6 +76,7 @@ fn packed_array_clone() { #[allow(clippy::redundant_clone)] let clone = array.clone(); array.set(0, 3); + assert_eq!(clone.get(0), 1); } @@ -83,9 +84,51 @@ fn packed_array_clone() { fn packed_array_slice() { let array = PackedByteArray::from(&[1, 2, 3]); let slice = array.slice(1, 2); + assert_eq!(slice.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] fn packed_array_get() { let array = PackedByteArray::from(&[1, 2]); From 2bfc4f9b2e5001a83dd281fff863efa771be3f59 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Mon, 12 Jun 2023 22:14:17 +0200 Subject: [PATCH 2/2] Rename slice[_deep|_shallow] -> subarray Makes it more distinct from PackedArray::as_slice() operations and true Rust slices. --- godot-core/src/builtin/array.rs | 32 +++++++++++++------------- godot-core/src/builtin/packed_array.rs | 12 +++++----- itest/rust/src/array_test.rs | 12 +++++----- itest/rust/src/packed_array_test.rs | 6 ++--- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index f9bf430d0..60dd09ae6 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -263,10 +263,9 @@ impl Array { 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, @@ -274,15 +273,15 @@ impl Array { /// /// 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) -> 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) -> 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, @@ -290,25 +289,26 @@ impl Array { /// /// 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) -> 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) -> Self { + self.subarray_impl(begin, end, step, true) } - fn slice_impl(&self, begin: usize, end: usize, step: Option, deep: bool) -> Self { - assert_ne!(step, Some(0), "slice: step cannot be zero"); + fn subarray_impl(&self, begin: usize, end: usize, step: Option, 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. diff --git a/godot-core/src/builtin/packed_array.rs b/godot-core/src/builtin/packed_array.rs index eb173aa77..d62030b5f 100644 --- a/godot-core/src/builtin/packed_array.rs +++ b/godot-core/src/builtin/packed_array.rs @@ -115,13 +115,13 @@ 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. + /// 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]. - pub fn slice(&self, begin: usize, end: usize) -> Self { + #[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); @@ -133,7 +133,7 @@ macro_rules! impl_packed_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 - /// [`slice`][Self::slice] to get a sub-array as a copy. + /// [`subarray`][Self::subarray] to get a sub-array as a copy. pub fn as_slice(&self) -> &[$Element] { if self.is_empty() { &[] @@ -153,7 +153,7 @@ macro_rules! impl_packed_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 - /// [`slice`][Self::slice] to get a sub-array as a copy. + /// [`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 [] diff --git a/itest/rust/src/array_test.rs b/itest/rust/src/array_test.rs index e8ac4e4a9..b247d1274 100644 --- a/itest/rust/src/array_test.rs +++ b/itest/rust/src/array_test.rs @@ -126,14 +126,14 @@ 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::::try_from_variant(&slice.get(0)) .unwrap() .set(0, 4); @@ -141,14 +141,14 @@ fn array_slice_shallow() { } #[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::::try_from_variant(&slice.get(0)) .unwrap() .set(0, 4); diff --git a/itest/rust/src/packed_array_test.rs b/itest/rust/src/packed_array_test.rs index 1c2a13b29..b93a38242 100644 --- a/itest/rust/src/packed_array_test.rs +++ b/itest/rust/src/packed_array_test.rs @@ -81,11 +81,11 @@ fn packed_array_clone() { } #[itest] -fn packed_array_slice() { +fn packed_array_subarray() { let array = PackedByteArray::from(&[1, 2, 3]); - let slice = array.slice(1, 2); + let subarray = array.subarray(1, 2); - assert_eq!(slice.to_vec(), vec![2]); + assert_eq!(subarray.to_vec(), vec![2]); } #[itest]