[Webkit-unassigned] [Bug 166695] [ESNext] Async iteration - Implement Async Generator

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=166695

--- Comment #52 from GSkachkov <gskachkov at gmail.com> ---
Comment on attachment 305500
  --> https://bugs.webkit.org/attachment.cgi?id=305500
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305500&action=review

>> Source/JavaScriptCore/builtins/AsyncGeneratorPrototype.js:117
>> +                                        } else {
> 
> So the last thing is these handlers.
> 
> You could simplify this:
> 
> ```
> wrappedValue. at promise.@then(
>     function(result) {
>         const isDelegetedYield = generator. at asyncGeneratorSuspendReason === @AsyncGeneratorSuspendReasonDelegatedYield;
>         generator. at asyncGeneratorSuspendReason = @AsyncGeneratorSuspendReasonNone;
>         if (isDelegetedYield) {
>             const isDone = false;
>             @asyncGeneratorResolve(generator, result.value, isDone);
>         }
>         return @asyncGeneratorResumeNext(generator);
>     },
>     function(error) {
>         generator. at generatorState = @AsyncGeneratorStateCompleted; // see note below
>         @asyncGeneratorReject(generator, error);
>         @asyncGeneratorResumeNext(generator);
>     }
> ```

Yeah, it seems my patch is not clear enough, because in this asyncGeneratorResume function in resolve callback for wrappedValue, is handled case when we suspend execution control flow because of await or yield*: AsyncGeneratorSuspendReasonAwait/AsyncGeneratorSuspendReasonDelegatedYield
For suspended await we need run current function again because because we need to continue execution of function until faced with 'yield' or new await.
For value from async generator function that invoked by yield* we need to unwrap value, because it returns promise.

>> Source/JavaScriptCore/builtins/AsyncGeneratorPrototype.js:123
>> +                                        generator. at generatorState = @AsyncGeneratorStateCompleted;
> 
> This actually looks wrong to me, per the current spec. Unless the committee decides to go with the "yielded rejections don't affect control flow of the generator" thing (which is admittedly pretty popular), closing the generator should be left up to @asyncGeneratorResume().
> 
> Even if the proposal is changed, this will prevent finally blocks from being reached.
> 
> E.g.
> 
> ```js
> let reject;
> async function* foo() {
>   try {
>     yield new Promise(function(unused, rej) { reject = rej; });
>   } finally {
>     doImportantCleanupStuff(); // In this patch, this appears to be unreachable
>     return 0;
>   }
> }
> 
> foo().next().then(x => print(x), e => print(e));
> reject("Oop!");
> ```
> 
> I could be wrong about this, but I _believe_ this is broken in this patch due to the generator state change here, and I think it is supposed to work in the current spec

I see following result:

  do important staff
  returned value
  Oop!

Is this result close to spec or 'important staff' should be returned after Oop!
To test I used little bit modified function:
```
async function* foo() {
  try {
    yield new Promise(function(unused, rej) { reject = rej; });
  } finally {
    print('do important staff');
    return 'returned value';
  }
}

var f= foo()
f.next().then(({value}) => print(value), e => print(e));
reject("Oop!");
f.next().then(({value}) => print(value), e => print(e));
drainMicrotasks();
```

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170329/cbf711ef/attachment.html>