builtins should be able to use async/await
Created attachment 358645 [details] Patch
Comment on attachment 358645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358645&action=review r=me but your changelog makes me feel like we should measure the perf impact. > Source/JavaScriptCore/ChangeLog:10 > + It's not clear if this substantially regresses the performance of our module loader > + code though. Can we determine if it does? Is it worth landing regardless? > Source/JavaScriptCore/builtins/BuiltinExecutables.cpp:88 > + // FIXME: Can we just make MetaData computation be constexpr and have the compiler do this for us? This would be super nice. You should file a bug. > Source/JavaScriptCore/builtins/BuiltinExecutables.cpp:-105 > - 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] == '('); I'm Sorry :( > Source/JavaScriptCore/builtins/ModuleLoader.js:341 > + let key = await this.resolve(moduleName, @undefined, fetcher); > + let entry = await this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher, new @Set); > + return entry.key; very nice > Source/JavaScriptCore/parser/Parser.cpp:282 > + // FIXME: We allow async to leak because it appearing as a closed variable is a side effect of trying to parse async arrow functions. I would also file a bug here or just remove the FIXME. This seems unlikely something we'll ever implement.
Comment on attachment 358645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358645&action=review >> Source/JavaScriptCore/ChangeLog:10 >> + code though. > > Can we determine if it does? Is it worth landing regardless? My very scientific experiments running our module loading tests before and after this change doesn't appear to significantly impact the end to end time. >> Source/JavaScriptCore/builtins/BuiltinExecutables.cpp:88 >> + // FIXME: Can we just make MetaData computation be constexpr and have the compiler do this for us? > > This would be super nice. You should file a bug. Done. >> Source/JavaScriptCore/parser/Parser.cpp:282 >> + // FIXME: We allow async to leak because it appearing as a closed variable is a side effect of trying to parse async arrow functions. > > I would also file a bug here or just remove the FIXME. This seems unlikely something we'll ever implement. I'll just remove the FIXME part but leave the comment.
Created attachment 358676 [details] Patch for landing
Comment on attachment 358676 [details] Patch for landing Rejecting attachment 358676 [details] from commit-queue. Number of test failures exceeded the failure limit. Full output: https://webkit-queues.webkit.org/results/10681459
Created attachment 358680 [details] Archive of layout-test-results from webkit-cq-02 for mac-sierra The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-02 Port: mac-sierra Platform: Mac OS X 10.12.6
Committed r239774: <https://trac.webkit.org/changeset/239774>
<rdar://problem/47148780>