fix(array): respect capacity when growing

Author: peter-jerry-ye

builtin/array.mbt

@@ -394,33 +394,6 @@ pub impl[T] Add for Array[T] with fn add(self, other) {
   }
 }
 
-///|
-/// Appends all elements from one array to the end of another array. The elements
-/// are added in-place, modifying the original array.
-///
-/// Parameters:
-///
-/// * `self` : The array to append to.
-/// * `other` : The array whose elements will be appended.
-///
-/// Example:
-///
-/// ```mbt check
-/// test {
-///   let v1 = [1, 2, 3]
-///   let v2 : ReadOnlyArray[Int] = [4, 5, 6]
-///   v1.append(v2)
-///   debug_inspect(v1, content="[1, 2, 3, 4, 5, 6]")
-///   let v1 = [1, 2, 3]
-///   let v2 : ReadOnlyArray[Int] = []
-///   v1.append(v2)
-///   debug_inspect(v1, content="[1, 2, 3]")
-/// }
-/// ```
-pub fn[T] Array::append(self : Array[T], other : ArrayView[T]) -> Unit {
-  other.blit_to(self, dst_offset=self.length())
-}
-
 ///|
 /// Iterates through each element of the array in order, applying the given
 /// function to each element.

builtin/array_block.mbt

@@ -107,106 +107,6 @@ pub fn[A] Array::unsafe_blit_fixed(
   )
 }
 
-///|
-/// Copies elements from one array to another array, with support for growing the
-/// destination array if needed. The arrays may overlap, in which case the copy
-/// is performed in a way that preserves the data.
-///
-/// Parameters:
-///
-/// * `self` : The array to copy elements from.
-/// * `dst` : The array to copy elements to. Will be automatically grown
-/// if needed to accommodate the copied elements.
-/// * `len` : The number of elements to copy.
-/// * `src_offset` : Starting index in the source array. Defaults to 0.
-/// * `dst_offset` : Starting index in the destination array. Defaults to
-/// 0.
-///
-/// Example:
-///
-/// ```mbt check
-/// test {
-///   let src = [1, 2, 3, 4, 5]
-///   let dst = [0, 0]
-///   src[:3].blit_to(dst, dst_offset=1)
-///   @debug.debug_inspect(dst, content="[0, 1, 2, 3]")
-/// }
-/// ```
-///
-/// Panics if:
-///
-/// * `len` is negative
-/// * `src_offset` is negative
-/// * `dst_offset` is negative
-/// * `dst_offset` exceeds the length of destination array
-/// * `src_offset + len` exceeds the length of source array
-// TODO: make len optional and deprecate it
-#label_migration(src_offset, fill=false, msg="Use ArrayView::blit_to instead")
-#label_migration(len, fill=false, msg="Use ArrayView::blit_to instead")
-pub fn[A] Array::blit_to(
-  self : Array[A],
-  dst : Array[A],
-  len? : Int = self.length(),
-  src_offset? : Int = 0,
-  dst_offset? : Int = 0,
-) -> Unit {
-  guard len >= 0 &&
-    dst_offset >= 0 &&
-    src_offset >= 0 &&
-    dst_offset <= dst.length() &&
-    src_offset + len <= self.length()
-  if dst_offset + len > dst.length() {
-    dst.unsafe_grow_to_length(dst_offset + len)
-  }
-  Array::unsafe_blit(dst, dst_offset, self, src_offset, len)
-}
-
-///|
-/// Copies all elements from an array view to a destination array, with support
-/// for growing the destination array if needed.
-///
-/// Parameters:
-///
-/// * `self` : The array view to copy elements from.
-/// * `dst` : The array to copy elements to. Will be automatically grown
-/// if needed to accommodate the copied elements.
-/// * `dst_offset` : Starting index in the destination array. Defaults to 0.
-///
-/// Example:
-///
-/// ```mbt check
-/// test {
-///   let src = [1, 2, 3, 4, 5]
-///   let view = src[1:4] // view = [2, 3, 4]
-///   let dst = [0, 0]
-///   view.blit_to(dst, dst_offset=1)
-///   @debug.debug_inspect(dst, content="[0, 2, 3, 4]")
-/// }
-/// ```
-///
-/// Panics if:
-///
-/// * `dst_offset` is negative
-/// * `dst_offset` exceeds the length of destination array
-pub fn[A] ArrayView::blit_to(
-  self : ArrayView[A],
-  dst : Array[A],
-  dst_offset? : Int = 0,
-) -> Unit {
-  let len = self.len()
-  guard dst_offset >= 0 && dst_offset <= dst.length()
-  if dst_offset + len > dst.length() {
-    dst.unsafe_grow_to_length(dst_offset + len)
-  }
-  UninitializedArray::unsafe_blit(
-    dst.buffer(),
-    dst_offset,
-    self.buf(),
-    self.start(),
-    len,
-  )
-}
-
 ///|
 test "Array::blit_to/basic" {
   let src = [1, 2, 3, 4, 5]
@@ -235,6 +135,18 @@ test "Array::blit_to/grow_destination" {
   assert_true(dst == [0, 1, 2, 3])
 }
 
+///|
+#warnings("-deprecated")
+test "Array::blit_to/grow_destination_directly" {
+  let src = [1, 2, 3, 4]
+  let dst = [0]
+  Array::blit_to(src, dst, len=2, src_offset=1, dst_offset=1)
+  assert_true(dst == [0, 2, 3])
+  let self = [1, 2, 3]
+  Array::blit_to(self, self, len=2, dst_offset=3)
+  assert_true(self == [1, 2, 3, 1, 2])
+}
+
 ///|
 test "Array::blit_to/edge_cases" {
   // Test with src_offset and dst_offset
@@ -289,6 +201,14 @@ test "panic Array::blit_to/boundary_cases4" {
   ignore(src[0:5].blit_to(dst, dst_offset=6))
 }
 
+///|
+#warnings("-deprecated")
+test "panic Array::blit_to/reject_overflowed_source_range" {
+  let src = [1]
+  let dst = [0]
+  Array::blit_to(src, dst, len=0x7fffffff, src_offset=1)
+}
+
 ///|
 /// TODO
 /// 1. allow skip

builtin/array_make_blit_bench_test.mbt

@@ -56,6 +56,56 @@ test "bench ArrayView::add Ref n=500000+500000" (it : @bench.T) {
   it.bench(fn() { it.keep(left_view + right_view) })
 }
 
+///|
+test "bench Array::blit_to Ref n=1000000 overwrite" (it : @bench.T) {
+  let src = make_blit_ref_array(make_blit_ref_bench_size)
+  let dst = Array::make(make_blit_ref_bench_size, "dst")
+  it.bench(fn() {
+    src.blit_to(dst)
+    it.keep(dst)
+  })
+}
+
+///|
+test "bench Array::blit_to Ref n=1000000 grow" (it : @bench.T) {
+  let src = make_blit_ref_array(make_blit_ref_bench_size)
+  it.bench(fn() {
+    let dst : Array[String] = []
+    src.blit_to(dst)
+    it.keep(dst)
+  })
+}
+
+///|
+test "bench ArrayView::blit_to Ref n=1000000 overwrite" (it : @bench.T) {
+  let src = make_blit_ref_array(make_blit_ref_bench_size)[:]
+  let dst = Array::make(make_blit_ref_bench_size, "dst")
+  it.bench(fn() {
+    src.blit_to(dst)
+    it.keep(dst)
+  })
+}
+
+///|
+test "bench ArrayView::blit_to Ref n=1000000 grow" (it : @bench.T) {
+  let src = make_blit_ref_array(make_blit_ref_bench_size)[:]
+  it.bench(fn() {
+    let dst : Array[String] = []
+    src.blit_to(dst)
+    it.keep(dst)
+  })
+}
+
+///|
+test "bench Array::append Ref n=1000000 grow" (it : @bench.T) {
+  let src = make_blit_ref_array(make_blit_ref_bench_size)[:]
+  it.bench(fn() {
+    let dst : Array[String] = []
+    dst.append(src)
+    it.keep(dst)
+  })
+}
+
 ///|
 test "bench Array::push Ref n=1000000 resize" (it : @bench.T) {
   it.bench(fn() {

builtin/array_nonjs_test.mbt

@@ -18,6 +18,34 @@ test "array_capacity" {
   assert_true(arr.capacity() >= 10)
 }
 
+///|
+test "array_append_reuses_capacity" {
+  let arr = Array::new(capacity=4)
+  arr.push(1)
+  arr.append([2, 3])
+  inspect(arr.capacity(), content="4")
+  debug_inspect(arr, content="[1, 2, 3]")
+}
+
+///|
+test "array_append_grows_geometrically" {
+  let arr = Array::new(capacity=4)
+  arr.append([1, 2, 3, 4, 5])
+  inspect(arr.capacity(), content="8")
+  debug_inspect(arr, content="[1, 2, 3, 4, 5]")
+}
+
+///|
+test "array_blit_to_reuses_and_grows_capacity" {
+  let dst = Array::new(capacity=4)
+  dst.push(0)
+  [1, 2][:].blit_to(dst, dst_offset=1)
+  inspect(dst.capacity(), content="4")
+  [3, 4][:].blit_to(dst, dst_offset=3)
+  inspect(dst.capacity(), content="8")
+  debug_inspect(dst, content="[0, 1, 2, 3, 4]")
+}
+
 ///|
 test "array_retain" {
   let arr = [1, 2, 3, 4, 5]

builtin/arraycore_js.mbt

@@ -50,6 +50,19 @@ extern "js" fn JSArray::set_length(self : JSArray, new_len : Int) -> Unit =
 extern "js" fn JSArray::push(self : JSArray, value : JSValue) -> Unit =
   #| (arr, val) => { arr.push(val); }
 
+///|
+extern "js" fn JSArray::append_view(
+  self : JSArray,
+  src : JSArray,
+  src_offset : Int,
+  len : Int,
+) -> Unit =
+  #| (dst, src, src_offset, len) => {
+  #|   for (let i = 0; i < len; i++) {
+  #|     dst.push(src[src_offset + i]);
+  #|   }
+  #| }
+
 ///|
 extern "js" fn JSArray::pop(self : JSArray) -> JSValue =
   #| (arr) => arr.pop()
@@ -248,6 +261,91 @@ pub fn[T] Array::push(self : Array[T], value : T) -> Unit {
   JSArray::ofAnyArray(self).push(JSValue::ofAny(value))
 }
 
+///|
+/// Appends all elements from one array to the end of another array. The elements
+/// are added in-place, modifying the original array.
+///
+/// Parameters:
+///
+/// * `self` : The array to append to.
+/// * `other` : The array whose elements will be appended.
+///
+/// Example:
+///
+/// ```mbt check
+/// test {
+///   let v1 = [1, 2, 3]
+///   let v2 : ReadOnlyArray[Int] = [4, 5, 6]
+///   v1.append(v2)
+///   debug_inspect(v1, content="[1, 2, 3, 4, 5, 6]")
+///   let v1 = [1, 2, 3]
+///   let v2 : ReadOnlyArray[Int] = []
+///   v1.append(v2)
+///   debug_inspect(v1, content="[1, 2, 3]")
+/// }
+/// ```
+pub fn[T] Array::append(self : Array[T], other : ArrayView[T]) -> Unit {
+  let old_len = self.length()
+  let append_len = other.len()
+  guard old_len + append_len >= old_len
+  JSArray::ofAnyArray(self).append_view(
+    JSArray::ofAnyFixedArray(other.buf().0),
+    other.start(),
+    append_len,
+  )
+}
+
+///|
+/// JavaScript arrays use their native length as the writable extent, so extend
+/// it before blitting into the new range.
+#label_migration(src_offset, fill=false, msg="Use ArrayView::blit_to instead")
+#label_migration(len, fill=false, msg="Use ArrayView::blit_to instead")
+pub fn[A] Array::blit_to(
+  self : Array[A],
+  dst : Array[A],
+  len? : Int = self.length(),
+  src_offset? : Int = 0,
+  dst_offset? : Int = 0,
+) -> Unit {
+  let old_len = dst.length()
+  guard len >= 0 &&
+    dst_offset >= 0 &&
+    src_offset >= 0 &&
+    dst_offset <= old_len &&
+    len <= self.length() - src_offset
+  let new_len = dst_offset + len
+  guard new_len >= 0
+  if new_len > old_len {
+    JSArray::ofAnyArray(dst).set_length(new_len)
+  }
+  Array::unsafe_blit(dst, dst_offset, self, src_offset, len)
+}
+
+///|
+/// JavaScript arrays use their native length as the writable extent, so extend
+/// it before blitting into the new range.
+pub fn[A] ArrayView::blit_to(
+  self : ArrayView[A],
+  dst : Array[A],
+  dst_offset? : Int = 0,
+) -> Unit {
+  let len = self.len()
+  let old_len = dst.length()
+  guard dst_offset >= 0 && dst_offset <= old_len
+  let new_len = dst_offset + len
+  guard new_len >= 0
+  if new_len > old_len {
+    JSArray::ofAnyArray(dst).set_length(new_len)
+  }
+  UninitializedArray::unsafe_blit(
+    dst.buffer(),
+    dst_offset,
+    self.buf(),
+    self.start(),
+    len,
+  )
+}
+
 ///|
 /// Removes the last element from an array and returns it, or `None` if it is empty.
 ///
@@ -354,18 +452,6 @@ pub fn[T] Array::insert(self : Array[T], index : Int, value : T) -> Unit {
   let _ = JSArray::ofAnyArray(self).splice1(index, 0, JSValue::ofAny(value))
 }
 
-///|
-/// Resize the array in-place so that `len` is equal to `new_len`.
-///
-/// If `new_len` is greater than `len`, the array will be extended by the
-/// difference, and the values in the new slots are left uninitilized.
-///  If `new_len` is less than `len`, it will panic
-///
-fn[T] Array::unsafe_grow_to_length(self : Array[T], new_len : Int) -> Unit {
-  guard new_len >= self.length()
-  JSArray::ofAnyArray(self).set_length(new_len)
-}
-
 ///|
 /// Fills an Array with a specified value.
 ///