Implemented parser part of the Async Generator
Created attachment 317298 [details] Patch Patch
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.
Patch applied https://trac.webkit.org/r220323
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
<rdar://problem/34158206>