Element iterators should use [[Get]] and not peculiarly ignore the prototype chain

RESOLVED FIXED in mozilla13

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

(Blocks: 1 bug)

Other Branch
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
This only matters in dumb corner cases, but basically:

    Array.prototype[2] = "potato";
    for (let x of [0, 1, , 3])
        print(x);

should print 0, 1, "potato", 3
rather than 0, 1, undefined, 3.
(Assignee)

Comment 1

6 years ago
Created attachment 595257 [details] [diff] [review]
v1

I don't know what I was thinking. Just randomly hating on holes.

The fast path for dense arrays could be thrown away too. It really depends on how we want to implement this.
Attachment #595257 - Flags: review?(jwalden+bmo)
Attachment #595257 - Attachment is patch: true
Comment on attachment 595257 [details] [diff] [review]
v1

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

Agreed about throwing out the dense array fast path, I think.  We can pick up fast paths when they're demonstrably necessary; in the meantime, clarity and concision are more important.

::: js/src/jsiter.cpp
@@ -1153,5 @@
> -        } else {
> -            Value v = DoubleValue(i);
> -            if (!js_ValueToStringId(cx, v, &id))
> -                goto error;
> -        }

Did I review this code and not require IndexToString?  :-(
Attachment #595257 - Flags: review?(jwalden+bmo) → review+
Oh, I express no position on whether it is better to have for-of look up the prototype chain (as happens with this patch) or look only at own properties (as happens currently).  Definitely whatever the rationale is should be recorded in this bug, tho.
(Assignee)

Comment 4

6 years ago
The rationale, in broad strokes:

  - it is more like the canonical for loop that way
  - nothing in the language currently does this kind of "get if own property";
    for better or worse, everything uses [[Get]]
  - just how unusual this is shows up as code complexity
  - everybody says ignoring the prototype chain won't make for-of any faster

In any case TC39 will kick this back and forth and eventually come to some decision, and we'll implement that.
(Assignee)

Comment 5

6 years ago
Incidentally, removing the "simple" fast path fixes bug 726212, so the patch I check in for that bug will just be a few tests.
(Assignee)

Comment 6

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/dac3cc61a1b0
https://hg.mozilla.org/mozilla-central/rev/dac3cc61a1b0
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Nice diffstat!
You need to log in before you can comment on or make changes to this bug.