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

Some iterator cleanup

RESOLVED FIXED in mozilla35

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla35
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

3 years ago
There's some cruft here, like js_IteratorMore taking a MutableHandleValue where it just wants a bool* and we can also remove jit::IteratorMore and js;:IteratorMore.
(Assignee)

Comment 1

3 years ago
Created attachment 8488076 [details] [diff] [review]
Part 1 - js_IteratorMore

Will post another patch to rename it to js::IteratorMore.
Attachment #8488076 - Flags: review?(bhackett1024)
(Assignee)

Updated

3 years ago
Blocks: 777596
Attachment #8488076 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 2

3 years ago
Part 1:

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a45ca4767de
Keywords: leave-open
(Assignee)

Comment 3

3 years ago
Created attachment 8488678 [details] [diff] [review]
Part 2 - Remove js::IteratorNext

js::IteratorNext has a path for native iterators and then calls js_IteratorNext. js_IteratorNext does exactly the same thing, so let's just use that one everywhere.
Attachment #8488678 - Flags: review?(bhackett1024)
Attachment #8488678 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 4

3 years ago
Created attachment 8488679 [details] [diff] [review]
Part 3 - Remove js_* prefix

Move js_IteratorMore, js_IteratorNext, js_Suppress* and js_ThrowStopIteration into the js namespace and drop the js_*.
Attachment #8488679 - Flags: review?(bhackett1024)
Attachment #8488679 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 5

3 years ago
Created attachment 8488725 [details] [diff] [review]
Part 4 - Move iterValue from JSContext to JSRuntime

I want to get rid of iterValue, but having it in the runtime simplifies some other patches before we are there.
Attachment #8488725 - Flags: review?(bhackett1024)
Comment on attachment 8488725 [details] [diff] [review]
Part 4 - Move iterValue from JSContext to JSRuntime

Review of attachment 8488725 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsiter.cpp
@@ +1019,5 @@
>      /*
>       * Make sure the more/next state machine doesn't get stuck. A value might
>       * be left in iterValue when a trace is left due to an interrupt after
>       * JSOP_MOREITER but before the value is picked up by FOR*.
>       */

Wow, this comment is an antique.
Attachment #8488725 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/8a45ca4767de
(Assignee)

Updated

3 years ago
Blocks: 831585
(Assignee)

Comment 8

3 years ago
Parts 2 and 3:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5d9353b3572c
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e47a9f5e048

Dropping part 4, bug 831585 removes cx->iterValue and moving it first is not necessary after all.
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/5d9353b3572c
https://hg.mozilla.org/mozilla-central/rev/7e47a9f5e048
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.