From 28f16a2905ab640ee94f71a217e45b11479c29b9 Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sat, 9 May 2026 10:02:44 +0100 Subject: [PATCH 1/2] Close source iterator when Iterator.prototype.take limit is reached (#591) Co-Authored-By: Claude Opus 4.6 (1M context) --- source/units/Goccia.Values.Iterator.Lazy.pas | 2 + tests/built-ins/Iterator/prototype/take.js | 83 +++++++++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/source/units/Goccia.Values.Iterator.Lazy.pas b/source/units/Goccia.Values.Iterator.Lazy.pas index b1ecd2d9..8700555d 100644 --- a/source/units/Goccia.Values.Iterator.Lazy.pas +++ b/source/units/Goccia.Values.Iterator.Lazy.pas @@ -273,6 +273,7 @@ function TGocciaLazyTakeIteratorValue.DoAdvanceNext: TGocciaObjectValue; if FIndex >= FLimit then begin FDone := True; + CloseIteratorPreservingError(FSourceIterator); Result := CreateIteratorResult(TGocciaUndefinedLiteralValue.UndefinedValue, True); Exit; end; @@ -292,6 +293,7 @@ function TGocciaLazyTakeIteratorValue.DoDirectNext(out ADone: Boolean): TGocciaV if FIndex >= FLimit then begin FDone := True; + CloseIteratorPreservingError(FSourceIterator); ADone := True; Result := TGocciaUndefinedLiteralValue.UndefinedValue; Exit; diff --git a/tests/built-ins/Iterator/prototype/take.js b/tests/built-ins/Iterator/prototype/take.js index e490b3c4..7e1f3450 100644 --- a/tests/built-ins/Iterator/prototype/take.js +++ b/tests/built-ins/Iterator/prototype/take.js @@ -27,7 +27,8 @@ describe("Iterator.prototype.take()", () => { expect(taken.next().value).toBe(10); expect(taken.next().value).toBe(20); expect(taken.next().done).toBe(true); - expect(source.next().value).toBe(30); + // Source is closed when the take limit is reached (IteratorClose per spec) + expect(source.next().done).toBe(true); }); test("take composes after drop", () => { @@ -39,6 +40,86 @@ describe("Iterator.prototype.take()", () => { expect(result).toEqual([3, 4, 5]); }); + test("take closes source iterator when limit is reached", () => { + let closed = false; + const source = { + [Symbol.iterator]() { + let i = 0; + return { + next() { + i++; + return { value: i, done: false }; + }, + return() { + closed = true; + return { value: undefined, done: true }; + }, + }; + }, + }; + + const result = Iterator.from(source[Symbol.iterator]()).take(2).toArray(); + expect(result).toEqual([1, 2]); + expect(closed).toBe(true); + }); + + test("take runs generator finally blocks when limit is reached", () => { + let finalized = false; + const gen = { + *go() { + try { yield 1; yield 2; yield 3; } + finally { finalized = true; } + }, + }.go; + + const arr = gen().take(1).toArray(); + expect(arr).toEqual([1]); + expect(finalized).toBe(true); + }); + + test("take does not close source when source is exhausted before limit", () => { + let closed = false; + const source = { + [Symbol.iterator]() { + let i = 0; + return { + next() { + i++; + if (i > 2) return { value: undefined, done: true }; + return { value: i, done: false }; + }, + return() { + closed = true; + return { value: undefined, done: true }; + }, + }; + }, + }; + + const result = Iterator.from(source[Symbol.iterator]()).take(5).toArray(); + expect(result).toEqual([1, 2]); + expect(closed).toBe(false); + }); + + test("take(0) closes source iterator immediately", () => { + let closed = false; + const source = { + [Symbol.iterator]() { + return { + next() { return { value: 1, done: false }; }, + return() { + closed = true; + return { value: undefined, done: true }; + }, + }; + }, + }; + + const result = Iterator.from(source[Symbol.iterator]()).take(0).toArray(); + expect(result).toEqual([]); + expect(closed).toBe(true); + }); + test("take can bound nested iterators inside helper chains", () => { const data = [5, 2, 8, 1, 9]; const result = Iterator.from(data[Symbol.iterator]()) From 3b34b74e74aea0685c0066c10000daa02e90bd93 Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sat, 9 May 2026 19:27:46 +0100 Subject: [PATCH 2/2] Use normal-completion CloseIterator on take exhaustion path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CloseIteratorPreservingError swallows errors from iter.return(), but take exhaustion is a normal completion — errors should propagate per ES2024 §7.4.10. Switch to CloseIterator and add a test verifying TypeError propagation. Also rename existing test to encode close semantics in name. Co-Authored-By: Claude Opus 4.6 (1M context) --- source/units/Goccia.Values.Iterator.Lazy.pas | 5 ++-- tests/built-ins/Iterator/prototype/take.js | 24 ++++++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/source/units/Goccia.Values.Iterator.Lazy.pas b/source/units/Goccia.Values.Iterator.Lazy.pas index 8700555d..6aab0e72 100644 --- a/source/units/Goccia.Values.Iterator.Lazy.pas +++ b/source/units/Goccia.Values.Iterator.Lazy.pas @@ -97,6 +97,7 @@ implementation Goccia.Values.FunctionBase, Goccia.Values.Iterator.Concrete, Goccia.Values.Iterator.Generic, + Goccia.Values.IteratorSupport, Goccia.Values.SymbolValue; { TGocciaLazyMapIteratorValue } @@ -273,7 +274,7 @@ function TGocciaLazyTakeIteratorValue.DoAdvanceNext: TGocciaObjectValue; if FIndex >= FLimit then begin FDone := True; - CloseIteratorPreservingError(FSourceIterator); + CloseIterator(FSourceIterator); Result := CreateIteratorResult(TGocciaUndefinedLiteralValue.UndefinedValue, True); Exit; end; @@ -293,7 +294,7 @@ function TGocciaLazyTakeIteratorValue.DoDirectNext(out ADone: Boolean): TGocciaV if FIndex >= FLimit then begin FDone := True; - CloseIteratorPreservingError(FSourceIterator); + CloseIterator(FSourceIterator); ADone := True; Result := TGocciaUndefinedLiteralValue.UndefinedValue; Exit; diff --git a/tests/built-ins/Iterator/prototype/take.js b/tests/built-ins/Iterator/prototype/take.js index 7e1f3450..b9ebee8b 100644 --- a/tests/built-ins/Iterator/prototype/take.js +++ b/tests/built-ins/Iterator/prototype/take.js @@ -20,14 +20,13 @@ describe("Iterator.prototype.take()", () => { expect(() => [1].values().take(-1)).toThrow(RangeError); }); - test("take only advances the source as needed", () => { + test("take closes the source when the limit is reached", () => { const source = [10, 20, 30, 40, 50].values(); const taken = source.take(2); expect(taken.next().value).toBe(10); expect(taken.next().value).toBe(20); expect(taken.next().done).toBe(true); - // Source is closed when the take limit is reached (IteratorClose per spec) expect(source.next().done).toBe(true); }); @@ -120,6 +119,27 @@ describe("Iterator.prototype.take()", () => { expect(closed).toBe(true); }); + test("take propagates errors from return() on normal completion", () => { + const source = { + [Symbol.iterator]() { + let i = 0; + return { + next() { + i++; + return { value: i, done: false }; + }, + return() { + throw new TypeError("close error"); + }, + }; + }, + }; + + expect(() => { + Iterator.from(source[Symbol.iterator]()).take(1).toArray(); + }).toThrow(TypeError); + }); + test("take can bound nested iterators inside helper chains", () => { const data = [5, 2, 8, 1, 9]; const result = Iterator.from(data[Symbol.iterator]())