Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Fix bug in yield* desugaring.
Browse files Browse the repository at this point in the history
In one corner case, we incorrectly returned a value without first wrapping it in
an iterator result object.

R=littledan@chromium.org
BUG=v8:5057

Review-Url: https://codereview.chromium.org/2034653002
Cr-Commit-Position: refs/heads/master@{#36676}
  • Loading branch information
GeorgNeis authored and Commit bot committed Jun 2, 2016
1 parent a2fef3a commit 8154d97
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 7 deletions.
23 changes: 16 additions & 7 deletions src/parsing/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6421,12 +6421,18 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
// following code:
//
// let iteratorReturn = iterator.return;
// if (IS_NULL_OR_UNDEFINED(iteratorReturn) return |input|;
// output = %_Call(iteratorReturn, iterator|, input|);
// if (IS_NULL_OR_UNDEFINED(iteratorReturn) {
// return {value: input, done: true};
// }
// output = %_Call(iteratorReturn, iterator, input);
// if (!IS_RECEIVER(output)) %ThrowIterResultNotAnObject(output);
//
// Here, |...| denotes optional parts, depending on the presence of the
// input variable. The reason for allowing input is that BuildIteratorClose
// When the input variable is not given, the return statement becomes
// return {value: undefined, done: true};
// and %_Call has only two arguments:
// output = %_Call(iteratorReturn, iterator);
//
// The reason for allowing input is that BuildIteratorClose
// can then be reused to handle the return case in yield*.
//

Expand All @@ -6450,7 +6456,9 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
get_return = factory->NewExpressionStatement(assignment, nopos);
}

// if (IS_NULL_OR_UNDEFINED(iteratorReturn) return |input|;
// if (IS_NULL_OR_UNDEFINED(iteratorReturn) {
// return {value: input, done: true};
// }
Statement* check_return;
{
Expression* condition = factory->NewCompareOperation(
Expand All @@ -6462,13 +6470,14 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
factory->NewVariableProxy(input.FromJust()))
: factory->NewUndefinedLiteral(nopos);

Statement* return_input = factory->NewReturnStatement(value, nopos);
Statement* return_input =
factory->NewReturnStatement(BuildIteratorResult(value, true), nopos);

check_return = factory->NewIfStatement(
condition, return_input, factory->NewEmptyStatement(nopos), nopos);
}

// output = %_Call(iteratorReturn, iterator, |input|);
// output = %_Call(iteratorReturn, iterator, input);
Statement* call_return;
{
auto args = new (zone) ZoneList<Expression*>(3, zone);
Expand Down
72 changes: 72 additions & 0 deletions test/mjsunit/harmony/iterator-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -1298,3 +1298,75 @@ function* g() { yield 42; return 88 };
}, 5);
assertEquals([1], log);
}


// yield*, argument's return method is "undefined".
function TestYieldStarWithoutReturn(get_iterable) {
assertTrue(get_iterable().return == undefined);

function* g() { yield* get_iterable() }

{
let gen = g();
assertEquals({value: 1, done: false}, gen.next());
assertEquals({value: undefined, done: true}, gen.return());
}

assertEquals(42, (() => {
for (let x of g()) break;
return 42;
})());

assertEquals(42, (() => {
for (let x of g()) return 42;
})());

assertThrowsEquals(() => {
for (let x of g()) throw 42;
}, 42);
}
{
let get_iterable1 = () => [1, 2];
let get_iterable2 = function*() { yield 1; yield 2 };
get_iterable2.prototype.return = null;
TestYieldStarWithoutReturn(get_iterable1);
TestYieldStarWithoutReturn(get_iterable2);
}


// yield*, argument's return method is defined.
{
let get_iterable = function*() { yield 1; yield 2 };
const obj = {};
get_iterable.prototype.return = (...args) => obj;

function* g() { yield* get_iterable() }

{
let gen = g();
assertEquals({value: 1, done: false}, gen.next());
assertSame(obj, gen.return());
assertSame(obj, gen.return());
assertSame(obj, gen.return());
assertEquals({value: 2, done: false}, gen.next());
assertSame(obj, gen.return());
assertSame(obj, gen.return());
assertSame(obj, gen.return());
assertEquals({value: undefined, done: true}, gen.next());
assertEquals({value: undefined, done: true}, gen.return());
assertEquals({value: undefined, done: true}, gen.return());
}

assertEquals(42, (() => {
for (let x of g()) break;
return 42;
})());

assertEquals(42, (() => {
for (let x of g()) return 42;
})());

assertThrowsEquals(() => {
for (let x of g()) throw 42;
}, 42);
}

0 comments on commit 8154d97

Please sign in to comment.