If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove use of expression closures in jstests/jit-test that's not testing expression closures.

RESOLVED FIXED in Firefox 40

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: arai, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
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

3 years ago
Created attachment 8586038 [details] [diff] [review]
Part 1: Remove some use of expression closure from jstests ecma_7/.

Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=11e843688748
Attachment #8586038 - Flags: review?(sphink)
(Reporter)

Comment 2

3 years ago
Created attachment 8586040 [details] [diff] [review]
Part 2: Remove some use of expression closure from jstests js1_8_5/extensions.
Attachment #8586040 - Flags: review?(luke)

Updated

3 years ago
Attachment #8586040 - Flags: review?(luke) → review+
(Reporter)

Comment 3

3 years ago
Created attachment 8586097 [details] [diff] [review]
Part 3: Remove some use of expression closure from jit-test

Thank you for reviewing :)
can I ask reviewing one more patch?
Attachment #8586097 - Flags: review?(luke)

Updated

3 years ago
Attachment #8586097 - Flags: review?(luke) → review+
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

3 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

3 years ago
Keywords: leave-open
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

3 years ago
rebased on bug 1149769

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d8a12b1e3b0
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/4d8a12b1e3b0
Status: NEW → RESOLVED
Last Resolved: 3 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.