Bug 202484 - Implement the Top-level await proposal
Summary: Implement the Top-level await proposal
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Enhancement
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-02 11:09 PDT by Myles Borins
Modified: 2021-02-26 09:50 PST (History)
21 users (show)

See Also:

Attachments
WIP (11.26 KB, patch)
2021-02-01 15:48 PST, Keith Miller 		no flags Details | Formatted Diff | Diff
WIP (63.52 KB, patch)
2021-02-21 13:56 PST, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch (81.38 KB, patch)
2021-02-21 15:34 PST, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch (80.16 KB, patch)
2021-02-21 15:44 PST, Keith Miller 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (80.93 KB, patch)
2021-02-21 15:53 PST, Keith Miller 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (82.05 KB, patch)
2021-02-21 16:02 PST, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch (101.97 KB, patch)
2021-02-21 16:05 PST, Keith Miller 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (93.58 KB, patch)
2021-02-21 16:36 PST, Keith Miller 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles Borins 2019-10-02 11:09:36 PDT 
The proposal is currently Stage-3 in the TC39 process which means it is ready for engines to implement.

https://github.com/tc39/proposal-top-level-await

I'm the champion from the TC39 committee so please feel free to ask me any questions you might have.
Comment 1 Radar WebKit Bug Importer 2019-10-04 17:08:27 PDT 
<rdar://problem/55999996>
Comment 2 Keith Miller 2021-02-01 15:48:35 PST 
Created attachment 418936 [details]
WIP
Comment 3 Keith Miller 2021-02-21 13:56:42 PST 
Created attachment 421151 [details]
WIP
Comment 4 Keith Miller 2021-02-21 15:34:43 PST 
Created attachment 421154 [details]
Patch
Comment 5 Keith Miller 2021-02-21 15:44:05 PST 
Created attachment 421155 [details]
Patch
Comment 6 Keith Miller 2021-02-21 15:53:44 PST 
Created attachment 421156 [details]
Patch
Comment 7 Keith Miller 2021-02-21 16:02:44 PST 
Created attachment 421157 [details]
Patch
Comment 8 Keith Miller 2021-02-21 16:05:41 PST 
Created attachment 421158 [details]
Patch
Comment 9 Yusuke Suzuki 2021-02-21 16:13:36 PST 
Comment on attachment 421156 [details]
Patch

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

Please check WebCore side too since now Workers support modules?

> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:261
> +    // FIXME: We should update the delegate callbacks for async modules.

Can you file a bug and paste URL?

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:958
> +        for (unsigned i = 0; i <= JSGenerator::Argument::NumberOfArguments; ++i)

Why isn't it `< jsgenerator::Argument::NumberOfArguments`? If it is because |this| parameter, I think,

initializeNextParameter(); // |this|
for (unsigned i = 0; i < JSGenerator::Argument::NumberOfArguments; ++i)
    initializeNextParameter(); 

would be better.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3747
> +    // RELEASE_ASSERT(m_scopeNode->isFunctionNode());

Why is  it necessary? If this comment-out is right, can you remove this assert?

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:5066
> +    // FIXME: It seems like this will create a lot of constants if there are many yield points. Maybe we should op_inc the old state.

Can you file a bug and paste URL?
Comment 10 Keith Miller 2021-02-21 16:32:51 PST 
Comment on attachment 421156 [details]
Patch

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

>> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:261
>> +    // FIXME: We should update the delegate callbacks for async modules.
> 
> Can you file a bug and paste URL?

Done.

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:958
>> +        for (unsigned i = 0; i <= JSGenerator::Argument::NumberOfArguments; ++i)
> 
> Why isn't it `< jsgenerator::Argument::NumberOfArguments`? If it is because |this| parameter, I think,
> 
> initializeNextParameter(); // |this|
> for (unsigned i = 0; i < JSGenerator::Argument::NumberOfArguments; ++i)
>     initializeNextParameter(); 
> 
> would be better.

Yeah, that's the reason. I'll change.

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3747
>> +    // RELEASE_ASSERT(m_scopeNode->isFunctionNode());
> 
> Why is  it necessary? If this comment-out is right, can you remove this assert?

Yeah missed that during cleanup. Deleted.

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:5066
>> +    // FIXME: It seems like this will create a lot of constants if there are many yield points. Maybe we should op_inc the old state.
> 
> Can you file a bug and paste URL?

Done.
Comment 11 Keith Miller 2021-02-21 16:36:10 PST 
Created attachment 421159 [details]
Patch for landing
Comment 12 Keith Miller 2021-02-21 16:41:55 PST 
Landed in r273225
Comment 13 Aakash Jain 2021-02-22 04:27:32 PST 
Seems like there was a follow-up fix: https://trac.webkit.org/r273226 (mentioning here just for reference)
Comment 14 Aakash Jain 2021-02-22 04:33:17 PST 
(In reply to Keith Miller from comment #12)
> Landed in r273225
This broke 5 layout-tests on mac and ios, as also indicated by ews. Should have waited for EWS to complete.

- imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/evaluation-order-4-tla.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-delayed.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-import.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-promise.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-immediate-promise.html
Comment 15 Keith Miller 2021-02-22 10:26:23 PST 
(In reply to Aakash Jain from comment #14)
> (In reply to Keith Miller from comment #12)
> > Landed in r273225
> This broke 5 layout-tests on mac and ios, as also indicated by ews. Should
> have waited for EWS to complete.
> 
> -
> imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-
> element/module/evaluation-order-4-tla.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-delayed.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-import.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-promise.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-immediate-promise.html

Weird that those didn't show up for me locally. They just need to be rebaselined.
Comment 16 Ryan Haddad 2021-02-22 10:41:39 PST 
(In reply to Aakash Jain from comment #14)
> (In reply to Keith Miller from comment #12)
> > Landed in r273225
> This broke 5 layout-tests on mac and ios, as also indicated by ews. Should
> have waited for EWS to complete.
Rebaselined these tests in r273261