From 3e287f997e493f7594195e8b5aeb713e6f8cbf52 Mon Sep 17 00:00:00 2001 From: Grant Nelson Date: Tue, 4 Jun 2024 11:24:44 -0600 Subject: [PATCH 1/3] Adding reflect Grow --- compiler/natives/src/reflect/reflect.go | 20 ++++++++ compiler/prelude/prelude.js | 63 +++++++++++++++---------- 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/compiler/natives/src/reflect/reflect.go b/compiler/natives/src/reflect/reflect.go index 47b93662e..12fd05de9 100644 --- a/compiler/natives/src/reflect/reflect.go +++ b/compiler/natives/src/reflect/reflect.go @@ -1325,6 +1325,26 @@ func getJsTag(tag string) string { return "" } +func (v Value) grow(n int) { + if n < 0 { + panic(`reflect.Value.Grow: negative len`) + } + + s := v.object() + len := s.Get(`$length`).Int() + if len+n < 0 { + panic(`reflect.Value.Grow: slice overflow`) + } + + cap := s.Get(`$capacity`).Int() + if len+n > cap { + ns := js.Global.Call("$growSlice", s, len+n) + s.Set(`$capacity`, ns.Get(`$capacity`)) + s.Set(`$array`, ns.Get(`$array`)) + s.Set(`$offset`, ns.Get(`$offset`)) + } +} + func (v Value) Index(i int) Value { switch k := v.kind(); k { case Array: diff --git a/compiler/prelude/prelude.js b/compiler/prelude/prelude.js index 0f6b9cb80..426aae032 100644 --- a/compiler/prelude/prelude.js +++ b/compiler/prelude/prelude.js @@ -433,37 +433,52 @@ var $internalAppend = (slice, array, offset, length) => { return slice; } - var newArray = slice.$array; - var newOffset = slice.$offset; var newLength = slice.$length + length; - var newCapacity = slice.$capacity; - - if (newLength > newCapacity) { - newOffset = 0; - newCapacity = Math.max(newLength, slice.$capacity < 1024 ? slice.$capacity * 2 : Math.floor(slice.$capacity * 5 / 4)); - - if (slice.$array.constructor === Array) { - newArray = slice.$array.slice(slice.$offset, slice.$offset + slice.$length); - newArray.length = newCapacity; - var zero = slice.constructor.elem.zero; - for (var i = slice.$length; i < newCapacity; i++) { - newArray[i] = zero(); - } - } else { - newArray = new slice.$array.constructor(newCapacity); - newArray.set(slice.$array.subarray(slice.$offset, slice.$offset + slice.$length)); - } - } - - $copyArray(newArray, array, newOffset + slice.$length, offset, length, slice.constructor.elem); + slice = $growSlice(slice, newLength); + + var newArray = slice.$array; + $copyArray(newArray, array, slice.$offset + slice.$length, offset, length, slice.constructor.elem); var newSlice = new slice.constructor(newArray); - newSlice.$offset = newOffset; + newSlice.$offset = slice.$offset; newSlice.$length = newLength; - newSlice.$capacity = newCapacity; + newSlice.$capacity = slice.$capacity; return newSlice; }; +const $calculateNewCapacity = (minCapacity, oldCapacity) => { + return Math.max(minCapacity, oldCapacity < 1024 ? oldCapacity * 2 : Math.floor(oldCapacity * 5 / 4)); +}; + +var $growSlice = (slice, minCapacity) => { + const oldCapacity = slice.$capacity; + if (minCapacity <= oldCapacity) { + return slice + } + + const newCapacity = $calculateNewCapacity(minCapacity, oldCapacity); + + let newArray; + if (slice.$array.constructor === Array) { + newArray = slice.$array.slice( slice.$offset, slice.$offset + slice.$length); + newArray.length = newCapacity; + let zero = slice.constructor.elem.zero; + for (let i = slice.$length; i < newCapacity; i++) { + newArray[i] = zero(); + } + } else { + newArray = new slice.$array.constructor(newCapacity); + newArray.set(slice.$array.subarray( slice.$offset, slice.$offset + slice.$length)); + } + + const oldLength = slice.$length; + slice = new slice.constructor(newArray); + slice.$offset = 0; + slice.$length = oldLength; + slice.$capacity = newCapacity; + return slice; +}; + var $equal = (a, b, type) => { if (type === $jsObjectPtr) { return a === b; From 9fdcb644273df90bc4b7f2a65f3fe57fb63044f4 Mon Sep 17 00:00:00 2001 From: Grant Nelson Date: Wed, 12 Jun 2024 15:39:41 -0600 Subject: [PATCH 2/3] Adding nil-check test --- compiler/natives/src/reflect/reflect.go | 10 +++++++ compiler/prelude/types.js | 1 + tests/arrays_test.go | 38 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/compiler/natives/src/reflect/reflect.go b/compiler/natives/src/reflect/reflect.go index 12fd05de9..cbc9cc54b 100644 --- a/compiler/natives/src/reflect/reflect.go +++ b/compiler/natives/src/reflect/reflect.go @@ -1830,3 +1830,13 @@ func verifyNotInHeapPtr(p uintptr) bool { // always return true. return true } + +// typedslicecopy is implemented in prelude.js as $copySlice +// +//gopherjs:purge +func typedslicecopy(elemType *rtype, dst, src unsafeheader.Slice) int + +// growslice is implemented in prelude.js as $growSlice. +// +//gopherjs:purge +func growslice(t *rtype, old unsafeheader.Slice, num int) unsafeheader.Slice diff --git a/compiler/prelude/types.js b/compiler/prelude/types.js index 9570b2fed..08be9593b 100644 --- a/compiler/prelude/types.js +++ b/compiler/prelude/types.js @@ -231,6 +231,7 @@ var $newType = (size, kind, string, named, pkg, exported, constructor) => { typ.comparable = false; typ.nativeArray = $nativeArray(elem.kind); typ.nil = new typ([]); + Object.freeze(typ.nil); }; break; diff --git a/tests/arrays_test.go b/tests/arrays_test.go index e79989991..feca983c5 100644 --- a/tests/arrays_test.go +++ b/tests/arrays_test.go @@ -83,3 +83,41 @@ func TestReflectArraySize(t *testing.T) { t.Errorf("array type size gave %v, want %v", got, want) } } + +func TestNilPrototypeNotModifiedByPointer(t *testing.T) { + const growth = 3 + + s1 := []int(nil) + p1 := &s1 + *p1 = make([]int, 0, growth) + if c := cap(s1); c != growth { + t.Errorf(`expected capacity of nil to increase to %d, got %d`, growth, c) + println("s1:", s1) + } + + s2 := []int(nil) + if c := cap(s2); c != 0 { + t.Errorf(`the capacity of nil must always be zero, it was %d`, c) + println("s1:", s1) + println("s2:", s2) + } +} + +func TestNilPrototypeNotModifiedByReflectGrow(t *testing.T) { + const growth = 3 + + s1 := []int(nil) + v1 := reflect.ValueOf(&s1).Elem() + v1.Grow(growth) + if c := cap(s1); c != growth { + t.Errorf(`expected capacity of nil to increase to %d, got %d`, growth, c) + println("s1:", s1) + } + + s2 := []int(nil) + if c := cap(s2); c != 0 { + t.Errorf(`the capacity of nil must always be zero, it was %d`, c) + println("s1:", s1) + println("s2:", s2) + } +} From 165ce970489e4c9dfa6a86cc5f56bb86395e840e Mon Sep 17 00:00:00 2001 From: Grant Nelson Date: Wed, 26 Jun 2024 15:51:11 -0600 Subject: [PATCH 3/3] Made changes per suggestions and help --- compiler/natives/src/reflect/reflect.go | 4 +- compiler/prelude/prelude.js | 82 +++++++++++++++---------- 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/compiler/natives/src/reflect/reflect.go b/compiler/natives/src/reflect/reflect.go index cbc9cc54b..eef450356 100644 --- a/compiler/natives/src/reflect/reflect.go +++ b/compiler/natives/src/reflect/reflect.go @@ -1339,9 +1339,7 @@ func (v Value) grow(n int) { cap := s.Get(`$capacity`).Int() if len+n > cap { ns := js.Global.Call("$growSlice", s, len+n) - s.Set(`$capacity`, ns.Get(`$capacity`)) - s.Set(`$array`, ns.Get(`$array`)) - s.Set(`$offset`, ns.Get(`$offset`)) + js.InternalObject(v.ptr).Call("$set", ns) } } diff --git a/compiler/prelude/prelude.js b/compiler/prelude/prelude.js index 426aae032..e925fc321 100644 --- a/compiler/prelude/prelude.js +++ b/compiler/prelude/prelude.js @@ -428,55 +428,75 @@ var $appendSlice = (slice, toAppend) => { return $internalAppend(slice, toAppend.$array, toAppend.$offset, toAppend.$length); }; +// Internal helper function for appending to a slice. +// The given slice will not be modified. +// +// If no values are being appended, the original slice will be returned. +// Otherwise, a new slice will be created with the appended values. +// +// If the underlying array has enough capacity, it will be used. +// Otherwise, a new array will be allocated with enough capacity to hold +// the new values and the original array will not be modified. var $internalAppend = (slice, array, offset, length) => { if (length === 0) { return slice; } - var newLength = slice.$length + length; - slice = $growSlice(slice, newLength); + let newLength = slice.$length + length; + let newSlice = $growSlice(slice, newLength); - var newArray = slice.$array; - $copyArray(newArray, array, slice.$offset + slice.$length, offset, length, slice.constructor.elem); - - var newSlice = new slice.constructor(newArray); - newSlice.$offset = slice.$offset; + let newArray = newSlice.$array; + $copyArray(newArray, array, newSlice.$offset + newSlice.$length, offset, length, newSlice.constructor.elem); + newSlice.$length = newLength; - newSlice.$capacity = slice.$capacity; return newSlice; }; +// Calculates the new capacity for a slice that is expected to grow to at least +// the given minCapacity. This follows the Go runtime's growth strategy. +// The oldCapacity is the current capacity of the slice that is being grown. const $calculateNewCapacity = (minCapacity, oldCapacity) => { return Math.max(minCapacity, oldCapacity < 1024 ? oldCapacity * 2 : Math.floor(oldCapacity * 5 / 4)); }; +// Potentially grows the slice to have a capacity of at least minCapacity. +// +// A new slice will always be returned, even if the given slice had the required capacity. +// If the slice didn't have enough capacity, the new slice will have a +// new array created for it with the required minimum capacity. +// +// This takes the place of the growSlice function in the reflect package. var $growSlice = (slice, minCapacity) => { - const oldCapacity = slice.$capacity; - if (minCapacity <= oldCapacity) { - return slice - } - - const newCapacity = $calculateNewCapacity(minCapacity, oldCapacity); - - let newArray; - if (slice.$array.constructor === Array) { - newArray = slice.$array.slice( slice.$offset, slice.$offset + slice.$length); - newArray.length = newCapacity; - let zero = slice.constructor.elem.zero; - for (let i = slice.$length; i < newCapacity; i++) { - newArray[i] = zero(); + let array = slice.$array; + let offset = slice.$offset; + const length = slice.$length; + let capacity = slice.$capacity; + + if (minCapacity > capacity) { + capacity = $calculateNewCapacity(minCapacity, capacity); + + let newArray; + if (array.constructor === Array) { + newArray = array.slice(offset, offset + length); + newArray.length = capacity; + const zero = slice.constructor.elem.zero; + for (let i = slice.$length; i < capacity; i++) { + newArray[i] = zero(); + } + } else { + newArray = new array.constructor(capacity); + newArray.set(array.subarray(offset, offset + length)); } - } else { - newArray = new slice.$array.constructor(newCapacity); - newArray.set(slice.$array.subarray( slice.$offset, slice.$offset + slice.$length)); + + array = newArray; + offset = 0; } - const oldLength = slice.$length; - slice = new slice.constructor(newArray); - slice.$offset = 0; - slice.$length = oldLength; - slice.$capacity = newCapacity; - return slice; + let newSlice = new slice.constructor(array); + newSlice.$offset = offset; + newSlice.$length = length; + newSlice.$capacity = capacity; + return newSlice; }; var $equal = (a, b, type) => {