[Webkit-unassigned] [Bug 156147] [JSC] implement async functions proposal

[bugzilla-daemon at webkit.org]

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

--- Comment #68 from Caitlin Potter (:caitp) <caitp at igalia.com> ---
Comment on attachment 277702
  --> https://bugs.webkit.org/attachment.cgi?id=277702
Patch

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

>>>> Source/JavaScriptCore/parser/Keywords.table:11
>>>> +await           AWAIT
>>> 
>>> Are async and await keywords? I think they are contextual keywords, correct?
>> 
>> A number of versions ago, they were treated as contextual keywords (and tokenized as IDENTs) --- but they are checked in so many contexts, that it seems worthwhile to internalize them and tokenize them separately from identifiers --- It may be a close call, but I think when the flag is enabled, it makes more sense to do this.
>> 
>> Contexts where `async` is potentially a keyword include any PrimaryExpression, any AssignmentExpression, and any StatementListItem (including in module code) --- nearly anywhere a BindingIdentifier is expected.
>> 
>> For `await`, it might make a bit more sense to use `matchContextualKeyword`, though, you're right.
>> 
>> Thanks for taking a look at this, I appreciate it Yusuke :)
> 
> I have some concerns about adding "async" and "await" into this keyword category.
> Currently, Keywords.table only holds keywords. Adding (strictly speaking)non-keyword words into this table seems confusing to readers.
> For example, "eval", "arguments" are handled like keywords under the strict mode, but it is not included in this table.
> 
> "async" is so widely accepted word. So "async" is frequently used as a variable, a method name, a parameter etc.
> If we don't include "async" in IDENT, once we use "IDENT" in the current parser, we easily miss "async" word, and it will break many pages.
> 
> Seeing http://tc39.github.io/ecmascript-asyncawait/#identifier-static-semantics-early-errors, "await" can be introduced into the keywords.
> But this handling does not include "async". I think it is better that "async" is handled in IDENT at least.

Okay --- I've changed it back to the contextual keyword style approach

>> Source/JavaScriptCore/parser/Parser.cpp:2130
>> +            AllowAwaitOverride allowAwait(this, !isAsyncFunctionParseMode(mode));
> 
> Nice. We can use SetForScope here :)

Switched to using this, so there are fewer lines added in Parser.h now

>> Source/JavaScriptCore/runtime/CodeCache.cpp:156
>>          source, name.string(), SourceCodeKey::FunctionType, 
> 
> We need to add RuntimeFlats into SourceCodeKey.

I've added it --- does it need to be used to calculate the hash, or just to determine if a given hash is the right one?

>> Source/JavaScriptCore/runtime/CodeCache.cpp:200
>> +    if (isAsyncFunctionParseMode(metadata->parseMode()))
> 
> Let's add generator and arrow function cases here.

I've changed this to a switch statement over the parseMode(), covering each type, and RELEASE_ASSERT_NOT_REACHED() for module/program code (I meant to remove RELEASE_* prefix, but maybe it doesn't matter a lot?).

>> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:490
>> +        putDirectWithoutTransition(vm, vm.propertyNames->AsyncFunction, asyncFunctionConstructor, DontEnum);
> 
> AsyncFunction constructor is not exposed as a global variable. (It is the same to the GeneratorFunction constructor).
> For example,
> 
> Object constructor,
> https://tc39.github.io/ecma262/#sec-object-constructor
> "... and the initial value of the Object property of the global object."
> 
> Array constructor
> https://tc39.github.io/ecma262/#sec-array-constructor
> "... and the initial value of the Array property of the global object."
> 
> But, GeneratorFunction constructor and AsyncFunction constructor do not have such statements.

Updated to no longer install AsyncFunction to the global object

>> Source/JavaScriptCore/tests/es6/async_arrow_functions_lexical_this_binding.js:27
>> +shouldBeAsync("barley", () => e.y("ley"));
> 
> Let's move these tests to tests/stress directory. es6 is used for kangax's compat-table tests.

They've been moved, and doing so found some new bugs, so that's good.

-- 
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/20160502/8a1dfe8e/attachment.html>