From 2bb0252ab48a97a72c33cef9cbe54e86563f15c9 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sat, 24 Mar 2018 12:23:14 +0100 Subject: Bug 1147371: Implement IteratorClose for array destructuring Issue #74 --- js/src/frontend/BytecodeEmitter.cpp | 218 ++++++++++++--------- js/src/frontend/BytecodeEmitter.h | 3 +- js/src/jit/JitFrames.cpp | 82 +++++--- js/src/jsscript.h | 3 +- js/src/shell/js.cpp | 35 +++- .../ecma_6/Destructuring/array-iterator-close.js | 77 ++++++++ js/src/vm/Interpreter.cpp | 39 +++- 7 files changed, 322 insertions(+), 135 deletions(-) create mode 100644 js/src/tests/ecma_6/Destructuring/array-iterator-close.js (limited to 'js') diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 2934f63a5..e2c906850 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -4947,7 +4947,7 @@ BytecodeEmitter::emitIteratorClose(Maybe yieldStarTryStart, bool all template bool -BytecodeEmitter::wrapWithIteratorCloseTryNote(int32_t iterDepth, InnerEmitter emitter) +BytecodeEmitter::wrapWithDestructuringIteratorCloseTryNote(int32_t iterDepth, InnerEmitter emitter) { MOZ_ASSERT(this->stackDepth >= iterDepth); @@ -4956,7 +4956,7 @@ BytecodeEmitter::wrapWithIteratorCloseTryNote(int32_t iterDepth, InnerEmitter em return false; ptrdiff_t end = offset(); if (start != end) - return tryNoteList.append(JSTRY_ITERCLOSE, iterDepth, start, end); + return tryNoteList.append(JSTRY_DESTRUCTURING_ITERCLOSE, iterDepth, start, end); return true; } @@ -5057,6 +5057,9 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav // Here's pseudo code for |let [a, b, , c=y, ...d] = x;| // + // Lines that are annotated "covered by trynote" mean that upon throwing + // an exception, IteratorClose is called on iter only if done is false. + // // let x, y; // let a, b, c, d; // let iter, lref, result, done, value; // stack values @@ -5064,7 +5067,7 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav // iter = x[Symbol.iterator](); // // // ==== emitted by loop for a ==== - // lref = GetReference(a); + // lref = GetReference(a); // covered by trynote // // result = iter.next(); // done = result.done; @@ -5074,10 +5077,10 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav // else // value = result.value; // - // SetOrInitialize(lref, value); + // SetOrInitialize(lref, value); // covered by trynote // // // ==== emitted by loop for b ==== - // lref = GetReference(b); + // lref = GetReference(b); // covered by trynote // // if (done) { // value = undefined; @@ -5090,7 +5093,7 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav // value = result.value; // } // - // SetOrInitialize(lref, value); + // SetOrInitialize(lref, value); // covered by trynote // // // ==== emitted by loop for elision ==== // if (done) { @@ -5105,7 +5108,7 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav // } // // // ==== emitted by loop for c ==== - // lref = GetReference(c); + // lref = GetReference(c); // covered by trynote // // if (done) { // value = undefined; @@ -5119,19 +5122,23 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav // } // // if (value === undefined) - // value = y; + // value = y; // covered by trynote // - // SetOrInitialize(lref, value); + // SetOrInitialize(lref, value); // covered by trynote // // // ==== emitted by loop for d ==== - // lref = GetReference(d); + // lref = GetReference(d); // covered by trynote // // if (done) // value = []; // else // value = [...iter]; // - // SetOrInitialize(lref, value); + // SetOrInitialize(lref, value); // covered by trynote + // + // // === emitted after loop === + // if (!done) + // IteratorClose(iter); // Use an iterator to destructure the RHS, instead of index lookup. We // must leave the *original* value on the stack. @@ -5140,25 +5147,61 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav if (!emitIterator()) // ... OBJ ITER return false; + // For an empty pattern [], call IteratorClose unconditionally. Nothing + // else needs to be done. + if (!pattern->pn_head) + return emitIteratorClose(); // ... OBJ + + // Push an initial FALSE value for DONE. + if (!emit1(JSOP_FALSE)) // ... OBJ ITER FALSE + return false; + + // JSTRY_DESTRUCTURING_ITERCLOSE expects the iterator and the done value + // to be the second to top and the top of the stack, respectively. + // IteratorClose is called upon exception only if done is false. + int32_t tryNoteDepth = stackDepth; + for (ParseNode* member = pattern->pn_head; member; member = member->pn_next) { - bool isHead = member == pattern->pn_head; - bool hasNext = !!member->pn_next; + bool isFirst = member == pattern->pn_head; + DebugOnly hasNext = !!member->pn_next; - if (member->isKind(PNK_SPREAD)) { - size_t emitted = 0; - if (!emitDestructuringLHSRef(member, &emitted)) // ... OBJ ITER ?DONE *LREF + size_t emitted = 0; + + // Spec requires LHS reference to be evaluated first. + ParseNode* lhsPattern = member; + if (lhsPattern->isKind(PNK_ASSIGN)) + lhsPattern = lhsPattern->pn_left; + + bool isElision = lhsPattern->isKind(PNK_ELISION); + if (!isElision) { + auto emitLHSRef = [lhsPattern, &emitted](BytecodeEmitter* bce) { + return bce->emitDestructuringLHSRef(lhsPattern, &emitted); // ... OBJ ITER DONE *LREF + }; + if (!wrapWithDestructuringIteratorCloseTryNote(tryNoteDepth, emitLHSRef)) + return false; + } + + // Pick the DONE value to the top of the stack. + if (emitted) { + if (!emit2(JSOP_PICK, emitted)) // ... OBJ ITER *LREF DONE return false; + } + if (isFirst) { + // If this element is the first, DONE is always FALSE, so pop it. + // + // Non-first elements should emit if-else depending on the + // member pattern, below. + if (!emit1(JSOP_POP)) // ... OBJ ITER *LREF + return false; + } + + if (member->isKind(PNK_SPREAD)) { IfThenElseEmitter ifThenElse(this); - if (!isHead) { + if (!isFirst) { // If spread is not the first element of the pattern, // iterator can already be completed. - // ... OBJ ITER DONE *LREF - if (emitted) { - if (!emit2(JSOP_PICK, emitted)) // ... OBJ ITER *LREF DONE - return false; - } - + // ... OBJ ITER *LREF DONE if (!ifThenElse.emitIfElse()) // ... OBJ ITER *LREF return false; @@ -5181,13 +5224,22 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav if (!emit1(JSOP_POP)) // ... OBJ ITER *LREF ARRAY return false; - if (!isHead) { + if (!isFirst) { if (!ifThenElse.emitEnd()) return false; MOZ_ASSERT(ifThenElse.pushed() == 1); } - if (!emitSetOrInitializeDestructuring(member, flav)) // ... OBJ ITER + // At this point the iterator is done. Unpick a TRUE value for DONE above ITER. + if (!emit1(JSOP_TRUE)) // ... OBJ ITER *LREF ARRAY TRUE + return false; + if (!emit2(JSOP_UNPICK, emitted + 1)) // ... OBJ ITER TRUE *LREF ARRAY + return false; + + auto emitAssignment = [member, flav](BytecodeEmitter* bce) { + return bce->emitSetOrInitializeDestructuring(member, flav); // ... OBJ ITER TRUE + }; + if (!wrapWithDestructuringIteratorCloseTryNote(tryNoteDepth, emitAssignment)) return false; MOZ_ASSERT(!hasNext); @@ -5195,63 +5247,30 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav } ParseNode* pndefault = nullptr; - ParseNode* subpattern = member; - if (subpattern->isKind(PNK_ASSIGN)) { - pndefault = subpattern->pn_right; - subpattern = subpattern->pn_left; - } - - bool isElision = subpattern->isKind(PNK_ELISION); - - MOZ_ASSERT(!subpattern->isKind(PNK_SPREAD)); + if (member->isKind(PNK_ASSIGN)) + pndefault = member->pn_right; - size_t emitted = 0; - if (!isElision) { - if (!emitDestructuringLHSRef(subpattern, &emitted)) // ... OBJ ITER ?DONE *LREF - return false; - } + MOZ_ASSERT(!member->isKind(PNK_SPREAD)); IfThenElseEmitter ifAlreadyDone(this); - if (!isHead) { - // If this element is not the first element of the pattern, - // iterator can already be completed. - // ... OBJ ITER DONE *LREF - if (emitted) { - if (hasNext) { - if (!emitDupAt(emitted)) // ... OBJ ITER DONE *LREF DONE - return false; - } else { - if (!emit2(JSOP_PICK, emitted)) // ... OBJ ITER *LREF DONE - return false; - } - } else { - if (hasNext) { - // The position of LREF in the following stack comment - // isn't accurate for the operation, but it's equivalent - // since LREF is nothing - if (!emit1(JSOP_DUP)) // ... OBJ ITER DONE *LREF DONE - return false; - } - } - if (!ifAlreadyDone.emitIfElse()) // ... OBJ ITER ?DONE *LREF + if (!isFirst) { + // ... OBJ ITER *LREF DONE + if (!ifAlreadyDone.emitIfElse()) // ... OBJ ITER *LREF return false; - if (!emit1(JSOP_UNDEFINED)) // ... OBJ ITER ?DONE *LREF UNDEF + if (!emit1(JSOP_UNDEFINED)) // ... OBJ ITER *LREF UNDEF return false; - if (!emit1(JSOP_NOP_DESTRUCTURING)) // ... OBJ ITER ?DONE *LREF UNDEF + if (!emit1(JSOP_NOP_DESTRUCTURING)) // ... OBJ ITER *LREF UNDEF return false; - if (!ifAlreadyDone.emitElse()) // ... OBJ ITER ?DONE *LREF + // The iterator is done. Unpick a TRUE value for DONE above ITER. + if (!emit1(JSOP_TRUE)) // ... OBJ ITER *LREF UNDEF TRUE + return false; + if (!emit2(JSOP_UNPICK, emitted + 1)) // ... OBJ ITER TRUE *LREF UNDEF return false; - if (hasNext) { - if (emitted) { - if (!emit2(JSOP_PICK, emitted)) // ... OBJ ITER *LREF DONE - return false; - } - if (!emit1(JSOP_POP)) // ... OBJ ITER *LREF - return false; - } + if (!ifAlreadyDone.emitElse()) // ... OBJ ITER *LREF + return false; } if (emitted) { @@ -5268,59 +5287,74 @@ BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlav if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ... OBJ ITER *LREF RESULT DONE return false; - if (hasNext) { - if (!emit1(JSOP_DUP)) // ... OBJ ITER *LREF RESULT DONE DONE - return false; - if (!emit2(JSOP_UNPICK, emitted + 2)) // ... OBJ ITER DONE *LREF RESULT DONE - return false; - } + if (!emit1(JSOP_DUP)) // ... OBJ ITER *LREF RESULT DONE DONE + return false; + if (!emit2(JSOP_UNPICK, emitted + 2)) // ... OBJ ITER DONE *LREF RESULT DONE + return false; IfThenElseEmitter ifDone(this); - if (!ifDone.emitIfElse()) // ... OBJ ITER ?DONE *LREF RESULT + if (!ifDone.emitIfElse()) // ... OBJ ITER DONE *LREF RESULT return false; - if (!emit1(JSOP_POP)) // ... OBJ ITER ?DONE *LREF + if (!emit1(JSOP_POP)) // ... OBJ ITER DONE *LREF return false; - if (!emit1(JSOP_UNDEFINED)) // ... OBJ ITER ?DONE *LREF UNDEF + if (!emit1(JSOP_UNDEFINED)) // ... OBJ ITER DONE *LREF UNDEF return false; - if (!emit1(JSOP_NOP_DESTRUCTURING)) // ... OBJ ITER ?DONE *LREF UNDEF + if (!emit1(JSOP_NOP_DESTRUCTURING)) // ... OBJ ITER DONE *LREF UNDEF return false; - if (!ifDone.emitElse()) // ... OBJ ITER ?DONE *LREF RESULT + if (!ifDone.emitElse()) // ... OBJ ITER DONE *LREF RESULT return false; - if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ... OBJ ITER ?DONE *LREF VALUE + if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ... OBJ ITER DONE *LREF VALUE return false; if (!ifDone.emitEnd()) return false; MOZ_ASSERT(ifDone.pushed() == 0); - if (!isHead) { + if (!isFirst) { if (!ifAlreadyDone.emitEnd()) return false; - MOZ_ASSERT(ifAlreadyDone.pushed() == 1); + MOZ_ASSERT(ifAlreadyDone.pushed() == 2); } if (pndefault) { - if (!emitDefault(pndefault, subpattern)) // ... OBJ ITER ?DONE *LREF VALUE + auto emitDefault = [pndefault, lhsPattern](BytecodeEmitter* bce) { + return bce->emitDefault(pndefault, lhsPattern); // ... OBJ ITER DONE *LREF VALUE + }; + + if (!wrapWithDestructuringIteratorCloseTryNote(tryNoteDepth, emitDefault)) return false; } if (!isElision) { - if (!emitSetOrInitializeDestructuring(subpattern, - flav)) // ... OBJ ITER ?DONE - { + auto emitAssignment = [lhsPattern, flav](BytecodeEmitter* bce) { + return bce->emitSetOrInitializeDestructuring(lhsPattern, flav); // ... OBJ ITER DONE + }; + + if (!wrapWithDestructuringIteratorCloseTryNote(tryNoteDepth, emitAssignment)) return false; - } } else { - if (!emit1(JSOP_POP)) // ... OBJ ITER ?DONE + if (!emit1(JSOP_POP)) // ... OBJ ITER DONE return false; } } + // The last DONE value is on top of the stack. If not DONE, call + // IteratorClose. + // ... OBJ ITER DONE + IfThenElseEmitter ifDone(this); + if (!ifDone.emitIfElse()) // ... OBJ ITER + return false; if (!emit1(JSOP_POP)) // ... OBJ return false; + if (!ifDone.emitElse()) // ... OBJ ITER + return false; + if (!emitIteratorClose()) // ... OBJ + return false; + if (!ifDone.emitEnd()) + return false; return true; } diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 156abedbe..78eb3510c 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -684,7 +684,8 @@ struct MOZ_STACK_CLASS BytecodeEmitter bool allowSelfHosted = false); template - MOZ_MUST_USE bool wrapWithIteratorCloseTryNote(int32_t iterDepth, InnerEmitter emitter); + MOZ_MUST_USE bool wrapWithDestructuringIteratorCloseTryNote(int32_t iterDepth, + InnerEmitter emitter); // Check if the value on top of the stack is "undefined". If so, replace // that value on the stack with the value defined by |defaultExpr|. diff --git a/js/src/jit/JitFrames.cpp b/js/src/jit/JitFrames.cpp index d6c6fc4c9..30bbd0b9b 100644 --- a/js/src/jit/JitFrames.cpp +++ b/js/src/jit/JitFrames.cpp @@ -330,28 +330,44 @@ NumArgAndLocalSlots(const InlineFrameIterator& frame) static void CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, JSTryNote* tn) { - MOZ_ASSERT(tn->kind == JSTRY_FOR_IN || tn->kind == JSTRY_ITERCLOSE); - MOZ_ASSERT(tn->stackDepth > 0); + MOZ_ASSERT(tn->kind == JSTRY_FOR_IN || + tn->kind == JSTRY_ITERCLOSE || + tn->kind == JSTRY_DESTRUCTURING_ITERCLOSE); + + bool isDestructuring = tn->kind == JSTRY_DESTRUCTURING_ITERCLOSE; + MOZ_ASSERT_IF(!isDestructuring, tn->stackDepth > 0); + MOZ_ASSERT_IF(isDestructuring, tn->stackDepth > 1); SnapshotIterator si = frame.snapshotIterator(); - // Skip stack slots until we reach the iterator object. + // Skip stack slots until we reach the iterator object on the stack. For + // the destructuring case, we also need to get the "done" value. uint32_t stackSlot = tn->stackDepth; - uint32_t skipSlots = NumArgAndLocalSlots(frame) + stackSlot - 1; + uint32_t adjust = isDestructuring ? 2 : 1; + uint32_t skipSlots = NumArgAndLocalSlots(frame) + stackSlot - adjust; for (unsigned i = 0; i < skipSlots; i++) si.skip(); Value v = si.read(); - RootedObject obj(cx, &v.toObject()); + RootedObject iterObject(cx, &v.toObject()); + + if (isDestructuring) { + Value v = si.read(); + MOZ_ASSERT(v.isBoolean()); + // Do not call IteratorClose if the destructuring iterator is already + // done. + if (v.isTrue()) + return; + } if (cx->isExceptionPending()) { if (tn->kind == JSTRY_FOR_IN) - UnwindIteratorForException(cx, obj); + UnwindIteratorForException(cx, iterObject); else - IteratorCloseForException(cx, obj); + IteratorCloseForException(cx, iterObject); } else { - UnwindIteratorForUncatchableException(cx, obj); + UnwindIteratorForUncatchableException(cx, iterObject); } } @@ -426,14 +442,12 @@ HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame, ResumeFromEx switch (tn->kind) { case JSTRY_FOR_IN: - case JSTRY_ITERCLOSE: { + case JSTRY_ITERCLOSE: + case JSTRY_DESTRUCTURING_ITERCLOSE: MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER); - MOZ_ASSERT(tn->stackDepth > 0); - CloseLiveIteratorIon(cx, frame, tn); break; - } case JSTRY_FOR_OF: case JSTRY_LOOP: @@ -607,28 +621,52 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen return true; } - case JSTRY_FOR_IN: - case JSTRY_ITERCLOSE: { + case JSTRY_FOR_IN: { uint8_t* framePointer; uint8_t* stackPointer; BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer, &stackPointer); - Value iterValue(*(reinterpret_cast(stackPointer))); + Value iterValue(*reinterpret_cast(stackPointer)); RootedObject iterObject(cx, &iterValue.toObject()); - bool ok; - if (tn->kind == JSTRY_FOR_IN) - ok = UnwindIteratorForException(cx, iterObject); - else - ok = IteratorCloseForException(cx, iterObject); - if (!ok) { + if (!UnwindIteratorForException(cx, iterObject)) { // See comment in the JSTRY_FOR_IN case in Interpreter.cpp's // ProcessTryNotes. SettleOnTryNote(cx, tn, frame, ei, rfe, pc); - MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, **pc == JSOP_ENDITER); + MOZ_ASSERT(**pc == JSOP_ENDITER); + return false; + } + break; + } + + case JSTRY_ITERCLOSE: { + uint8_t* framePointer; + uint8_t* stackPointer; + BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer, &stackPointer); + Value iterValue(*reinterpret_cast(stackPointer)); + RootedObject iterObject(cx, &iterValue.toObject()); + if (!IteratorCloseForException(cx, iterObject)) { + SettleOnTryNote(cx, tn, frame, ei, rfe, pc); return false; } break; } + case JSTRY_DESTRUCTURING_ITERCLOSE: { + uint8_t* framePointer; + uint8_t* stackPointer; + BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer, &stackPointer); + Value doneValue(*(reinterpret_cast(stackPointer))); + MOZ_ASSERT(doneValue.isBoolean()); + if (doneValue.isFalse()) { + Value iterValue(*(reinterpret_cast(stackPointer) + 1)); + RootedObject iterObject(cx, &iterValue.toObject()); + if (!IteratorCloseForException(cx, iterObject)) { + SettleOnTryNote(cx, tn, frame, ei, rfe, pc); + return false; + } + } + break; + } + case JSTRY_FOR_OF: case JSTRY_LOOP: break; diff --git a/js/src/jsscript.h b/js/src/jsscript.h index b48d48686..8bba3ec39 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -85,7 +85,8 @@ enum JSTryNoteKind { JSTRY_FOR_IN, JSTRY_FOR_OF, JSTRY_LOOP, - JSTRY_ITERCLOSE + JSTRY_ITERCLOSE, + JSTRY_DESTRUCTURING_ITERCLOSE }; /* diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 294dd935d..6fe01de22 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -2585,12 +2585,28 @@ Notes(JSContext* cx, unsigned argc, Value* vp) return true; } -JS_STATIC_ASSERT(JSTRY_CATCH == 0); -JS_STATIC_ASSERT(JSTRY_FINALLY == 1); -JS_STATIC_ASSERT(JSTRY_FOR_IN == 2); +static const char* +TryNoteName(JSTryNoteKind kind) +{ + switch (kind) { + case JSTRY_CATCH: + return "catch"; + case JSTRY_FINALLY: + return "finally"; + case JSTRY_FOR_IN: + return "for-in"; + case JSTRY_FOR_OF: + return "for-of"; + case JSTRY_LOOP: + return "loop"; + case JSTRY_ITERCLOSE: + return "iterclose"; + case JSTRY_DESTRUCTURING_ITERCLOSE: + return "dstr-iterclose"; + } -static const char* const TryNoteNames[] = { "catch", "finally", "for-in", "for-of", "loop", - "iterclose" }; + MOZ_CRASH("Bad JSTryNoteKind"); +} static MOZ_MUST_USE bool TryNotes(JSContext* cx, HandleScript script, Sprinter* sp) @@ -2598,17 +2614,16 @@ TryNotes(JSContext* cx, HandleScript script, Sprinter* sp) if (!script->hasTrynotes()) return true; - if (sp->put("\nException table:\nkind stack start end\n") < 0) + if (sp->put("\nException table:\nkind stack start end\n") < 0) return false; JSTryNote* tn = script->trynotes()->vector; JSTryNote* tnlimit = tn + script->trynotes()->length; do { - MOZ_ASSERT(tn->kind < ArrayLength(TryNoteNames)); uint32_t startOff = script->pcToOffset(script->main()) + tn->start; - if (!sp->jsprintf(" %-9s %6u %8u %8u\n", - TryNoteNames[tn->kind], tn->stackDepth, - startOff, startOff + tn->length)) + if (!sp->jsprintf(" %-14s %6u %8u %8u\n", + TryNoteName(static_cast(tn->kind)), + tn->stackDepth, startOff, startOff + tn->length)) { return false; } diff --git a/js/src/tests/ecma_6/Destructuring/array-iterator-close.js b/js/src/tests/ecma_6/Destructuring/array-iterator-close.js new file mode 100644 index 000000000..f7805540d --- /dev/null +++ b/js/src/tests/ecma_6/Destructuring/array-iterator-close.js @@ -0,0 +1,77 @@ +// Tests that IteratorClose is called in array destructuring patterns. + +function test() { + var returnCalled = 0; + var returnCalledExpected = 0; + var iterable = {}; + + // empty [] calls IteratorClose regardless of "done" on the result. + iterable[Symbol.iterator] = makeIterator({ + next: function() { + return { done: true }; + }, + ret: function() { + returnCalled++; + return {}; + } + }); + var [] = iterable; + assertEq(returnCalled, ++returnCalledExpected); + + iterable[Symbol.iterator] = makeIterator({ + ret: function() { + returnCalled++; + return {}; + } + }); + var [] = iterable; + assertEq(returnCalled, ++returnCalledExpected); + + // Non-empty destructuring calls IteratorClose if iterator is not done by + // the end of destructuring. + var [a,b] = iterable; + assertEq(returnCalled, ++returnCalledExpected); + var [c,] = iterable; + assertEq(returnCalled, ++returnCalledExpected); + + // throw in lhs ref calls IteratorClose + function throwlhs() { + throw "in lhs"; + } + assertThrowsValue(function() { + 0, [...{}[throwlhs()]] = iterable; + }, "in lhs"); + assertEq(returnCalled, ++returnCalledExpected); + + // throw in iter.next doesn't call IteratorClose + iterable[Symbol.iterator] = makeIterator({ + next: function() { + throw "in next"; + }, + ret: function() { + returnCalled++; + return {}; + } + }); + assertThrowsValue(function() { + var [d] = iterable; + }, "in next"); + assertEq(returnCalled, returnCalledExpected); + + // "return" must return an Object. + iterable[Symbol.iterator] = makeIterator({ + ret: function() { + returnCalled++; + return 42; + } + }); + assertThrowsInstanceOf(function() { + var [] = iterable; + }, TypeError); + assertEq(returnCalled, ++returnCalledExpected); +} + +test(); + +if (typeof reportCompare === "function") + reportCompare(0, 0); diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 489d0a155..923c824ce 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1171,19 +1171,13 @@ ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs) SettleOnTryNote(cx, tn, ei, regs); return FinallyContinuation; - case JSTRY_FOR_IN: - case JSTRY_ITERCLOSE: { + case JSTRY_FOR_IN: { /* This is similar to JSOP_ENDITER in the interpreter loop. */ DebugOnly pc = regs.fp()->script()->main() + tn->start + tn->length; - MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, JSOp(*pc) == JSOP_ENDITER); + MOZ_ASSERT(JSOp(*pc) == JSOP_ENDITER); Value* sp = regs.spForStackDepth(tn->stackDepth); RootedObject obj(cx, &sp[-1].toObject()); - bool ok; - if (tn->kind == JSTRY_FOR_IN) - ok = UnwindIteratorForException(cx, obj); - else - ok = IteratorCloseForException(cx, obj); - if (!ok) { + if (!UnwindIteratorForException(cx, obj)) { // We should only settle on the note only if // UnwindIteratorForException itself threw, as // onExceptionUnwind should be called anew with the new @@ -1195,6 +1189,33 @@ ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs) break; } + case JSTRY_ITERCLOSE: { + // The iterator object is at the top of the stack. + Value* sp = regs.spForStackDepth(tn->stackDepth); + RootedObject iterObject(cx, &sp[-1].toObject()); + if (!IteratorCloseForException(cx, iterObject)) { + SettleOnTryNote(cx, tn, ei, regs); + return ErrorReturnContinuation; + } + break; + } + + case JSTRY_DESTRUCTURING_ITERCLOSE: { + // Whether the destructuring iterator is done is at the top of the + // stack. The iterator object is second from the top. + MOZ_ASSERT(tn->stackDepth > 1); + Value* sp = regs.spForStackDepth(tn->stackDepth); + MOZ_ASSERT(sp[-1].isBoolean()); + if (sp[-1].isFalse()) { + RootedObject iterObject(cx, &sp[-2].toObject()); + if (!IteratorCloseForException(cx, iterObject)) { + SettleOnTryNote(cx, tn, ei, regs); + return ErrorReturnContinuation; + } + } + break; + } + case JSTRY_FOR_OF: case JSTRY_LOOP: break; -- cgit v1.2.3