Add evaluating-async module state to match the spec
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox104 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
()
Details
Attachments
(3 files)
Top level await added this new module state to the spec. It is purely editorial and has no function impact but it would be helpful to have it to make it clearer that our implementation matches the spec.
|
||
Updated•2 years ago
|
|
Assignee | |
Comment 1•2 years ago
|
||
I originally thought adding an extra status would allow for stronger assertions
but now I think it has made state checks more verbose and harder to compare
with the spec.
This patch makes ModuleObject::status return ModuleStatus::Evaluated when the
status is Evaluated_Error so that this method matches the spec's concept of the
value of the ModuleStatus field.
This turned up one place where we diverge from the spec. In ModuleEvaluate step
3 we don't get the cycle root if we are in the error state. This is currently
required otherwise some tests fail. This patch preserves the current behvaiour,
which is now explicit.
|
||
Updated•2 years ago
|
|
|
Assignee | |
Comment 2•2 years ago
|
||
This adds ModuleStatus::EvaluatingAsync, splitting it off from
ModuleStatus::Evaluated. This allows us to rewrite parts of the code to be
closer to the spec.
Depends on D151506
|
|
Assignee | |
Comment 3•2 years ago
|
||
This refactors the extra check to make it clearer what's going on and adds a
simple test case for the issue.
Depends on D151507
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a3211c84154b Part 1: Hide Evaluated_Error module state r=yulia https://hg.mozilla.org/integration/autoland/rev/8580c5c39eec Part 2: Add evaluating-async module status r=yulia https://hg.mozilla.org/integration/autoland/rev/2517efe4191b Part 3: Refactor extra error check at the start of ModuleEvaluate r=yulia
|
||
Comment 5•2 years ago
|
||
Backed out for causing failures at ModuleObject.cpp.
Backout link: https://hg.mozilla.org/integration/autoland/rev/8e64ff82c3eb2666eaa4550a3664257d03db8b1c
Push where the failures started: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=NhIQ7hS8Q4WeILDhbC_Sqw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=ed5f1dbedf5506438d3b846649ab80e48db98e89
Failure log:
https://treeherder.mozilla.org/logviewer?job_id=384006405&repo=autoland&lineNumber=8017
https://treeherder.mozilla.org/logviewer?job_id=384004872&repo=autoland&lineNumber=12734
|
|
Assignee | |
Comment 6•2 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #1)
This turned up one place where we diverge from the spec. In ModuleEvaluate step
3 we don't get the cycle root if we are in the error state. This is currently
required otherwise some tests fail. This patch preserves the current behvaiour,
which is now explicit.
I've filed a spec issue about this: https://github.com/tc39/ecma262/issues/2823
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/23e793db636d Part 1: Hide Evaluated_Error module state r=yulia https://hg.mozilla.org/integration/autoland/rev/2e18ce531c64 Part 2: Add evaluating-async module status r=yulia https://hg.mozilla.org/integration/autoland/rev/17cdb35dc893 Part 3: Refactor extra error check at the start of ModuleEvaluate r=yulia
|
||
Comment 8•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/23e793db636d
https://hg.mozilla.org/mozilla-central/rev/2e18ce531c64
https://hg.mozilla.org/mozilla-central/rev/17cdb35dc893
Description
•