From f5372c78cafff64bfda5849386538f806916049a Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Tue, 21 Mar 2023 11:57:35 -0700 Subject: [PATCH] [Fix] close underlying iterator when helper is closed See https://github.com/tc39/proposal-iterator-helpers/pull/267 --- .eslintrc | 1 + Iterator.prototype.drop/implementation.js | 6 +- Iterator.prototype.filter/implementation.js | 6 +- Iterator.prototype.flatMap/implementation.js | 6 +- Iterator.prototype.map/implementation.js | 6 +- Iterator.prototype.take/implementation.js | 6 +- IteratorHelperPrototype/index.js | 24 +++- aos/CreateIteratorFromClosure.js | 30 ++++- test/Iterator.prototype.drop.js | 122 +++++++++++++++++++ test/Iterator.prototype.filter.js | 70 +++++++++++ 10 files changed, 263 insertions(+), 14 deletions(-) diff --git a/.eslintrc b/.eslintrc index 62b6a21..1df2437 100644 --- a/.eslintrc +++ b/.eslintrc @@ -28,6 +28,7 @@ "GetIteratorDirect", "GetIteratorFlattenable", "GetMethod", + "IsArray", "IsCallable", "IteratorClose", "IteratorComplete", diff --git a/Iterator.prototype.drop/implementation.js b/Iterator.prototype.drop/implementation.js index fede5d2..980d14d 100644 --- a/Iterator.prototype.drop/implementation.js +++ b/Iterator.prototype.drop/implementation.js @@ -79,5 +79,9 @@ module.exports = function drop(limit) { SLOT.set(closure, '[[Sentinel]]', sentinel); // for the userland implementation SLOT.set(closure, '[[CloseIfAbrupt]]', closeIfAbrupt); // for the userland implementation - return CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto); // step 4 + var result = CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto, ['[[UnderlyingIterator]]']); // step 4 + + SLOT.set(result, '[[UnderlyingIterator]]', iterated); // step 5 + + return result; // step 6 }; diff --git a/Iterator.prototype.filter/implementation.js b/Iterator.prototype.filter/implementation.js index 0d7aeb9..5f814b0 100644 --- a/Iterator.prototype.filter/implementation.js +++ b/Iterator.prototype.filter/implementation.js @@ -66,5 +66,9 @@ module.exports = function filter(predicate) { SLOT.set(closure, '[[Sentinel]]', sentinel); // for the userland implementation SLOT.set(closure, '[[CloseIfAbrupt]]', closeIfAbrupt); // for the userland implementation - return CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto); // step 4 + var result = CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto, ['[[UnderlyingIterator]]']); // step 4 + + SLOT.set(result, '[[UnderlyingIterator]]', iterated); // step 5 + + return result; // step 6 }; diff --git a/Iterator.prototype.flatMap/implementation.js b/Iterator.prototype.flatMap/implementation.js index 0d4ed26..99556de 100644 --- a/Iterator.prototype.flatMap/implementation.js +++ b/Iterator.prototype.flatMap/implementation.js @@ -84,5 +84,9 @@ module.exports = function flatMap(mapper) { SLOT.set(closure, '[[Sentinel]]', sentinel); // for the userland implementation SLOT.set(closure, '[[CloseIfAbrupt]]', closeIfAbrupt); // for the userland implementation - return CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto); // step 4 + var result = CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto, ['[[UnderlyingIterator]]']); // step 4 + + SLOT.set(result, '[[UnderlyingIterator]]', iterated); // step 5 + + return result; // step 6 }; diff --git a/Iterator.prototype.map/implementation.js b/Iterator.prototype.map/implementation.js index c6016f8..2d2da32 100644 --- a/Iterator.prototype.map/implementation.js +++ b/Iterator.prototype.map/implementation.js @@ -63,5 +63,9 @@ module.exports = function map(mapper) { SLOT.set(closure, '[[Sentinel]]', sentinel); // for the userland implementation SLOT.set(closure, '[[CloseIfAbrupt]]', closeIfAbrupt); // for the userland implementation - return CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto); // step 4 + var result = CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto, ['[[UnderlyingIterator]]']); // step 4 + + SLOT.set(result, '[[UnderlyingIterator]]', iterated); // step 5 + + return result; // step 6 }; diff --git a/Iterator.prototype.take/implementation.js b/Iterator.prototype.take/implementation.js index 829e7ed..124bcd8 100644 --- a/Iterator.prototype.take/implementation.js +++ b/Iterator.prototype.take/implementation.js @@ -71,5 +71,9 @@ module.exports = function take(limit) { SLOT.set(closure, '[[Sentinel]]', sentinel); // for the userland implementation SLOT.set(closure, '[[CloseIfAbrupt]]', closeIfAbrupt); // for the userland implementation - return CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto); // step 4 + var result = CreateIteratorFromClosure(closure, 'Iterator Helper', iterHelperProto, ['[[UnderlyingIterator]]']); // step 4 + + SLOT.set(result, '[[UnderlyingIterator]]', iterated); // step 5 + + return result; // step 6 }; diff --git a/IteratorHelperPrototype/index.js b/IteratorHelperPrototype/index.js index 02146b1..92bf3e8 100644 --- a/IteratorHelperPrototype/index.js +++ b/IteratorHelperPrototype/index.js @@ -2,12 +2,15 @@ var setToStringTag = require('es-set-tostringtag'); var hasProto = require('has-proto')(); +var iterProto = require('iterator.prototype'); +var SLOT = require('internal-slot'); var CompletionRecord = require('es-abstract/2022/CompletionRecord'); +var CreateIterResultObject = require('es-abstract/2015/CreateIterResultObject'); var GeneratorResume = require('../aos/GeneratorResume'); var GeneratorResumeAbrupt = require('../aos/GeneratorResumeAbrupt'); - -var iterProto = require('iterator.prototype'); +var IteratorClose = require('../aos/IteratorClose'); +var NormalCompletion = require('es-abstract/2022/NormalCompletion'); var implementation; if (hasProto) { @@ -17,8 +20,21 @@ if (hasProto) { return GeneratorResume(this, void undefined, 'Iterator Helper'); }, 'return': function () { - var C = new CompletionRecord('return', void undefined); // step 1 - return GeneratorResumeAbrupt(this, C, 'Iterator Helper'); + var O = this; // step 1 + + SLOT.assert(O, '[[UnderlyingIterator]]'); // step 2 + + SLOT.assert(O, '[[GeneratorState]]'); // step 3 + + if (SLOT.get(O, '[[GeneratorState]]') === 'suspendedStart') { // step 4 + SLOT.set(O, '[[GeneratorState]]', 'completed'); // step 4.a + IteratorClose(SLOT.get(O, '[[UnderlyingIterator]]'), NormalCompletion('unused')); // step 4.c + return CreateIterResultObject(void undefined, true); // step 4.d + } + + var C = new CompletionRecord('return', void undefined); // step 5 + + return GeneratorResumeAbrupt(O, C, 'Iterator Helper'); // step 6 } }; setToStringTag(implementation, 'Iterator Helper'); diff --git a/aos/CreateIteratorFromClosure.js b/aos/CreateIteratorFromClosure.js index 075d6c7..1b58fe0 100644 --- a/aos/CreateIteratorFromClosure.js +++ b/aos/CreateIteratorFromClosure.js @@ -4,22 +4,42 @@ var GetIntrinsic = require('get-intrinsic'); var $TypeError = GetIntrinsic('%TypeError%'); +var GeneratorStart = require('./GeneratorStart'); +var IsArray = require('es-abstract/2022/IsArray'); var IsCallable = require('es-abstract/2022/IsCallable'); var OrdinaryObjectCreate = require('es-abstract/2022/OrdinaryObjectCreate'); +var Type = require('es-abstract/2022/Type'); -var GeneratorStart = require('./GeneratorStart'); +var every = require('es-abstract/helpers/every'); +var callBound = require('call-bind/callBound'); var SLOT = require('internal-slot'); -module.exports = function CreateIteratorFromClosure(closure, brand, proto) { +var $concat = callBound('Array.prototype.concat'); + +var isString = function isString(slot) { + return Type(slot) === 'String'; +}; + +module.exports = function CreateIteratorFromClosure(closure, generatorBrand, proto) { if (!IsCallable(closure)) { throw new $TypeError('`closure` must be a function'); } - var generator = OrdinaryObjectCreate(proto, ['[[GeneratorContext]]', '[[GeneratorBrand]]', '[[GeneratorState]]']); // steps 3, 5 - SLOT.set(generator, '[[GeneratorBrand]]', brand); // step 4 + if (Type(generatorBrand) !== 'String') { + throw new $TypeError('`generatorBrand` must be a string'); + } + var extraSlots = arguments.length > 3 ? arguments[3] : []; + if (arguments.length > 3) { + if (!IsArray(extraSlots) || !every(extraSlots, isString)) { + throw new $TypeError('`extraSlots` must be a List of String internal slot names'); + } + } + var internalSlotsList = $concat(extraSlots, ['[[GeneratorContext]]', '[[GeneratorBrand]]', '[[GeneratorState]]']); // step 3 + var generator = OrdinaryObjectCreate(proto, internalSlotsList); // steps 4, 6 + SLOT.set(generator, '[[GeneratorBrand]]', generatorBrand); // step 5 SLOT.assert(closure, '[[Sentinel]]'); // our userland slot - SLOT.set(generator, '[[Sentinel]]'); // our userland slot + SLOT.set(generator, '[[Sentinel]]', SLOT.get(closure, '[[Sentinel]]')); // our userland slot SLOT.assert(closure, '[[CloseIfAbrupt]]'); // our second userland slot SLOT.set(generator, '[[CloseIfAbrupt]]', SLOT.get(closure, '[[CloseIfAbrupt]]')); // our second userland slot diff --git a/test/Iterator.prototype.drop.js b/test/Iterator.prototype.drop.js index 9769724..c61a52d 100644 --- a/test/Iterator.prototype.drop.js +++ b/test/Iterator.prototype.drop.js @@ -83,6 +83,128 @@ module.exports = { st.end(); }); + + t.test('262: test/built-ins/Iterator/prototype/drop/return-is-forwarded', function (st) { + var returnCount = 0; + + var makeBadIterator = function makeBadIterator() { + return { + next: function next() { + return { + done: false, + value: 1 + }; + }, + 'return': function () { + returnCount += 1; + return {}; + } + }; + }; + + var iter1 = drop(makeBadIterator(), 0); + st.equal(returnCount, 0, 'iter1, before return()'); + iter1['return'](); + st.equal(returnCount, 1, 'iter1, after return()'); + + var iter2 = drop(makeBadIterator(), 1); + st.equal(returnCount, 1, 'iter2, before return()'); + iter2['return'](); + st.equal(returnCount, 2, 'iter2, after return()'); + + // 5 drops (i wish i had pipeline) + var iter3 = drop( + drop( + drop( + drop( + drop( + makeBadIterator(), + 1 + ), + 1 + ), + 1 + ), + 1 + ), + 1 + ); + st.equal(returnCount, 2, 'iter3, before return()'); + iter3['return'](); + st.equal(returnCount, 3, 'iter3, after return()'); + + st.end(); + }); + + t.test('262: test/built-ins/Iterator/prototype/drop/return-is-not-forwarded-after-exhaustion', { skip: !hasPropertyDescriptors }, function (st) { + var makeBadIterator = function makeBadIterator() { + return { + next: function next() { + return { + done: true, + value: undefined + }; + }, + 'return': function () { + throw new SyntaxError(); + } + }; + }; + + var iter1 = drop(makeBadIterator(), 0); + st['throws']( + function () { iter1['return'](); }, + SyntaxError, + 'iter1, return() throws' + ); + iter1.next(); + iter1['return'](); + + var iter2 = drop(makeBadIterator(), 1); + st['throws']( + function () { iter2['return'](); }, + SyntaxError, + 'iter2, return() throws' + ); + iter2.next(); + iter2['return'](); + + // 5 drops (i wish i had pipeline) + var iter3 = drop( + drop( + drop( + drop( + drop( + makeBadIterator(), + 1 + ), + 1 + ), + 1 + ), + 1 + ), + 1 + ); + st['throws']( + function () { iter3['return'](); }, + SyntaxError, + 'iter3, return() throws' + ); + iter3.next(); + iter3['return'](); + + var iter4 = drop(makeBadIterator(), 10); + st['throws']( + function () { iter4['return'](); }, + SyntaxError, + 'iter4, return() throws' + ); + iter4.next(); + iter4['return'](); + + st.end(); + }); }, index: function () { test('Iterator.prototype.' + fnName + ': index', function (t) { diff --git a/test/Iterator.prototype.filter.js b/test/Iterator.prototype.filter.js index 9e3d96e..f77557b 100644 --- a/test/Iterator.prototype.filter.js +++ b/test/Iterator.prototype.filter.js @@ -180,6 +180,76 @@ module.exports = { st.end(); }); + + t.test('262: test/built-ins/Iterator/prototype/drop/return-is-forwarded', function (st) { + var returnCount = 0; + + var badIterator = { + next: function next() { + return { + done: false, + value: 1 + }; + }, + 'return': function () { + returnCount += 1; + return {}; + } + }; + + var iter1 = filter(badIterator, function () { return false; }); + st.equal(returnCount, 0, 'iter1, before return()'); + iter1['return'](); + st.equal(returnCount, 1, 'iter1, after return()'); + + st.end(); + }); + + t.test('262: test/built-ins/Iterator/prototype/drop/return-is-not-forwarded-after-exhaustion', { skip: !hasPropertyDescriptors }, function (st) { + var makeBadIterator = function makeBadIterator() { + return { + next: function next() { + return { + done: true, + value: undefined + }; + }, + 'return': function () { + throw new SyntaxError(); + } + }; + }; + + var iter1 = filter(makeBadIterator(), function () { return true; }); + st['throws']( + function () { iter1['return'](); }, + SyntaxError, + 'iter1, return() throws' + ); + iter1.next(); + iter1['return'](); + + // 3 filters (i wish i had pipeline) + var iter2 = filter( + filter( + filter( + makeBadIterator(), + function () { return true; } + ), + function () { return true; } + ), + function () { return true; } + ); + st['throws']( + function () { iter2['return'](); }, + SyntaxError, + 'iter2, return() throws' + ); + iter2.next(); + iter2['return'](); + + st.end(); + }); }, index: function () { test('Iterator.prototype.' + fnName + ': index', function (t) {