Implement Module execution and Loader's ready / link phase
Now, I'm now implementing the rapid prototype. And later, I'll refactor/redesign/optimize it.
Created attachment 259754 [details] Patch WIP
WIP, still a lot of weird things. 1. C++ recursion exists 2. Looking up the hash tables repeatedly. It may be combined. 3. Not implemented the execution part (environment, instance, namespace object are partially done) I'll complete the implementation first, and later, I'll refactor them and submit the r? patch here.
Created attachment 259794 [details] Patch WIP
Created attachment 259861 [details] Patch WIP: part2
Created attachment 259868 [details] Patch WIP: part3
Created attachment 259925 [details] Patch WIP: part4, namespace object is correctly handled
Created attachment 259984 [details] Patch WIP: part5, import / export bindings are correctly supported. making dynamic handling correct under the module enviornment
Created attachment 260009 [details] Patch WIP: part6, functionality is done. Let's refactor and break this into several patches
Here, I'll just submit the patch to add the template for the link/ready phase. I'll upload the large (the big picture of the module implementation) patch to the original meta bug.
Created attachment 260028 [details] Patch
Comment on attachment 260028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260028&action=review Add comments for the ease of the review. > Source/JavaScriptCore/builtins/ModuleLoaderObject.js:352 > + let pair = entry.dependencies[i]; This was the bug. Change var to let to construct the lexical variable for this `pair`. > Source/JavaScriptCore/builtins/ModuleLoaderObject.js:-372 > - // https://github.com/whatwg/loader/pull/66 This issue is fixed in the draft. (PR is merged.) > Source/JavaScriptCore/runtime/JSModuleRecord.cpp:94 > +{ The implementation will be implemented in the subsequent patch. > Source/JavaScriptCore/runtime/JSModuleRecord.cpp:99 > + return jsUndefined(); Ditto.
Comment on attachment 260028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260028&action=review r=me >> Source/JavaScriptCore/builtins/ModuleLoaderObject.js:352 >> + let pair = entry.dependencies[i]; > > This was the bug. Change var to let to construct the lexical variable for this `pair`. There is a fixme in the parser that may need to be fixed for this to work. We do some validation when parsing builtins and we probably don't validate properly for "let" variables. Can you take a look at this maybe and make sure it's okay? > Source/JavaScriptCore/builtins/ModuleLoaderObject.js:451 > + entry.state = this.Ready; Even though we know we will succeed, why not just setStateToMax to be consistent?
Comment on attachment 260028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260028&action=review Thanks, helpful comments. >>> Source/JavaScriptCore/builtins/ModuleLoaderObject.js:352 >>> + let pair = entry.dependencies[i]; >> >> This was the bug. Change var to let to construct the lexical variable for this `pair`. > > There is a fixme in the parser that may need to be fixed for this to work. > We do some validation when parsing builtins and we probably don't validate > properly for "let" variables. Can you take a look at this maybe and make > sure it's okay? I've ensured this code itself is ok. In addition to that, I've tested the top-level let variable that is captured by the closure function, and it does not raise the assertion. This is because `capturedVariables` only collects the captured declared variables. lexical variables are ignored. `if (!m_declaredVariables.contains(*ptr))` purges the lexical variables. Is it intentional? >> Source/JavaScriptCore/builtins/ModuleLoaderObject.js:451 >> + entry.state = this.Ready; > > Even though we know we will succeed, why not just setStateToMax to be consistent? This is because we know `Ready` is the highest state value. So when performing `setStateToMax` with the `Ready` state, the result is always `Ready`. But, for the consistency and the future extension that may have the higher value than `Ready`, I'll change here to `setStateToMax`.
Committed r189088: <http://trac.webkit.org/changeset/189088>