[webkit-reviews] review granted: [Bug 175240] [ESNext] Async iteration - Implement Async Generator - runtime : [Attachment 318795] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 23 07:18:40 PDT 2017


Yusuke Suzuki <utatane.tea at gmail.com> has granted GSkachkov
<gskachkov at gmail.com>'s request for review:
Bug 175240: [ESNext] Async iteration - Implement Async Generator - runtime
https://bugs.webkit.org/show_bug.cgi?id=175240

Attachment 318795: Patch

https://bugs.webkit.org/attachment.cgi?id=318795&action=review




--- Comment #20 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 318795
  --> https://bugs.webkit.org/attachment.cgi?id=318795
Patch

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

I think we need to update test262 too.

> Source/JavaScriptCore/builtins/AsyncGeneratorPrototype.js:72
> +    if (queue.first === null) return null;

Style: use webkit style here.

if (queue.first === null)
    return null;

> Source/JavaScriptCore/builtins/AsyncGeneratorPrototype.js:77
> +    if (queue.first === null) queue.last = null;

Ditto.

> Source/JavaScriptCore/builtins/AsyncGeneratorPrototype.js:263
> +	       } else {

This else should be removed since the previous if-brace ends with `return`.

if (...) {
   ...
   return @undefined;
}
@assert(...);
return ...;

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2874
> +    auto varCreateAsyncFromSyncIterator =
variable(propertyNames().builtinNames().createAsyncGeneratorQueuePrivateName())
;
> +    RefPtr<RegisterID> scope = newTemporary();
> +    RefPtr<RegisterID> queue = newTemporary();
> +    moveToDestinationIfNeeded(scope.get(), emitResolveScope(scope.get(),
varCreateAsyncFromSyncIterator));
> +    RefPtr<RegisterID> createAsyncFromSyncIterator =
emitGetFromScope(newTemporary(), scope.get(), varCreateAsyncFromSyncIterator,
ThrowIfNotFound);
> +
> +    CallArguments args(*this, nullptr, 0);
> +    emitLoad(args.thisRegister(), jsUndefined());
> +
> +    emitCall(queue.get(), createAsyncFromSyncIterator.get(),
NoExpectedFunction, args, divot, divot, divot, DebuggableCall::No);
> +

We should use createAsyncGeneratorQueue, correct?

> JSTests/stress/async-iteration-async-from-sync.js:2
> +//@ skip

Let's remove this @skip.

> JSTests/stress/async-iteration-basic.js:2
> +//@ skip

Let's remove this @skip.

> JSTests/stress/async-iteration-syntax.js:2
>  //@ skip

Let's remove this @skip. (And comment).

> JSTests/stress/async-iteration-yield-promise.js:2
> +//@ skip

Let's remove this @skip.

> JSTests/stress/async-iteration-yield-star-interface.js:2
> +// This test requires enableAsyncIterator to be enabled at run time.
> +//@ skip

Let's remove this @skip.

> JSTests/stress/async-iteration-yield-star.js:2
> +// This test requires enableAsyncIterator to be enabled at run time.
> +//@ skip

Let's remove this @skip.


More information about the webkit-reviews mailing list