WebKit Bugzilla
Attachment 358645 Details for
Bug 193263
: builtins should be able to use async/await
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193263-20190108155804.patch (text/plain), 19.15 KB, created by
Keith Miller
on 2019-01-08 15:58:05 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Keith Miller
Created:
2019-01-08 15:58:05 PST
Size:
19.15 KB
patch
obsolete
>Subversion Revision: 239663 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index ef46089a850bdc2a4206b553d0b522e5b16d5ed7..e51011cbdb2fb589227829489ac505aa977b10f0 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,35 @@ >+2019-01-08 Keith Miller <keith_miller@apple.com> >+ >+ builtins should be able to use async/await >+ https://bugs.webkit.org/show_bug.cgi?id=193263 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ This patch makes it possible to use async functions when writing builtin JS code. >+ It's not clear if this substantially regresses the performance of our module loader >+ code though. >+ >+ * Scripts/wkbuiltins/builtins_generator.py: >+ (BuiltinsGenerator.generate_embedded_code_string_section_for_function): >+ * Scripts/wkbuiltins/builtins_model.py: >+ (BuiltinFunction.__init__): >+ (BuiltinFunction.fromString): >+ (BuiltinFunction.__str__): >+ * builtins/BuiltinExecutables.cpp: >+ (JSC::BuiltinExecutables::createExecutable): >+ * builtins/ModuleLoader.js: >+ (requestInstantiate): >+ (async.loadModule): >+ (async.loadAndEvaluateModule): >+ (async.requestImportModule): >+ (loadModule): Deleted. >+ (): Deleted. >+ (loadAndEvaluateModule): Deleted. >+ * bytecompiler/BytecodeGenerator.cpp: >+ (JSC::BytecodeGenerator::BytecodeGenerator): >+ * parser/Parser.cpp: >+ (JSC::Parser<LexerType>::parseInner): >+ > 2019-01-04 Tadeu Zagallo <tzagallo@apple.com> > > Baseline version of get_by_id may corrupt metadata >diff --git a/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py b/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py >index 1f2e80ea361a9e2169710d3bd57e42dc0bee4bfb..9fa94547a25f7ce69c1203cfd40da9cd322ab846 100644 >--- a/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py >+++ b/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py >@@ -114,7 +114,11 @@ class BuiltinsGenerator: > def generate_embedded_code_string_section_for_function(self, function): > text = function.function_source > # Wrap it in parens to avoid adding to global scope. >- text = "(function " + text[text.index("("):] + ")" >+ function_type_string = "function " >+ if function.is_async: >+ function_type_string = "async " + function_type_string >+ >+ text = "(" + function_type_string + text[text.index("("):] + ")" > embeddedSourceLength = len(text) + 1 # For extra \n. > # Lazy way to escape quotes, I think? > textLines = json.dumps(text)[1:-1].split("\\n") >diff --git a/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py b/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py >index 1f49a55ddb9c4adeda0d32415b32fdc7d18395bf..5792481340f2f5d09b3f9c46aea284e632f3ab66 100644 >--- a/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py >+++ b/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_model.py >@@ -42,14 +42,15 @@ _FRAMEWORK_CONFIG_MAP = { > }, > } > >-functionHeadRegExp = re.compile(r"(?:@[\w|=\[\] \"\.]+\s*\n)*function\s+\w+\s*\(.*?\)", re.MULTILINE | re.DOTALL) >+functionHeadRegExp = re.compile(r"(?:@[\w|=\[\] \"\.]+\s*\n)*(?:async\s+)?function\s+\w+\s*\(.*?\)", re.MULTILINE | re.DOTALL) > functionGlobalPrivateRegExp = re.compile(r".*^@globalPrivate", re.MULTILINE | re.DOTALL) > functionIntrinsicRegExp = re.compile(r".*^@intrinsic=(\w+)", re.MULTILINE | re.DOTALL) > functionIsConstructorRegExp = re.compile(r".*^@constructor", re.MULTILINE | re.DOTALL) > functionIsGetterRegExp = re.compile(r".*^@getter", re.MULTILINE | re.DOTALL) >+functionIsAsyncRegExp = re.compile(r"(async)?\s*function", re.MULTILINE | re.DOTALL) > functionNameRegExp = re.compile(r"function\s+(\w+)\s*\(", re.MULTILINE | re.DOTALL) > functionOverriddenNameRegExp = re.compile(r".*^@overriddenName=(\".+\")$", re.MULTILINE | re.DOTALL) >-functionParameterFinder = re.compile(r"^function\s+(?:\w+)\s*\(((?:\s*\w+)?\s*(?:\s*,\s*\w+)*)?\s*\)", re.MULTILINE | re.DOTALL) >+functionParameterFinder = re.compile(r"^(?:async\s+)?function\s+(?:\w+)\s*\(((?:\s*\w+)?\s*(?:\s*,\s*\w+)*)?\s*\)", re.MULTILINE | re.DOTALL) > > multilineCommentRegExp = re.compile(r"\/\*.*?\*\/", re.MULTILINE | re.DOTALL) > singleLineCommentRegExp = re.compile(r"\/\/.*?\n", re.MULTILINE | re.DOTALL) >@@ -100,10 +101,11 @@ class BuiltinObject: > > > class BuiltinFunction: >- def __init__(self, function_name, function_source, parameters, is_constructor, is_global_private, intrinsic, overridden_name): >+ def __init__(self, function_name, function_source, parameters, is_async, is_constructor, is_global_private, intrinsic, overridden_name): > self.function_name = function_name > self.function_source = function_source > self.parameters = parameters >+ self.is_async = is_async > self.is_constructor = is_constructor > self.is_global_private = is_global_private > self.intrinsic = intrinsic >@@ -133,6 +135,8 @@ class BuiltinFunction: > function_source = multipleEmptyLinesRegExp.sub("\n", function_source) > > function_name = functionNameRegExp.findall(function_source)[0] >+ async_match = functionIsAsyncRegExp.match(function_source) >+ is_async = async_match != None and async_match.group(1) == "async" > is_constructor = functionIsConstructorRegExp.match(function_source) != None > is_getter = functionIsGetterRegExp.match(function_source) != None > is_global_private = functionGlobalPrivateRegExp.match(function_source) != None >@@ -146,13 +150,16 @@ class BuiltinFunction: > if not overridden_name: > overridden_name = "static_cast<const char*>(nullptr)" > >- return BuiltinFunction(function_name, function_source, parameters, is_constructor, is_global_private, intrinsic, overridden_name) >+ return BuiltinFunction(function_name, function_source, parameters, is_async, is_constructor, is_global_private, intrinsic, overridden_name) > > def __str__(self): > interface = "%s(%s)" % (self.function_name, ', '.join(self.parameters)) > if self.is_constructor: > interface = interface + " [Constructor]" > >+ if self.is_async: >+ interface = "async " + interface >+ > return interface > > def __lt__(self, other): >diff --git a/Source/JavaScriptCore/builtins/BuiltinExecutables.cpp b/Source/JavaScriptCore/builtins/BuiltinExecutables.cpp >index f2e81e97194dfb3f3eed62448b5d9cc926d4ca56..5828b47b11e4efa83efc5f43ad6173dbdfc87709 100644 >--- a/Source/JavaScriptCore/builtins/BuiltinExecutables.cpp >+++ b/Source/JavaScriptCore/builtins/BuiltinExecutables.cpp >@@ -85,47 +85,42 @@ UnlinkedFunctionExecutable* createBuiltinExecutable(VM& vm, const SourceCode& co > > UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const SourceCode& source, const Identifier& name, ConstructorKind constructorKind, ConstructAbility constructAbility) > { >+ // FIXME: Can we just make MetaData computation be constexpr and have the compiler do this for us? > // Someone should get mad at me for writing this code. But, it prevents us from recursing into > // the parser, and hence, from throwing stack overflow when parsing a builtin. > StringView view = source.view(); > RELEASE_ASSERT(!view.isNull()); > RELEASE_ASSERT(view.is8Bit()); > auto* characters = view.characters8(); >- RELEASE_ASSERT(view.length() >= 15); // strlen("(function (){})") == 15 >- RELEASE_ASSERT(characters[0] == '('); >- RELEASE_ASSERT(characters[1] == 'f'); >- RELEASE_ASSERT(characters[2] == 'u'); >- RELEASE_ASSERT(characters[3] == 'n'); >- RELEASE_ASSERT(characters[4] == 'c'); >- RELEASE_ASSERT(characters[5] == 't'); >- RELEASE_ASSERT(characters[6] == 'i'); >- RELEASE_ASSERT(characters[7] == 'o'); >- RELEASE_ASSERT(characters[8] == 'n'); >- RELEASE_ASSERT(characters[9] == ' '); >- RELEASE_ASSERT(characters[10] == '('); >- >+ const char* regularFunctionBegin = "(function ("; >+ const char* asyncFunctionBegin = "(async function ("; >+ RELEASE_ASSERT(view.length() >= strlen("(function (){})")); >+ bool isAsyncFunction = view.length() >= strlen("(async function (){})") && !memcmp(characters, asyncFunctionBegin, strlen(asyncFunctionBegin)); >+ RELEASE_ASSERT(isAsyncFunction || !memcmp(characters, regularFunctionBegin, strlen(regularFunctionBegin))); >+ >+ unsigned asyncOffset = isAsyncFunction ? strlen("async ") : 0; >+ unsigned parametersStart = strlen("function (") + asyncOffset; > JSTokenLocation start; > start.line = -1; > start.lineStartOffset = std::numeric_limits<unsigned>::max(); >- start.startOffset = 10; >+ start.startOffset = parametersStart; > start.endOffset = std::numeric_limits<unsigned>::max(); > > JSTokenLocation end; > end.line = 1; > end.lineStartOffset = 0; >- end.startOffset = 1; >+ end.startOffset = strlen("(") + asyncOffset; > end.endOffset = std::numeric_limits<unsigned>::max(); > >- unsigned startColumn = 10; // strlen("function (") == 10 >- int functionKeywordStart = 1; // (f >- int functionNameStart = 10; >- int parametersStart = 10; >+ unsigned startColumn = parametersStart; >+ int functionKeywordStart = strlen("(") + asyncOffset; >+ int functionNameStart = parametersStart; > bool isInStrictContext = false; > bool isArrowFunctionBodyExpression = false; > > unsigned parameterCount; > { >- unsigned i = 11; >+ unsigned i = parametersStart + 1; > unsigned commas = 0; > bool sawOneParam = false; > bool hasRestParam = false; >@@ -199,14 +194,15 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const S > positionBeforeLastNewline.offset = offsetOfLastNewline; > positionBeforeLastNewline.lineStartOffset = positionBeforeLastNewlineLineStartOffset; > >- SourceCode newSource = source.subExpression(10, view.length() - closeBraceOffsetFromEnd, 0, 10); >+ SourceCode newSource = source.subExpression(parametersStart, view.length() - closeBraceOffsetFromEnd, 0, parametersStart); > bool isBuiltinDefaultClassConstructor = constructorKind != ConstructorKind::None; > UnlinkedFunctionKind kind = isBuiltinDefaultClassConstructor ? UnlinkedNormalFunction : UnlinkedBuiltinFunction; > >+ SourceParseMode parseMode = isAsyncFunction ? SourceParseMode::AsyncFunctionMode : SourceParseMode::NormalFunctionMode; > FunctionMetadataNode metadata( > start, end, startColumn, endColumn, functionKeywordStart, functionNameStart, parametersStart, > isInStrictContext, constructorKind, constructorKind == ConstructorKind::Extends ? SuperBinding::Needed : SuperBinding::NotNeeded, >- parameterCount, SourceParseMode::NormalFunctionMode, isArrowFunctionBodyExpression); >+ parameterCount, parseMode, isArrowFunctionBodyExpression); > > metadata.finishParsing(newSource, Identifier(), FunctionMode::FunctionExpression); > metadata.overrideName(name); >@@ -242,6 +238,14 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const S > metadataFromParser->setEndPosition(positionBeforeLastNewlineFromParser); > > if (metadata != *metadataFromParser || positionBeforeLastNewlineFromParser != positionBeforeLastNewline) { >+ dataLogLn("Expected Metadata:\n", metadata); >+ dataLogLn("Metadata from parser:\n", *metadataFromParser); >+ dataLogLn("positionBeforeLastNewlineFromParser.line ", positionBeforeLastNewlineFromParser.line); >+ dataLogLn("positionBeforeLastNewlineFromParser.offset ", positionBeforeLastNewlineFromParser.offset); >+ dataLogLn("positionBeforeLastNewlineFromParser.lineStartOffset ", positionBeforeLastNewlineFromParser.lineStartOffset); >+ dataLogLn("positionBeforeLastNewline.line ", positionBeforeLastNewline.line); >+ dataLogLn("positionBeforeLastNewline.offset ", positionBeforeLastNewline.offset); >+ dataLogLn("positionBeforeLastNewline.lineStartOffset ", positionBeforeLastNewline.lineStartOffset); > WTFLogAlways("Metadata of parser and hand rolled parser don't match\n"); > CRASH(); > } >diff --git a/Source/JavaScriptCore/builtins/ModuleLoader.js b/Source/JavaScriptCore/builtins/ModuleLoader.js >index eef91eb86551fa93197c14d80908dd69f56dc74a..1ebe2c02b64e2b574c6fc3990ea720ee203c7884 100644 >--- a/Source/JavaScriptCore/builtins/ModuleLoader.js >+++ b/Source/JavaScriptCore/builtins/ModuleLoader.js >@@ -187,32 +187,32 @@ function requestInstantiate(entry, parameters, fetcher) > if (entry.instantiate) > return entry.instantiate; > >- var instantiatePromise = this.requestFetch(entry, parameters, fetcher).then((source) => { >+ var instantiatePromise = (async () => { >+ var source = await this.requestFetch(entry, parameters, fetcher); > // https://html.spec.whatwg.org/#fetch-a-single-module-script > // Now fetching request succeeds. Then even if instantiation fails, we should cache it. > // Instantiation won't be retried. > if (entry.instantiate) >- return entry.instantiate; >+ return await entry.instantiate; > entry.instantiate = instantiatePromise; > > var key = entry.key; >- return this.parseModule(key, source).then((moduleRecord) => { >- var dependenciesMap = moduleRecord.dependenciesMap; >- var requestedModules = this.requestedModules(moduleRecord); >- var dependencies = @newArrayWithSize(requestedModules.length); >- for (var i = 0, length = requestedModules.length; i < length; ++i) { >- var depName = requestedModules[i]; >- var depKey = this.resolveSync(depName, key, fetcher); >- var depEntry = this.ensureRegistered(depKey); >- @putByValDirect(dependencies, i, depEntry); >- dependenciesMap.@set(depName, depEntry); >- } >- entry.dependencies = dependencies; >- entry.module = moduleRecord; >- @setStateToMax(entry, @ModuleSatisfy); >- return entry; >- }); >- }); >+ var moduleRecord = await this.parseModule(key, source); >+ var dependenciesMap = moduleRecord.dependenciesMap; >+ var requestedModules = this.requestedModules(moduleRecord); >+ var dependencies = @newArrayWithSize(requestedModules.length); >+ for (var i = 0, length = requestedModules.length; i < length; ++i) { >+ var depName = requestedModules[i]; >+ var depKey = this.resolveSync(depName, key, fetcher); >+ var depEntry = this.ensureRegistered(depKey); >+ @putByValDirect(dependencies, i, depEntry); >+ dependenciesMap.@set(depName, depEntry); >+ } >+ entry.dependencies = dependencies; >+ entry.module = moduleRecord; >+ @setStateToMax(entry, @ModuleSatisfy); >+ return entry; >+ })(); > return instantiatePromise; > } > >@@ -328,7 +328,7 @@ function provideFetch(key, value) > this.fulfillFetch(entry, value); > } > >-function loadModule(moduleName, parameters, fetcher) >+async function loadModule(moduleName, parameters, fetcher) > { > "use strict"; > >@@ -336,11 +336,9 @@ function loadModule(moduleName, parameters, fetcher) > // resolve: moduleName => Promise(moduleKey) > // Take the name and resolve it to the unique identifier for the resource location. > // For example, take the "jquery" and return the URL for the resource. >- return this.resolve(moduleName, @undefined, fetcher).then((key) => { >- return this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher, new @Set); >- }).then((entry) => { >- return entry.key; >- }); >+ let key = await this.resolve(moduleName, @undefined, fetcher); >+ let entry = await this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher, new @Set); >+ return entry.key; > } > > function linkAndEvaluateModule(key, fetcher) >@@ -355,21 +353,19 @@ function linkAndEvaluateModule(key, fetcher) > return this.moduleEvaluation(entry, fetcher); > } > >-function loadAndEvaluateModule(moduleName, parameters, fetcher) >+async function loadAndEvaluateModule(moduleName, parameters, fetcher) > { > "use strict"; > >- return this.loadModule(moduleName, parameters, fetcher).then((key) => { >- return this.linkAndEvaluateModule(key, fetcher); >- }); >+ let key = await this.loadModule(moduleName, parameters, fetcher); >+ return await this.linkAndEvaluateModule(key, fetcher); > } > >-function requestImportModule(key, parameters, fetcher) >+async function requestImportModule(key, parameters, fetcher) > { > "use strict"; > >- return this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher, new @Set).then((entry) => { >- this.linkAndEvaluateModule(entry.key, fetcher); >- return this.getModuleNamespaceObject(entry.module); >- }); >+ let entry = await this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher, new @Set); >+ this.linkAndEvaluateModule(entry.key, fetcher); >+ return this.getModuleNamespaceObject(entry.module); > } >diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp >index 71eff15247eb8f976da169f43b17e984a6fcaf1b..8ec5bc95ee38daaff5997506ac85211124821cbb 100644 >--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp >+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp >@@ -692,7 +692,8 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke > CallArguments args(*this, nullptr, 1); > emitLoad(args.thisRegister(), jsUndefined()); > >- auto varPromiseConstructor = variable(propertyNames().builtinNames().PromisePrivateName()); >+ auto& builtinNames = propertyNames().builtinNames(); >+ auto varPromiseConstructor = variable(m_isBuiltinFunction ? builtinNames.InternalPromisePrivateName() : builtinNames.PromisePrivateName()); > move(scope.get(), emitResolveScope(scope.get(), varPromiseConstructor)); > emitGetFromScope(args.argumentRegister(0), scope.get(), varPromiseConstructor, ThrowIfNotFound); > >diff --git a/Source/JavaScriptCore/parser/Parser.cpp b/Source/JavaScriptCore/parser/Parser.cpp >index 9e2f9d8c0b9c23f8181a0e41a80e3c36959239e5..922f41dccc1ba59e11cd2eaeca1c3249679afdba 100644 >--- a/Source/JavaScriptCore/parser/Parser.cpp >+++ b/Source/JavaScriptCore/parser/Parser.cpp >@@ -279,7 +279,8 @@ String Parser<LexerType>::parseInner(const Identifier& calleeName, SourceParseMo > VariableEnvironment& lexicalVariables = scope->lexicalVariables(); > const HashSet<UniquedStringImpl*>& closedVariableCandidates = scope->closedVariableCandidates(); > for (UniquedStringImpl* candidate : closedVariableCandidates) { >- if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !candidate->isSymbol()) { >+ // FIXME: We allow async to leak because it appearing as a closed variable is a side effect of trying to parse async arrow functions. >+ if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !candidate->isSymbol() && candidate != m_vm->propertyNames->async.impl()) { > dataLog("Bad global capture in builtin: '", candidate, "'\n"); > dataLog(m_source->view()); > CRASH();
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=358645">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193263
:
358645
|
358676
|
358680