diff --git a/src/generics/lib.generics.ts b/src/generics/lib.generics.ts index 1f87e4c..f59534c 100644 --- a/src/generics/lib.generics.ts +++ b/src/generics/lib.generics.ts @@ -71,7 +71,17 @@ namespace __Array { end = this.length; } - memmove(Ref(this[target]), Ref(this[start]), sizeof() * (end - start)); + // element by element, not memmove: under rc an element may own what it holds, and an + // assignment is what takes a reference to the copy and gives up the one overwritten. + // Backwards when the ranges overlap that way, as memmove would. + let count = end - start; + if (count > this.length - target) count = this.length - target; + if (target > start) { + for (let i = count - 1; i >= 0; i--) this[target + i] = this[start + i]; + } else { + for (let i = 0; i < count; i++) this[target + i] = this[start + i]; + } + return this; } @@ -82,12 +92,12 @@ namespace __Array { count += item.length; let newArray: T[] = []; newArray.length = count; + // element by element, not memcpy: under rc an element may own what it holds, and the new + // array's store is what takes the reference its release will give back let index = 0; - memcpy(Ref(newArray[index]), Ref(this[0]), sizeof() * this.length); - index += this.length; + for (const v of this) newArray[index++] = v; for (const item of other) { - memcpy(Ref(newArray[index]), Ref(item[0]), sizeof() * item.length); - index += item.length; + for (const v of item) newArray[index++] = v; } return newArray; @@ -131,7 +141,7 @@ namespace __Array { end = this.length; } - for (let i = start; i <= end; i++) + for (let i = start; i < end; i++) newArray[i] = value; return newArray; @@ -340,7 +350,8 @@ namespace __Array { let newArray: T[] = []; newArray.length = end - start; - memcpy(Ref(newArray[0]), Ref(this[start]), sizeof() * (end - start)); + // element by element, not memcpy (see concat) + for (let i = start; i < end; i++) newArray[i - start] = this[i]; return newArray; } @@ -821,7 +832,10 @@ class Map { entries.length = newSize; const count = this.count; - memcpy(Ref(entries[0]), Ref(this._entries[0]), sizeof() * count); + // entry by entry, not memcpy: under rc an entry owns its key and value, and the old array + // gives them up when it is replaced below - the new one has to have taken its own + const oldEntries = this._entries; + for (let i = 0; i < count; i++) entries[i] = oldEntries[i]; if (forceNewHashCodes) { this.newHashCodes(entries); @@ -1243,7 +1257,10 @@ class Set { entries.length = newSize; const count = this.count; - memcpy(Ref(entries[0]), Ref(this._entries[0]), sizeof() * count); + // entry by entry, not memcpy: under rc an entry owns its key and value, and the old array + // gives them up when it is replaced below - the new one has to have taken its own + const oldEntries = this._entries; + for (let i = 0; i < count; i++) entries[i] = oldEntries[i]; if (forceNewHashCodes) { this.newHashCodes(entries); diff --git a/src/lib.ts b/src/lib.ts index d0c91f5..4e19ade 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -842,7 +842,9 @@ namespace __String { const newString = this.clone().resize(newSize); let index = this.length; const byteSize = sizeof() * this.length; - for (let i = 0; i < count; i++) { + // the clone holds the first copy already: count - 1 more fill the string, and one more + // wrote past its end + for (let i = 1; i < count; i++) { memcpy(Ref(newString[index]), this, byteSize); index += this.length; } diff --git a/tests/array_fill.ts b/tests/array_fill.ts index 4d9986d..bdef201 100644 --- a/tests/array_fill.ts +++ b/tests/array_fill.ts @@ -2,14 +2,27 @@ function main() { const array1 = [1, 2, 3, 4]; // Fill with 0 from position 2 until position 4 - console.log(array1.fill(0, 2, 4)); + const zeros = array1.fill(0, 2, 4); + console.log(zeros); // Expected output: Array [1, 2, 0, 0] + assert(zeros.length == 4 && zeros[0] == 1 && zeros[1] == 2 && zeros[2] == 0 && zeros[3] == 0); // Fill with 5 from position 1 - console.log(array1.fill(5, 1)); + const fives = array1.fill(5, 1); + console.log(fives); // Expected output: Array [1, 5, 5, 5] + assert(fives.length == 4 && fives[0] == 1 && fives[1] == 5 && fives[2] == 5 && fives[3] == 5); - console.log(array1.fill(6)); + const sixes = array1.fill(6); + console.log(sixes); // Expected output: Array [6, 6, 6, 6] + assert(sixes.length == 4 && sixes[0] == 6 && sixes[1] == 6 && sixes[2] == 6 && sixes[3] == 6); + + // `end` is not filled: this wrote one past it, and past the end of the array when `end` is its length + const inner = [1, 2, 3].fill(9, 1, 2); + console.log(inner); + // Expected output: Array [1, 9, 3] + assert(inner.length == 3 && inner[0] == 1 && inner[1] == 9 && inner[2] == 3); + console.log("ALL DONE"); } diff --git a/tests/owning_copies.ts b/tests/owning_copies.ts new file mode 100644 index 0000000..9ba5ade --- /dev/null +++ b/tests/owning_copies.ts @@ -0,0 +1,57 @@ +// Copies of elements that own what they hold. Array.concat, slice and copyWithin, and Set and Map +// growing their entries, copied elements with memcpy - under rc that duplicated references nobody +// took, so the copy and the original released the same strings and the heap was corrupted. Every +// string here is built at run time: a literal is immortal and would hide it. + +function made(prefix: string, n: int): string[] { + const result: string[] = []; + for (let i = 0; i < n; i++) result.push(prefix + i); + return result; +} + +function concatHoldsItsOwn() { + const a = made("a", 3); + const joined = a.concat(made("b", 2), made("c", 1)); + return joined.join() == "a0,a1,a2,b0,b1,c0" && a.join() == "a0,a1,a2"; +} + +function sliceHoldsItsOwn() { + const a = made("s", 5); + const part = a.slice(1, 4); + return part.join() == "s1,s2,s3" && a.join() == "s0,s1,s2,s3,s4"; +} + +function copyWithinKeepsEveryElement() { + const a = made("w", 5); + a.copyWithin(0, 3); + const b = made("v", 5); + b.copyWithin(2, 0); + return a.join() == "w3,w4,w2,w3,w4" && b.join() == "v0,v1,v0,v1,v2"; +} + +function setGrowsWithItsStrings() { + const s = new Set(); + for (const v of made("e", 20)) s.add(v); + let all = ""; + for (const v of s) all += v; + return s.size == 20 && s.has("e7") && s.has("e19") && all.length == 50; +} + +function mapGrowsWithItsStrings() { + const m = new Map(); + for (let i = 0; i < 20; i++) m.set("k" + i, "v" + i); + return m.size == 20 && m.get("k7") == "v7" && m.get("k19") == "v19"; +} + +function main() { + // twice over, so the second round allocates over whatever the first freed + for (let round = 0; round < 2; round++) { + assert(concatHoldsItsOwn(), "concat"); + assert(sliceHoldsItsOwn(), "slice"); + assert(copyWithinKeepsEveryElement(), "copyWithin"); + assert(setGrowsWithItsStrings(), "Set growth"); + assert(mapGrowsWithItsStrings(), "Map growth"); + } + + console.log("ALL DONE"); +} diff --git a/tests/string_repeat.ts b/tests/string_repeat.ts index 4b8315b..33c4d5c 100644 --- a/tests/string_repeat.ts +++ b/tests/string_repeat.ts @@ -6,6 +6,11 @@ function main() { console.log("abc".repeat(0)); // '' console.log("abc".repeat(1)); // 'abc' - console.log("abc".repeat(2)); // 'abcabc' + console.log("abc".repeat(2)); // 'abcabc' + + assert(mood.repeat(3) == "Happy! Happy! Happy! "); + assert("abc".repeat(0) == ""); + assert("abc".repeat(1) == "abc"); + assert("abc".repeat(2) == "abcabc"); console.log("ALL DONE"); }