Closed
Bug 1382306
Opened 7 years ago
Closed 7 years ago
async function not working in the module type script
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: nullizer, Assigned: anba)
References
Details
Attachments
(1 file, 1 obsolete file)
4.73 KB,
patch
|
anba
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0 Build ID: 20170630112252 Steps to reproduce: 1. Set dom.moduleScripts.enabled to true 2. Write a html like this: <!DOCTYPE html> <html lang="en"> <head> <meta charset="UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> <meta http-equiv="X-UA-Compatible" content="ie=edge"> <title>Document</title> </head> <body> <script type="module"> async function run() { console.log('console output not working') } const r = run() console.log(r) </script> </body> </html> 3. Test it with firefox 54.0.1, firefox nightly and chrome dev version Actual results: Firefox not execute the async function, the console.log statement not working. Chrome does it correct. Expected results: Should display text "console output not working" in the developer console panel
Updated•7 years ago
|
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
Assignee | ||
Comment 1•7 years ago
|
||
The bug is also reproducible on the shell: async function run() { print('called run()'); } var r = run(); print("run() returned:", r); Prints "run() returned: [object Generator]", which indicates the async function's inner generator function is exposed as |run| instead of the actual async function.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 2•7 years ago
|
||
If I understand the BytecodeEmitter code for async functions correctly, we just need to call WrapAsyncFunction/WrapAsyncGenerator in ModuleObject::instantiateFunctionDeclarations(). And it seems like there was another issue in ModuleObject::instantiateFunctionDeclarations, the method calls Lambda() to clone the function, but then installs the original JSFunction into the module environment (it has |value = ObjectValue(*fun)|, but I think that should be |value = ObjectValue(*obj)|, shouldn't it?). And while touching this code, I've also moved the definition for |RootedObject obj| outside of the loop and removed what looks like unnecessary rooting in js::Lambda() and js::LambdaArrow().
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Attachment #8888043 -
Flags: review?(jcoppeard)
Comment 3•7 years ago
|
||
Comment on attachment 8888043 [details] [diff] [review] bug1382306.patch Review of attachment 8888043 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, but can you add tests for the other cases too? ::: js/src/builtin/ModuleObject.cpp @@ +904,5 @@ > + obj = WrapAsyncFunction(cx, obj.as<JSFunction>()); > + } > + } > + > + value = ObjectValue(*obj); Thanks, you're correct. This was meant to be |obj| not |fun|.
Attachment #8888043 -
Flags: review?(jcoppeard) → review+
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #3) > This looks good, but can you add tests for the other cases too? Do you mean just adding a test for async generator declarations or something else?
Comment 5•7 years ago
|
||
(In reply to André Bargull from comment #4) Yes, for async generator declarations if that's possible.
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #5) > (In reply to André Bargull from comment #4) > Yes, for async generator declarations if that's possible. Sure thing, update on the way!
Assignee | ||
Comment 7•7 years ago
|
||
Updated patch to include test case for async generators (and made the test case compatible for browsers builds by replacing assertEventuallyEq).
Attachment #8888043 -
Attachment is obsolete: true
Attachment #8891635 -
Flags: review+
Assignee | ||
Comment 8•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e537243fed2613487c724651f0b7e7473ed83eb
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/81c056ef9e19 Create async function wrapper when instantiating module functions. r=jonco
Keywords: checkin-needed
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/81c056ef9e19
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•