From 7fa33fc250407b6e8d5eb4d09bebf94007e620a8 Mon Sep 17 00:00:00 2001
From: Alvydas Vitkauskas <alvydas.vitkauskas@gmail.com>
Date: Thu, 31 Oct 2019 14:46:50 +0200
Subject: [PATCH] array: fix and document array functions

---
 vlib/builtin/array.v      | 201 ++++++++++++++++++++++++--------------
 vlib/builtin/array_test.v | 124 +++++++++++++++++++----
 2 files changed, 233 insertions(+), 92 deletions(-)

diff --git a/vlib/builtin/array.v b/vlib/builtin/array.v
index 7c8ea72107..ea43958999 100644
--- a/vlib/builtin/array.v
+++ b/vlib/builtin/array.v
@@ -58,8 +58,26 @@ fn new_array_from_c_array_no_alloc(len, cap, elm_size int, c_array voidptr) arra
 	return arr
 }
 
+// Private function. Doubles array capacity if needed
+fn (a mut array) ensure_cap(required int) {
+	if required > a.cap {
+		mut cap := if a.cap == 0 { 2 } else { a.cap * 2 }
+		for required > cap { cap *= 2 }
+		if a.cap == 0 {
+			a.data = calloc(cap * a.element_size)
+		}
+		else {
+			a.data = C.realloc(a.data, cap * a.element_size)
+		}
+		a.cap = cap
+	}
+}
+
 // Private function, used by V  (`[0; 100]`)
 fn array_repeat_old(val voidptr, nr_repeats, elm_size int) array {
+	if nr_repeats < 0 {
+		panic('[0; len]: `len` is negative (len == $nr_repeats)')
+	}
 	arr := array {
 		len: nr_repeats
 		cap: nr_repeats
@@ -72,149 +90,169 @@ fn array_repeat_old(val voidptr, nr_repeats, elm_size int) array {
 	return arr
 }
 
+// array.repeat returns new array with the given array elements 
+// repeated `nr_repeat` times
 pub fn (a array) repeat(nr_repeats int) array {
-	arr := array {
-		len: nr_repeats
-		cap: nr_repeats
-		element_size: a.element_size
-		data: calloc(nr_repeats * a.element_size)
+	if nr_repeats < 0 {
+		panic('array.repeat: count is negative (count == $nr_repeats)')
+	}
+	arr := array {
+		len: nr_repeats * a.len
+		cap: nr_repeats * a.len
+		element_size: a.element_size
+		data: calloc(nr_repeats * a.len * a.element_size)
 	}
-	val := a.data + 0 //nr_repeats * a.element_size
 	for i := 0; i < nr_repeats; i++ {
-		C.memcpy(arr.data + i * a.element_size, val, a.element_size)
+		C.memcpy(arr.data + i * a.len * a.element_size, a.data, a.len * a.element_size)
 	}
 	return arr
 }
 
+// array.sort sorts array in-place using given `compare` function as comparator
 pub fn (a mut array) sort_with_compare(compare voidptr) {
 	C.qsort(a.data, a.len, a.element_size, compare)
 }
 
+// TODO array.insert is broken
+// Cannot pass literal or primitive type as it cannot be cast to voidptr.
+// In the current state only that would work:
+//   i := 3
+//	 a.insert(0, &i)
+// ----------------------------
 pub fn (a mut array) insert(i int, val voidptr) {
-	if i >= a.len {
-		panic('array.insert: index larger than length')
+	if i < 0 || i > a.len {
+		panic('array.insert: index out of range (i == $i, a.len == $a.len)')
 	}
-	a.push(val)
+	a.ensure_cap(a.len + 1)
 	size := a.element_size
 	C.memmove(a.data + (i + 1) * size, a.data + i * size, (a.len - i) * size)
-	a.set(i, val)
+	C.memcpy(a.data + i * size, val, size)
+	a.len++
 }
 
+// TODO array.prepend is broken
+// It depends on array.insert
+// -----------------------------
 pub fn (a mut array) prepend(val voidptr) {
 	a.insert(0, val)
 }
 
-pub fn (a mut array) delete(idx int) {
+// array.delete deletes array element at the given index
+pub fn (a mut array) delete(i int) {
+	if i < 0 || i >= a.len {
+		panic('array.delete: index out of range (i == $i, a.len == $a.len)')
+	}
 	size := a.element_size
-	C.memmove(a.data + idx * size, a.data + (idx + 1) * size, (a.len - idx) * size)
+	C.memmove(a.data + i * size, a.data + (i + 1) * size, (a.len - i) * size)
 	a.len--
-	a.cap--
 }
 
+// Private function. Used to implement array[] operator
 fn (a array) get(i int) voidptr {
 	if i < 0 || i >= a.len {
-		panic('array index out of range: $i/$a.len')
+		panic('array.get: index out of range (i == $i, a.len == $a.len)')
 	}
 	return a.data + i * a.element_size
 }
 
+// array.first gives the first element of the array
 pub fn (a array) first() voidptr {
 	if a.len == 0 {
-		panic('array.first: empty array')
+		panic('array.first: array is empty')
 	}
 	return a.data + 0
 }
 
+// array.last gives the last element of the array
 pub fn (a array) last() voidptr {
 	if a.len == 0 {
-		panic('array.last: empty array')
+		panic('array.last: array is empty')
 	}
 	return a.data + (a.len - 1) * a.element_size
 }
 
-pub fn (s array) left(n int) array {
-	if n >= s.len {
-		return s
+// array.left returns a new array using the same buffer as the given array
+// with the first `n` elements of the given array.
+pub fn (a array) left(n int) array {
+	if n < 0 {
+		panic('array.left: index is negative (n == $n)')
 	}
-	return s.slice(0, n)
+	if n >= a.len {
+		return a.slice(0, a.len)
+	}
+	return a.slice(0, n)
 }
 
-pub fn (s array) right(n int) array {
-	if n >= s.len {
-		return new_array(0, 0, s.element_size)
+// array.right returns an array using same buffer as the given array
+// but starting with the element of the given array beyond the index `n`. 
+// If `n` is bigger or equal to the length of the given array,
+// returns an empty array of the same type as the given array.
+pub fn (a array) right(n int) array {
+	if n < 0 {
+		panic('array.right: index is negative (n == $n)')
 	}
-	return s.slice(n, s.len)
+	if n >= a.len {
+		return new_array(0, 0, a.element_size)
+	}
+	return a.slice(n, a.len)
 }
 
 // used internally for [2..4]
-fn (s array) slice2(start, _end int, end_max bool) array {
-	end := if end_max { s.len } else { _end }
-	return s.slice(start, end)
+fn (a array) slice2(start, _end int, end_max bool) array {
+	end := if end_max { a.len } else { _end }
+	return a.slice(start, end)
 }
 
-pub fn (s array) slice(start, _end int) array {
+// array.slice returns an array using the same buffer as original array
+// but starting from the `start` element and ending with the element before 
+// the `end` element of the original array with the length and capacity 
+// set to the number of the elements in the slice.
+pub fn (a array) slice(start, _end int) array {
 	mut end := _end
 	if start > end {
-		panic('invalid slice index: $start > $end')
+		panic('array.slice: invalid slice index ($start > $end)')
 	}
-	if end > s.len {
-		panic('runtime error: slice bounds out of range ($end >= $s.len)')
+	if end > a.len {
+		panic('array.slice: slice bounds out of range ($end >= $a.len)')
 	}
 	if start < 0 {
-		panic('runtime error: slice bounds out of range ($start < 0)')
+		panic('array.slice: slice bounds out of range ($start < 0)')
 	}
 	l := end - start
 	res := array {
-		element_size: s.element_size
-		data: s.data + start * s.element_size
+		element_size: a.element_size
+		data: a.data + start * a.element_size
 		len: l
 		cap: l
-		//is_slice: true
 	}
 	return res
 }
 
-fn (a mut array) set(idx int, val voidptr) {
-	if idx < 0 || idx >= a.len {
-		panic('array index out of range: $idx / $a.len')
+// Private function. Used to implement assigment to the array element.
+fn (a mut array) set(i int, val voidptr) {
+	if i < 0 || i >= a.len {
+		panic('array.set: index out of range (i == $i, a.len == $a.len)')
 	}
-	C.memcpy(a.data + a.element_size * idx, val, a.element_size)
+	C.memcpy(a.data + a.element_size * i, val, a.element_size)
 }
 
-fn (arr mut array) push(val voidptr) {
-	if arr.len >= arr.cap - 1 {
-		cap := (arr.len + 1) * 2
-		// println('_push: realloc, new cap=$cap')
-		if arr.cap == 0 {
-			arr.data = calloc(cap * arr.element_size)
-		}
-		else {
-			arr.data = C.realloc(arr.data, cap * arr.element_size)
-		}
-		arr.cap = cap
-	}
-	C.memcpy(arr.data + arr.element_size * arr.len, val, arr.element_size)
-	arr.len++
+// TODO push(val) is the same as push_many(val, 1), can be eliminated
+fn (a mut array) push(val voidptr) {
+	a.ensure_cap(a.len + 1)
+	C.memcpy(a.data + a.element_size * a.len, val, a.element_size)
+	a.len++
 }
 
 // `val` is array.data
 // TODO make private, right now it's used by strings.Builder
-pub fn (arr mut array) push_many(val voidptr, size int) {
-	if arr.len >= arr.cap - size {
-		cap := (arr.len + size) * 2
-		// println('_push: realloc, new cap=$cap')
-		if arr.cap == 0 {
-			arr.data = calloc(cap * arr.element_size)
-		}
-		else {
-			arr.data = C.realloc(arr.data, cap * arr.element_size)
-		}
-		arr.cap = cap
-	}
-	C.memcpy(arr.data + arr.element_size * arr.len, val, arr.element_size * size)
-	arr.len += size
+pub fn (a mut array) push_many(val voidptr, size int) {
+	a.ensure_cap(a.len + size)
+	C.memcpy(a.data + a.element_size * a.len, val, a.element_size * size)
+	a.len += size
 }
 
+// array.reverse returns a new array with the elements of 
+// the original array in reverse order.
 pub fn (a array) reverse() array {
 	arr := array {
 		len: a.len
@@ -228,6 +266,7 @@ pub fn (a array) reverse() array {
 	return arr
 }
 
+// array.clone returns an independent copy of a given array
 pub fn (a array) clone() array {
 	arr := array {
 		len: a.len
@@ -248,6 +287,7 @@ pub fn (a array) free() {
 	C.free(a.data)
 }
 
+// []string.str returns a string representation of the array of strings
 // "[ 'a', 'b', 'c' ]"
 pub fn (a []string) str() string {
 	mut sb := strings.new_builder(a.len * 3)
@@ -265,6 +305,7 @@ pub fn (a []string) str() string {
 	return sb.str()
 }
 
+// []bool.str returns a string representation of the array of bools
 // "[true, true, false]"
 pub fn (a []bool) str() string {
 	mut sb := strings.new_builder(a.len * 3)
@@ -284,6 +325,8 @@ pub fn (a []bool) str() string {
 	return sb.str()
 }
 
+// []byte.hex returns a string with the hexadecimal representation 
+// of the byte elements of the array
 pub fn (b []byte) hex() string {
 	mut hex := malloc(b.len*2+1)
 	mut ptr := &hex[0]
@@ -293,6 +336,9 @@ pub fn (b []byte) hex() string {
 	return string(hex)
 }
 
+// copy copies the `src` byte array elements to the `dst` byte array.
+// The number of the elements copied is the minimum of the length of both arrays.
+// Returns the number of elements copied.
 // TODO: implement for all types
 pub fn copy(dst, src []byte) int {
 	if dst.len > 0 && src.len > 0 {
@@ -303,7 +349,8 @@ pub fn copy(dst, src []byte) int {
 	return 0
 }
 
-fn compare_ints(a, b &int) int {
+// Private function. Comparator for int type.
+fn compare_ints(a, b int) int {
 	if a < b {
 		return -1
 	}
@@ -313,12 +360,13 @@ fn compare_ints(a, b &int) int {
 	return 0
 }
 
+// []int.sort sorts array of int in place in ascending order.
 pub fn (a mut []int) sort() {
 	a.sort_with_compare(compare_ints)
 }
 
-// Looking for an array index based on value.
-// If there is, it will return the index and if not, it will return `-1`
+// []string.index returns the index of the first element equal to the given value,
+// or -1 if the value is not found in the array.
 pub fn (a []string) index(v string) int {
 	for i := 0; i < a.len; i++ {
 		if a[i] == v {
@@ -328,6 +376,8 @@ pub fn (a []string) index(v string) int {
 	return -1
 }
 
+// []int.index returns the index of the first element equal to the given value,
+// or -1 if the value is not found in the array.
 pub fn (a []int) index(v int) int {
 	for i := 0; i < a.len; i++ {
 		if a[i] == v {
@@ -337,6 +387,8 @@ pub fn (a []int) index(v int) int {
 	return -1
 }
 
+// []byte.index returns the index of the first element equal to the given value,
+// or -1 if the value is not found in the array.
 pub fn (a []byte) index(v byte) int {
 	for i := 0; i < a.len; i++ {
 		if a[i] == v {
@@ -346,6 +398,9 @@ pub fn (a []byte) index(v byte) int {
 	return -1
 }
 
+// []char.index returns the index of the first element equal to the given value,
+// or -1 if the value is not found in the array.
+// TODO is `char` type yet in the language?
 pub fn (a []char) index(v char) int {
 	for i := 0; i < a.len; i++ {
 		if a[i] == v {
@@ -355,7 +410,7 @@ pub fn (a []char) index(v char) int {
 	return -1
 }
 
-// Executes a reducer function (that you provide) on each element of the array,
+// []int.reduce executes a given reducer function on each element of the array,
 // resulting in a single output value.
 pub fn (a []int) reduce(iter fn (accum, curr int) int, accum_start int) int {
 	mut _accum := 0
@@ -366,6 +421,7 @@ pub fn (a []int) reduce(iter fn (accum, curr int) int, accum_start int) int {
 	return _accum
 }
 
+// array_eq<T> checks if two arrays contain all the same elements in the same order.
 // []int == []int (also for: i64, f32, f64, byte, string)
 fn array_eq<T>(a1, a2 []T) bool {
 	if a1.len != a2.len {
@@ -398,4 +454,3 @@ pub fn (a []byte) eq(a2 []byte) bool {
 pub fn (a []f32) eq(a2 []f32) bool { 
 	return array_eq(a, a2) 
 }
-
diff --git a/vlib/builtin/array_test.v b/vlib/builtin/array_test.v
index 28f8f12320..cf632f1645 100644
--- a/vlib/builtin/array_test.v
+++ b/vlib/builtin/array_test.v
@@ -32,6 +32,10 @@ fn test_deleting() {
 	a.delete(1)
 	assert a.str() == '[5, 3, 4]'
 	assert a.len == 3
+
+    a.delete(a.len - 1)
+	assert a.str() == '[5, 3]'
+	assert a.len == 2
 }
 
 fn test_short() {
@@ -80,37 +84,115 @@ fn test_push() {
 	assert a.str() == '[1, 3]'
 }
 
+// TODO array.insert is broken
+// Cannot pass literal or primitive type as it cannot be cast to voidptr.
+// In the current state only that would work:
+//   i := 3
+//	 a.insert(0, &i)
+// ----------------------------
+// fn test_insert() {
+//     mut a := [1, 2]
+
+//     a.insert(0, 3)
+//     assert a[0] == 3
+//     assert a[2] == 2
+//     assert a.len == 3
+
+//     a.insert(1, 4)
+//     assert a[1] == 4
+//     assert a[2] == 1
+//     assert a.len == 4
+
+//     a.insert(4, 5)
+//     assert a[4] == 5
+//     assert a[3] == 2
+//     assert a.len == 5
+
+//     mut b := []f64
+//     assert b.len == 0
+//     b.insert(0, f64(1.1))
+//     assert b.len == 1
+//     assert b[0] == f64(1.1)
+// }
+
+// TODO array.prepend is broken
+// It depends on array.insert
+// -----------------------------
+// fn test_prepend() {
+//     mut a := []int
+//     assert a.len == 0
+//     a.prepend(1)
+//     assert a.len == 1
+//     assert a[0] == 1
+
+//     mut b := []f64
+//     assert b.len == 0
+//     b.prepend(f64(1.1))
+//     assert b.len == 1
+//     assert b[0] == f64(1.1)
+// }
+
 fn test_strings() {
 	a := ['a', 'b', 'c']
 	assert a.str() == '["a", "b", "c"]'
 }
 
-fn test_repeat() {
-	a := [0].repeat(5)
-	assert a.len == 5
-	assert a[0] == 0 && a[1] == 0 && a[2] == 0 && a[3] == 0 && a[4] == 0
+fn test_compare_ints() {
+    assert compare_ints(1, 2) == -1
+    assert compare_ints(2, 1) == 1
+    assert compare_ints(0, 0) == 0
 
-	b := [7].repeat(3)
-	assert b.len == 3
-	assert b[0] == 7 && b[1] == 7 && b[2] == 7
+    a := 1
+    b := 2
+    assert compare_ints(a, b) == -1
+    assert compare_ints(b, a) == 1
+    assert compare_ints(a, a) == 0
+}
+
+fn test_repeat() {
 	{
-		mut aa := [1.1].repeat(10)
+		a := [0].repeat(5)
+		assert a.len == 5
+		assert a[0] == 0 && a[1] == 0 && a[2] == 0 && a[3] == 0 && a[4] == 0
+	}
+	{
+		a := [1.1].repeat(10)
 		// FIXME: assert aa[0] == 1.1 will fail, need fix
-		assert aa[0] == f32(1.1)
-		assert aa[5] == f32(1.1)
-		assert aa[9] == f32(1.1)
+		assert a[0] == f32(1.1)
+		assert a[5] == f32(1.1)
+		assert a[9] == f32(1.1)
 	}
 	{
-		mut aa := [f32(1.1)].repeat(10)
-		assert aa[0] == f32(1.1)
-		assert aa[5] == f32(1.1)
-		assert aa[9] == f32(1.1)
+		a := [f32(1.1)].repeat(10)
+		assert a[0] == f32(1.1)
+		assert a[5] == f32(1.1)
+		assert a[9] == f32(1.1)
 	}
 	{
-		aa := [f64(1.1)].repeat(10)
-		assert aa[0] == f64(1.1)
-		assert aa[5] == f64(1.1)
-		assert aa[9] == f64(1.1)
+		a := [f64(1.1)].repeat(10)
+		assert a[0] == f64(1.1)
+		assert a[5] == f64(1.1)
+		assert a[9] == f64(1.1)
+	}
+	{
+		a := [1, 2].repeat(2)
+		assert a[0] == 1
+		assert a[1] == 2
+		assert a[2] == 1
+		assert a[3] == 2
+	}
+	{
+		a := ['1', 'abc'].repeat(2)
+		assert a[0] == '1'
+		assert a[1] == 'abc'
+		assert a[2] == '1'
+		assert a[3] == 'abc'
+	}
+	{
+		mut a := ['1', 'abc'].repeat(0)
+		assert a.len == 0
+		a << 'abc'
+		assert a[0] == 'abc'
 	}
 }
 
@@ -149,12 +231,16 @@ fn test_left() {
 	b := a.left(2)
 	c := a[0..2]
 	d := a[..2]
+    e := a.left(4)
 	assert b[0] == 1
 	assert b[1] == 2
 	assert c[0] == 1
 	assert c[1] == 2
 	assert d[0] == 1
 	assert d[1] == 2
+    assert e[0] == 1
+    assert e[2] == 3
+    assert e.len == 3
 }
 
 fn test_slice() {