implement yield* operator

RESOLVED FIXED in mozilla27

Status

()

RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: dherman, Assigned: wingo)

Tracking

({dev-doc-complete})

unspecified
mozilla27
x86
macOS
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox 27+)

Details

(Whiteboard: [DocArea=JS], URL)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

8 years ago
The yield* operator delegates to a subgenerator and produces the result that was thrown from the subgenerator. The semantics is described via a desugaring at the attached URL.

We could implement this via a desugaring, or try to build in native support for compound generators. I'm not sure what the best plan of attack is.

Dave
(Assignee)

Comment 1

6 years ago
I'm on it; I think via adding a flag to PNK_YIELD, and doing the yield* logic in BytecodeEmitter.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 907738
Keywords: dev-doc-needed
(Assignee)

Comment 3

6 years ago
Posted patch Implemement yield* (obsolete) — Splinter Review
(Assignee)

Updated

6 years ago
Assignee: general → wingo
(Assignee)

Updated

6 years ago
Attachment #797840 - Flags: review?(jorendorff)
(Assignee)

Updated

6 years ago
Attachment #797840 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 4

6 years ago
The one thing this patch doesn't do is call @@iterator on the incoming object.  That's pretty easy to add but there are a couple of problems:

  1. @@iterator isn't specified yet, AFAIK

  2. There is ".iterator()" currently, but that uses the StopIteration protocol.

So for now I left it off.  See bugs 907077 and 907717.
André thinks, and I agree, that there's a bug in the runtime semantics in the current draft spec:
  https://bugs.ecmascript.org/show_bug.cgi?id=1633

Taking a look, but I don't think I'll finish today :(
Comment on attachment 797840 [details] [diff] [review]
Implemement yield*

Review of attachment 797840 [details] [diff] [review]:
-----------------------------------------------------------------

I'll review more tomorrow.

The changes to iteration.js are great, but this may require more tests...

We decided to migrate for-of to "@@iterator" with the new protocol, and retire the "iterator" stuff. Can we start doing that in this bug?

::: js/src/frontend/BytecodeEmitter.cpp
@@ +4926,5 @@
> +
> +    // Catch location.
> +    // result = iter.throw(exception)                            // ITER
> +    bce->stackDepth = (unsigned) depth;
> +    if (Emit1(cx, bce, JSOP_EXCEPTION) < 0)                      // EXCEPTION ITER

Generally the convention we use on comments like this is that the top of the stack is to the right, so this would be
    // ITER EXCEPTION
and so on.
Comment on attachment 797840 [details] [diff] [review]
Implemement yield*

Review of attachment 797840 [details] [diff] [review]:
-----------------------------------------------------------------

All right, one more comment today:

::: js/src/frontend/BytecodeEmitter.cpp
@@ +4935,5 @@
> +        return false;
> +    if (Emit1(cx, bce, JSOP_DUP) < 0)                            // ITER ITER ITER EXCEPTION
> +        return false;
> +    if (!EmitAtomOp(cx, cx->names().throw_, JSOP_CALLPROP, bce)) // THROW ITER ITER EXCEPTION
> +        return false;

The spec requires testing `"throw" in iter` before proceeding here.

Looks like we need tests for what happens if you
- delete Generator.prototype.throw
- or replace it with a different function
- or define a "throw" method on a generator's .prototype object
- or on an actual generator-iterator instance

(Of course ultimately we also want this to work with random other iterator objects that *normally* don't have a throw method, but that can wait for another bug.)
(Assignee)

Comment 8

6 years ago
(In reply to Jason Orendorff [:jorendorff] from comment #7)
> ::: js/src/frontend/BytecodeEmitter.cpp
> The spec requires testing `"throw" in iter` before proceeding here.

Will fix.

> Looks like we need tests for what happens if you
> - delete Generator.prototype.throw

Interesting question: what are the flags on this property?  For legacy generators they are read-only and not configurable.  Star generators are currently the same, so I should probably relax them; however, http://people.mozilla.org/~jorendorff/es6-draft.html#sec-15.19.4 does not seem to indicate what the flags should be.

For now I'll make them read-only, but configurable, I guess.
The introduction to clause 15 specifies this:
> Every other data property described in this clause has the attributes { [[Writable]]: true,
> [[Enumerable]]: false, [[Configurable]]: true } unless otherwise specified.
(Assignee)

Comment 10

6 years ago
Posted patch Implement yield* v2 (obsolete) — Splinter Review
Attached updated patch addressing nits so far.  I didn't mark the old one as obsolete as the review isn't finished yet.
(Assignee)

Comment 11

6 years ago
Regarding @@iterator, I think I would use a separate bug for that, if that's fine with you.  I'll take a look.
(In reply to Andy Wingo from comment #11)
> Regarding @@iterator, I think I would use a separate bug for that, if that's
> fine with you.  I'll take a look.

Absolutely.
Attachment #797840 - Attachment is obsolete: true
Attachment #797840 - Flags: review?(jwalden+bmo)
Attachment #797840 - Flags: review?(jorendorff)
Attachment #799413 - Flags: review?(jorendorff)
Attachment #799413 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 13

6 years ago
bog 907077 for for-of and @@iterator
(Assignee)

Comment 14

6 years ago
bug 907077, rather
Comment on attachment 799413 [details] [diff] [review]
Implement yield* v2

Very nice patch, very readable.

If you think it would save any code to emit all this by last-minute
desugaring (that is, creating parse nodes in the emitter), feel free to
try. I don't think it will, though.

In BytecodeEmitter.cpp:
>+    if (Emit1(cx, bce, JSOP_IN) < 0)                             // EXCEPTION ITER THROW?
>+        return false;
>+    ptrdiff_t checkThrow = EmitJump(cx, bce, JSOP_IFNE, 0);      // if (THROW?) goto delegate
>+    if (checkThrow < 0)
>+        return false;
>+    if (Emit1(cx, bce, JSOP_POP) < 0)                            // EXCEPTION
>+        return false;
>+    if (Emit1(cx, bce, JSOP_THROW) < 0)                          // throw EXCEPTION
>+        return false;

If I understand your style right, I think the two comments
"if (THROW?) goto delegate" and "throw EXCEPTION" belong on the left,
not the right, and the "if (THROW?)" comment should be replaced
with a snapshot of the operand stack. But not a big deal either way.

>+    if (!EmitAtomOp(cx, cx->names().throw_, JSOP_CALLPROP, bce)) // EXCEPTION ITER ITER THROW
>+        return false;
>+    if (Emit1(cx, bce, JSOP_SWAP) < 0)                           // EXCEPTION ITER THROW ITER
>+        return false;
>+    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER EXCEPTION
>+        return false;
>+    if (Emit3(cx, bce, JSOP_CALL, ARGC_HI(1), ARGC_LO(1)) < 0)   // ITER RESULT
>+        return false;

For consistency's sake, please emit JSOP_NOTEARG after JSOP_PICK here
(even though Ion doesn't support generators and may not support them
anytime soon).

>+    if (!EmitAtomOp(cx, cx->names().next, JSOP_CALLPROP, bce))   // RECEIVED ITER ITER THROW
>+        return false;
>+    if (Emit1(cx, bce, JSOP_SWAP) < 0)                           // RECEIVED ITER THROW ITER
>+        return false;
>+    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER RECEIVED
>+        return false;

In these three comments, THROW should be NEXT.

Add a JSOP_NOTEARG here too.

In jsast.tbl:
> ASTDEF(AST_YIELD_EXPR,            "YieldExpression",                "yieldExpression")
>+ASTDEF(AST_YIELD_STAR_EXPR,       "YieldStarExpression",            "yieldStarExpression")

Instead please add a boolean parameter named "delegate" to
YieldExpressions. (That's what Esprima does, and I think it's more like
the way the grammar is written in the spec.)

In jsiter.cpp:
> #define JSPROP_ROPERM   (JSPROP_READONLY | JSPROP_PERMANENT)
> #define JS_METHOD(name, T, impl, len, perms) JS_FN(name, (NativeMethod<T,impl>), len, perms)

While we're here, please rename `perms` to `attrs`.

> static const JSFunctionSpec star_generator_methods[] = {
>     JS_FN("iterator", iterator_iterator, 0, 0),
>-    JS_METHOD("next", StarGeneratorObject, star_generator_next, 1, JSPROP_ROPERM),
>-    JS_METHOD("throw", StarGeneratorObject, star_generator_throw, 1, JSPROP_ROPERM),
>+    JS_METHOD("next", StarGeneratorObject, star_generator_next, 1, JSPROP_READONLY),
>+    JS_METHOD("throw", StarGeneratorObject, star_generator_throw, 1, JSPROP_READONLY),

Good fix. These shouldn't be readonly either, though. Per the next-to-last
paragraph of clause 17, attrs should be 0.

In js/src/tests/ecma_6/Generators/iteration.js:
>+    // What would be an uncaught delegated throw, but with a monkeypatched iterator.
>[...]
>+    Object.defineProperty(inner, 'throw', { value: function(e) { return e*2; } });

You can just
    inner.throw = function(e) { ... };

If that doesn't work, it's a bug somewhere.

Good tests generally. Please also test:

- monkeypunching inner.next, as you've done for .throw

- more generally, yield* with an operand that is just an iterable, like an array,
  and not necessarily a generator (you can check in these commented-out if they
  are not going to work until bug 907077 lands)

- that calling inner.next() directly while outer exists works correctly
  (that is, outer.next() isn't broken but it does observe that inner adanced)

- that delegate(delegate(delegate(delegate(...innermost...)))) works
  for long chains

- that each yield* checks .done but not .value. You can check by making
  innermost.next() return an iterator-result object with getters for
  .done and .value that logs calls to it:

    var log = "";
    ...

        return {
            _done: false,
            _value: 42,
            get done() { log += "d"; return this._done; }
            get value() { log += "v"; return this._value; }
        }

    ...
    assertEq(log, "ddddddddv");

- the operand to yield* can be a Proxy

- that multiple delegate(inner) objects can exist for the same inner
  generator, and you can interleave calls to them

- that yield* works in a loop (so far I think all the tests only
  have each yield* expression executing once per generator instance)

- multiple `yield` and `yield*` expressions in a single generator

- that `yield* EXPR;` is a valid statement, in a generator

- that things like `f(yield* g())` and `(yield* f()) + (yield* g())`
  work (the syntactic context of yield* expressions in these tests has
  been limited to `return _` which is awfully narrow.)

- that an exception that occurs while getting .next, .throw, .done, or
  .value accessor in a yield* expression does *not* cause inner.throw()
  to be called (in other words, testing the boundaries of the implicit
  try-block around the yield)

- that the number of arguments actually passed to .next() is always 1,
  including for the first call from a given yield*.

Some of those scenarios are not things that would likely break under the
current implementation approach, but let's make sure future
optimizations continue to get this stuff right.

Also, and I'm sorry to hit you with this now:

- interactions between generators and the Debugger object. You can add
  those tests in js/src/jit-test/tests/debug, and we can chat about how
  the Debugger object works and how to test it. In particular I want to
  test:

    - that the stack observed by the debugger is correct when
      generators, including delegating generators, are on the stack

    - that frame.eval() works in generator frames, including
      non-top generator frames

    - that frame.eval("yield 0") and similar are correctly
      rejected as SyntaxErrors

    - that breakpoints work in generators

    - that the debugger statement works in generators

    - that the onPop hook works for generator frames -- I don't remember
      exactly what it's supposed to do, but at least, it mustn't crash
Comment on attachment 799413 [details] [diff] [review]
Implement yield* v2

r=me with the remaining nits addressed, and the requested tests.

No need for another review pass, imho; it's up to your judgment.
Attachment #799413 - Flags: review?(jorendorff) → review+
Comment on attachment 799413 [details] [diff] [review]
Implement yield* v2

Review of attachment 799413 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/BytecodeEmitter.cpp
@@ +4909,5 @@
> +        return false;
> +
> +    // Try prologue.                                             // ITER RESULT
> +    StmtInfoBCE stmtInfo(cx);
> +    PushStatementBCE(bce, &stmtInfo, STMT_TRY, bce->offset());

I am not going to claim knowledge of exactly how try-blocks are supposed to be structured at the bytecode level.  I'm a little surprised it's fine to goto into the middle, ish, of a try block (that is, after the try).  You're sure this is right?

I am also not going to claim I understand, or attempted to understand, the exact bytecode differences, offsets, source notes, etc. involved in constructing this try-catch sequence here.

@@ +4916,5 @@
> +        return false;
> +    ptrdiff_t tryStart = bce->offset();                          // tryStart:
> +    JS_ASSERT(bce->stackDepth == depth + 1);
> +
> +    // received = %yield result

I'd remove the % and put a ; at the end, myself.

@@ +4966,5 @@
> +    if (Emit1(cx, bce, JSOP_SWAP) < 0)                           // EXCEPTION ITER THROW ITER
> +        return false;
> +    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER EXCEPTION
> +        return false;
> +    if (Emit3(cx, bce, JSOP_CALL, ARGC_HI(1), ARGC_LO(1)) < 0)   // ITER RESULT

unsigned throwArgc = 1; as suggested below

@@ +4971,5 @@
> +        return false;
> +    JS_ASSERT(bce->stackDepth == depth + 1);
> +    ptrdiff_t checkResult = -1;
> +    if (EmitBackPatchOp(cx, bce, &checkResult) < 0)              // goto checkResult
> +        return false;

I might be misreading the ES6 semantics.  But don't they say that if the result of one of the implicit yields of the generator expression throws (yieldCompletion.[[type]] is "throw"), then you call iter.throw(thrown value), propagate the exception if that throws, then proceed to return yieldCompletion -- that is, to throw the value thrown by the implicit yield?

This might be an ES6 draft spec bug, because it seems to be saying that if you have a yield* inside a generator, and you call .throw on that outer generator when it's in the middle of execution, that'll call .throw on the nested generator, then ignore a successful completion and throw the value passed in.  Please clarify with the spec peoples on the list.

@@ +4985,5 @@
> +
> +    // After the try/catch block: send the received value to the iterator.
> +    if (!BackPatch(cx, bce, initialSend, bce->code().end(), JSOP_GOTO)) // initialSend:
> +        return false;
> +    if (!BackPatch(cx, bce, subsequentSend, bce->code().end(), JSOP_GOTO)) // subsequentSend:

I don't understand why both initialSend and subsequentSend are necessary.  Shouldn't only one of them be necessary?

@@ +4989,5 @@
> +    if (!BackPatch(cx, bce, subsequentSend, bce->code().end(), JSOP_GOTO)) // subsequentSend:
> +        return false;
> +
> +    // Send location.
> +    // result = iter.send(received)                              // ITER RECEIVED

iter.next

@@ +5000,5 @@
> +    if (!EmitAtomOp(cx, cx->names().next, JSOP_CALLPROP, bce))   // RECEIVED ITER ITER THROW
> +        return false;
> +    if (Emit1(cx, bce, JSOP_SWAP) < 0)                           // RECEIVED ITER THROW ITER
> +        return false;
> +    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER RECEIVED

Maybe it's just been too long (not long enough?) since I read bytecode stacks.  But why is this named THROW and not NEXT?

@@ +5002,5 @@
> +    if (Emit1(cx, bce, JSOP_SWAP) < 0)                           // RECEIVED ITER THROW ITER
> +        return false;
> +    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER RECEIVED
> +        return false;
> +    if (Emit3(cx, bce, JSOP_CALL, ARGC_HI(1), ARGC_LO(1)) < 0)   // ITER RESULT

I might be inclined to have |unsigned nextArgc = 1;| and then use that instead of magic 1s here.

@@ +5003,5 @@
> +        return false;
> +    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER RECEIVED
> +        return false;
> +    if (Emit3(cx, bce, JSOP_CALL, ARGC_HI(1), ARGC_LO(1)) < 0)   // ITER RESULT
> +        return false;

So you're doing a call.  Is it really the case that you don't need JSOP_NOTEARGs on ITER and RECEIVED here?  I'm pretty sure you do.  I'm told that NOTEARG-ness "sticks" to pushed values even through picks and swaps.  So ITER at top needs a NOTEARG, and the initial send at top needs a NOTEARG.  And a bit more of the same for the throw-call, too.

That said, I think this only matters for JITs, so you're kind of saved right now.  But this needs a followup too.

@@ +5004,5 @@
> +    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER RECEIVED
> +        return false;
> +    if (Emit3(cx, bce, JSOP_CALL, ARGC_HI(1), ARGC_LO(1)) < 0)   // ITER RESULT
> +        return false;
> +    JS_ASSERT(bce->stackDepth == depth + 1);

IteratorNext is supposed to throw a TypeError if Type(result) is not Object.  I don't see that happening anywhere here.  This probably can be a followup, but jorendorff might see some reason why it must be done now, and can't be deferred at all.

::: js/src/frontend/Parser.cpp
@@ +4585,5 @@
>          Node exprNode = assignExpr();
>          if (!exprNode)
>              return null();
>  
> +        return handler.newUnary(kind, JSOP_YIELD, begin, exprNode);

I think you can/should pass along JSOP_NOP here -- we're trying to get rid of pn->getOp() in favor of pn->getKind(), so making op meaningless whenever possible prevents new dependencies (and there aren't any, here, because the emitter is distinguishing only based on kind) showing up.

::: js/src/jsiter.cpp
@@ +1808,5 @@
>  
>  static const JSFunctionSpec star_generator_methods[] = {
>      JS_FN("iterator", iterator_iterator, 0, 0),
> +    JS_METHOD("next", StarGeneratorObject, star_generator_next, 1, JSPROP_READONLY),
> +    JS_METHOD("throw", StarGeneratorObject, star_generator_throw, 1, JSPROP_READONLY),

Why are these non-writable?  In the latest draft, these are 25.4.1.2/3 (Generator.prototype.next/throw), right?  I don't see any notes about their being non-writable anywhere.

::: js/src/tests/ecma_6/Generators/iteration.js
@@ -53,1 @@
>              assertThrowsInstanceOf(function () { iter.next(); }, TypeError);

No comment on the tests in this file, but this file is getting pretty large for a "single" test.  It would be good to split it up a bit so that anyone trying to debug an issue here doesn't have to wade through so much code.

@@ +389,5 @@
> +    Object.defineProperty(inner, 'throw', { value: function(e) { return e*2; } });
> +    assertEq(84, outer.throw(42));
> +    assertIteratorResult(undefined, true, outer.next());
> +
> +    // What would be a caught delegated throw, but with a monkeypunched prototype.

haha, monkeypunched
Attachment #799413 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 18

6 years ago
Thanks for the review.  Will update and hit it with a try build.
(Assignee)

Comment 19

6 years ago
Posted patch Implemement yield* (obsolete) — Splinter Review
(Assignee)

Updated

6 years ago
Attachment #799413 - Attachment is obsolete: true
(Assignee)

Comment 20

6 years ago
Comment on attachment 806714 [details] [diff] [review]
Implemement yield*

Updated patch addresses comments and adds gobs of tests.  I'll look at Jeff's spec comments tomorrow morning, but I wanted to push out a try build first with this patch.  Propagating r=jorendorff and r=Waldo.
Attachment #806714 - Flags: review+
(Assignee)

Comment 21

6 years ago
Comment on attachment 806714 [details] [diff] [review]
Implemement yield*

Review of attachment 806714 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsast.tbl
@@ +40,1 @@
>  ASTDEF(AST_LET_EXPR,              "LetExpression",                  "letExpression")

looks like I have to fix this one

::: js/src/jsreflect.cpp
@@ +1248,1 @@
>  {

NB!  Here I chose to break current users of the builder interface.  I didn't need to change any tests though.
(Assignee)

Comment 23

6 years ago
Hi,

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #17)
> Comment on attachment 799413 [details] [diff] [review]
> Implement yield* v2
> ::: js/src/frontend/BytecodeEmitter.cpp
> @@ +4909,5 @@
> > +        return false;
> > +
> > +    // Try prologue.                                             // ITER RESULT
> > +    StmtInfoBCE stmtInfo(cx);
> > +    PushStatementBCE(bce, &stmtInfo, STMT_TRY, bce->offset());
> 
> I am not going to claim knowledge of exactly how try-blocks are supposed to
> be structured at the bytecode level.  I'm a little surprised it's fine to
> goto into the middle, ish, of a try block (that is, after the try).  You're
> sure this is right?

I believe so.  At runtime, there is no data structure that tracks the current try block stack, and no opcode to push a try block; instead the PC -> try mapping is computed at compile time and stored in a table.  Additionally it does pass tests.

> @@ +4916,5 @@
> > +        return false;
> > +    ptrdiff_t tryStart = bce->offset();                          // tryStart:
> > +    JS_ASSERT(bce->stackDepth == depth + 1);
> > +
> > +    // received = %yield result
> 
> I'd remove the % and put a ; at the end, myself.

I left it as it was because it doesn't do a normal re-boxing yield -- it yield result as-is, so it's some kind of more primitive yield.

> @@ +4966,5 @@
> > +    if (Emit1(cx, bce, JSOP_SWAP) < 0)                           // EXCEPTION ITER THROW ITER
> > +        return false;
> > +    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER EXCEPTION
> > +        return false;
> > +    if (Emit3(cx, bce, JSOP_CALL, ARGC_HI(1), ARGC_LO(1)) < 0)   // ITER RESULT
> 
> unsigned throwArgc = 1; as suggested below

Good idea, though that borks the indentation.  Instead I added a little EmitCall() helper that just takes argc, and that way the condition still fits on a line with the stack simulation off to the right.

> @@ +4971,5 @@
> > +        return false;
> > +    JS_ASSERT(bce->stackDepth == depth + 1);
> > +    ptrdiff_t checkResult = -1;
> > +    if (EmitBackPatchOp(cx, bce, &checkResult) < 0)              // goto checkResult
> > +        return false;
> 
> I might be misreading the ES6 semantics.  But don't they say that if the
> result of one of the implicit yields of the generator expression throws
> (yieldCompletion.[[type]] is "throw"), then you call iter.throw(thrown
> value), propagate the exception if that throws, then proceed to return
> yieldCompletion -- that is, to throw the value thrown by the implicit yield?

https://bugs.ecmascript.org/show_bug.cgi?id=1633

> @@ +4985,5 @@
> > +
> > +    // After the try/catch block: send the received value to the iterator.
> > +    if (!BackPatch(cx, bce, initialSend, bce->code().end(), JSOP_GOTO)) // initialSend:
> > +        return false;
> > +    if (!BackPatch(cx, bce, subsequentSend, bce->code().end(), JSOP_GOTO)) // subsequentSend:
> 
> I don't understand why both initialSend and subsequentSend are necessary. 
> Shouldn't only one of them be necessary?

It's one location, but two places that can reach there -- one initially and one to jump out of the try{}.  I thought it made more sense making the pseudo-label names match the names of the patch location variables; YMMV.

> @@ +5004,5 @@
> > +    if (Emit2(cx, bce, JSOP_PICK, (jsbytecode)3) < 0)            // ITER THROW ITER RECEIVED
> > +        return false;
> > +    if (Emit3(cx, bce, JSOP_CALL, ARGC_HI(1), ARGC_LO(1)) < 0)   // ITER RESULT
> > +        return false;
> > +    JS_ASSERT(bce->stackDepth == depth + 1);
> 
> IteratorNext is supposed to throw a TypeError if Type(result) is not Object.
> I don't see that happening anywhere here.  This probably can be a followup,
> but jorendorff might see some reason why it must be done now, and can't be
> deferred at all.

It's not happening; I think this is new language in the spec relative to a few months ago.  https://bugs.ecmascript.org/show_bug.cgi?id=1901.  Will watch and possibly fix in a followup.

I addressed the other nits in the patch above.  Thanks very much for the detailed review!
(Assignee)

Comment 24

6 years ago
Posted patch Implemement yield* (obsolete) — Splinter Review
(Assignee)

Updated

6 years ago
Attachment #806714 - Attachment is obsolete: true
(Assignee)

Comment 25

6 years ago
Comment on attachment 807121 [details] [diff] [review]
Implemement yield*

marking r=jorendorff, r=Waldo
Attachment #807121 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Needs rebasing against inbound.
Keywords: checkin-needed
(Assignee)

Comment 27

6 years ago
(Assignee)

Updated

6 years ago
Attachment #807121 - Attachment is obsolete: true
(Assignee)

Comment 28

6 years ago
Comment on attachment 807182 [details] [diff] [review]
Implemement yield*

Rebased on inbound; did a quick build and test and things seem to be super duper.
Attachment #807182 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Thank you kind sir :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/89406858afdf
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/89406858afdf
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
relnote-firefox: --- → ?
relnote-firefox: ? → 27+
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.