Closed
Bug 1149015
Opened 9 years ago
Closed 9 years ago
Remove use of expression closures in jstests/jit-test that's not testing expression closures.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: arai, Unassigned)
References
Details
Attachments
(3 files)
11.12 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
116.92 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
27.17 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
There are several tests use expression closure in jstests and jit-test, and some of them are not testing expression closure itself, so it would be better to switch to one of function declaration, function expression, or arrow function.
for example, js/src/tests/js1_8_5/extensions/reflect-parse.js uses a lot of expression closures, but most of them can be function declarations.
> function program(elts) Pattern({ type: "Program", body: elts })
> function exprStmt(expr) Pattern({ type: "ExpressionStatement", expression: expr })
> function throwStmt(expr) Pattern({ type: "ThrowStatement", argument: expr })
> function returnStmt(expr) Pattern({ type: "ReturnStatement", argument: expr })
> ...
Reporter | ||
Comment 1•9 years ago
|
||
Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=11e843688748
Attachment #8586038 -
Flags: review?(sphink)
Reporter | ||
Comment 2•9 years ago
|
||
Attachment #8586040 -
Flags: review?(luke)
Updated•9 years ago
|
Attachment #8586040 -
Flags: review?(luke) → review+
Reporter | ||
Comment 3•9 years ago
|
||
Thank you for reviewing :) can I ask reviewing one more patch?
Attachment #8586097 -
Flags: review?(luke)
Updated•9 years ago
|
Attachment #8586097 -
Flags: review?(luke) → review+
Comment 4•9 years ago
|
||
Comment on attachment 8586038 [details] [diff] [review] Part 1: Remove some use of expression closure from jstests ecma_7/. Review of attachment 8586038 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! I've been meaning to do this, at least for the typed array stuff, to permit these tests to be shared with other engines.
Attachment #8586038 -
Flags: review?(sphink) → review+
Reporter | ||
Comment 5•9 years ago
|
||
Thank you! Landed without reflect-parse.js and shell.js in Part 2. I'll land it as Part 4 after rebasing on bug 1149769, since it should be easier than the reverse case. https://hg.mozilla.org/integration/mozilla-inbound/rev/45ecfb3e9f6f https://hg.mozilla.org/integration/mozilla-inbound/rev/a9a98220bb3f https://hg.mozilla.org/integration/mozilla-inbound/rev/09bdf23bd4b2
Reporter | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/45ecfb3e9f6f https://hg.mozilla.org/mozilla-central/rev/a9a98220bb3f https://hg.mozilla.org/mozilla-central/rev/09bdf23bd4b2
Reporter | ||
Comment 7•9 years ago
|
||
rebased on bug 1149769 https://hg.mozilla.org/integration/mozilla-inbound/rev/4d8a12b1e3b0
Keywords: leave-open
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4d8a12b1e3b0
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•