Closed Bug 875433 Opened 8 years ago Closed 5 years ago

Array.prototype[@@iterator] should be the same function object as Array.prototype.values

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25
Tracking Status
firefox48 --- fixed

People

(Reporter: bbenvie, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 1 obsolete file)

The ES6 spec states:

> Array.prototype[@@iterator]()
> The initial value of the @@iterator property is the same function object as the initial value of the Array.prototype.values property.

> Map.prototype[@@iterator]()
> The initial value of the @@iterator property is the same function object as the initial value of the Map.prototype.entries property.

> Set.prototype[@@iterator ]()
> The initial value of the @@iterator property is the same function object as the initial value of the Set.prototype.values property.


Ignoring the fact that SpiderMonkey currently isn't using a Symbol for the @@iterator property under the assumption that the string keyed "iterator" is a temporary stand-in for it, these departures from the ES6 spec can be observed:

> Array.prototype.entries !== Array.prototype.iterator;
> Array.prototype.iterator.name !== "entries";

> Map.prototype.entries !== Map.prototype.iterator;
> Map.prototype.iterator.name !== "entries";

(Set.{keys, values, entries} is not implemented yet, bug 869996)
Apologies, a correction. This should be observed for Array:

> Array.prototype.values !== Array.prototype.iterator;
> Array.prototype.iterator.name !== "values";
Attached patch patch for Array.prototype (obsolete) — Splinter Review
This should patch should fix the Array.prototype deviations from the ES6 spec.

The rest of the problem for Map and Set should go away once the patch on the bug 869996 has been landed.
Assignee: general → sankha93
Status: NEW → ASSIGNED
Attachment #754135 - Flags: review?(jorendorff)
Depends on: 869996
Attachment #754135 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/5d35dc039af7
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Depends on: 881782
Adding Array.prototype.values caused some surprising compatibility issues (bug 881782, bug 883914).  I think we need to do a partial backout before FF24.  I intend to do so tomorrow unless the plot develops further.

I've mentioned this to es-discuss, as well -- the method we added that seems to be causing the problems is standard-track. Presumably that will change.
Depends on: 883914
Backed out for now. No real discussion on es-discuss yet.
  https://hg.mozilla.org/integration/mozilla-inbound/rev/38f0844975f2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Jason Orendorff [:jorendorff] from comment #5)
> Backed out for now. No real discussion on es-discuss yet.
>   https://hg.mozilla.org/integration/mozilla-inbound/rev/38f0844975f2

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/38f0844975f2
[clearing milestone and in-testsuite, since this has been backed out]
Flags: in-testsuite+ → in-testsuite?
Target Milestone: mozilla24 → ---
I'm sure Sencha is working on fixing Ext JS but that won't fix existing web sites without further outreach.

The attitude on es-discuss seems to be "wait and see", so this could be in limbo for a long time. Setting need-info from dherman, one of our TC39 members.
Flags: needinfo?(dherman)
Having Array.prototype.values has limited usefulness as long as you have Array.prototype.@@iterator (or just "iterator" in our case currently). Perhaps we could spin this bug off into two bugs: one for Array.prototype.values which is in limbo, and one for the rest which is not.
Oh nevermind, Map/Set was done elsewhere. Ignore my last comment.
Summary: iterator methods of [Array, Map, Set].prototype should be same function object as entries/values → Array.prototype.iterator should be the same function object as Array.prototype.values
Please use hg out to check the commit message before you push. This one got a bit munged.
So, is there a plan to address the site regressions from comment 4?  Are those going to become tech evang issues?
(In reply to Daniel Holbert [:dholbert] from comment #13)
> So, is there a plan to address the site regressions from comment 4?  Are
> those going to become tech evang issues?

Sencha is aware of the issue and working on a fix. Once they have that, a roll out to regressing sites is hopefully going to be fairly painless, and hence quick. Hopefully.
https://hg.mozilla.org/mozilla-central/rev/b4426d926b31
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Depends on: 892225
https://hg.mozilla.org/integration/mozilla-inbound/rev/22c3433bdbc9
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Just for completeness, and because it confused someone on another bug, here's the m-c changeset for Comment 16's backout:
 https://hg.mozilla.org/mozilla-central/rev/22c3433bdbc9
Should this bug be closed (or morphed?) as a result of bug 881782 comment 22?
No, keep this open.

TC39 is designing a truly gruesome workaround for this. Eventually it'll be done and we'll implement, and then we can re-land the patch here.
Keywords: site-compat
We can try relanding this once bug 1054759 is fixed.
Depends on: 1054759
Flags: needinfo?(dherman)
See Also: → 1067049
Depends on: 1258163
Next time let's land this together with bug 1258163.
Assignee: sankha93 → evilpies
Attachment #754135 - Attachment is obsolete: true
Attachment #8732611 - Flags: review?(jorendorff)
Summary: Array.prototype.iterator should be the same function object as Array.prototype.values → Array.prototype[@@iterator[ should be the same function object as Array.prototype.values
Summary: Array.prototype[@@iterator[ should be the same function object as Array.prototype.values → Array.prototype[@@iterator] should be the same function object as Array.prototype.values
Comment on attachment 8732611 [details] [diff] [review]
Add Array.prototype.values

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

r=me. I had this in my patch for a while, but didn't realize we had _SetCanonicalName -- so I didn't expect it to work -- so I removed it. Nice!
Attachment #8732611 - Flags: review?(jorendorff) → review+
I'll land this when I reland @@unscopables, hopefully Monday.
I think I forgot to add values to test_xrayToJS.xul.
Yeah, I expected that. I'll add it before pushing to the try server.
Attachment #8732889 - Flags: review?(bobbyholley)
Assignee: evilpies → jorendorff
Status: REOPENED → ASSIGNED
Comment on attachment 8732889 [details] [diff] [review]
Implement Array.prototype.values

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

r=me on the xray bits. Everything else looks fine, but I don't know the spec well enough to confirm that this matches it.
Attachment #8732889 - Flags: review?(bobbyholley) → review+
I added the site-compat keyword 2 years ago but is there still any chance of breaking site compatibility here?
Not with @@unscopables in place, no.
Then removing the keyword. Thanks.
Keywords: site-compat
https://hg.mozilla.org/mozilla-central/rev/ee890a109435
Status: ASSIGNED → RESOLVED
Closed: 8 years ago5 years ago
Resolution: --- → FIXED
Depends on: 1299593
You need to log in before you can comment on or make changes to this bug.