[webkit-reviews] review canceled: [Bug 166698] [ESNext] Async iteration - Implement async iteration statement: for-await-of : [Attachment 319202] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <utatane.tea at gmail.com> has canceled GSkachkov
<gskachkov at gmail.com>'s request for review:
Bug 166698: [ESNext] Async iteration - Implement async iteration statement:
for-await-of
https://bugs.webkit.org/show_bug.cgi?id=166698

Attachment 319202: Patch

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




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

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

Nice! I think we have several bugs.

> Source/JavaScriptCore/parser/Parser.cpp:1490
> +	   ASSERT(!isAwaitFor);

In this `enumerationLoop:` labelled case, we do not check isAwaitFor.
This means that `for await (expr of expr);` can be parsed. (And this assertion
could fail.).
Let's fix INTOKEN parsing as the same to `for await (var xxx of xxx)` case.
And could you add tests for `for await (expr of expr);` and `for await (expr in
expr);` cases?

> Source/JavaScriptCore/parser/Parser.h:734
> +	   m_isAsyncFunctionBoundary = false;

Why is this necessary? If it is necessary, is this enough? (We only set false
for arrow functions. Why is this unnecessary for the other types of
functions?).