From 25550ce903d01f31bead59de945e4adf86819440 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Thu, 15 Mar 2018 21:12:03 +0100 Subject: Close iterator after error in Array.from Issue #17 --- js/src/builtin/Array.js | 46 ++++-- js/src/tests/ecma_6/Array/from-iterator-close.js | 183 +++++++++++++++++++++++ 2 files changed, 215 insertions(+), 14 deletions(-) create mode 100644 js/src/tests/ecma_6/Array/from-iterator-close.js diff --git a/js/src/builtin/Array.js b/js/src/builtin/Array.js index 54b47b72f..5ab0b71be 100644 --- a/js/src/builtin/Array.js +++ b/js/src/builtin/Array.js @@ -784,7 +784,7 @@ function ArrayKeys() { return CreateArrayIterator(this, ITEM_KIND_KEY); } -// ES6 draft rev31 (2015/01/15) 22.1.2.1 Array.from(source[, mapfn[, thisArg]]). +// ES 2017 draft 0f10dba4ad18de92d47d421f378233a2eae8f077 22.1.2.1 function ArrayFrom(items, mapfn=undefined, thisArg=undefined) { // Step 1. var C = this; @@ -795,43 +795,61 @@ function ArrayFrom(items, mapfn=undefined, thisArg=undefined) { ThrowTypeError(JSMSG_NOT_FUNCTION, DecompileArg(1, mapfn)); var T = thisArg; - // Steps 4-5. + // Step 4. var usingIterator = GetMethod(items, std_iterator); - // Step 6. + // Step 5. if (usingIterator !== undefined) { - // Steps 6.a-c. + // Steps 5.a-c. var A = IsConstructor(C) ? new C() : []; - // Steps 6.d-e. + // Step 5.c. var iterator = GetIterator(items, usingIterator); - // Step 6.f. + // Step 5.d. var k = 0; - // Step 6.g. + // Step 5.e. // These steps cannot be implemented using a for-of loop. // See . while (true) { - // Steps 6.g.i-iii. + // Step 5.e.i. + // Disabled for performance reason. We won't hit this case on + // normal array, since _DefineDataProperty will throw before it. + // We could hit this when |A| is a proxy and it ignores + // |_DefineDataProperty|, but it happens only after too long loop. + /* + if (k >= 0x1fffffffffffff) { + IteratorCloseThrow(iterator); + ThrowTypeError(JSMSG_TOO_LONG_ARRAY); + } + */ + + // Step 5.e.iii. var next = callContentFunction(iterator.next, iterator); if (!IsObject(next)) ThrowTypeError(JSMSG_NEXT_RETURNED_PRIMITIVE); - // Step 6.g.iv. + // Step 5.e.iv. if (next.done) { A.length = k; return A; } - // Steps 6.g.v-vi. + // Steps 5.e.v. var nextValue = next.value; - // Steps 6.g.vii-viii. - var mappedValue = mapping ? callContentFunction(mapfn, thisArg, nextValue, k) : nextValue; + // Steps 5.e.vi-vii. + try { + var mappedValue = mapping ? callContentFunction(mapfn, thisArg, nextValue, k) : nextValue; - // Steps 6.g.ix-xi. - _DefineDataProperty(A, k++, mappedValue); + // Steps 5.e.ii (reordered), 5.e.viii. + _DefineDataProperty(A, k++, mappedValue); + } catch (e) { + // Steps 5.e.vi.2, 5.e.ix. + IteratorCloseThrow(iterator); + throw e; + } } } diff --git a/js/src/tests/ecma_6/Array/from-iterator-close.js b/js/src/tests/ecma_6/Array/from-iterator-close.js new file mode 100644 index 000000000..fa97ea282 --- /dev/null +++ b/js/src/tests/ecma_6/Array/from-iterator-close.js @@ -0,0 +1,183 @@ +var BUGNUMBER = 1180306; +var summary = 'Array.from should close iterator on error'; + +print(BUGNUMBER + ": " + summary); + +function test(ctor, { mapVal=undefined, + nextVal=undefined, + nextThrowVal=undefined, + modifier=undefined, + exceptionVal=undefined, + exceptionType=undefined, + closed=true }) { + let iterable = { + closed: false, + [Symbol.iterator]() { + let iterator = { + first: true, + next() { + if (this.first) { + this.first = false; + if (nextThrowVal) + throw nextThrowVal; + return nextVal; + } + return { value: undefined, done: true }; + }, + return() { + iterable.closed = true; + return {}; + } + }; + if (modifier) + modifier(iterator, iterable); + + return iterator; + } + }; + if (exceptionVal) { + let caught = false; + try { + ctor.from(iterable, mapVal); + } catch (e) { + assertEq(e, exceptionVal); + caught = true; + } + assertEq(caught, true); + } else if (exceptionType) { + assertThrowsInstanceOf(() => ctor.from(iterable, mapVal), exceptionType); + } else { + ctor.from(iterable, mapVal); + } + assertEq(iterable.closed, closed); +} + +// == Error cases with close == + +// ES 2017 draft 22.1.2.1 step 5.e.i.1. +// Cannot test. + +// ES 2017 draft 22.1.2.1 step 5.e.vi.2. +test(Array, { + mapVal: () => { throw "map throws"; }, + nextVal: { value: 1, done: false }, + exceptionVal: "map throws", + closed: true, +}); + +// ES 2017 draft 22.1.2.1 step 5.e.ix. +class MyArray extends Array { + constructor() { + return new Proxy({}, { + defineProperty() { + throw "defineProperty throws"; + } + }); + } +} +test(MyArray, { + nextVal: { value: 1, done: false }, + exceptionVal: "defineProperty throws", + closed: true, +}); + +// ES 2017 draft 7.4.6 step 3. +// if GetMethod fails, the thrown value should be used. +test(MyArray, { + nextVal: { value: 1, done: false }, + modifier: (iterator, iterable) => { + Object.defineProperty(iterator, "return", { + get: function() { + iterable.closed = true; + throw "return getter throws"; + } + }); + }, + exceptionVal: "return getter throws", + closed: true, +}); +test(MyArray, { + nextVal: { value: 1, done: false }, + modifier: (iterator, iterable) => { + Object.defineProperty(iterator, "return", { + get: function() { + iterable.closed = true; + return "non object"; + } + }); + }, + exceptionType: TypeError, + closed: true, +}); +test(MyArray, { + nextVal: { value: 1, done: false }, + modifier: (iterator, iterable) => { + Object.defineProperty(iterator, "return", { + get: function() { + iterable.closed = true; + // Non callable. + return {}; + } + }); + }, + exceptionType: TypeError, + closed: true, +}); + +// ES 2017 draft 7.4.6 steps 6. +// if return method throws, the thrown value should be ignored. +test(MyArray, { + nextVal: { value: 1, done: false }, + modifier: (iterator, iterable) => { + iterator.return = function() { + iterable.closed = true; + throw "return throws"; + }; + }, + exceptionVal: "defineProperty throws", + closed: true, +}); + +test(MyArray, { + nextVal: { value: 1, done: false }, + modifier: (iterator, iterable) => { + iterator.return = function() { + iterable.closed = true; + return "non object"; + }; + }, + exceptionVal: "defineProperty throws", + closed: true, +}); + +// == Error cases without close == + +// ES 2017 draft 22.1.2.1 step 5.e.iii. +test(Array, { + nextThrowVal: "next throws", + exceptionVal: "next throws", + closed: false, +}); + +test(Array, { + nextVal: { value: {}, get done() { throw "done getter throws"; } }, + exceptionVal: "done getter throws", + closed: false, +}); + +// ES 2017 draft 22.1.2.1 step 5.e.v. +test(Array, { + nextVal: { get value() { throw "value getter throws"; }, done: false }, + exceptionVal: "value getter throws", + closed: false, +}); + +// == Successful cases == + +test(Array, { + nextVal: { value: 1, done: false }, + closed: false, +}); + +if (typeof reportCompare === 'function') + reportCompare(true, true); -- cgit v1.2.3