From 955fa125c4e957a34218bef33866826377c5f18b Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sat, 27 Jun 2026 19:34:52 +0100 Subject: [PATCH 1/4] perf(engine): back strong Map/Set with a SameValueZero-keyed ordered hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strong Map and Set stored entries in a flat list and located keys by a linear SameValueZero scan, making get/set/has/delete O(n), build/probe loops O(n^2), and set-algebra O(n*m). Back both with a single new TGocciaOrderedValueMap — the first production consumer of the generic insertion-ordered TOrderedMap (ADR 0019) — whose HashKey/KeysEqual are overridden with a hash exactly consistent with IsSameValueZero. Ops are now O(1) amortized and set-algebra O(n+m); insertion order is preserved. Deletes tombstone instead of shifting and iterators hold a physical cursor, matching the spec [[MapData]] model. Compaction (which renumbers entries) is gated by a per-store live-iterator counter via a new CanCompact virtual on TOrderedMap, so live cursors never observe a renumbering. Insertion canonicalizes -0 to +0 (ES2026 §24.5.1). Adopting the spec model fixed four pre-existing conformance bugs the flat-list code carried, none covered by the prior suite: -0 keys stored un-normalized; forEach/iterators not seeing entries appended during iteration; deleting during forEach throwing "Argument out of range"; and deleting the current key during for...of skipping the next entry. JS regression tests for all four ship here, plus a Pascal invariant test for the hash/equality consistency. Closes #807 Co-Authored-By: Claude Opus 4.8 --- .../0077-samevaluezero-ordered-collections.md | 15 + docs/adr/README.md | 1 + source/shared/OrderedMap.pas | 13 +- source/units/Goccia.Builtins.GlobalMap.pas | 7 +- source/units/Goccia.Builtins.Globals.pas | 22 +- source/units/Goccia.Evaluator.Comparison.pas | 29 +- source/units/Goccia.REPL.Formatter.pas | 38 +- .../units/Goccia.Values.Iterator.Concrete.pas | 102 ++++-- source/units/Goccia.Values.MapValue.pas | 215 ++++++----- .../Goccia.Values.OrderedValueMap.Test.pas | 343 ++++++++++++++++++ .../units/Goccia.Values.OrderedValueMap.pas | 206 +++++++++++ source/units/Goccia.Values.SetValue.pas | 276 ++++++++------ tests/built-ins/Map/prototype/entries.js | 29 ++ tests/built-ins/Map/prototype/forEach.js | 46 +++ tests/built-ins/Map/prototype/set.js | 46 +++ tests/built-ins/Set/prototype/add.js | 24 ++ tests/built-ins/Set/prototype/forEach.js | 31 ++ tests/built-ins/Set/prototype/values.js | 22 ++ 18 files changed, 1176 insertions(+), 289 deletions(-) create mode 100644 docs/adr/0077-samevaluezero-ordered-collections.md create mode 100644 source/units/Goccia.Values.OrderedValueMap.Test.pas create mode 100644 source/units/Goccia.Values.OrderedValueMap.pas diff --git a/docs/adr/0077-samevaluezero-ordered-collections.md b/docs/adr/0077-samevaluezero-ordered-collections.md new file mode 100644 index 000000000..85abc8d93 --- /dev/null +++ b/docs/adr/0077-samevaluezero-ordered-collections.md @@ -0,0 +1,15 @@ +# SameValueZero-keyed ordered store for Map and Set + +**Date:** 2026-06-27 +**Area:** `data-structures` / `engine` +**Issue:** [#807](https://github.com/frostney/GocciaScript/issues/807) + +Strong `Map` and `Set` stored their entries in a flat list and located keys by a linear SameValueZero scan, so every `get`/`set`/`has`/`delete` was O(n) and any build-or-probe loop was O(n²); set-algebra (`union`/`intersection`/…) was O(n·m). `WeakMap`/`WeakSet` already use [`THashMap`](0019-custom-hash-maps-over-tdictionary.md) keyed by pointer identity, but strong collections cannot reuse it: their keys compare by **SameValueZero** (ES2026 §7.2.11) — `NaN` equals `NaN`, `-0` equals `+0`, strings and BigInts compare by content, and only objects/functions/symbols compare by reference. Identity hashing would split equal primitives across buckets. The distinction is therefore **content-hash (strong Map/Set) vs identity-hash (weak collections)**, and the two stores stay separate. + +We back both `Map` and `Set` with a single new type, `TGocciaOrderedValueMap` (`source/units/Goccia.Values.OrderedValueMap.pas`) — the first production consumer of the generic insertion-ordered `TOrderedMap` (ADR 0019). It overrides the map's protected `HashKey`/`KeysEqual` with a hash that is **exactly consistent with `IsSameValueZero`** (number with NaN- and signed-zero canonicalization over the IEEE-754 bits, string/BigInt by content, primitives by category, objects by pointer — the GC never relocates, so pointers are stable) and `IsSameValueZero` itself for equality. `Set` reuses the same type with the element stored as both key and value, so there is one store, one hash, and one set of tests rather than a per-collection subclass or a parallel structure. Operations become O(1) amortized and set-algebra O(n+m). The hash/equality pair is the load-bearing invariant — an inconsistent hash silently loses entries — so it is guarded by a Pascal invariant test (`Goccia.Values.OrderedValueMap.Test.pas`) in addition to the JavaScript suite. + +Live-iterator semantics follow the spec's `[[MapData]]` model: the entry list is append-only and `delete` writes a tombstone rather than shifting, so a Map/Set iterator (and `forEach`) holds a physical cursor that skips tombstones and sees entries appended mid-iteration (ES2026 §24.1.3.5). The one hazard is the inherited tombstone-reclaiming compaction, which renumbers entries; we add a `CanCompact` virtual to `TOrderedMap` (default unchanged) and gate it on a per-store live-iterator counter (`RetainIterator`/`ReleaseIterator`), so while any iteration is in flight the store only grows (which preserves indices) and never renumbers, then reclaims tombstones once no iteration is active. This is the same shape as engines that only shrink a keyed collection's storage once no iterator references it. The residual gap is an iterator that is partially consumed and then abandoned (never exhausted or closed) before being garbage-collected: it leaves the gate held, so that store stops reclaiming tombstones until it is cleared, and sustained `set`+`delete` churn on it grows storage meanwhile. That is a safe degradation, not a correctness bug — the common cases (full `for...of`, spread, `forEach`, an exhausted or `return()`-closed iterator) always release the gate — and releasing it for abandoned iterators needs GC-driven notification (the iterator cannot safely touch its source store from a sweep-time destructor), which is left as a follow-up. Insertion also applies CanonicalizeKeyedCollectionKey (ES2026 §24.5.1), storing `-0` as `+0`. + +Adopting the spec model fixed four pre-existing conformance bugs the flat-list implementation carried (none covered by the prior suite): `-0` keys were stored un-normalized; `forEach`/iterators did not visit entries appended during iteration; deleting during `forEach` threw `RangeError: Argument out of range`; and deleting the current key during `for...of` skipped the next entry and left `size` wrong. Regression tests for all four ship with this change. + +Alternatives rejected: **extending `THashMap` with SameValueZero** — it is unordered, tombstone-free, and identity-tuned, so it would need insertion ordering and content equality bolted on, i.e. effectively this store; a **purpose-built standalone store** modelled on `TOrderedStringMap` — avoids virtual `HashKey` dispatch but adds a fourth map implementation to maintain and leaves `TOrderedMap` with no production user; **keeping the flat list with a side hash index** — leaves `delete` O(n) and does not fix the live-iteration bugs; and **compacting eagerly always** (renumbers live cursors) or **never** (unbounded growth under sustained `set`+`delete` churn). See [ADR 0019](0019-custom-hash-maps-over-tdictionary.md) and [docs/value-system.md](../value-system.md). diff --git a/docs/adr/README.md b/docs/adr/README.md index 1be3f122d..f424b7be7 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -86,3 +86,4 @@ Durable architecture and implementation decisions for GocciaScript. New ADRs use - [0074 — Deferred bytecode call-stack frames](0074-deferred-bytecode-call-stack-frames.md) - [0075 — ShadowRealm full test262 conformance](0075-shadowrealm-conformance.md) - [0076 — Same-runner benchmark comparison](0076-same-runner-benchmark-comparison.md) +- [0077 — SameValueZero-keyed ordered store for Map and Set](0077-samevaluezero-ordered-collections.md) diff --git a/source/shared/OrderedMap.pas b/source/shared/OrderedMap.pas index 37bd1bd80..03cf19ae0 100644 --- a/source/shared/OrderedMap.pas +++ b/source/shared/OrderedMap.pas @@ -72,6 +72,12 @@ TEnumerator = record protected function HashKey(const AKey: TKey): Cardinal; virtual; function KeysEqual(const A, B: TKey): Boolean; virtual; + // Gate for the tombstone-reclaiming Compact in Add. Default True. + // Subclasses whose entries are observed by live, index-based cursors + // (e.g. JS Map/Set iterators) override this to suppress compaction + // while a cursor is active: Compact renumbers FEntries, but Grow does + // not, so returning False keeps physical indices stable for the cursor. + function CanCompact: Boolean; virtual; function GetCount: Integer; override; function GetValue(const AKey: TKey): TValue; override; @@ -140,6 +146,11 @@ function TOrderedMap.KeysEqual(const A, B: TKey): Boolean; Result := CompareMem(@A, @B, SizeOf(TKey)); end; +function TOrderedMap.CanCompact: Boolean; +begin + Result := True; +end; + { Probe } function TOrderedMap.FindBucket(const AKey: TKey; AHash: Cardinal; @@ -282,7 +293,7 @@ procedure TOrderedMap.Add(const AKey: TKey; const AValue: TValue); if (FEntryCount + 1) * 100 > FBucketCount * LOAD_FACTOR_PERCENT then begin - if FCount < FEntryCount div 2 then + if (FCount < FEntryCount div 2) and CanCompact then Compact else Grow; diff --git a/source/units/Goccia.Builtins.GlobalMap.pas b/source/units/Goccia.Builtins.GlobalMap.pas index 50162dcb5..3e8dc4710 100644 --- a/source/units/Goccia.Builtins.GlobalMap.pas +++ b/source/units/Goccia.Builtins.GlobalMap.pas @@ -63,7 +63,7 @@ function TGocciaGlobalMap.MapGroupBy(const AArgs: TGocciaArgumentsCollection; co procedure AddToGroup(const AValue: TGocciaValue; const AIndex: Integer); var - EntryIndex: Integer; + ExistingGroup: TGocciaValue; begin CallArgs := TGocciaArgumentsCollection.Create; try @@ -75,9 +75,8 @@ function TGocciaGlobalMap.MapGroupBy(const AArgs: TGocciaArgumentsCollection; co end; // Use Map's SameValueZero lookup to preserve key identity (1 vs "1", etc.) - EntryIndex := ResultMap.FindEntry(GroupKey); - if EntryIndex >= 0 then - GroupArray := TGocciaArrayValue(ResultMap.Entries[EntryIndex].Value) + if ResultMap.TryGetValue(GroupKey, ExistingGroup) then + GroupArray := TGocciaArrayValue(ExistingGroup) else begin GroupArray := TGocciaArrayValue.Create; diff --git a/source/units/Goccia.Builtins.Globals.pas b/source/units/Goccia.Builtins.Globals.pas index 34a6c2e3a..541c852c7 100644 --- a/source/units/Goccia.Builtins.Globals.pas +++ b/source/units/Goccia.Builtins.Globals.pas @@ -835,31 +835,31 @@ function CloneArray(const AArr: TGocciaArrayValue; function CloneMap(const AMap: TGocciaMapValue; const AMemory: THashMap): TGocciaMapValue; var - I: Integer; - Entry: TGocciaMapEntry; + Cursor: Integer; + Key, Value: TGocciaValue; begin Result := TGocciaMapValue.Create; AMemory.Add(AMap, Result); - for I := 0 to AMap.Entries.Count - 1 do - begin - Entry := AMap.Entries[I]; + Cursor := 0; + while AMap.NextEntry(Cursor, Key, Value) do Result.SetEntry( - StructuredCloneValue(Entry.Key, AMemory), - StructuredCloneValue(Entry.Value, AMemory)); - end; + StructuredCloneValue(Key, AMemory), + StructuredCloneValue(Value, AMemory)); end; function CloneSet(const ASet: TGocciaSetValue; const AMemory: THashMap): TGocciaSetValue; var - I: Integer; + Cursor: Integer; + Item: TGocciaValue; begin Result := TGocciaSetValue.Create; AMemory.Add(ASet, Result); - for I := 0 to ASet.Items.Count - 1 do - Result.AddItem(StructuredCloneValue(ASet.Items[I], AMemory)); + Cursor := 0; + while ASet.NextItem(Cursor, Item) do + Result.AddItem(StructuredCloneValue(Item, AMemory)); end; function CloneArrayBuffer(const ABuf: TGocciaArrayBufferValue; diff --git a/source/units/Goccia.Evaluator.Comparison.pas b/source/units/Goccia.Evaluator.Comparison.pas index b875d88f3..e2a3655b7 100644 --- a/source/units/Goccia.Evaluator.Comparison.pas +++ b/source/units/Goccia.Evaluator.Comparison.pas @@ -79,6 +79,8 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue; ActualKeys, ExpectedKeys: TArray; I: Integer; Key: string; + CursorA, CursorB: Integer; + LeftKey, LeftValue, RightKey, RightValue: TGocciaValue; begin // Base case: strict equality (handles primitives and same object references) if IsStrictEqual(AActual, AExpected) then @@ -142,10 +144,10 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue; Exit; end; - // Handle Sets + // Handle Sets — compared by insertion order, element for element. if (AActual is TGocciaSetValue) and (AExpected is TGocciaSetValue) then begin - if TGocciaSetValue(AActual).Items.Count <> TGocciaSetValue(AExpected).Items.Count then + if TGocciaSetValue(AActual).Count <> TGocciaSetValue(AExpected).Count then begin Result := False; Exit; @@ -156,10 +158,12 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue; Exit; end; AddComparedPair(AComparedPairs, AActual, AExpected); - for I := 0 to TGocciaSetValue(AActual).Items.Count - 1 do + CursorA := 0; + CursorB := 0; + while TGocciaSetValue(AActual).NextItem(CursorA, LeftValue) do begin - if not IsDeepEqualInternal(TGocciaSetValue(AActual).Items[I], - TGocciaSetValue(AExpected).Items[I], AComparedPairs) then + TGocciaSetValue(AExpected).NextItem(CursorB, RightValue); + if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then begin Result := False; Exit; @@ -169,10 +173,10 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue; Exit; end; - // Handle Maps + // Handle Maps — compared by insertion order, entry for entry. if (AActual is TGocciaMapValue) and (AExpected is TGocciaMapValue) then begin - if TGocciaMapValue(AActual).Entries.Count <> TGocciaMapValue(AExpected).Entries.Count then + if TGocciaMapValue(AActual).Count <> TGocciaMapValue(AExpected).Count then begin Result := False; Exit; @@ -183,16 +187,17 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue; Exit; end; AddComparedPair(AComparedPairs, AActual, AExpected); - for I := 0 to TGocciaMapValue(AActual).Entries.Count - 1 do + CursorA := 0; + CursorB := 0; + while TGocciaMapValue(AActual).NextEntry(CursorA, LeftKey, LeftValue) do begin - if not IsDeepEqualInternal(TGocciaMapValue(AActual).Entries[I].Key, - TGocciaMapValue(AExpected).Entries[I].Key, AComparedPairs) then + TGocciaMapValue(AExpected).NextEntry(CursorB, RightKey, RightValue); + if not IsDeepEqualInternal(LeftKey, RightKey, AComparedPairs) then begin Result := False; Exit; end; - if not IsDeepEqualInternal(TGocciaMapValue(AActual).Entries[I].Value, - TGocciaMapValue(AExpected).Entries[I].Value, AComparedPairs) then + if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then begin Result := False; Exit; diff --git a/source/units/Goccia.REPL.Formatter.pas b/source/units/Goccia.REPL.Formatter.pas index eec20c0a3..aceee2b84 100644 --- a/source/units/Goccia.REPL.Formatter.pas +++ b/source/units/Goccia.REPL.Formatter.pas @@ -73,28 +73,32 @@ function FormatMapValue(const AMap: TGocciaMapValue; const AUseColor: Boolean): string; var SB: TStringBuffer; - I, Remaining: Integer; + I, Total, Cursor: Integer; + Key, Value: TGocciaValue; begin SB := TStringBuffer.Create; - SB.Append('Map(' + IntToStr(AMap.Entries.Count) + ')'); - if AMap.Entries.Count = 0 then + Total := AMap.Count; + SB.Append('Map(' + IntToStr(Total) + ')'); + if Total = 0 then SB.Append(' {}') else begin SB.Append(' { '); - for I := 0 to AMap.Entries.Count - 1 do + I := 0; + Cursor := 0; + while AMap.NextEntry(Cursor, Key, Value) do begin if I >= MAX_INSPECT_ITEMS then begin - Remaining := AMap.Entries.Count - MAX_INSPECT_ITEMS; - SB.Append(', ... ' + IntToStr(Remaining) + ' more'); + SB.Append(', ... ' + IntToStr(Total - MAX_INSPECT_ITEMS) + ' more'); Break; end; if I > 0 then SB.Append(', '); - SB.Append(FormatREPLValue(AMap.Entries[I].Key, AUseColor)); + SB.Append(FormatREPLValue(Key, AUseColor)); SB.Append(' => '); - SB.Append(FormatREPLValue(AMap.Entries[I].Value, AUseColor)); + SB.Append(FormatREPLValue(Value, AUseColor)); + Inc(I); end; SB.Append(' }'); end; @@ -105,26 +109,30 @@ function FormatSetValue(const ASet: TGocciaSetValue; const AUseColor: Boolean): string; var SB: TStringBuffer; - I, Remaining: Integer; + I, Total, Cursor: Integer; + Item: TGocciaValue; begin SB := TStringBuffer.Create; - SB.Append('Set(' + IntToStr(ASet.Items.Count) + ')'); - if ASet.Items.Count = 0 then + Total := ASet.Count; + SB.Append('Set(' + IntToStr(Total) + ')'); + if Total = 0 then SB.Append(' {}') else begin SB.Append(' { '); - for I := 0 to ASet.Items.Count - 1 do + I := 0; + Cursor := 0; + while ASet.NextItem(Cursor, Item) do begin if I >= MAX_INSPECT_ITEMS then begin - Remaining := ASet.Items.Count - MAX_INSPECT_ITEMS; - SB.Append(', ... ' + IntToStr(Remaining) + ' more'); + SB.Append(', ... ' + IntToStr(Total - MAX_INSPECT_ITEMS) + ' more'); Break; end; if I > 0 then SB.Append(', '); - SB.Append(FormatREPLValue(ASet.Items[I], AUseColor)); + SB.Append(FormatREPLValue(Item, AUseColor)); + Inc(I); end; SB.Append(' }'); end; diff --git a/source/units/Goccia.Values.Iterator.Concrete.pas b/source/units/Goccia.Values.Iterator.Concrete.pas index 52ee6f593..dccd12d29 100644 --- a/source/units/Goccia.Values.Iterator.Concrete.pas +++ b/source/units/Goccia.Values.Iterator.Concrete.pas @@ -43,12 +43,18 @@ TGocciaStringIteratorValue = class(TGocciaIteratorValue) TGocciaMapIteratorValue = class(TGocciaIteratorValue) private FSource: TGocciaValue; - FIndex: Integer; + // Physical cursor into the Map's live entry array (skips tombstones, sees + // appends); paired with RetainIterator/ReleaseIterator so compaction never + // renumbers it mid-iteration. + FCursor: Integer; + FReleased: Boolean; FKind: TGocciaMapIteratorKind; + procedure ReleaseSource; public constructor Create(const ASource: TGocciaValue; const AKind: TGocciaMapIteratorKind); function AdvanceNext: TGocciaObjectValue; override; function DirectNext(out ADone: Boolean): TGocciaValue; override; + procedure Close; override; function ToStringTag: string; override; procedure MarkReferences; override; end; @@ -58,12 +64,15 @@ TGocciaMapIteratorValue = class(TGocciaIteratorValue) TGocciaSetIteratorValue = class(TGocciaIteratorValue) private FSource: TGocciaValue; - FIndex: Integer; + FCursor: Integer; + FReleased: Boolean; FKind: TGocciaSetIteratorKind; + procedure ReleaseSource; public constructor Create(const ASource: TGocciaValue; const AKind: TGocciaSetIteratorKind); function AdvanceNext: TGocciaObjectValue; override; function DirectNext(out ADone: Boolean): TGocciaValue; override; + procedure Close; override; function ToStringTag: string; override; procedure MarkReferences; override; end; @@ -377,16 +386,29 @@ constructor TGocciaMapIteratorValue.Create(const ASource: TGocciaValue; const AK if Assigned(SharedPrototype) then FPrototype := SharedPrototype; FSource := ASource; - FIndex := 0; + FCursor := 0; + FReleased := False; FKind := AKind; + // Hold compaction off while this iterator is live so FCursor stays valid. + TGocciaMapValue(ASource).RetainIterator; finally RemoveTempRootIfNeeded(ASource, SourceWasRooted); end; end; +procedure TGocciaMapIteratorValue.ReleaseSource; +begin + if not FReleased then + begin + FReleased := True; + TGocciaMapValue(FSource).ReleaseIterator; + end; +end; + function TGocciaMapIteratorValue.AdvanceNext: TGocciaObjectValue; var MapVal: TGocciaMapValue; + Key, Value: TGocciaValue; EntryArray: TGocciaArrayValue; begin if FDone then @@ -396,9 +418,10 @@ function TGocciaMapIteratorValue.AdvanceNext: TGocciaObjectValue; end; MapVal := TGocciaMapValue(FSource); - if FIndex >= MapVal.Entries.Count then + if not MapVal.NextEntry(FCursor, Key, Value) then begin FDone := True; + ReleaseSource; Result := CreateIteratorResult(TGocciaUndefinedLiteralValue.UndefinedValue, True); Exit; end; @@ -407,21 +430,21 @@ function TGocciaMapIteratorValue.AdvanceNext: TGocciaObjectValue; mkEntries: begin EntryArray := TGocciaArrayValue.Create; - EntryArray.Elements.Add(MapVal.Entries[FIndex].Key); - EntryArray.Elements.Add(MapVal.Entries[FIndex].Value); + EntryArray.Elements.Add(Key); + EntryArray.Elements.Add(Value); Result := CreateIteratorResult(EntryArray, False); end; mkKeys: - Result := CreateIteratorResult(MapVal.Entries[FIndex].Key, False); + Result := CreateIteratorResult(Key, False); mkValues: - Result := CreateIteratorResult(MapVal.Entries[FIndex].Value, False); + Result := CreateIteratorResult(Value, False); end; - Inc(FIndex); end; function TGocciaMapIteratorValue.DirectNext(out ADone: Boolean): TGocciaValue; var MapVal: TGocciaMapValue; + Key, Value: TGocciaValue; EntryArray: TGocciaArrayValue; begin if FDone then @@ -432,9 +455,10 @@ function TGocciaMapIteratorValue.DirectNext(out ADone: Boolean): TGocciaValue; end; MapVal := TGocciaMapValue(FSource); - if FIndex >= MapVal.Entries.Count then + if not MapVal.NextEntry(FCursor, Key, Value) then begin FDone := True; + ReleaseSource; ADone := True; Result := TGocciaUndefinedLiteralValue.UndefinedValue; Exit; @@ -445,16 +469,22 @@ function TGocciaMapIteratorValue.DirectNext(out ADone: Boolean): TGocciaValue; mkEntries: begin EntryArray := TGocciaArrayValue.Create; - EntryArray.Elements.Add(MapVal.Entries[FIndex].Key); - EntryArray.Elements.Add(MapVal.Entries[FIndex].Value); + EntryArray.Elements.Add(Key); + EntryArray.Elements.Add(Value); Result := EntryArray; end; mkKeys: - Result := MapVal.Entries[FIndex].Key; + Result := Key; mkValues: - Result := MapVal.Entries[FIndex].Value; + Result := Value; end; - Inc(FIndex); +end; + +procedure TGocciaMapIteratorValue.Close; +begin + FDone := True; + ReleaseSource; + inherited Close; end; function TGocciaMapIteratorValue.ToStringTag: string; @@ -485,16 +515,28 @@ constructor TGocciaSetIteratorValue.Create(const ASource: TGocciaValue; const AK if Assigned(SharedPrototype) then FPrototype := SharedPrototype; FSource := ASource; - FIndex := 0; + FCursor := 0; + FReleased := False; FKind := AKind; + TGocciaSetValue(ASource).RetainIterator; finally RemoveTempRootIfNeeded(ASource, SourceWasRooted); end; end; +procedure TGocciaSetIteratorValue.ReleaseSource; +begin + if not FReleased then + begin + FReleased := True; + TGocciaSetValue(FSource).ReleaseIterator; + end; +end; + function TGocciaSetIteratorValue.AdvanceNext: TGocciaObjectValue; var SetVal: TGocciaSetValue; + Item: TGocciaValue; EntryArray: TGocciaArrayValue; begin if FDone then @@ -504,30 +546,31 @@ function TGocciaSetIteratorValue.AdvanceNext: TGocciaObjectValue; end; SetVal := TGocciaSetValue(FSource); - if FIndex >= SetVal.Items.Count then + if not SetVal.NextItem(FCursor, Item) then begin FDone := True; + ReleaseSource; Result := CreateIteratorResult(TGocciaUndefinedLiteralValue.UndefinedValue, True); Exit; end; case FKind of skValues: - Result := CreateIteratorResult(SetVal.Items[FIndex], False); + Result := CreateIteratorResult(Item, False); skEntries: begin EntryArray := TGocciaArrayValue.Create; - EntryArray.Elements.Add(SetVal.Items[FIndex]); - EntryArray.Elements.Add(SetVal.Items[FIndex]); + EntryArray.Elements.Add(Item); + EntryArray.Elements.Add(Item); Result := CreateIteratorResult(EntryArray, False); end; end; - Inc(FIndex); end; function TGocciaSetIteratorValue.DirectNext(out ADone: Boolean): TGocciaValue; var SetVal: TGocciaSetValue; + Item: TGocciaValue; EntryArray: TGocciaArrayValue; begin if FDone then @@ -538,9 +581,10 @@ function TGocciaSetIteratorValue.DirectNext(out ADone: Boolean): TGocciaValue; end; SetVal := TGocciaSetValue(FSource); - if FIndex >= SetVal.Items.Count then + if not SetVal.NextItem(FCursor, Item) then begin FDone := True; + ReleaseSource; ADone := True; Result := TGocciaUndefinedLiteralValue.UndefinedValue; Exit; @@ -549,16 +593,22 @@ function TGocciaSetIteratorValue.DirectNext(out ADone: Boolean): TGocciaValue; ADone := False; case FKind of skValues: - Result := SetVal.Items[FIndex]; + Result := Item; skEntries: begin EntryArray := TGocciaArrayValue.Create; - EntryArray.Elements.Add(SetVal.Items[FIndex]); - EntryArray.Elements.Add(SetVal.Items[FIndex]); + EntryArray.Elements.Add(Item); + EntryArray.Elements.Add(Item); Result := EntryArray; end; end; - Inc(FIndex); +end; + +procedure TGocciaSetIteratorValue.Close; +begin + FDone := True; + ReleaseSource; + inherited Close; end; function TGocciaSetIteratorValue.ToStringTag: string; diff --git a/source/units/Goccia.Values.MapValue.pas b/source/units/Goccia.Values.MapValue.pas index 7b6e045e5..b8a133c30 100644 --- a/source/units/Goccia.Values.MapValue.pas +++ b/source/units/Goccia.Values.MapValue.pas @@ -5,25 +5,19 @@ interface uses - Generics.Collections, - Goccia.Arguments.Collection, Goccia.ObjectModel, Goccia.SharedPrototype, Goccia.Values.ArrayValue, Goccia.Values.ClassValue, Goccia.Values.ObjectValue, + Goccia.Values.OrderedValueMap, Goccia.Values.Primitives; type - TGocciaMapEntry = record - Key: TGocciaValue; - Value: TGocciaValue; - end; - TGocciaMapValue = class(TGocciaInstanceValue) private - FEntries: TList; + FStore: TGocciaOrderedValueMap; public function MapGet(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; function MapSet(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; @@ -39,11 +33,23 @@ TGocciaMapValue = class(TGocciaInstanceValue) function MapGetOrInsertComputed(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; procedure InitializePrototype; public - function FindEntry(const AKey: TGocciaValue): Integer; constructor Create(const AClass: TGocciaClassValue = nil); destructor Destroy; override; + // Insert or in-place update (keeps insertion position), with -0 -> +0 + // key canonicalization. procedure SetEntry(const AKey, AValue: TGocciaValue); + // SameValueZero lookup. Returns False (and AValue undefined) when absent. + function TryGetValue(const AKey: TGocciaValue; out AValue: TGocciaValue): Boolean; + + // Live, insertion-ordered cursor used by the Map iterators / forEach. + // Seed ACursor with 0; new entries appended mid-iteration are visited and + // deleted entries are skipped. Bracket external iteration with + // RetainIterator / ReleaseIterator so compaction cannot renumber entries. + function NextEntry(var ACursor: Integer; out AKey, AValue: TGocciaValue): Boolean; + procedure RetainIterator; + procedure ReleaseIterator; + function Count: Integer; function GetProperty(const AName: string): TGocciaValue; override; function GetPropertyWithContext(const AName: string; const AThisContext: TGocciaValue): TGocciaValue; override; @@ -54,14 +60,11 @@ TGocciaMapValue = class(TGocciaInstanceValue) procedure MarkReferences; override; class procedure ExposePrototype(const AConstructor: TGocciaValue); - - property Entries: TList read FEntries; end; implementation uses - Goccia.Arithmetic, Goccia.Constants.ConstructorNames, Goccia.Constants.PropertyNames, Goccia.Error.Messages, @@ -97,7 +100,7 @@ constructor TGocciaMapValue.Create(const AClass: TGocciaClassValue = nil); Shared: TGocciaSharedPrototype; begin inherited Create(AClass); - FEntries := TList.Create; + FStore := TGocciaOrderedValueMap.Create; InitializePrototype; Shared := GetMapShared; if not Assigned(AClass) and Assigned(Shared) then @@ -164,7 +167,7 @@ class procedure TGocciaMapValue.ExposePrototype(const AConstructor: TGocciaValue destructor TGocciaMapValue.Destroy; begin - FEntries.Free; + FStore.Free; inherited; end; @@ -202,48 +205,52 @@ procedure TGocciaMapValue.InitializeNativeFromArguments(const AArguments: TGocci procedure TGocciaMapValue.MarkReferences; var - I: Integer; + Cursor: Integer; + Key, Value: TGocciaValue; begin if GCMarked then Exit; inherited; - for I := 0 to FEntries.Count - 1 do + Cursor := 0; + while FStore.NextEntry(Cursor, Key, Value) do begin - if Assigned(FEntries[I].Key) then - FEntries[I].Key.MarkReferences; - if Assigned(FEntries[I].Value) then - FEntries[I].Value.MarkReferences; + if Assigned(Key) then + Key.MarkReferences; + if Assigned(Value) then + Value.MarkReferences; end; end; -function TGocciaMapValue.FindEntry(const AKey: TGocciaValue): Integer; -var - I: Integer; +procedure TGocciaMapValue.SetEntry(const AKey, AValue: TGocciaValue); begin - for I := 0 to FEntries.Count - 1 do - begin - if IsSameValueZero(FEntries[I].Key, AKey) then - begin - Result := I; - Exit; - end; - end; - Result := -1; + FStore.SetEntry(AKey, AValue); end; -procedure TGocciaMapValue.SetEntry(const AKey, AValue: TGocciaValue); -var - Index: Integer; - Entry: TGocciaMapEntry; +function TGocciaMapValue.TryGetValue(const AKey: TGocciaValue; + out AValue: TGocciaValue): Boolean; begin - Index := FindEntry(AKey); - Entry.Key := AKey; - Entry.Value := AValue; + Result := FStore.TryGetValue(AKey, AValue); +end; - if Index >= 0 then - FEntries[Index] := Entry - else - FEntries.Add(Entry); +function TGocciaMapValue.NextEntry(var ACursor: Integer; + out AKey, AValue: TGocciaValue): Boolean; +begin + Result := FStore.NextEntry(ACursor, AKey, AValue); +end; + +procedure TGocciaMapValue.RetainIterator; +begin + FStore.RetainIterator; +end; + +procedure TGocciaMapValue.ReleaseIterator; +begin + FStore.ReleaseIterator; +end; + +function TGocciaMapValue.Count: Integer; +begin + Result := FStore.Count; end; function TGocciaMapValue.GetProperty(const AName: string): TGocciaValue; @@ -254,22 +261,24 @@ function TGocciaMapValue.GetProperty(const AName: string): TGocciaValue; function TGocciaMapValue.GetPropertyWithContext(const AName: string; const AThisContext: TGocciaValue): TGocciaValue; begin if AName = PROP_SIZE then - Result := TGocciaNumberLiteralValue.Create(FEntries.Count) + Result := TGocciaNumberLiteralValue.Create(FStore.Count) else Result := inherited GetPropertyWithContext(AName, AThisContext); end; function TGocciaMapValue.ToArray: TGocciaArrayValue; var - I: Integer; + Cursor: Integer; + Key, Value: TGocciaValue; EntryArr: TGocciaArrayValue; begin Result := TGocciaArrayValue.Create; - for I := 0 to FEntries.Count - 1 do + Cursor := 0; + while FStore.NextEntry(Cursor, Key, Value) do begin EntryArr := TGocciaArrayValue.Create; - EntryArr.Elements.Add(FEntries[I].Key); - EntryArr.Elements.Add(FEntries[I].Value); + EntryArr.Elements.Add(Key); + EntryArr.Elements.Add(Value); Result.Elements.Add(EntryArr); end; end; @@ -285,26 +294,20 @@ function TGocciaMapValue.ToStringTag: string; function TGocciaMapValue.MapGet(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var M: TGocciaMapValue; - Index: Integer; + Value: TGocciaValue; begin // Step 1: Let M be the this value // Steps 2-3: If M does not have a [[MapData]] internal slot, throw a TypeError if not (AThisValue is TGocciaMapValue) then ThrowTypeError(SErrorMapGetNonMap, SSuggestMapThisType); M := TGocciaMapValue(AThisValue); - if AArgs.Length > 0 then - begin - // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do - // Step 4a: If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, return p.[[Value]] - Index := M.FindEntry(AArgs.GetElement(0)); - if Index >= 0 then - begin - Result := M.Entries[Index].Value; - Exit; - end; - end; - // Step 5: Return undefined - Result := TGocciaUndefinedLiteralValue.UndefinedValue; + // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do + // Step 4a: If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, return p.[[Value]] + if (AArgs.Length > 0) and M.TryGetValue(AArgs.GetElement(0), Value) then + Result := Value + else + // Step 5: Return undefined + Result := TGocciaUndefinedLiteralValue.UndefinedValue; end; // ES2026 §24.1.3.9 Map.prototype.set(key, value) @@ -325,7 +328,7 @@ function TGocciaMapValue.MapSet(const AArgs: TGocciaArgumentsCollection; const A // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, // set p.[[Value]] to value and return M - // Step 5: If key is -0, set key to +0 + // Step 5: Set key to CanonicalizeKeyedCollectionKey(key) (-0 -> +0) // Step 6: Let p be the Record { [[Key]]: key, [[Value]]: value } // Step 7: Append p to M.[[MapData]] M.SetEntry(MapKey, MapValue); @@ -338,6 +341,7 @@ function TGocciaMapValue.MapSet(const AArgs: TGocciaArgumentsCollection; const A function TGocciaMapValue.MapHas(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var M: TGocciaMapValue; + Value: TGocciaValue; begin // Step 1: Let M be the this value // Steps 2-3: If M does not have a [[MapData]] internal slot, throw a TypeError @@ -346,7 +350,7 @@ function TGocciaMapValue.MapHas(const AArgs: TGocciaArgumentsCollection; const A M := TGocciaMapValue(AThisValue); // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, return true - if (AArgs.Length > 0) and (M.FindEntry(AArgs.GetElement(0)) >= 0) then + if (AArgs.Length > 0) and M.TryGetValue(AArgs.GetElement(0), Value) then Result := TGocciaBooleanLiteralValue.TrueValue else // Step 5: Return false @@ -357,29 +361,20 @@ function TGocciaMapValue.MapHas(const AArgs: TGocciaArgumentsCollection; const A function TGocciaMapValue.MapDelete(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var M: TGocciaMapValue; - Index: Integer; begin // Step 1: Let M be the this value // Steps 2-3: If M does not have a [[MapData]] internal slot, throw a TypeError if not (AThisValue is TGocciaMapValue) then ThrowTypeError(SErrorMapDeleteNonMap, SSuggestMapThisType); M := TGocciaMapValue(AThisValue); - // Step 5 (early): Default return false - Result := TGocciaBooleanLiteralValue.FalseValue; - if AArgs.Length > 0 then - begin - // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do - Index := M.FindEntry(AArgs.GetElement(0)); - if Index >= 0 then - begin - // Step 4a: If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true - // Step 4a.i: Set p.[[Key]] to empty / Step 4a.ii: Set p.[[Value]] to empty - M.FEntries.Delete(Index); - // Step 4a.iii: Return true - Result := TGocciaBooleanLiteralValue.TrueValue; - end; - end; - // Step 5: Return false + // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do + // If SameValueZero(p.[[Key]], key) is true, set p.[[Key]]/[[Value]] to + // empty (tombstone) and return true + if (AArgs.Length > 0) and M.FStore.Remove(AArgs.GetElement(0)) then + Result := TGocciaBooleanLiteralValue.TrueValue + else + // Step 5: Return false + Result := TGocciaBooleanLiteralValue.FalseValue; end; // ES2026 §24.1.3.1 Map.prototype.clear() @@ -391,7 +386,7 @@ function TGocciaMapValue.MapClear(const AArgs: TGocciaArgumentsCollection; const ThrowTypeError(SErrorMapClearNonMap, SSuggestMapThisType); // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // Set p.[[Key]] to empty, set p.[[Value]] to empty - TGocciaMapValue(AThisValue).FEntries.Clear; + TGocciaMapValue(AThisValue).FStore.Clear; // Step 5: Return undefined Result := TGocciaUndefinedLiteralValue.UndefinedValue; end; @@ -400,10 +395,10 @@ function TGocciaMapValue.MapClear(const AArgs: TGocciaArgumentsCollection; const function TGocciaMapValue.MapForEach(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var M: TGocciaMapValue; - Callback, ThisArg: TGocciaValue; + Callback, ThisArg, Key, Value: TGocciaValue; TypedCallback: TGocciaFunctionBase; CallArgs: TGocciaArgumentsCollection; - I: Integer; + Cursor: Integer; MapRoot, CallbackRoot, ThisRoot: TGocciaTempRoot; begin // Steps 1-3: If M does not have a [[MapData]] internal slot, throw a TypeError @@ -435,12 +430,17 @@ function TGocciaMapValue.MapForEach(const AArgs: TGocciaArgumentsCollection; con AddTempRootIfNeeded(MapRoot, M); AddTempRootIfNeeded(CallbackRoot, Callback); AddTempRootIfNeeded(ThisRoot, ThisArg); + // Hold compaction off so the cursor's entry indices stay valid even if the + // callback adds or deletes entries (ES2026 §24.1.3.5: numEntries is re-read + // each step; appended keys are visited, deleted keys are skipped). + M.RetainIterator; try - // Step 6: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do - for I := 0 to M.Entries.Count - 1 do + // Step 6: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do + Cursor := 0; + while M.NextEntry(Cursor, Key, Value) do begin // Step 6b: Call(callbackfn, thisArg, « p.[[Value]], p.[[Key]], M ») - CallArgs := TGocciaArgumentsCollection.Create([M.Entries[I].Value, M.Entries[I].Key, M]); + CallArgs := TGocciaArgumentsCollection.Create([Value, Key, M]); try if Assigned(TypedCallback) then TypedCallback.Call(CallArgs, ThisArg) @@ -454,6 +454,7 @@ function TGocciaMapValue.MapForEach(const AArgs: TGocciaArgumentsCollection; con // Step 7: Return undefined Result := TGocciaUndefinedLiteralValue.UndefinedValue; finally + M.ReleaseIterator; RemoveTempRootIfNeeded(ThisRoot); RemoveTempRootIfNeeded(CallbackRoot); RemoveTempRootIfNeeded(MapRoot); @@ -508,34 +509,26 @@ function TGocciaMapValue.MapSymbolIterator(const AArgs: TGocciaArgumentsCollecti function TGocciaMapValue.MapGetOrInsert(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var M: TGocciaMapValue; - MapKey, DefaultValue: TGocciaValue; - Index: Integer; + MapKey, DefaultValue, Existing: TGocciaValue; begin // Steps 1-3: If M does not have a [[MapData]] internal slot, throw a TypeError if not (AThisValue is TGocciaMapValue) then ThrowTypeError(SErrorMapGetOrInsertNonMap, SSuggestMapThisType); M := TGocciaMapValue(AThisValue); - if AArgs.Length < 2 then - begin - if AArgs.Length > 0 then - MapKey := AArgs.GetElement(0) - else - MapKey := TGocciaUndefinedLiteralValue.UndefinedValue; - Index := M.FindEntry(MapKey); - if Index >= 0 then - Exit(M.Entries[Index].Value); - DefaultValue := TGocciaUndefinedLiteralValue.UndefinedValue; - M.SetEntry(MapKey, DefaultValue); - Exit(DefaultValue); - end; + if AArgs.Length > 0 then + MapKey := AArgs.GetElement(0) + else + MapKey := TGocciaUndefinedLiteralValue.UndefinedValue; - MapKey := AArgs.GetElement(0); - DefaultValue := AArgs.GetElement(1); // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // If SameValueZero(p.[[Key]], key) is true, return p.[[Value]] - Index := M.FindEntry(MapKey); - if Index >= 0 then - Exit(M.Entries[Index].Value); + if M.TryGetValue(MapKey, Existing) then + Exit(Existing); + + if AArgs.Length >= 2 then + DefaultValue := AArgs.GetElement(1) + else + DefaultValue := TGocciaUndefinedLiteralValue.UndefinedValue; // Step 5: Set key to CanonicalizeKeyedCollectionKey(key) // Step 6: Append Record { [[Key]]: key, [[Value]]: value } to M.[[MapData]] M.SetEntry(MapKey, DefaultValue); @@ -547,10 +540,9 @@ function TGocciaMapValue.MapGetOrInsert(const AArgs: TGocciaArgumentsCollection; function TGocciaMapValue.MapGetOrInsertComputed(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var M: TGocciaMapValue; - MapKey, CallbackArg, ComputedValue: TGocciaValue; + MapKey, CallbackArg, ComputedValue, Existing: TGocciaValue; TypedCallback: TGocciaFunctionBase; CallArgs: TGocciaArgumentsCollection; - Index: Integer; MapRoot, KeyRoot, CallbackRoot: TGocciaTempRoot; begin // Steps 1-3: If M does not have a [[MapData]] internal slot, throw a TypeError @@ -572,9 +564,8 @@ function TGocciaMapValue.MapGetOrInsertComputed(const AArgs: TGocciaArgumentsCol // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // If SameValueZero(p.[[Key]], key) is true, return p.[[Value]] - Index := M.FindEntry(MapKey); - if Index >= 0 then - Exit(M.Entries[Index].Value); + if M.TryGetValue(MapKey, Existing) then + Exit(Existing); InitializeTempRoot(MapRoot); InitializeTempRoot(KeyRoot); diff --git a/source/units/Goccia.Values.OrderedValueMap.Test.pas b/source/units/Goccia.Values.OrderedValueMap.Test.pas new file mode 100644 index 000000000..518b8bf91 --- /dev/null +++ b/source/units/Goccia.Values.OrderedValueMap.Test.pas @@ -0,0 +1,343 @@ +program Goccia.Values.OrderedValueMap.Test; + +{$I Goccia.inc} + +uses + Math, + SysUtils, + + BigInteger, + TestingPascalLibrary, + + Goccia.TestSetup, + Goccia.Values.BigIntValue, + Goccia.Values.OrderedValueMap, + Goccia.Values.Primitives; + +type + TOrderedValueMapTests = class(TTestSuite) + private + { Hash/equality consistency: SameValueZero-equal keys (distinct instances) + must collapse to one entry. A hash inconsistent with the equality would + route them to different buckets and silently keep both. } + procedure TestNaNKeysCollapse; + procedure TestSignedZeroKeysCollapse; + procedure TestEqualNumberInstancesCollapse; + procedure TestEqualStringInstancesCollapse; + procedure TestEqualBigIntInstancesCollapse; + procedure TestLookupWithDistinctEqualInstance; + + { No false collapse: SameValueZero-distinct keys stay separate. } + procedure TestNumberAndStringAreDistinct; + procedure TestDistinctNumbersAreDistinct; + procedure TestBooleanAndNumberAreDistinct; + procedure TestNullAndUndefinedAreDistinct; + + { -0 canonicalization (ES2026 §24.5.1). } + procedure TestNegativeZeroStoredAsPositiveZero; + procedure TestCanonicalizeKeyMapsNegativeZero; + + { Insertion-ordered cursor with tombstone deletes. } + procedure TestCursorVisitsInInsertionOrder; + procedure TestCursorSkipsTombstonesAndReaddAppends; + + { Compaction gating for live iterators. } + procedure TestIteratorRetainReleaseCounter; + public + procedure SetupTests; override; + end; + +procedure TOrderedValueMapTests.SetupTests; +begin + Test('NaN keys collapse to one entry', TestNaNKeysCollapse); + Test('-0 and +0 collapse to one entry', TestSignedZeroKeysCollapse); + Test('Equal number instances collapse', TestEqualNumberInstancesCollapse); + Test('Equal string instances collapse', TestEqualStringInstancesCollapse); + Test('Equal BigInt instances collapse', TestEqualBigIntInstancesCollapse); + Test('Lookup succeeds with a distinct equal instance', TestLookupWithDistinctEqualInstance); + Test('Number 1 and string "1" are distinct keys', TestNumberAndStringAreDistinct); + Test('Distinct numbers are distinct keys', TestDistinctNumbersAreDistinct); + Test('Boolean and number are distinct keys', TestBooleanAndNumberAreDistinct); + Test('null and undefined are distinct keys', TestNullAndUndefinedAreDistinct); + Test('-0 is stored as +0', TestNegativeZeroStoredAsPositiveZero); + Test('CanonicalizeKey maps -0 to +0', TestCanonicalizeKeyMapsNegativeZero); + Test('Cursor visits entries in insertion order', TestCursorVisitsInInsertionOrder); + Test('Cursor skips tombstones; re-add appends at end', TestCursorSkipsTombstonesAndReaddAppends); + Test('RetainIterator/ReleaseIterator track active count', TestIteratorRetainReleaseCounter); +end; + +function Num(const AValue: Double): TGocciaValue; inline; +begin + Result := TGocciaNumberLiteralValue.Create(AValue); +end; + +function Str(const AValue: string): TGocciaValue; inline; +begin + Result := TGocciaStringLiteralValue.Create(AValue); +end; + +procedure TOrderedValueMapTests.TestNaNKeysCollapse; +var + Store: TGocciaOrderedValueMap; + V: TGocciaValue; +begin + Store := TGocciaOrderedValueMap.Create; + try + // Two distinct NaN instances. + Store.SetEntry(Num(Math.NaN), Str('first')); + Store.SetEntry(Num(Math.NaN), Str('second')); + Expect(Store.Count).ToBe(1); + Expect(Store.TryGetValue(Num(Math.NaN), V)).ToBe(True); + Expect(TGocciaStringLiteralValue(V).Value).ToBe('second'); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestSignedZeroKeysCollapse; +var + Store: TGocciaOrderedValueMap; + V: TGocciaValue; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(TGocciaNumberLiteralValue.NegativeZeroValue, Str('neg')); + Store.SetEntry(TGocciaNumberLiteralValue.ZeroValue, Str('pos')); + Expect(Store.Count).ToBe(1); + Expect(Store.TryGetValue(Num(0), V)).ToBe(True); + Expect(TGocciaStringLiteralValue(V).Value).ToBe('pos'); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestEqualNumberInstancesCollapse; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(Num(42), Str('a')); + Store.SetEntry(Num(42), Str('b')); + Expect(Store.Count).ToBe(1); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestEqualStringInstancesCollapse; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + // Distinct string instances with equal content (no string interning): + // proves content hashing, not reference identity. + Store.SetEntry(Str('ab'), Str('a')); + Store.SetEntry(Str('a' + 'b'), Str('b')); + Expect(Store.Count).ToBe(1); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestEqualBigIntInstancesCollapse; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(TGocciaBigIntValue.Create(TBigInteger.FromInt64(123)), Str('a')); + Store.SetEntry(TGocciaBigIntValue.Create(TBigInteger.FromInt64(123)), Str('b')); + Expect(Store.Count).ToBe(1); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestLookupWithDistinctEqualInstance; +var + Store: TGocciaOrderedValueMap; + V: TGocciaValue; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(Str('key'), Str('value')); + Expect(Store.TryGetValue(Str('key'), V)).ToBe(True); + Expect(TGocciaStringLiteralValue(V).Value).ToBe('value'); + Expect(Store.ContainsKey(Str('absent'))).ToBe(False); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestNumberAndStringAreDistinct; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(Num(1), Str('num')); + Store.SetEntry(Str('1'), Str('str')); + Expect(Store.Count).ToBe(2); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestDistinctNumbersAreDistinct; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(Num(1), Str('one')); + Store.SetEntry(Num(2), Str('two')); + Expect(Store.Count).ToBe(2); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestBooleanAndNumberAreDistinct; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(TGocciaBooleanLiteralValue.TrueValue, Str('bool')); + Store.SetEntry(Num(1), Str('num')); + Expect(Store.Count).ToBe(2); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestNullAndUndefinedAreDistinct; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(TGocciaNullLiteralValue.NullValue, Str('null')); + Store.SetEntry(TGocciaUndefinedLiteralValue.UndefinedValue, Str('undef')); + Expect(Store.Count).ToBe(2); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestNegativeZeroStoredAsPositiveZero; +var + Store: TGocciaOrderedValueMap; + Cursor: Integer; + Key, Value: TGocciaValue; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(TGocciaNumberLiteralValue.NegativeZeroValue, Str('z')); + Cursor := 0; + Expect(Store.NextEntry(Cursor, Key, Value)).ToBe(True); + Expect(TGocciaNumberLiteralValue(Key).IsNegativeZero).ToBe(False); + Expect(TGocciaNumberLiteralValue(Key).Value).ToBe(0); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestCanonicalizeKeyMapsNegativeZero; +var + Canonical: TGocciaValue; +begin + Canonical := TGocciaOrderedValueMap.CanonicalizeKey( + TGocciaNumberLiteralValue.NegativeZeroValue); + Expect(TGocciaNumberLiteralValue(Canonical).IsNegativeZero).ToBe(False); + // A non -0 key is returned unchanged. + Expect(TGocciaOrderedValueMap.CanonicalizeKey(Num(5)) = nil).ToBe(False); +end; + +procedure TOrderedValueMapTests.TestCursorVisitsInInsertionOrder; +var + Store: TGocciaOrderedValueMap; + Cursor: Integer; + Key, Value: TGocciaValue; + Order: string; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(Str('a'), Num(1)); + Store.SetEntry(Str('b'), Num(2)); + Store.SetEntry(Str('c'), Num(3)); + // Updating b keeps its position. + Store.SetEntry(Str('b'), Num(99)); + Order := ''; + Cursor := 0; + while Store.NextEntry(Cursor, Key, Value) do + Order := Order + TGocciaStringLiteralValue(Key).Value; + Expect(Order).ToBe('abc'); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestCursorSkipsTombstonesAndReaddAppends; +var + Store: TGocciaOrderedValueMap; + Cursor: Integer; + Key, Value: TGocciaValue; + Order: string; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(Str('a'), Num(1)); + Store.SetEntry(Str('b'), Num(2)); + Store.SetEntry(Str('c'), Num(3)); + Expect(Store.Remove(Str('b'))).ToBe(True); + Expect(Store.Count).ToBe(2); + + Order := ''; + Cursor := 0; + while Store.NextEntry(Cursor, Key, Value) do + Order := Order + TGocciaStringLiteralValue(Key).Value; + Expect(Order).ToBe('ac'); + + // Re-adding a removed key appends it at the end. + Store.SetEntry(Str('b'), Num(4)); + Order := ''; + Cursor := 0; + while Store.NextEntry(Cursor, Key, Value) do + Order := Order + TGocciaStringLiteralValue(Key).Value; + Expect(Order).ToBe('acb'); + finally + Store.Free; + end; +end; + +procedure TOrderedValueMapTests.TestIteratorRetainReleaseCounter; +var + Store: TGocciaOrderedValueMap; +begin + Store := TGocciaOrderedValueMap.Create; + try + Expect(Store.ActiveIterators).ToBe(0); + Store.RetainIterator; + Store.RetainIterator; + Expect(Store.ActiveIterators).ToBe(2); + Store.ReleaseIterator; + Expect(Store.ActiveIterators).ToBe(1); + Store.ReleaseIterator; + Expect(Store.ActiveIterators).ToBe(0); + // Release never drives the counter negative. + Store.ReleaseIterator; + Expect(Store.ActiveIterators).ToBe(0); + finally + Store.Free; + end; +end; + +begin + TestRunnerProgram.AddSuite(TOrderedValueMapTests.Create('OrderedValueMap')); + TestRunnerProgram.Run; + + ExitCode := TestResultToExitCode; +end. diff --git a/source/units/Goccia.Values.OrderedValueMap.pas b/source/units/Goccia.Values.OrderedValueMap.pas new file mode 100644 index 000000000..46ba941b8 --- /dev/null +++ b/source/units/Goccia.Values.OrderedValueMap.pas @@ -0,0 +1,206 @@ +{ + TGocciaOrderedValueMap — the SameValueZero-keyed, insertion-ordered store + that backs strong `Map` and `Set`. + + It is the single specialization of the generic insertion-ordered hash map + (TOrderedMap, see ADR 0019) for ECMAScript keyed collections: it overrides + HashKey/KeysEqual with a hash that is exactly consistent with + SameValueZero (ES2026 §7.2.11 / Goccia.Arithmetic.IsSameValueZero), and + overrides CanCompact so live, index-based Map/Set iterators never observe a + renumbering compaction. + + Both Map (key -> value) and Set (key -> key) use this one type; there is no + per-collection subclass. Identity-keyed weak collections keep using THashMap + (WeakMap/WeakSet) — the content-vs-identity distinction is recorded in + docs/adr/0077. + + Hashing must stay consistent with IsSameValueZero. The invariant + "IsSameValueZero(a, b) implies HashValue(a) = HashValue(b)" is enforced by + Goccia.Values.OrderedValueMap.Test.pas; an inconsistent hash silently loses + entries, so change the two in lockstep. +} + +unit Goccia.Values.OrderedValueMap; + +{$I Goccia.inc} + +interface + +uses + OrderedMap, + + Goccia.Values.Primitives; + +type + TGocciaOrderedValueMap = class(TOrderedMap) + private + // While > 0 a live Map/Set iterator (or forEach) holds index-based + // cursors into the entry array; compaction (which renumbers entries) is + // suppressed until every cursor is released so the cursors stay valid. + FActiveIterators: Integer; + protected + function HashKey(const AKey: TGocciaValue): Cardinal; override; + function KeysEqual(const A, B: TGocciaValue): Boolean; override; + function CanCompact: Boolean; override; + public + // ES2026 §24.5.1 CanonicalizeKeyedCollectionKey: -0 is stored as +0 so + // keys()/values()/entries()/forEach observe +0. SameValueZero already + // treats -0 and +0 as equal for lookup, so callers only need this for the + // value that gets stored. + class function CanonicalizeKey(const AKey: TGocciaValue): TGocciaValue; static; inline; + + // Insert or in-place update (keeping the original insertion position), + // canonicalizing the key first. + procedure SetEntry(const AKey, AValue: TGocciaValue); + + // Live, insertion-ordered cursor over active entries. ACursor is a + // physical index; seed it with 0 and keep calling until False. New + // entries appended during iteration are visited and deleted (tombstoned) + // entries are skipped, matching the spec [[MapData]] iteration model. + function NextEntry(var ACursor: Integer; + out AKey, AValue: TGocciaValue): Boolean; inline; + + // Bracket a live iteration so compaction cannot renumber entries mid-walk. + procedure RetainIterator; inline; + procedure ReleaseIterator; inline; + + property ActiveIterators: Integer read FActiveIterators; + end; + +implementation + +uses + BigInteger, + + Goccia.Arithmetic, + Goccia.Values.BigIntValue; + +const + // Distinct seeds per primitive category. Cross-type values are never + // SameValueZero-equal, so these only need to be deterministic; the seeds + // just keep categories from colliding for better probe distribution. + HASH_UNDEFINED = Cardinal($9E3779B1); + HASH_NULL = Cardinal($85EBCA77); + HASH_TRUE = Cardinal($C2B2AE3D); + HASH_FALSE = Cardinal($27D4EB2F); + HASH_ZERO = Cardinal($165667B1); + HASH_NAN = Cardinal($D6E8FEB8); + +{$PUSH}{$R-}{$Q-} +function TGocciaOrderedValueMap.HashKey(const AKey: TGocciaValue): Cardinal; +var + Num: TGocciaNumberLiteralValue; + D: Double; + Bits: QWord; + S: string; + I: Integer; +begin + // Numbers: canonicalize NaN (all NaNs are SameValueZero-equal) and signed + // zero (-0 and +0 are equal); otherwise mix the raw IEEE-754 bits, which + // are identical for any two doubles that compare equal (except +0/-0, just + // handled). + if AKey is TGocciaNumberLiteralValue then + begin + Num := TGocciaNumberLiteralValue(AKey); + if Num.IsNaN then + Exit(HASH_NAN); + D := Num.Value; + if D = 0 then + Exit(HASH_ZERO); + Bits := PQWord(@D)^; + Bits := (Bits xor (Bits shr 4)) * QWord(11400714819323198485); + Exit(Cardinal(Bits xor (Bits shr 32))); + end; + + // Strings: DJB2 over the content, mirroring OrderedStringMap so string keys + // hash identically to the engine's other string-keyed maps. + if AKey is TGocciaStringLiteralValue then + begin + S := TGocciaStringLiteralValue(AKey).Value; + Result := 5381; + for I := 1 to Length(S) do + Result := Result * 33 + Ord(S[I]); + Exit; + end; + + if AKey is TGocciaBooleanLiteralValue then + begin + if TGocciaBooleanLiteralValue(AKey).Value then + Exit(HASH_TRUE) + else + Exit(HASH_FALSE); + end; + + // BigInt: hash the canonical decimal string. TBigInteger is normalized, so + // Value.Equal(other) holds iff the decimal strings match. + if AKey is TGocciaBigIntValue then + begin + S := TGocciaBigIntValue(AKey).Value.ToString; + Result := 5381; + for I := 1 to Length(S) do + Result := Result * 33 + Ord(S[I]); + Exit(Result xor Cardinal($9747B28C)); + end; + + if AKey is TGocciaUndefinedLiteralValue then + Exit(HASH_UNDEFINED); + if AKey is TGocciaNullLiteralValue then + Exit(HASH_NULL); + + // Objects, functions, symbols: SameValueZero is reference identity, so hash + // the pointer. The GC never relocates objects, so the pointer is stable for + // the entry's lifetime. + Bits := QWord(PtrUInt(AKey)); + Bits := (Bits xor (Bits shr 4)) * QWord(11400714819323198485); + Result := Cardinal(Bits xor (Bits shr 32)); +end; +{$POP} + +function TGocciaOrderedValueMap.KeysEqual(const A, B: TGocciaValue): Boolean; +begin + // The equality half of the hash/equality pair must be the exact function + // SameValueZero hashing above is consistent with. + Result := IsSameValueZero(A, B); +end; + +function TGocciaOrderedValueMap.CanCompact: Boolean; +begin + Result := FActiveIterators = 0; +end; + +class function TGocciaOrderedValueMap.CanonicalizeKey( + const AKey: TGocciaValue): TGocciaValue; +begin + if (AKey is TGocciaNumberLiteralValue) and + TGocciaNumberLiteralValue(AKey).IsNegativeZero then + Result := TGocciaNumberLiteralValue.ZeroValue + else + Result := AKey; +end; + +procedure TGocciaOrderedValueMap.SetEntry(const AKey, AValue: TGocciaValue); +begin + // inherited Add updates the value in place when the key already exists, + // preserving the entry's original insertion position (Map.set/Set.add + // semantics). + Add(CanonicalizeKey(AKey), AValue); +end; + +function TGocciaOrderedValueMap.NextEntry(var ACursor: Integer; + out AKey, AValue: TGocciaValue): Boolean; +begin + Result := GetNextEntry(ACursor, AKey, AValue); +end; + +procedure TGocciaOrderedValueMap.RetainIterator; +begin + Inc(FActiveIterators); +end; + +procedure TGocciaOrderedValueMap.ReleaseIterator; +begin + if FActiveIterators > 0 then + Dec(FActiveIterators); +end; + +end. diff --git a/source/units/Goccia.Values.SetValue.pas b/source/units/Goccia.Values.SetValue.pas index 60668e288..74f216097 100644 --- a/source/units/Goccia.Values.SetValue.pas +++ b/source/units/Goccia.Values.SetValue.pas @@ -11,12 +11,15 @@ interface Goccia.Values.ArrayValue, Goccia.Values.ClassValue, Goccia.Values.ObjectValue, + Goccia.Values.OrderedValueMap, Goccia.Values.Primitives; type TGocciaSetValue = class(TGocciaInstanceValue) private - FItems: TGocciaValueList; + // Backed by the shared SameValueZero ordered store, keyed by the element + // with the element itself as the value (key = value). + FStore: TGocciaOrderedValueMap; public function SetHas(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; function SetAdd(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; @@ -43,6 +46,12 @@ TGocciaSetValue = class(TGocciaInstanceValue) function ContainsValue(const AValue: TGocciaValue): Boolean; procedure AddItem(const AValue: TGocciaValue); + // Live, insertion-ordered cursor over active members. Seed ACursor with 0. + function NextItem(var ACursor: Integer; out AValue: TGocciaValue): Boolean; + procedure RetainIterator; + procedure ReleaseIterator; + function Count: Integer; + function GetProperty(const AName: string): TGocciaValue; override; function GetPropertyWithContext(const AName: string; const AThisContext: TGocciaValue): TGocciaValue; override; function ToArray: TGocciaArrayValue; @@ -52,8 +61,6 @@ TGocciaSetValue = class(TGocciaInstanceValue) procedure MarkReferences; override; class procedure ExposePrototype(const AConstructor: TGocciaValue); - - property Items: TGocciaValueList read FItems; end; implementation @@ -61,7 +68,6 @@ implementation uses SysUtils, - Goccia.Arithmetic, Goccia.Constants.ConstructorNames, Goccia.Constants.PropertyNames, Goccia.Error.Messages, @@ -100,23 +106,9 @@ function GetSetShared: TGocciaSharedPrototype; inline; Result := nil; end; -function SetDataIndex(const AItems: TGocciaValueList; const AValue: TGocciaValue): Integer; -var - I: Integer; -begin - for I := 0 to AItems.Count - 1 do - if IsSameValueZero(AItems[I], AValue) then - Exit(I); - Result := -1; -end; - procedure RemoveSetItem(const ASet: TGocciaSetValue; const AValue: TGocciaValue); -var - Index: Integer; begin - Index := SetDataIndex(ASet.FItems, AValue); - if Index >= 0 then - ASet.FItems.Delete(Index); + ASet.FStore.Remove(AValue); end; function GetSetRecord(const AValue: TGocciaValue; const AMethodName: string): TGocciaSetRecord; @@ -210,7 +202,7 @@ constructor TGocciaSetValue.Create(const AClass: TGocciaClassValue = nil); Shared: TGocciaSharedPrototype; begin inherited Create(AClass); - FItems := TGocciaValueList.Create(False); + FStore := TGocciaOrderedValueMap.Create; InitializePrototype; Shared := GetSetShared; if not Assigned(AClass) and Assigned(Shared) then @@ -280,7 +272,7 @@ class procedure TGocciaSetValue.ExposePrototype(const AConstructor: TGocciaValue destructor TGocciaSetValue.Destroy; begin - FItems.Free; + FStore.Free; inherited; end; @@ -288,6 +280,9 @@ procedure TGocciaSetValue.InitializeNativeFromArguments(const AArguments: TGocci var InitArg: TGocciaValue; ArrValue: TGocciaArrayValue; + OtherSet: TGocciaSetValue; + Cursor: Integer; + Item: TGocciaValue; I: Integer; begin if AArguments.Length = 0 then @@ -302,34 +297,63 @@ procedure TGocciaSetValue.InitializeNativeFromArguments(const AArguments: TGocci end else if InitArg is TGocciaSetValue then begin - for I := 0 to TGocciaSetValue(InitArg).Items.Count - 1 do - AddItem(TGocciaSetValue(InitArg).Items[I]); + OtherSet := TGocciaSetValue(InitArg); + Cursor := 0; + while OtherSet.NextItem(Cursor, Item) do + AddItem(Item); end; end; procedure TGocciaSetValue.MarkReferences; var - I: Integer; + Cursor: Integer; + Item: TGocciaValue; begin if GCMarked then Exit; inherited; - for I := 0 to FItems.Count - 1 do - begin - if Assigned(FItems[I]) then - FItems[I].MarkReferences; - end; + Cursor := 0; + while NextItem(Cursor, Item) do + if Assigned(Item) then + Item.MarkReferences; end; function TGocciaSetValue.ContainsValue(const AValue: TGocciaValue): Boolean; begin - Result := SetDataIndex(FItems, AValue) >= 0; + Result := FStore.ContainsKey(AValue); end; procedure TGocciaSetValue.AddItem(const AValue: TGocciaValue); +var + Canonical: TGocciaValue; +begin + // Store the canonicalized element as both key and value so that iteration + // yields the canonical form (-0 observed as +0). SetEntry leaves an existing + // member in place, matching Set.add (no reordering on re-add). + Canonical := TGocciaOrderedValueMap.CanonicalizeKey(AValue); + FStore.SetEntry(Canonical, Canonical); +end; + +function TGocciaSetValue.NextItem(var ACursor: Integer; out AValue: TGocciaValue): Boolean; +var + Key: TGocciaValue; +begin + Result := FStore.NextEntry(ACursor, Key, AValue); +end; + +procedure TGocciaSetValue.RetainIterator; +begin + FStore.RetainIterator; +end; + +procedure TGocciaSetValue.ReleaseIterator; begin - if not ContainsValue(AValue) then - FItems.Add(AValue); + FStore.ReleaseIterator; +end; + +function TGocciaSetValue.Count: Integer; +begin + Result := FStore.Count; end; function TGocciaSetValue.GetProperty(const AName: string): TGocciaValue; @@ -340,18 +364,20 @@ function TGocciaSetValue.GetProperty(const AName: string): TGocciaValue; function TGocciaSetValue.GetPropertyWithContext(const AName: string; const AThisContext: TGocciaValue): TGocciaValue; begin if AName = PROP_SIZE then - Result := TGocciaNumberLiteralValue.Create(FItems.Count) + Result := TGocciaNumberLiteralValue.Create(FStore.Count) else Result := inherited GetPropertyWithContext(AName, AThisContext); end; function TGocciaSetValue.ToArray: TGocciaArrayValue; var - I: Integer; + Cursor: Integer; + Item: TGocciaValue; begin Result := TGocciaArrayValue.Create; - for I := 0 to FItems.Count - 1 do - Result.Elements.Add(FItems[I]); + Cursor := 0; + while NextItem(Cursor, Item) do + Result.Elements.Add(Item); end; function TGocciaSetValue.ToStringTag: string; @@ -404,32 +430,20 @@ function TGocciaSetValue.SetAdd(const AArgs: TGocciaArgumentsCollection; const A function TGocciaSetValue.SetDelete(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var S: TGocciaSetValue; - I: Integer; begin // Step 1: Let S be the this value // Steps 2-3: If S does not have a [[SetData]] internal slot, throw a TypeError if not (AThisValue is TGocciaSetValue) then ThrowTypeError(SErrorSetDeleteNonSet, SSuggestSetThisType); S := TGocciaSetValue(AThisValue); - // Step 5 (early): Default return false - Result := TGocciaBooleanLiteralValue.FalseValue; - if AArgs.Length > 0 then - begin - // Step 4: For each element e of S.[[SetData]], do - for I := 0 to S.FItems.Count - 1 do - begin - // Step 4a: If e is not empty and SameValueZero(e, value) is true - if IsSameValueZero(S.FItems[I], AArgs.GetElement(0)) then - begin - // Step 4a.i: Replace the element of S.[[SetData]] with empty - S.FItems.Delete(I); - // Step 4a.ii: Return true - Result := TGocciaBooleanLiteralValue.TrueValue; - Exit; - end; - end; - end; - // Step 5: Return false + // Step 4: For each element e of S.[[SetData]], do + // If SameValueZero(e, value) is true, replace e with empty (tombstone) + // and return true + if (AArgs.Length > 0) and S.FStore.Remove(AArgs.GetElement(0)) then + Result := TGocciaBooleanLiteralValue.TrueValue + else + // Step 5: Return false + Result := TGocciaBooleanLiteralValue.FalseValue; end; // ES2026 §24.2.3.2 Set.prototype.clear() @@ -440,7 +454,7 @@ function TGocciaSetValue.SetClear(const AArgs: TGocciaArgumentsCollection; const if not (AThisValue is TGocciaSetValue) then ThrowTypeError(SErrorSetClearNonSet, SSuggestSetThisType); // Step 4: For each element e of S.[[SetData]], replace e with empty - TGocciaSetValue(AThisValue).FItems.Clear; + TGocciaSetValue(AThisValue).FStore.Clear; // Step 5: Return undefined Result := TGocciaUndefinedLiteralValue.UndefinedValue; end; @@ -449,10 +463,10 @@ function TGocciaSetValue.SetClear(const AArgs: TGocciaArgumentsCollection; const function TGocciaSetValue.SetForEach(const AArgs: TGocciaArgumentsCollection; const AThisValue: TGocciaValue): TGocciaValue; var S: TGocciaSetValue; - Callback, ThisArg: TGocciaValue; + Callback, ThisArg, Item: TGocciaValue; TypedCallback: TGocciaFunctionBase; CallArgs: TGocciaArgumentsCollection; - I: Integer; + Cursor: Integer; SetRoot, CallbackRoot, ThisRoot: TGocciaTempRoot; begin // Steps 1-3: If S does not have a [[SetData]] internal slot, throw a TypeError @@ -484,12 +498,16 @@ function TGocciaSetValue.SetForEach(const AArgs: TGocciaArgumentsCollection; con AddTempRootIfNeeded(SetRoot, S); AddTempRootIfNeeded(CallbackRoot, Callback); AddTempRootIfNeeded(ThisRoot, ThisArg); + // Keep entry indices stable across callback mutation (appended values are + // visited, deleted values are skipped — §24.2.3.6 re-reads numEntries). + S.RetainIterator; try - // Step 6: For each element e of S.[[SetData]], do - for I := 0 to S.FItems.Count - 1 do + // Step 6: For each element e of S.[[SetData]], do + Cursor := 0; + while S.NextItem(Cursor, Item) do begin // Step 6b: Call(callbackfn, thisArg, « e, e, S ») - CallArgs := TGocciaArgumentsCollection.Create([S.FItems[I], S.FItems[I], S]); + CallArgs := TGocciaArgumentsCollection.Create([Item, Item, S]); try if Assigned(TypedCallback) then TypedCallback.Call(CallArgs, ThisArg) @@ -503,6 +521,7 @@ function TGocciaSetValue.SetForEach(const AArgs: TGocciaArgumentsCollection; con // Step 7: Return undefined Result := TGocciaUndefinedLiteralValue.UndefinedValue; finally + S.ReleaseIterator; RemoveTempRootIfNeeded(ThisRoot); RemoveTempRootIfNeeded(CallbackRoot); RemoveTempRootIfNeeded(SetRoot); @@ -558,9 +577,9 @@ function TGocciaSetValue.SetUnion(const AArgs: TGocciaArgumentsCollection; const ThisSet, ResultSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue: TGocciaValue; + NextValue, Item: TGocciaValue; Done, WasResultRooted, WasIteratorRooted: Boolean; - I: Integer; + Cursor: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -577,8 +596,9 @@ function TGocciaSetValue.SetUnion(const AArgs: TGocciaArgumentsCollection; const if Assigned(GC) and not WasResultRooted then GC.AddTempRoot(ResultSet); try - for I := 0 to ThisSet.FItems.Count - 1 do - ResultSet.AddItem(ThisSet.FItems[I]); + Cursor := 0; + while ThisSet.NextItem(Cursor, Item) do + ResultSet.AddItem(Item); // Step 6: Let keysIter be GetIteratorFromSetLike(otherRec) Iterator := GetSetRecordKeysIterator(OtherRecord, 'union'); WasIteratorRooted := Assigned(GC) and GC.IsTempRoot(Iterator); @@ -614,9 +634,9 @@ function TGocciaSetValue.SetIntersection(const AArgs: TGocciaArgumentsCollection ThisSet, ResultSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue: TGocciaValue; + NextValue, Item: TGocciaValue; Done, WasResultRooted, WasIteratorRooted: Boolean; - I: Integer; + Cursor: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -633,13 +653,20 @@ function TGocciaSetValue.SetIntersection(const AArgs: TGocciaArgumentsCollection if Assigned(GC) and not WasResultRooted then GC.AddTempRoot(ResultSet); try - if ThisSet.FItems.Count <= OtherRecord.Size then + if ThisSet.Count <= OtherRecord.Size then begin // Step 6: For each element e of O.[[SetData]], do - for I := 0 to ThisSet.FItems.Count - 1 do - // Step 6a: If e is not empty and SetDataHas(otherRec, e) is true, append e - if SetRecordHas(OtherRecord, ThisSet.FItems[I]) then - ResultSet.AddItem(ThisSet.FItems[I]); + // (SetRecordHas calls user code; hold compaction so the cursor stays valid) + ThisSet.RetainIterator; + try + Cursor := 0; + while ThisSet.NextItem(Cursor, Item) do + // Step 6a: If e is not empty and SetDataHas(otherRec, e) is true, append e + if SetRecordHas(OtherRecord, Item) then + ResultSet.AddItem(Item); + finally + ThisSet.ReleaseIterator; + end; end else begin @@ -677,9 +704,9 @@ function TGocciaSetValue.SetDifference(const AArgs: TGocciaArgumentsCollection; ThisSet, ResultSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue: TGocciaValue; + NextValue, Item: TGocciaValue; Done, WasResultRooted, WasIteratorRooted: Boolean; - I: Integer; + Cursor: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -696,18 +723,26 @@ function TGocciaSetValue.SetDifference(const AArgs: TGocciaArgumentsCollection; if Assigned(GC) and not WasResultRooted then GC.AddTempRoot(ResultSet); try - if ThisSet.FItems.Count <= OtherRecord.Size then + if ThisSet.Count <= OtherRecord.Size then begin // Step 6: For each element e of resultSetData, do - for I := 0 to ThisSet.FItems.Count - 1 do - // Step 6a: If e is not empty and SetDataHas(otherRec, e) is true, remove e - if not SetRecordHas(OtherRecord, ThisSet.FItems[I]) then - ResultSet.AddItem(ThisSet.FItems[I]); + // (SetRecordHas calls user code; hold compaction so the cursor stays valid) + ThisSet.RetainIterator; + try + Cursor := 0; + while ThisSet.NextItem(Cursor, Item) do + // Step 6a: If e is not empty and SetDataHas(otherRec, e) is true, remove e + if not SetRecordHas(OtherRecord, Item) then + ResultSet.AddItem(Item); + finally + ThisSet.ReleaseIterator; + end; end else begin - for I := 0 to ThisSet.FItems.Count - 1 do - ResultSet.AddItem(ThisSet.FItems[I]); + Cursor := 0; + while ThisSet.NextItem(Cursor, Item) do + ResultSet.AddItem(Item); Iterator := GetSetRecordKeysIterator(OtherRecord, 'difference'); WasIteratorRooted := Assigned(GC) and GC.IsTempRoot(Iterator); @@ -742,9 +777,9 @@ function TGocciaSetValue.SetSymmetricDifference(const AArgs: TGocciaArgumentsCol ThisSet, ResultSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue: TGocciaValue; + NextValue, Item: TGocciaValue; Done, WasResultRooted, WasIteratorRooted: Boolean; - I: Integer; + Cursor: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -761,8 +796,9 @@ function TGocciaSetValue.SetSymmetricDifference(const AArgs: TGocciaArgumentsCol if Assigned(GC) and not WasResultRooted then GC.AddTempRoot(ResultSet); try - for I := 0 to ThisSet.FItems.Count - 1 do - ResultSet.AddItem(ThisSet.FItems[I]); + Cursor := 0; + while ThisSet.NextItem(Cursor, Item) do + ResultSet.AddItem(Item); // Step 7: Let keysIter be GetIteratorFromSetLike(otherRec) Iterator := GetSetRecordKeysIterator(OtherRecord, 'symmetricDifference'); @@ -801,7 +837,9 @@ function TGocciaSetValue.SetIsSubsetOf(const AArgs: TGocciaArgumentsCollection; var ThisSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; - I: Integer; + Item: TGocciaValue; + Cursor: Integer; + IsSubset: Boolean; begin // Step 1: Let O be the this value // Steps 2-3: If O does not have a [[SetData]] internal slot, throw a TypeError @@ -812,23 +850,33 @@ function TGocciaSetValue.SetIsSubsetOf(const AArgs: TGocciaArgumentsCollection; OtherRecord := GetSetRecord(AArgs.GetElement(0), 'isSubsetOf'); // Step 5: If SetDataSize(O) > otherRec.[[Size]], return false (optimization) - if ThisSet.FItems.Count > OtherRecord.Size then + if ThisSet.Count > OtherRecord.Size then begin Result := TGocciaBooleanLiteralValue.FalseValue; Exit; end; + IsSubset := True; // Step 6: For each element e of O.[[SetData]], do - for I := 0 to ThisSet.FItems.Count - 1 do - // Step 6a: If SetDataHas(otherRec, e) is false, return false - if not SetRecordHas(OtherRecord, ThisSet.FItems[I]) then - begin - Result := TGocciaBooleanLiteralValue.FalseValue; - Exit; - end; + ThisSet.RetainIterator; + try + Cursor := 0; + while ThisSet.NextItem(Cursor, Item) do + // Step 6a: If SetDataHas(otherRec, e) is false, return false + if not SetRecordHas(OtherRecord, Item) then + begin + IsSubset := False; + Break; + end; + finally + ThisSet.ReleaseIterator; + end; // Step 7: Return true - Result := TGocciaBooleanLiteralValue.TrueValue; + if IsSubset then + Result := TGocciaBooleanLiteralValue.TrueValue + else + Result := TGocciaBooleanLiteralValue.FalseValue; end; // ES2026 §24.2.3.9 Set.prototype.isSupersetOf(other) @@ -850,7 +898,7 @@ function TGocciaSetValue.SetIsSupersetOf(const AArgs: TGocciaArgumentsCollection OtherRecord := GetSetRecord(AArgs.GetElement(0), 'isSupersetOf'); // Step 5: If SetDataSize(O) < otherRec.[[Size]], return false (optimization) - if ThisSet.FItems.Count < OtherRecord.Size then + if ThisSet.Count < OtherRecord.Size then begin Result := TGocciaBooleanLiteralValue.FalseValue; Exit; @@ -891,9 +939,9 @@ function TGocciaSetValue.SetIsDisjointFrom(const AArgs: TGocciaArgumentsCollecti ThisSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue: TGocciaValue; - Done, WasIteratorRooted: Boolean; - I: Integer; + NextValue, Item: TGocciaValue; + Done, WasIteratorRooted, IsDisjoint: Boolean; + Cursor: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -904,16 +952,28 @@ function TGocciaSetValue.SetIsDisjointFrom(const AArgs: TGocciaArgumentsCollecti // Step 4: Let otherRec be GetSetRecord(other) OtherRecord := GetSetRecord(AArgs.GetElement(0), 'isDisjointFrom'); - if ThisSet.FItems.Count <= OtherRecord.Size then + if ThisSet.Count <= OtherRecord.Size then begin + IsDisjoint := True; // Step 5: For each element e of O.[[SetData]], do - for I := 0 to ThisSet.FItems.Count - 1 do - // Step 5a: If e is not empty and SetDataHas(otherRec, e) is true, return false - if SetRecordHas(OtherRecord, ThisSet.FItems[I]) then - begin - Result := TGocciaBooleanLiteralValue.FalseValue; - Exit; - end; + ThisSet.RetainIterator; + try + Cursor := 0; + while ThisSet.NextItem(Cursor, Item) do + // Step 5a: If e is not empty and SetDataHas(otherRec, e) is true, return false + if SetRecordHas(OtherRecord, Item) then + begin + IsDisjoint := False; + Break; + end; + finally + ThisSet.ReleaseIterator; + end; + if not IsDisjoint then + begin + Result := TGocciaBooleanLiteralValue.FalseValue; + Exit; + end; end else begin diff --git a/tests/built-ins/Map/prototype/entries.js b/tests/built-ins/Map/prototype/entries.js index b3c8faec6..4df16f12a 100644 --- a/tests/built-ins/Map/prototype/entries.js +++ b/tests/built-ins/Map/prototype/entries.js @@ -43,6 +43,35 @@ test("spread on map produces entries", () => { expect([...map]).toEqual([["a", 1], ["b", 2]]); }); +test("live iterator skips a not-yet-visited key deleted after it starts", () => { + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const iter = map.entries(); + expect(iter.next().value).toEqual(["a", 1]); + map.delete("c"); + expect(iter.next().value).toEqual(["b", 2]); + expect(iter.next().done).toBe(true); +}); + +test("live iterator yields a key appended after it starts", () => { + const map = new Map([["a", 1]]); + const iter = map.entries(); + expect(iter.next().value).toEqual(["a", 1]); + map.set("b", 2); + expect(iter.next().value).toEqual(["b", 2]); + expect(iter.next().done).toBe(true); +}); + +test("for...of deleting the current key visits every original key", () => { + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const seen = []; + for (const [key] of map) { + seen.push(key); + map.delete(key); + } + expect(seen).toEqual(["a", "b", "c"]); + expect(map.size).toBe(0); +}); + test("entries throws TypeError when called on non-Map", () => { const entries = Map.prototype.entries; expect(() => entries.call(Map.prototype)).toThrow(TypeError); diff --git a/tests/built-ins/Map/prototype/forEach.js b/tests/built-ins/Map/prototype/forEach.js index cecfa9ec9..7a699e9c6 100644 --- a/tests/built-ins/Map/prototype/forEach.js +++ b/tests/built-ins/Map/prototype/forEach.js @@ -69,3 +69,49 @@ test("forEach passes map as third callback argument", () => { }); expect(receivedMap).toBe(map); }); + +test("forEach visits entries appended during iteration", () => { + const map = new Map([["a", 1]]); + const seen = []; + map.forEach((value, key) => { + seen.push(key); + if (key === "a") map.set("b", 2); + }); + expect(seen).toEqual(["a", "b"]); +}); + +test("forEach does not visit an entry deleted before it is reached", () => { + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const seen = []; + map.forEach((value, key) => { + seen.push(key); + if (key === "a") map.delete("b"); + }); + expect(seen).toEqual(["a", "c"]); +}); + +test("forEach deleting the current key does not skip the next", () => { + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const seen = []; + map.forEach((value, key) => { + seen.push(key); + map.delete(key); + }); + expect(seen).toEqual(["a", "b", "c"]); + expect(map.size).toBe(0); +}); + +test("forEach revisits a key deleted after visiting then re-added", () => { + const map = new Map([["a", 1], ["b", 2]]); + const seen = []; + let readded = false; + map.forEach((value, key) => { + seen.push(key); + if (key === "b" && !readded) { + readded = true; + map.delete("a"); + map.set("a", 1); + } + }); + expect(seen).toEqual(["a", "b", "a"]); +}); diff --git a/tests/built-ins/Map/prototype/set.js b/tests/built-ins/Map/prototype/set.js index f81c5a221..7fba06140 100644 --- a/tests/built-ins/Map/prototype/set.js +++ b/tests/built-ins/Map/prototype/set.js @@ -42,6 +42,52 @@ test("set with NaN key", () => { expect(map.get(NaN)).toBe("second"); }); +test("set normalizes a -0 key to +0", () => { + const map = new Map(); + map.set(-0, "neg"); + expect(map.has(0)).toBe(true); + expect(map.get(0)).toBe("neg"); + expect(map.size).toBe(1); + const key = [...map.keys()][0]; + expect(Object.is(key, 0)).toBe(true); + expect(Object.is(key, -0)).toBe(false); +}); + +test("set treats +0 and -0 as the same key", () => { + const map = new Map(); + map.set(0, "a"); + map.set(-0, "b"); + expect(map.size).toBe(1); + expect(map.get(0)).toBe("b"); +}); + +test("set keys by string content, not string identity", () => { + const map = new Map(); + map.set("a" + "b", 1); + map.set("ab", 2); + expect(map.size).toBe(1); + expect(map.get("ab")).toBe(2); +}); + +test("set treats number 1 and string '1' as distinct keys", () => { + const map = new Map(); + map.set(1, "num"); + map.set("1", "str"); + expect(map.size).toBe(2); + expect(map.get(1)).toBe("num"); + expect(map.get("1")).toBe("str"); +}); + +test("set uses reference identity for object keys", () => { + const map = new Map(); + const a = {}; + map.set(a, 1); + map.set(a, 2); + map.set({}, 3); + expect(map.size).toBe(2); + expect(map.get(a)).toBe(2); +}); + test("throws TypeError when called on non-Map", () => { const set = Map.prototype.set; expect(() => set.call(Map.prototype, "k", "v")).toThrow(TypeError); diff --git a/tests/built-ins/Set/prototype/add.js b/tests/built-ins/Set/prototype/add.js index 21fc139fc..d05ec36e0 100644 --- a/tests/built-ins/Set/prototype/add.js +++ b/tests/built-ins/Set/prototype/add.js @@ -62,6 +62,30 @@ test("add object references", () => { expect(set.size).toBe(2); }); +test("add normalizes -0 to +0", () => { + const set = new Set(); + set.add(-0); + expect(set.has(0)).toBe(true); + expect(set.size).toBe(1); + const value = [...set][0]; + expect(Object.is(value, 0)).toBe(true); + expect(Object.is(value, -0)).toBe(false); +}); + +test("add treats +0 and -0 as the same value", () => { + const set = new Set(); + set.add(0); + set.add(-0); + expect(set.size).toBe(1); +}); + +test("add dedupes by string content, not string identity", () => { + const set = new Set(); + set.add("a" + "b"); + set.add("ab"); + expect(set.size).toBe(1); +}); + test("throws TypeError when called on non-Set", () => { const add = Set.prototype.add; expect(() => add.call(Set.prototype, 1)).toThrow(TypeError); diff --git a/tests/built-ins/Set/prototype/forEach.js b/tests/built-ins/Set/prototype/forEach.js index 4b88ffc19..47d86b320 100644 --- a/tests/built-ins/Set/prototype/forEach.js +++ b/tests/built-ins/Set/prototype/forEach.js @@ -66,3 +66,34 @@ test("forEach passes set as third callback argument", () => { }); expect(receivedSet).toBe(set); }); + +test("forEach visits values appended during iteration", () => { + const set = new Set([1]); + const seen = []; + set.forEach((value) => { + seen.push(value); + if (value === 1) set.add(2); + }); + expect(seen).toEqual([1, 2]); +}); + +test("forEach does not visit a value deleted before it is reached", () => { + const set = new Set([1, 2, 3]); + const seen = []; + set.forEach((value) => { + seen.push(value); + if (value === 1) set.delete(2); + }); + expect(seen).toEqual([1, 3]); +}); + +test("forEach deleting the current value does not skip the next", () => { + const set = new Set([1, 2, 3]); + const seen = []; + set.forEach((value) => { + seen.push(value); + set.delete(value); + }); + expect(seen).toEqual([1, 2, 3]); + expect(set.size).toBe(0); +}); diff --git a/tests/built-ins/Set/prototype/values.js b/tests/built-ins/Set/prototype/values.js index 734e02c20..35b1a80ad 100644 --- a/tests/built-ins/Set/prototype/values.js +++ b/tests/built-ins/Set/prototype/values.js @@ -68,6 +68,28 @@ test("Set.entries() iterator with next()", () => { expect(iter.next().done).toBe(true); }); +test("live iterator reflects delete and add during iteration", () => { + const set = new Set([1, 2, 3]); + const seen = []; + for (const value of set) { + seen.push(value); + if (value === 1) { + set.delete(2); + set.add(4); + } + } + expect(seen).toEqual([1, 3, 4]); +}); + +test("live iterator skips a not-yet-visited value deleted after it starts", () => { + const set = new Set([1, 2, 3]); + const iter = set.values(); + expect(iter.next().value).toBe(1); + set.delete(3); + expect(iter.next().value).toBe(2); + expect(iter.next().done).toBe(true); +}); + test("values throws TypeError when called on non-Set", () => { const values = Set.prototype.values; expect(() => values.call(Set.prototype)).toThrow(TypeError); From af71aa2cc651fed62ed05e468199949ba4fe4cdf Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sat, 27 Jun 2026 20:53:11 +0100 Subject: [PATCH 2/4] fix(engine): harden Map/Set iteration and key/set-operation edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review findings on the SameValueZero Map/Set store: - Retain the source store across the structuredClone, deep-equal, and REPL cursor walks: those recurse into user getters that can mutate the collection mid-walk, which could otherwise let compaction renumber entries and invalidate the cursor. Deep-equal now also confirms the expected-side cursor advances before comparing. - Treat an omitted Map/Set argument as the `undefined` key/value instead of a no-op: Map get/set/has/delete and Set has/add/delete read the argument directly (GetElement already yields undefined for out-of-range), so `m.set("k")`, `m.get()`, and `s.add()` behave per spec. - Move the Set canonicalization invariant into the store via AddSetMember, so the element is stored as both key and value without callers pre-canonicalizing. - Make Set difference/intersection/isSubsetOf/isDisjointFrom iterate a snapshot taken before any set-like `has` callback runs (ES2026 §24.2.4): elements appended during a mutating callback are no longer visited; the live ones re-check membership so elements deleted during a callback are skipped. Regression tests added for the argument handling and the mutating-callback set operations, plus a Pascal test for AddSetMember. Co-Authored-By: Claude Opus 4.8 --- source/units/Goccia.Builtins.Globals.pas | 26 ++- source/units/Goccia.Evaluator.Comparison.pas | 62 ++++++-- source/units/Goccia.REPL.Formatter.pas | 54 ++++--- source/units/Goccia.Values.MapValue.pas | 31 ++-- .../Goccia.Values.OrderedValueMap.Test.pas | 23 +++ .../units/Goccia.Values.OrderedValueMap.pas | 14 ++ source/units/Goccia.Values.SetValue.pas | 150 ++++++++++-------- tests/built-ins/Map/prototype/set.js | 13 ++ tests/built-ins/Set/prototype/add.js | 10 ++ tests/built-ins/Set/prototype/difference.js | 19 +++ tests/built-ins/Set/prototype/intersection.js | 19 +++ 11 files changed, 293 insertions(+), 128 deletions(-) diff --git a/source/units/Goccia.Builtins.Globals.pas b/source/units/Goccia.Builtins.Globals.pas index 541c852c7..42f9fd1c1 100644 --- a/source/units/Goccia.Builtins.Globals.pas +++ b/source/units/Goccia.Builtins.Globals.pas @@ -841,11 +841,18 @@ function CloneMap(const AMap: TGocciaMapValue; Result := TGocciaMapValue.Create; AMemory.Add(AMap, Result); + // StructuredCloneValue can run user getters that mutate AMap; retain it so + // compaction cannot renumber entries mid-walk and invalidate Cursor. Cursor := 0; - while AMap.NextEntry(Cursor, Key, Value) do - Result.SetEntry( - StructuredCloneValue(Key, AMemory), - StructuredCloneValue(Value, AMemory)); + AMap.RetainIterator; + try + while AMap.NextEntry(Cursor, Key, Value) do + Result.SetEntry( + StructuredCloneValue(Key, AMemory), + StructuredCloneValue(Value, AMemory)); + finally + AMap.ReleaseIterator; + end; end; function CloneSet(const ASet: TGocciaSetValue; @@ -857,9 +864,16 @@ function CloneSet(const ASet: TGocciaSetValue; Result := TGocciaSetValue.Create; AMemory.Add(ASet, Result); + // StructuredCloneValue can run user getters that mutate ASet; retain it so + // compaction cannot renumber entries mid-walk and invalidate Cursor. Cursor := 0; - while ASet.NextItem(Cursor, Item) do - Result.AddItem(StructuredCloneValue(Item, AMemory)); + ASet.RetainIterator; + try + while ASet.NextItem(Cursor, Item) do + Result.AddItem(StructuredCloneValue(Item, AMemory)); + finally + ASet.ReleaseIterator; + end; end; function CloneArrayBuffer(const ABuf: TGocciaArrayBufferValue; diff --git a/source/units/Goccia.Evaluator.Comparison.pas b/source/units/Goccia.Evaluator.Comparison.pas index e2a3655b7..3a6f1bcfa 100644 --- a/source/units/Goccia.Evaluator.Comparison.pas +++ b/source/units/Goccia.Evaluator.Comparison.pas @@ -160,14 +160,28 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue; AddComparedPair(AComparedPairs, AActual, AExpected); CursorA := 0; CursorB := 0; - while TGocciaSetValue(AActual).NextItem(CursorA, LeftValue) do - begin - TGocciaSetValue(AExpected).NextItem(CursorB, RightValue); - if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then + // Recursive comparison can run user getters that mutate either set; retain + // both so cursors stay valid, and confirm the expected side advances before + // comparing (avoids comparing stale out values). + TGocciaSetValue(AActual).RetainIterator; + TGocciaSetValue(AExpected).RetainIterator; + try + while TGocciaSetValue(AActual).NextItem(CursorA, LeftValue) do begin - Result := False; - Exit; + if not TGocciaSetValue(AExpected).NextItem(CursorB, RightValue) then + begin + Result := False; + Exit; + end; + if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then + begin + Result := False; + Exit; + end; end; + finally + TGocciaSetValue(AExpected).ReleaseIterator; + TGocciaSetValue(AActual).ReleaseIterator; end; Result := True; Exit; @@ -189,19 +203,33 @@ function IsDeepEqualInternal(const AActual, AExpected: TGocciaValue; AddComparedPair(AComparedPairs, AActual, AExpected); CursorA := 0; CursorB := 0; - while TGocciaMapValue(AActual).NextEntry(CursorA, LeftKey, LeftValue) do - begin - TGocciaMapValue(AExpected).NextEntry(CursorB, RightKey, RightValue); - if not IsDeepEqualInternal(LeftKey, RightKey, AComparedPairs) then - begin - Result := False; - Exit; - end; - if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then + // Recursive comparison can run user getters that mutate either map; retain + // both so cursors stay valid, and confirm the expected side advances before + // comparing (avoids comparing stale out values). + TGocciaMapValue(AActual).RetainIterator; + TGocciaMapValue(AExpected).RetainIterator; + try + while TGocciaMapValue(AActual).NextEntry(CursorA, LeftKey, LeftValue) do begin - Result := False; - Exit; + if not TGocciaMapValue(AExpected).NextEntry(CursorB, RightKey, RightValue) then + begin + Result := False; + Exit; + end; + if not IsDeepEqualInternal(LeftKey, RightKey, AComparedPairs) then + begin + Result := False; + Exit; + end; + if not IsDeepEqualInternal(LeftValue, RightValue, AComparedPairs) then + begin + Result := False; + Exit; + end; end; + finally + TGocciaMapValue(AExpected).ReleaseIterator; + TGocciaMapValue(AActual).ReleaseIterator; end; Result := True; Exit; diff --git a/source/units/Goccia.REPL.Formatter.pas b/source/units/Goccia.REPL.Formatter.pas index aceee2b84..b06619939 100644 --- a/source/units/Goccia.REPL.Formatter.pas +++ b/source/units/Goccia.REPL.Formatter.pas @@ -84,21 +84,28 @@ function FormatMapValue(const AMap: TGocciaMapValue; else begin SB.Append(' { '); + // FormatREPLValue can recurse into user getters that mutate AMap; retain it + // so compaction cannot renumber entries and invalidate Cursor mid-walk. I := 0; Cursor := 0; - while AMap.NextEntry(Cursor, Key, Value) do - begin - if I >= MAX_INSPECT_ITEMS then + AMap.RetainIterator; + try + while AMap.NextEntry(Cursor, Key, Value) do begin - SB.Append(', ... ' + IntToStr(Total - MAX_INSPECT_ITEMS) + ' more'); - Break; + if I >= MAX_INSPECT_ITEMS then + begin + SB.Append(', ... ' + IntToStr(Total - MAX_INSPECT_ITEMS) + ' more'); + Break; + end; + if I > 0 then + SB.Append(', '); + SB.Append(FormatREPLValue(Key, AUseColor)); + SB.Append(' => '); + SB.Append(FormatREPLValue(Value, AUseColor)); + Inc(I); end; - if I > 0 then - SB.Append(', '); - SB.Append(FormatREPLValue(Key, AUseColor)); - SB.Append(' => '); - SB.Append(FormatREPLValue(Value, AUseColor)); - Inc(I); + finally + AMap.ReleaseIterator; end; SB.Append(' }'); end; @@ -120,19 +127,26 @@ function FormatSetValue(const ASet: TGocciaSetValue; else begin SB.Append(' { '); + // FormatREPLValue can recurse into user getters that mutate ASet; retain it + // so compaction cannot renumber entries and invalidate Cursor mid-walk. I := 0; Cursor := 0; - while ASet.NextItem(Cursor, Item) do - begin - if I >= MAX_INSPECT_ITEMS then + ASet.RetainIterator; + try + while ASet.NextItem(Cursor, Item) do begin - SB.Append(', ... ' + IntToStr(Total - MAX_INSPECT_ITEMS) + ' more'); - Break; + if I >= MAX_INSPECT_ITEMS then + begin + SB.Append(', ... ' + IntToStr(Total - MAX_INSPECT_ITEMS) + ' more'); + Break; + end; + if I > 0 then + SB.Append(', '); + SB.Append(FormatREPLValue(Item, AUseColor)); + Inc(I); end; - if I > 0 then - SB.Append(', '); - SB.Append(FormatREPLValue(Item, AUseColor)); - Inc(I); + finally + ASet.ReleaseIterator; end; SB.Append(' }'); end; diff --git a/source/units/Goccia.Values.MapValue.pas b/source/units/Goccia.Values.MapValue.pas index b8a133c30..b76bbb39d 100644 --- a/source/units/Goccia.Values.MapValue.pas +++ b/source/units/Goccia.Values.MapValue.pas @@ -303,7 +303,8 @@ function TGocciaMapValue.MapGet(const AArgs: TGocciaArgumentsCollection; const A M := TGocciaMapValue(AThisValue); // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // Step 4a: If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, return p.[[Value]] - if (AArgs.Length > 0) and M.TryGetValue(AArgs.GetElement(0), Value) then + // An omitted argument is the value `undefined` (GetElement yields undefined). + if M.TryGetValue(AArgs.GetElement(0), Value) then Result := Value else // Step 5: Return undefined @@ -321,18 +322,16 @@ function TGocciaMapValue.MapSet(const AArgs: TGocciaArgumentsCollection; const A if not (AThisValue is TGocciaMapValue) then ThrowTypeError(SErrorMapSetNonMap, SSuggestMapThisType); M := TGocciaMapValue(AThisValue); - if AArgs.Length >= 2 then - begin - MapKey := AArgs.GetElement(0); - MapValue := AArgs.GetElement(1); - // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do - // If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, - // set p.[[Value]] to value and return M - // Step 5: Set key to CanonicalizeKeyedCollectionKey(key) (-0 -> +0) - // Step 6: Let p be the Record { [[Key]]: key, [[Value]]: value } - // Step 7: Append p to M.[[MapData]] - M.SetEntry(MapKey, MapValue); - end; + // Omitted arguments are the value `undefined` (GetElement yields undefined). + MapKey := AArgs.GetElement(0); + MapValue := AArgs.GetElement(1); + // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do + // If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, + // set p.[[Value]] to value and return M + // Step 5: Set key to CanonicalizeKeyedCollectionKey(key) (-0 -> +0) + // Step 6: Let p be the Record { [[Key]]: key, [[Value]]: value } + // Step 7: Append p to M.[[MapData]] + M.SetEntry(MapKey, MapValue); // Step 8: Return M Result := AThisValue; end; @@ -350,7 +349,8 @@ function TGocciaMapValue.MapHas(const AArgs: TGocciaArgumentsCollection; const A M := TGocciaMapValue(AThisValue); // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, return true - if (AArgs.Length > 0) and M.TryGetValue(AArgs.GetElement(0), Value) then + // An omitted argument is the value `undefined` (GetElement yields undefined). + if M.TryGetValue(AArgs.GetElement(0), Value) then Result := TGocciaBooleanLiteralValue.TrueValue else // Step 5: Return false @@ -370,7 +370,8 @@ function TGocciaMapValue.MapDelete(const AArgs: TGocciaArgumentsCollection; cons // Step 4: For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // If SameValueZero(p.[[Key]], key) is true, set p.[[Key]]/[[Value]] to // empty (tombstone) and return true - if (AArgs.Length > 0) and M.FStore.Remove(AArgs.GetElement(0)) then + // An omitted argument is the value `undefined` (GetElement yields undefined). + if M.FStore.Remove(AArgs.GetElement(0)) then Result := TGocciaBooleanLiteralValue.TrueValue else // Step 5: Return false diff --git a/source/units/Goccia.Values.OrderedValueMap.Test.pas b/source/units/Goccia.Values.OrderedValueMap.Test.pas index 518b8bf91..fb27b500c 100644 --- a/source/units/Goccia.Values.OrderedValueMap.Test.pas +++ b/source/units/Goccia.Values.OrderedValueMap.Test.pas @@ -36,6 +36,7 @@ TOrderedValueMapTests = class(TTestSuite) { -0 canonicalization (ES2026 §24.5.1). } procedure TestNegativeZeroStoredAsPositiveZero; procedure TestCanonicalizeKeyMapsNegativeZero; + procedure TestAddSetMemberStoresCanonicalKeyAndValue; { Insertion-ordered cursor with tombstone deletes. } procedure TestCursorVisitsInInsertionOrder; @@ -61,6 +62,7 @@ procedure TOrderedValueMapTests.SetupTests; Test('null and undefined are distinct keys', TestNullAndUndefinedAreDistinct); Test('-0 is stored as +0', TestNegativeZeroStoredAsPositiveZero); Test('CanonicalizeKey maps -0 to +0', TestCanonicalizeKeyMapsNegativeZero); + Test('AddSetMember stores the canonical element as key and value', TestAddSetMemberStoresCanonicalKeyAndValue); Test('Cursor visits entries in insertion order', TestCursorVisitsInInsertionOrder); Test('Cursor skips tombstones; re-add appends at end', TestCursorSkipsTombstonesAndReaddAppends); Test('RetainIterator/ReleaseIterator track active count', TestIteratorRetainReleaseCounter); @@ -256,6 +258,27 @@ procedure TOrderedValueMapTests.TestCanonicalizeKeyMapsNegativeZero; Expect(TGocciaOrderedValueMap.CanonicalizeKey(Num(5)) = nil).ToBe(False); end; +procedure TOrderedValueMapTests.TestAddSetMemberStoresCanonicalKeyAndValue; +var + Store: TGocciaOrderedValueMap; + Cursor: Integer; + Key, Value: TGocciaValue; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.AddSetMember(TGocciaNumberLiteralValue.NegativeZeroValue); + Expect(Store.Count).ToBe(1); + Cursor := 0; + Expect(Store.NextEntry(Cursor, Key, Value)).ToBe(True); + // Both the key and the value slot hold the canonical +0 (not -0). + Expect(TGocciaNumberLiteralValue(Key).IsNegativeZero).ToBe(False); + Expect(TGocciaNumberLiteralValue(Value).IsNegativeZero).ToBe(False); + Expect(Key = Value).ToBe(True); + finally + Store.Free; + end; +end; + procedure TOrderedValueMapTests.TestCursorVisitsInInsertionOrder; var Store: TGocciaOrderedValueMap; diff --git a/source/units/Goccia.Values.OrderedValueMap.pas b/source/units/Goccia.Values.OrderedValueMap.pas index 46ba941b8..501ce6a21 100644 --- a/source/units/Goccia.Values.OrderedValueMap.pas +++ b/source/units/Goccia.Values.OrderedValueMap.pas @@ -53,6 +53,12 @@ TGocciaOrderedValueMap = class(TOrderedMap) // canonicalizing the key first. procedure SetEntry(const AKey, AValue: TGocciaValue); + // Insert a Set member: the canonical element is stored as both key and + // value, so iteration (values/entries/forEach) observes the canonical form + // (-0 as +0). Keeping this in the store means Set callers cannot forget to + // canonicalize the value slot. + procedure AddSetMember(const AValue: TGocciaValue); + // Live, insertion-ordered cursor over active entries. ACursor is a // physical index; seed it with 0 and keep calling until False. New // entries appended during iteration are visited and deleted (tombstoned) @@ -186,6 +192,14 @@ procedure TGocciaOrderedValueMap.SetEntry(const AKey, AValue: TGocciaValue); Add(CanonicalizeKey(AKey), AValue); end; +procedure TGocciaOrderedValueMap.AddSetMember(const AValue: TGocciaValue); +var + Canonical: TGocciaValue; +begin + Canonical := CanonicalizeKey(AValue); + Add(Canonical, Canonical); +end; + function TGocciaOrderedValueMap.NextEntry(var ACursor: Integer; out AKey, AValue: TGocciaValue): Boolean; begin diff --git a/source/units/Goccia.Values.SetValue.pas b/source/units/Goccia.Values.SetValue.pas index 74f216097..0912f0f4d 100644 --- a/source/units/Goccia.Values.SetValue.pas +++ b/source/units/Goccia.Values.SetValue.pas @@ -111,6 +111,27 @@ procedure RemoveSetItem(const ASet: TGocciaSetValue; const AValue: TGocciaValue) ASet.FStore.Remove(AValue); end; +// Copy the set's current members into a plain array. The set-operation methods +// iterate this snapshot rather than a live cursor: their `has`/`keys` callbacks +// can run user code that mutates the receiver, and the spec (ES2026 §24.2.4) +// bounds these loops to the elements present when the operation began — +// elements appended during a callback must not be visited. +function SnapshotSetItems(const ASet: TGocciaSetValue): TArray; +var + Cursor, Count: Integer; + Item: TGocciaValue; +begin + SetLength(Result, ASet.Count); + Count := 0; + Cursor := 0; + while ASet.NextItem(Cursor, Item) do + begin + Result[Count] := Item; + Inc(Count); + end; + SetLength(Result, Count); +end; + function GetSetRecord(const AValue: TGocciaValue; const AMethodName: string): TGocciaSetRecord; var RawSize: TGocciaValue; @@ -324,14 +345,11 @@ function TGocciaSetValue.ContainsValue(const AValue: TGocciaValue): Boolean; end; procedure TGocciaSetValue.AddItem(const AValue: TGocciaValue); -var - Canonical: TGocciaValue; begin - // Store the canonicalized element as both key and value so that iteration - // yields the canonical form (-0 observed as +0). SetEntry leaves an existing - // member in place, matching Set.add (no reordering on re-add). - Canonical := TGocciaOrderedValueMap.CanonicalizeKey(AValue); - FStore.SetEntry(Canonical, Canonical); + // The store canonicalizes and stores the element as both key and value, so + // iteration yields the canonical form (-0 observed as +0). An existing member + // is left in place, matching Set.add (no reordering on re-add). + FStore.AddSetMember(AValue); end; function TGocciaSetValue.NextItem(var ACursor: Integer; out AValue: TGocciaValue): Boolean; @@ -399,7 +417,8 @@ function TGocciaSetValue.SetHas(const AArgs: TGocciaArgumentsCollection; const A S := TGocciaSetValue(AThisValue); // Step 4: For each element e of S.[[SetData]], do // If e is not empty and SameValueZero(e, value) is true, return true - if (AArgs.Length > 0) and S.ContainsValue(AArgs.GetElement(0)) then + // An omitted argument is the value `undefined` (GetElement yields undefined). + if S.ContainsValue(AArgs.GetElement(0)) then Result := TGocciaBooleanLiteralValue.TrueValue else // Step 5: Return false @@ -420,8 +439,8 @@ function TGocciaSetValue.SetAdd(const AArgs: TGocciaArgumentsCollection; const A // If e is not empty and SameValueZero(e, value) is true, return S // Step 5: If value is -0, set value to +0 // Step 6: Append value to S.[[SetData]] - if AArgs.Length > 0 then - S.AddItem(AArgs.GetElement(0)); + // An omitted argument is the value `undefined` (GetElement yields undefined). + S.AddItem(AArgs.GetElement(0)); // Step 7: Return S Result := AThisValue; end; @@ -439,7 +458,8 @@ function TGocciaSetValue.SetDelete(const AArgs: TGocciaArgumentsCollection; cons // Step 4: For each element e of S.[[SetData]], do // If SameValueZero(e, value) is true, replace e with empty (tombstone) // and return true - if (AArgs.Length > 0) and S.FStore.Remove(AArgs.GetElement(0)) then + // An omitted argument is the value `undefined` (GetElement yields undefined). + if S.FStore.Remove(AArgs.GetElement(0)) then Result := TGocciaBooleanLiteralValue.TrueValue else // Step 5: Return false @@ -634,9 +654,10 @@ function TGocciaSetValue.SetIntersection(const AArgs: TGocciaArgumentsCollection ThisSet, ResultSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue, Item: TGocciaValue; + NextValue: TGocciaValue; Done, WasResultRooted, WasIteratorRooted: Boolean; - Cursor: Integer; + Snapshot: TArray; + I: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -655,18 +676,14 @@ function TGocciaSetValue.SetIntersection(const AArgs: TGocciaArgumentsCollection try if ThisSet.Count <= OtherRecord.Size then begin - // Step 6: For each element e of O.[[SetData]], do - // (SetRecordHas calls user code; hold compaction so the cursor stays valid) - ThisSet.RetainIterator; - try - Cursor := 0; - while ThisSet.NextItem(Cursor, Item) do - // Step 6a: If e is not empty and SetDataHas(otherRec, e) is true, append e - if SetRecordHas(OtherRecord, Item) then - ResultSet.AddItem(Item); - finally - ThisSet.ReleaseIterator; - end; + // Step 6: For each element e of O.[[SetData]], do. Iterate a snapshot: + // SetRecordHas runs user code that may mutate O, and elements appended + // during the callback must not be visited (§24.2.4.6). + Snapshot := SnapshotSetItems(ThisSet); + for I := 0 to High(Snapshot) do + // Step 6a: If e is not empty (still a member) and SetDataHas(otherRec, e), append e + if ThisSet.ContainsValue(Snapshot[I]) and SetRecordHas(OtherRecord, Snapshot[I]) then + ResultSet.AddItem(Snapshot[I]); end else begin @@ -706,7 +723,8 @@ function TGocciaSetValue.SetDifference(const AArgs: TGocciaArgumentsCollection; Iterator: TGocciaIteratorValue; NextValue, Item: TGocciaValue; Done, WasResultRooted, WasIteratorRooted: Boolean; - Cursor: Integer; + Cursor, I: Integer; + Snapshot: TArray; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -725,18 +743,15 @@ function TGocciaSetValue.SetDifference(const AArgs: TGocciaArgumentsCollection; try if ThisSet.Count <= OtherRecord.Size then begin - // Step 6: For each element e of resultSetData, do - // (SetRecordHas calls user code; hold compaction so the cursor stays valid) - ThisSet.RetainIterator; - try - Cursor := 0; - while ThisSet.NextItem(Cursor, Item) do - // Step 6a: If e is not empty and SetDataHas(otherRec, e) is true, remove e - if not SetRecordHas(OtherRecord, Item) then - ResultSet.AddItem(Item); - finally - ThisSet.ReleaseIterator; - end; + // Step 5/6: resultSetData is a copy of O.[[SetData]]; remove members that + // are in other. The spec operates on that copy, so iterate a snapshot + // taken before any SetRecordHas callback runs (§24.2.4.5) — mutations to O + // during the callback do not affect the result. + Snapshot := SnapshotSetItems(ThisSet); + for I := 0 to High(Snapshot) do + // Step 6a: If e is not in other, keep it. + if not SetRecordHas(OtherRecord, Snapshot[I]) then + ResultSet.AddItem(Snapshot[I]); end else begin @@ -837,8 +852,8 @@ function TGocciaSetValue.SetIsSubsetOf(const AArgs: TGocciaArgumentsCollection; var ThisSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; - Item: TGocciaValue; - Cursor: Integer; + Snapshot: TArray; + I: Integer; IsSubset: Boolean; begin // Step 1: Let O be the this value @@ -857,20 +872,17 @@ function TGocciaSetValue.SetIsSubsetOf(const AArgs: TGocciaArgumentsCollection; end; IsSubset := True; - // Step 6: For each element e of O.[[SetData]], do - ThisSet.RetainIterator; - try - Cursor := 0; - while ThisSet.NextItem(Cursor, Item) do - // Step 6a: If SetDataHas(otherRec, e) is false, return false - if not SetRecordHas(OtherRecord, Item) then - begin - IsSubset := False; - Break; - end; - finally - ThisSet.ReleaseIterator; - end; + // Step 6: For each element e of O.[[SetData]] (snapshot; SetRecordHas can run + // user code that mutates O — elements appended during a callback must not be + // checked, and elements deleted during a callback are skipped). + Snapshot := SnapshotSetItems(ThisSet); + for I := 0 to High(Snapshot) do + // Step 6a: If e is still a member and SetDataHas(otherRec, e) is false, return false + if ThisSet.ContainsValue(Snapshot[I]) and not SetRecordHas(OtherRecord, Snapshot[I]) then + begin + IsSubset := False; + Break; + end; // Step 7: Return true if IsSubset then @@ -939,9 +951,10 @@ function TGocciaSetValue.SetIsDisjointFrom(const AArgs: TGocciaArgumentsCollecti ThisSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue, Item: TGocciaValue; + NextValue: TGocciaValue; Done, WasIteratorRooted, IsDisjoint: Boolean; - Cursor: Integer; + Snapshot: TArray; + I: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -955,20 +968,17 @@ function TGocciaSetValue.SetIsDisjointFrom(const AArgs: TGocciaArgumentsCollecti if ThisSet.Count <= OtherRecord.Size then begin IsDisjoint := True; - // Step 5: For each element e of O.[[SetData]], do - ThisSet.RetainIterator; - try - Cursor := 0; - while ThisSet.NextItem(Cursor, Item) do - // Step 5a: If e is not empty and SetDataHas(otherRec, e) is true, return false - if SetRecordHas(OtherRecord, Item) then - begin - IsDisjoint := False; - Break; - end; - finally - ThisSet.ReleaseIterator; - end; + // Step 5: For each element e of O.[[SetData]] (snapshot; SetRecordHas can + // run user code that mutates O — appended elements must not be checked, + // deleted elements are skipped). + Snapshot := SnapshotSetItems(ThisSet); + for I := 0 to High(Snapshot) do + // Step 5a: If e is still a member and SetDataHas(otherRec, e) is true, return false + if ThisSet.ContainsValue(Snapshot[I]) and SetRecordHas(OtherRecord, Snapshot[I]) then + begin + IsDisjoint := False; + Break; + end; if not IsDisjoint then begin Result := TGocciaBooleanLiteralValue.FalseValue; diff --git a/tests/built-ins/Map/prototype/set.js b/tests/built-ins/Map/prototype/set.js index 7fba06140..5925a2e75 100644 --- a/tests/built-ins/Map/prototype/set.js +++ b/tests/built-ins/Map/prototype/set.js @@ -88,6 +88,19 @@ test("set uses reference identity for object keys", () => { expect(map.get(a)).toBe(2); }); +test("treats omitted arguments as the undefined key and value", () => { + const m = new Map(); + m.set(); + expect(m.size).toBe(1); + expect(m.has()).toBe(true); + expect(m.get()).toBe(undefined); + m.set("k"); + expect(m.has("k")).toBe(true); + expect(m.get("k")).toBe(undefined); + expect(m.delete()).toBe(true); + expect(m.has()).toBe(false); +}); + test("throws TypeError when called on non-Map", () => { const set = Map.prototype.set; expect(() => set.call(Map.prototype, "k", "v")).toThrow(TypeError); diff --git a/tests/built-ins/Set/prototype/add.js b/tests/built-ins/Set/prototype/add.js index d05ec36e0..21623b507 100644 --- a/tests/built-ins/Set/prototype/add.js +++ b/tests/built-ins/Set/prototype/add.js @@ -86,6 +86,16 @@ test("add dedupes by string content, not string identity", () => { expect(set.size).toBe(1); }); +test("treats an omitted argument as the undefined value", () => { + const s = new Set(); + s.add(); + expect(s.size).toBe(1); + expect(s.has(undefined)).toBe(true); + expect(s.has()).toBe(true); + expect(s.delete()).toBe(true); + expect(s.size).toBe(0); +}); + test("throws TypeError when called on non-Set", () => { const add = Set.prototype.add; expect(() => add.call(Set.prototype, 1)).toThrow(TypeError); diff --git a/tests/built-ins/Set/prototype/difference.js b/tests/built-ins/Set/prototype/difference.js index b86f14356..638d2caa3 100644 --- a/tests/built-ins/Set/prototype/difference.js +++ b/tests/built-ins/Set/prototype/difference.js @@ -20,6 +20,25 @@ describe("Set.prototype.difference", () => { expect(a.difference(b).size).toBe(0); }); + test("ignores elements appended by a mutating has callback", () => { + const a = new Set([1, 2]); + const setLike = { + size: 5, + has(value) { + a.add(99); + return value === 1; + }, + keys() { + return [].values(); + }, + }; + const result = a.difference(setLike); + expect(result.has(2)).toBe(true); + expect(result.has(1)).toBe(false); + expect(result.has(99)).toBe(false); + expect(result.size).toBe(1); + }); + test("accepts set-like object", () => { const setLike = { size: 10, diff --git a/tests/built-ins/Set/prototype/intersection.js b/tests/built-ins/Set/prototype/intersection.js index 7065167be..5f82529da 100644 --- a/tests/built-ins/Set/prototype/intersection.js +++ b/tests/built-ins/Set/prototype/intersection.js @@ -19,6 +19,25 @@ describe("Set.prototype.intersection", () => { expect(a.intersection(new Set()).size).toBe(0); }); + test("skips an element deleted by a mutating has callback", () => { + const a = new Set([1, 2, 3]); + const setLike = { + size: 5, + has(value) { + if (value === 1) a.delete(2); + return true; + }, + keys() { + return [].values(); + }, + }; + const result = a.intersection(setLike); + expect(result.has(1)).toBe(true); + expect(result.has(3)).toBe(true); + expect(result.has(2)).toBe(false); + expect(result.size).toBe(2); + }); + test("accepts set-like object", () => { const setLike = { size: 10, From e7388273925e49bd216df5ad10f642fbba0bd65c Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sat, 27 Jun 2026 21:14:52 +0100 Subject: [PATCH 3/4] fix(engine): make mutating-callback Set operations spec-exact via bounded iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier snapshot+ContainsValue recheck for intersection/isSubsetOf/ isDisjointFrom could not distinguish a still-live original member from a delete-then-readd of the same value during a mutating `has` callback — membership flips back to true, so the re-added entry was wrongly visited. Replace it with bounded iteration over the original physical slots: capture the store's slot count before the callbacks (EntrySlotCount), retain the store so compaction cannot renumber slots, and walk with NextEntryBounded. Tombstones below the bound are skipped, appended members land at slots >= the bound and are ignored, and a delete-then-readd lands past the bound — matching ES2026 §24.2.4 exactly. `difference` keeps the value snapshot because the spec builds its result from a copy of O.[[SetData]]. Adds EntrySlotCount/NextEntryBounded to the shared TOrderedMap, a Pascal test for them, and a JS regression test for the delete-then-readd case. Co-Authored-By: Claude Opus 4.8 --- source/shared/OrderedMap.pas | 36 +++++ .../Goccia.Values.OrderedValueMap.Test.pas | 32 +++++ source/units/Goccia.Values.SetValue.pas | 128 ++++++++++++------ tests/built-ins/Set/prototype/intersection.js | 24 ++++ 4 files changed, 177 insertions(+), 43 deletions(-) diff --git a/source/shared/OrderedMap.pas b/source/shared/OrderedMap.pas index 03cf19ae0..20f0c151a 100644 --- a/source/shared/OrderedMap.pas +++ b/source/shared/OrderedMap.pas @@ -99,6 +99,17 @@ TEnumerator = record function GetEnumerator: TEnumerator; inline; function EntryAt(AIndex: Integer): TBaseMap.TKeyValuePair; + // Number of physical entry slots, including tombstones — the upper bound of + // valid physical indices. Stable while compaction is suppressed; grows only + // by appends. Callers that need "the entries present at a point in time" + // (e.g. spec-bounded Set operations) capture this, then iterate with + // NextEntryBounded so entries appended later are not visited. + function EntrySlotCount: Integer; + // Like GetNextEntry, but stops once the next active slot would be at or past + // ALimit. Skips tombstones below ALimit. + function NextEntryBounded(var AIterState: Integer; ALimit: Integer; + out AKey: TKey; out AValue: TValue): Boolean; + property Capacity: Integer read FBucketCount; end; @@ -439,6 +450,31 @@ function TOrderedMap.GetNextEntry(var AIterState: Integer; Result := False; end; +function TOrderedMap.EntrySlotCount: Integer; +begin + Result := FEntryCount; +end; + +function TOrderedMap.NextEntryBounded(var AIterState: Integer; + ALimit: Integer; out AKey: TKey; out AValue: TValue): Boolean; +begin + if ALimit > FEntryCount then + ALimit := FEntryCount; + while AIterState < ALimit do + begin + if FEntries[AIterState].Active then + begin + AKey := FEntries[AIterState].Key; + AValue := FEntries[AIterState].Value; + Inc(AIterState); + Result := True; + Exit; + end; + Inc(AIterState); + end; + Result := False; +end; + function TOrderedMap.EntryAt( AIndex: Integer): TBaseMap.TKeyValuePair; var diff --git a/source/units/Goccia.Values.OrderedValueMap.Test.pas b/source/units/Goccia.Values.OrderedValueMap.Test.pas index fb27b500c..8a8c2cd5d 100644 --- a/source/units/Goccia.Values.OrderedValueMap.Test.pas +++ b/source/units/Goccia.Values.OrderedValueMap.Test.pas @@ -42,6 +42,9 @@ TOrderedValueMapTests = class(TTestSuite) procedure TestCursorVisitsInInsertionOrder; procedure TestCursorSkipsTombstonesAndReaddAppends; + { Bounded iteration for spec-bounded Set operations. } + procedure TestNextEntryBoundedSkipsTombstonesAndExcludesAppends; + { Compaction gating for live iterators. } procedure TestIteratorRetainReleaseCounter; public @@ -65,6 +68,7 @@ procedure TOrderedValueMapTests.SetupTests; Test('AddSetMember stores the canonical element as key and value', TestAddSetMemberStoresCanonicalKeyAndValue); Test('Cursor visits entries in insertion order', TestCursorVisitsInInsertionOrder); Test('Cursor skips tombstones; re-add appends at end', TestCursorSkipsTombstonesAndReaddAppends); + Test('NextEntryBounded skips tombstones and excludes appends past the limit', TestNextEntryBoundedSkipsTombstonesAndExcludesAppends); Test('RetainIterator/ReleaseIterator track active count', TestIteratorRetainReleaseCounter); end; @@ -336,6 +340,34 @@ procedure TOrderedValueMapTests.TestCursorSkipsTombstonesAndReaddAppends; end; end; +procedure TOrderedValueMapTests.TestNextEntryBoundedSkipsTombstonesAndExcludesAppends; +var + Store: TGocciaOrderedValueMap; + Cursor, Limit: Integer; + Key, Value: TGocciaValue; + Order: string; +begin + Store := TGocciaOrderedValueMap.Create; + try + Store.SetEntry(Str('a'), Num(1)); + Store.SetEntry(Str('b'), Num(2)); + Store.SetEntry(Str('c'), Num(3)); + Limit := Store.EntrySlotCount; + Expect(Limit).ToBe(3); + // Tombstone a slot below the limit and append one past it. + Expect(Store.Remove(Str('b'))).ToBe(True); + Store.SetEntry(Str('d'), Num(4)); + Order := ''; + Cursor := 0; + while Store.NextEntryBounded(Cursor, Limit, Key, Value) do + Order := Order + TGocciaStringLiteralValue(Key).Value; + // 'b' skipped (tombstone), 'd' excluded (appended at a slot >= Limit). + Expect(Order).ToBe('ac'); + finally + Store.Free; + end; +end; + procedure TOrderedValueMapTests.TestIteratorRetainReleaseCounter; var Store: TGocciaOrderedValueMap; diff --git a/source/units/Goccia.Values.SetValue.pas b/source/units/Goccia.Values.SetValue.pas index 0912f0f4d..8123dea9e 100644 --- a/source/units/Goccia.Values.SetValue.pas +++ b/source/units/Goccia.Values.SetValue.pas @@ -48,6 +48,12 @@ TGocciaSetValue = class(TGocciaInstanceValue) // Live, insertion-ordered cursor over active members. Seed ACursor with 0. function NextItem(var ACursor: Integer; out AValue: TGocciaValue): Boolean; + // Like NextItem, but stops at physical slot ALimit (captured via + // EntrySlotCount before a set operation that runs user callbacks), so + // members appended during a callback are not visited. + function NextItemBounded(var ACursor: Integer; ALimit: Integer; + out AValue: TGocciaValue): Boolean; + function EntrySlotCount: Integer; procedure RetainIterator; procedure ReleaseIterator; function Count: Integer; @@ -111,11 +117,12 @@ procedure RemoveSetItem(const ASet: TGocciaSetValue; const AValue: TGocciaValue) ASet.FStore.Remove(AValue); end; -// Copy the set's current members into a plain array. The set-operation methods -// iterate this snapshot rather than a live cursor: their `has`/`keys` callbacks -// can run user code that mutates the receiver, and the spec (ES2026 §24.2.4) -// bounds these loops to the elements present when the operation began — -// elements appended during a callback must not be visited. +// Copy the set's current members into a plain array. `difference` iterates this +// snapshot because the spec (ES2026 §24.2.4.5) builds its result from a copy of +// O.[[SetData]] taken before any `has` callback runs, so later mutations of the +// receiver — including delete-then-readd — do not affect the result. The +// live-iterating operations (intersection/isSubsetOf/isDisjointFrom) instead use +// NextItemBounded over the original physical slots. function SnapshotSetItems(const ASet: TGocciaSetValue): TArray; var Cursor, Count: Integer; @@ -359,6 +366,19 @@ function TGocciaSetValue.NextItem(var ACursor: Integer; out AValue: TGocciaValue Result := FStore.NextEntry(ACursor, Key, AValue); end; +function TGocciaSetValue.NextItemBounded(var ACursor: Integer; ALimit: Integer; + out AValue: TGocciaValue): Boolean; +var + Key: TGocciaValue; +begin + Result := FStore.NextEntryBounded(ACursor, ALimit, Key, AValue); +end; + +function TGocciaSetValue.EntrySlotCount: Integer; +begin + Result := FStore.EntrySlotCount; +end; + procedure TGocciaSetValue.RetainIterator; begin FStore.RetainIterator; @@ -654,10 +674,9 @@ function TGocciaSetValue.SetIntersection(const AArgs: TGocciaArgumentsCollection ThisSet, ResultSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue: TGocciaValue; + NextValue, Item: TGocciaValue; Done, WasResultRooted, WasIteratorRooted: Boolean; - Snapshot: TArray; - I: Integer; + Cursor, Limit: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -676,14 +695,22 @@ function TGocciaSetValue.SetIntersection(const AArgs: TGocciaArgumentsCollection try if ThisSet.Count <= OtherRecord.Size then begin - // Step 6: For each element e of O.[[SetData]], do. Iterate a snapshot: - // SetRecordHas runs user code that may mutate O, and elements appended - // during the callback must not be visited (§24.2.4.6). - Snapshot := SnapshotSetItems(ThisSet); - for I := 0 to High(Snapshot) do - // Step 6a: If e is not empty (still a member) and SetDataHas(otherRec, e), append e - if ThisSet.ContainsValue(Snapshot[I]) and SetRecordHas(OtherRecord, Snapshot[I]) then - ResultSet.AddItem(Snapshot[I]); + // Step 6: For each element e of O.[[SetData]] present when the operation + // began. SetRecordHas runs user code that may mutate O; retain the store + // so physical slot indices stay stable, and bound iteration to the + // original slot count — appended members are not visited, deleted members + // are skipped, and a delete-then-readd lands past the bound (§24.2.4.6). + Limit := ThisSet.EntrySlotCount; + ThisSet.RetainIterator; + try + Cursor := 0; + while ThisSet.NextItemBounded(Cursor, Limit, Item) do + // Step 6a: If SetDataHas(otherRec, e) is true, append e + if SetRecordHas(OtherRecord, Item) then + ResultSet.AddItem(Item); + finally + ThisSet.ReleaseIterator; + end; end else begin @@ -852,8 +879,8 @@ function TGocciaSetValue.SetIsSubsetOf(const AArgs: TGocciaArgumentsCollection; var ThisSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; - Snapshot: TArray; - I: Integer; + Item: TGocciaValue; + Cursor, Limit: Integer; IsSubset: Boolean; begin // Step 1: Let O be the this value @@ -872,17 +899,25 @@ function TGocciaSetValue.SetIsSubsetOf(const AArgs: TGocciaArgumentsCollection; end; IsSubset := True; - // Step 6: For each element e of O.[[SetData]] (snapshot; SetRecordHas can run - // user code that mutates O — elements appended during a callback must not be - // checked, and elements deleted during a callback are skipped). - Snapshot := SnapshotSetItems(ThisSet); - for I := 0 to High(Snapshot) do - // Step 6a: If e is still a member and SetDataHas(otherRec, e) is false, return false - if ThisSet.ContainsValue(Snapshot[I]) and not SetRecordHas(OtherRecord, Snapshot[I]) then - begin - IsSubset := False; - Break; - end; + // Step 6: For each element e of O.[[SetData]] present when the operation + // began. SetRecordHas runs user code that may mutate O; retain the store and + // bound iteration to the original slot count so appended members are not + // checked, deleted members are skipped, and a delete-then-readd lands past + // the bound (§24.2.3.8). + Limit := ThisSet.EntrySlotCount; + ThisSet.RetainIterator; + try + Cursor := 0; + while ThisSet.NextItemBounded(Cursor, Limit, Item) do + // Step 6a: If SetDataHas(otherRec, e) is false, return false + if not SetRecordHas(OtherRecord, Item) then + begin + IsSubset := False; + Break; + end; + finally + ThisSet.ReleaseIterator; + end; // Step 7: Return true if IsSubset then @@ -951,10 +986,9 @@ function TGocciaSetValue.SetIsDisjointFrom(const AArgs: TGocciaArgumentsCollecti ThisSet: TGocciaSetValue; OtherRecord: TGocciaSetRecord; Iterator: TGocciaIteratorValue; - NextValue: TGocciaValue; + NextValue, Item: TGocciaValue; Done, WasIteratorRooted, IsDisjoint: Boolean; - Snapshot: TArray; - I: Integer; + Cursor, Limit: Integer; GC: TGarbageCollector; begin // Step 1: Let O be the this value @@ -968,17 +1002,25 @@ function TGocciaSetValue.SetIsDisjointFrom(const AArgs: TGocciaArgumentsCollecti if ThisSet.Count <= OtherRecord.Size then begin IsDisjoint := True; - // Step 5: For each element e of O.[[SetData]] (snapshot; SetRecordHas can - // run user code that mutates O — appended elements must not be checked, - // deleted elements are skipped). - Snapshot := SnapshotSetItems(ThisSet); - for I := 0 to High(Snapshot) do - // Step 5a: If e is still a member and SetDataHas(otherRec, e) is true, return false - if ThisSet.ContainsValue(Snapshot[I]) and SetRecordHas(OtherRecord, Snapshot[I]) then - begin - IsDisjoint := False; - Break; - end; + // Step 5: For each element e of O.[[SetData]] present when the operation + // began. SetRecordHas runs user code that may mutate O; retain the store and + // bound iteration to the original slot count so appended elements are not + // checked, deleted elements are skipped, and a delete-then-readd lands past + // the bound (§24.2.3.6). + Limit := ThisSet.EntrySlotCount; + ThisSet.RetainIterator; + try + Cursor := 0; + while ThisSet.NextItemBounded(Cursor, Limit, Item) do + // Step 5a: If SetDataHas(otherRec, e) is true, return false + if SetRecordHas(OtherRecord, Item) then + begin + IsDisjoint := False; + Break; + end; + finally + ThisSet.ReleaseIterator; + end; if not IsDisjoint then begin Result := TGocciaBooleanLiteralValue.FalseValue; diff --git a/tests/built-ins/Set/prototype/intersection.js b/tests/built-ins/Set/prototype/intersection.js index 5f82529da..575a6b890 100644 --- a/tests/built-ins/Set/prototype/intersection.js +++ b/tests/built-ins/Set/prototype/intersection.js @@ -38,6 +38,30 @@ describe("Set.prototype.intersection", () => { expect(result.size).toBe(2); }); + test("does not revisit a member deleted then re-added during a has callback", () => { + const a = new Set([1, 2, 3]); + const setLike = { + size: 5, + has(value) { + if (value === 1) { + a.delete(3); + a.add(3); + } + return true; + }, + keys() { + return [].values(); + }, + }; + const result = a.intersection(setLike); + // 3's original entry was emptied and the re-add is past the start-of-call + // bound, so it is not visited. + expect(result.has(1)).toBe(true); + expect(result.has(2)).toBe(true); + expect(result.has(3)).toBe(false); + expect(result.size).toBe(2); + }); + test("accepts set-like object", () => { const setLike = { size: 10, From 7b65c465d0ca45556a6953ae84296c00f2ed1b1f Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sun, 28 Jun 2026 01:07:22 +0100 Subject: [PATCH 4/4] fix(engine): keep Set.difference members rooted across mutating has callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set.difference's small branch called the set-like `has` (user code) on each snapshot member before copying it into the temp-rooted ResultSet. SnapshotSetItems returns a plain array, which is not a GC root, so a `has` callback that deleted a not-yet-processed member from the receiver and triggered a GC could free that member while it was still referenced only by the snapshot, dangling a later Snapshot[I]. Copy the snapshot into the temp-rooted ResultSet first, then remove the members present in other. The members stay reachable via ResultSet for the duration of the callbacks, and this matches the spec more literally (resultSetData is a copy of O.[[SetData]] that has matching elements removed, §24.2.4.5). Co-Authored-By: Claude Opus 4.8 --- source/units/Goccia.Values.SetValue.pas | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/source/units/Goccia.Values.SetValue.pas b/source/units/Goccia.Values.SetValue.pas index 8123dea9e..21f478865 100644 --- a/source/units/Goccia.Values.SetValue.pas +++ b/source/units/Goccia.Values.SetValue.pas @@ -771,14 +771,18 @@ function TGocciaSetValue.SetDifference(const AArgs: TGocciaArgumentsCollection; if ThisSet.Count <= OtherRecord.Size then begin // Step 5/6: resultSetData is a copy of O.[[SetData]]; remove members that - // are in other. The spec operates on that copy, so iterate a snapshot - // taken before any SetRecordHas callback runs (§24.2.4.5) — mutations to O - // during the callback do not affect the result. + // are in other. Copy into the temp-rooted ResultSet first so the snapshot + // members stay reachable even if a SetRecordHas callback deletes one from + // O and triggers a GC, then remove those present in other (§24.2.4.5). + // Operating on the copy also means mutations to O during the callback do + // not affect the result. Snapshot := SnapshotSetItems(ThisSet); for I := 0 to High(Snapshot) do - // Step 6a: If e is not in other, keep it. - if not SetRecordHas(OtherRecord, Snapshot[I]) then - ResultSet.AddItem(Snapshot[I]); + ResultSet.AddItem(Snapshot[I]); + for I := 0 to High(Snapshot) do + // Step 6a: If e is in other, remove it from the result. + if SetRecordHas(OtherRecord, Snapshot[I]) then + RemoveSetItem(ResultSet, Snapshot[I]); end else begin