Bug 175210 – [ESNext] Async iteration - Implement Async Generator - parser

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[GSkachkov]

Implemented parser part of the Async Generator

[GSkachkov]

Created attachment 317298 [details]
Patch

Patch

[Yusuke Suzuki]

Comment on attachment 317298 [details]
Patch

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

r=+ with nits.

> Source/JavaScriptCore/parser/ASTBuilder.h:440
> +        SourceParseMode sourceParseMode;

Let's rename this to `bodySourceParseMode` and set `bodySourceParseMode = mode;` first.

> Source/JavaScriptCore/parser/ASTBuilder.h:451
> +        } else
> +            sourceParseMode = mode;

Drop this.

> Source/JavaScriptCore/parser/Parser.cpp:2866
> +            if (isAsyncMethodParseMode(parseMode) || isAsyncGeneratorMethodParseMode(parseMode)) {
>                  isConstructor = false;
> -                parseMode = SourceParseMode::AsyncMethodMode;
> -                semanticFailIfTrue(*ident == m_vm->propertyNames->prototype, "Cannot declare an async method named 'prototype'");
> -                semanticFailIfTrue(*ident == m_vm->propertyNames->constructor, "Cannot declare an async method named 'constructor'");
> -            } else if (isGenerator) {
> +                semanticFailIfTrue(*ident == m_vm->propertyNames->prototype, "Cannot declare ", stringArticleForFunctionMode(parseMode), stringForFunctionMode(parseMode), " named 'prototype'");
> +                semanticFailIfTrue(*ident == m_vm->propertyNames->constructor, "Cannot declare ", stringArticleForFunctionMode(parseMode), stringForFunctionMode(parseMode), " named 'constructor'");
> +            } else if (isGeneratorMethodParseMode(parseMode)) {
>                  isConstructor = false;
>                  parseMode = SourceParseMode::GeneratorWrapperMethodMode;
>                  semanticFailIfTrue(*ident == m_vm->propertyNames->prototype, "Cannot declare a generator named 'prototype'");
>                  semanticFailIfTrue(*ident == m_vm->propertyNames->constructor, "Cannot declare a generator named 'constructor'");
>              }

I think this `if (isGeneratorMethodParseMode(parseMode))` case can be merged into the above async case if you add a generator case to stringArticleForFunctionMode and stringForFunctionMode.
And we do not need to execute `parseMode = SourceParseMode::GeneratorWrapperMethodMode;` since parseMode is already SourceParseMode::GeneratorWrapperMethodMode.

> Source/JavaScriptCore/parser/Parser.cpp:3858
> +    bool isIdentSet = false;

I think this is always `false`, right? Nobody changes this isIdentSet.
What is the purpose of this variable?

> Source/JavaScriptCore/parser/Parser.cpp:3861
> +    const Identifier* ident;
> +    unsigned getterOrSetterStartOffset;
> +    JSToken identToken;

These changes are not necessary. Let's revert this.

> Source/JavaScriptCore/parser/Parser.cpp:3896
> -        JSToken identToken = m_token;
> -        const Identifier* ident = m_token.m_data.ident;
> -        unsigned getterOrSetterStartOffset = tokenStart();
> +        identToken = m_token;
> +        ident = m_token.m_data.ident;
> +        getterOrSetterStartOffset = tokenStart();

I don't think these changes are necessary. Let's revert this.

> Source/JavaScriptCore/parser/Parser.cpp:3900
> +        else if (UNLIKELY(!isIdentSet))

This is always `false`, right?

> Source/JavaScriptCore/parser/Parser.h:1514
> +    ALWAYS_INLINE SourceParseMode getAsynFunctionBodyParseMode(SourceParseMode parseMode)
> +    {
> +        if (isAsyncGeneratorFunctionParseMode(parseMode))
> +            return SourceParseMode::AsyncGeneratorBodyMode;
> +
> +        if (parseMode == SourceParseMode::AsyncArrowFunctionMode)
> +            return SourceParseMode::AsyncArrowFunctionBodyMode;
> +        
> +        return SourceParseMode::AsyncFunctionBodyMode;
> +    }

Let's put it in Parser.cpp as static function insead of Parser's member function.

[GSkachkov]

Patch applied https://trac.webkit.org/r220323

[GSkachkov]

Comment on attachment 317298 [details]
Patch

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

>> Source/JavaScriptCore/parser/ASTBuilder.h:440
>> +        SourceParseMode sourceParseMode;
> 
> Let's rename this to `bodySourceParseMode` and set `bodySourceParseMode = mode;` first.

Done

>> Source/JavaScriptCore/parser/ASTBuilder.h:451
>> +            sourceParseMode = mode;
> 
> Drop this.

Done

[Radar WebKit Bug Importer]

<rdar://problem/34158206>