From a899d824b8f382e665920bd51b3adba0da1dd2ff Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sat, 14 Nov 2026 16:28:47 +0000 Subject: [PATCH] Map constructor keeps a repeated key's last value; a looping chain throws new Map([["a", 1], ["a", 3]]).get("a") was 1: the constructor inserted with ThrowOnExisting, whose branch was an empty stub that skipped the entry. It inserts with OverwriteExisting now, so the last value wins and the key keeps its first place, as in JavaScript. ThrowOnExisting is gone: nothing else needed it. Set's constructor and add passed it too, and for them an equal value was already left alone; they pass None. The six places that find a chain of entries going round (a Map or a Set changed by another thread while this one reads it) were comments saying "throw exception". Two of them, in the insert loops, did nothing at all, so the loop would not have ended. They throw an Error now. map_constructor_duplicates.ts covers the constructors. Tested in release on Windows x64 for gc, rc, none and own, compiled and under the JIT; debug, Linux, x86 and wasm were not built locally. Co-Authored-By: Claude Fable 5.2 --- src/generics/lib.generics.ts | 44 +++++++++++++---------------- tests/map_constructor_duplicates.ts | 21 ++++++++++++++ 2 files changed, 41 insertions(+), 24 deletions(-) create mode 100644 tests/map_constructor_duplicates.ts diff --git a/src/generics/lib.generics.ts b/src/generics/lib.generics.ts index 236f91d..2f0ea7b 100644 --- a/src/generics/lib.generics.ts +++ b/src/generics/lib.generics.ts @@ -2139,10 +2139,14 @@ interface PromiseFulfilledResult { value: T; } +// thrown where a chain of entries is found to go round, which only a change made by another thread +// while this one reads can do +const CHAIN_LOOPS = "Map or Set changed by another thread while in use"; + +// what an insert does with a key already there: leaves its value, or replaces it enum InsertionBehavior { None, OverwriteExisting, - ThrowOnExisting, } namespace HashHelpers { @@ -2217,7 +2221,8 @@ class Map { } else { this.initialize(values.length); - for (const [k, v] of values) this.tryInsert(k, v, InsertionBehavior.ThrowOnExisting); + // a key given twice keeps its last value, as in JavaScript + for (const [k, v] of values) this.tryInsert(k, v, InsertionBehavior.OverwriteExisting); } } @@ -2364,15 +2369,13 @@ class Map { i = this._entries[i].next; collisionCount++; if (collisionCount > this._entries.length) { - // throw exception + // the chain goes round: another thread changed it under this one. The loop + // would never end + throw new Error(CHAIN_LOOPS); } } if (found >= 0 && behavior != InsertionBehavior.OverwriteExisting) { - if (behavior == InsertionBehavior.ThrowOnExisting) { - // throw error - } - return false; } @@ -2448,8 +2451,7 @@ class Map { // The chain of entries forms a loop; which means a concurrent update has happened. // Break out of the loop and throw, rather than looping forever. - // TODO: throw exception - return undefined; + throw new Error(CHAIN_LOOPS); } private hasValue(key: K): boolean { @@ -2478,8 +2480,7 @@ class Map { // The chain of entries forms a loop; which means a concurrent update has happened. // Break out of the loop and throw, rather than looping forever. - // TODO: throw exception - return false; + throw new Error(CHAIN_LOOPS); } private removeValue(key: K): boolean { @@ -2515,8 +2516,7 @@ class Map { if (collisionCount > entries.length) { // The chain of entries forms a loop; which means a concurrent update has happened. // Break out of the loop and throw, rather than looping forever. - // throw exception - return false; + throw new Error(CHAIN_LOOPS); } } @@ -2621,7 +2621,7 @@ class Set { } else { this.initialize(values.length); - for (const v of values) this.AddIfNotPresent(v, InsertionBehavior.ThrowOnExisting); + for (const v of values) this.AddIfNotPresent(v, InsertionBehavior.None); } } @@ -2647,7 +2647,7 @@ class Set { } add(v: V) { - this.AddIfNotPresent(v, InsertionBehavior.ThrowOnExisting); + this.AddIfNotPresent(v, InsertionBehavior.None); return this; } @@ -2845,17 +2845,15 @@ class Set { i = this._entries[i].next; collisionCount++; if (collisionCount > this._entries.length) { - // throw exception + // the chain goes round: another thread changed it under this one. The loop + // would never end + throw new Error(CHAIN_LOOPS); } } // outside the loop: under -mm=own the path that does not take the value releases it if (found) { // a value equal to one in the set is not put in again, whatever the behavior - if (behavior == InsertionBehavior.ThrowOnExisting) { - // throw error - } - return false; } @@ -2917,8 +2915,7 @@ class Set { // The chain of entries forms a loop; which means a concurrent update has happened. // Break out of the loop and throw, rather than looping forever. - // TODO: throw exception - return false; + throw new Error(CHAIN_LOOPS); } private removeValue(value: V): boolean { @@ -2954,8 +2951,7 @@ class Set { if (collisionCount > entries.length) { // The chain of entries forms a loop; which means a concurrent update has happened. // Break out of the loop and throw, rather than looping forever. - // throw exception - return false; + throw new Error(CHAIN_LOOPS); } } diff --git a/tests/map_constructor_duplicates.ts b/tests/map_constructor_duplicates.ts new file mode 100644 index 0000000..4177846 --- /dev/null +++ b/tests/map_constructor_duplicates.ts @@ -0,0 +1,21 @@ +// A key given twice to the Map constructor keeps its last value and its first place; a value given +// twice to the Set constructor is in the set once. +function main() { + const map = new Map([["a", 1], ["b", 2], ["a", 3]]); + assert(map.size == 2, "two keys"); + assert(map.get("a") == 3, "the last value given for a key"); + assert(map.get("b") == 2, "the others as they were"); + let order = ""; + for (const [k, v] of map) order += `${k}${v}`; + assert(order == "a3b2", "the key keeps its first place"); + + const numbers = new Map([[1, "one"], [1, "uno"], [2, "two"], [1, "ein"]]); + assert(numbers.size == 2 && numbers.get(1) == "ein", "number keys"); + + const set = new Set(["x", "y", "x"]); + assert(set.size == 2 && set.has("x") && set.has("y"), "a set of two"); + set.add("x"); + assert(set.size == 2, "add of a value already there"); + + console.log("ALL DONE"); +}