handle foreach properly with Proxy.iterate()

NEW
Unassigned

Status

()

Core
JavaScript Engine
8 years ago
4 years ago

People

(Reporter: gal, Unassigned)

Tracking

Other Branch
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

8 years ago
I am not exactly sure how to fix this. I can fix it for the internal implementation that delegates to enumerate(), but scripted proxies can't see the flags that tell us what kind of enumeration we want (keys, values, [key, value]).

These are non-standard features, only keys is standard in the language, and that works.

Updated

8 years ago
Blocks: 549143
(Reporter)

Comment 1

8 years ago
This is a SpiderMonkey-only issue and doesn't affect the spec.

Maybe mark and tom have a hint for us what to do here. I am leaning towards supporting this properly as long there is no scripted iterate() hook. Scripted hooks will always return an iterator object that returns keys on next(), which will produce unexpected results for "for each" loops on proxy objects.

I could pass flags to iterate() that indicate whether the VM wants keys, values, or [key, value], but I rather do not want to extend the scripted proxy implementation with non-standard features (I will extended the internal C++ API so we can properly implement our wrappers).
|for each (x in o) ...| is from E4X, ECMA-357. It's something we've talked about but today a bunch of said "no, not in Harmony, do not want."

Dave eloquently said something like "iterators and modules make it easy to say what you mean without adding an each pseudo-keyword":

for (let k in keys(o)) ...
for (let v in values(o)) ...
for (let [k, v] in props(o)) ...

(assuming some imports from an iteration module). Who needs |for each|?

It's true that many Rhino and SpiderMonkey users, and AS3 users in Flash, like |for each| and use it. We can't remove it any time soon. Also, in SpiderMonkey and Rhino it is meta-programmable via __iterator__, which gets a flag param, true for keysOnly and false to mean [key, value] pairs are needed. |for each| needs both, not just value, in order to do shadowing.

So for our implementation, a keysOnly flag param would suffice but it would be an illicit extension. ES5 and IIRC previous editions said "don't add non-spec parameters to spec built-ins".

/be
(In reply to comment #2)
> |for each (x in o) ...| is from E4X, ECMA-357. It's something we've talked
> about but today a bunch of said "no, not in Harmony, do not want."

"bunch of us", of course.

/be

Comment 4

8 years ago
I don't see too many other alternatives:
1. you could add a completely separate trap (|iterateEach|?)
2. you could extend the API of the iterator object returned from a call to 'iterate()'. For example, it could support, in addition to the standard 'next()' method, another method 'nextPair()' that is supposed to return the next [key,value] pair. 'for-in' loops iteratively call 'next', while 'for-each' loops could iteratively call 'nextPair'.
(In reply to comment #4)
> I don't see too many other alternatives:
> 1. you could add a completely separate trap (|iterateEach|?)

And enumerateEach. Argh.

> 2. you could extend the API of the iterator object returned from a call to
> 'iterate()'. For example, it could support, in addition to the standard
> 'next()' method, another method 'nextPair()' that is supposed to return the
> next [key,value] pair. 'for-in' loops iteratively call 'next', while 'for-each'
> loops could iteratively call 'nextPair'.

This is worse as cure than any disease, including leaving for-each out of the new meta-programming API.

We can carry the old __iterator__ hack along with for-each for a while, but not support any new proxy-based for-each customization. That seems best, it might even wean our smaller, non-web (but still large/distributed/absent) community of hackers off of for-each.

/be

Updated

8 years ago
No longer blocks: 549143
Still a valid non-TM bug?
Luke, ping?

Comment 8

6 years ago
The bug concerns proxies and non-standard for-each.  I don't think it has been fixed and if we get the whole new slew of for-of that I'm seeing in ES6 future-talk by Brendan I suspect it will have to be fixed.
Summary: TM: handle foreach properly with Proxy.iterate() → handle foreach properly with Proxy.iterate()
I think this works fine for non-scripted proxies. Two things don't work:

1. for-each-in and scripted proxies (no test case, just a hunch)

2. for-in and for-each-in when the iterator itself is a proxy (scripted or non-scripted) that does not have a .next method.

It might be that case 2 can't actually happen currently, because all iterators either have a .next method, or they're native enumerators, which are copied across compartment boundaries rather than being wrapped (see CanReify in jswrapper.cpp).

My for-of patch in bug 699565 is running up against case 2 though. More about this tomorrow.
I don't know exactly what I did here. It works somehow?

Anyway - we are going to float some changes to the semantics of the for-of loop in TC39 that would extricate for-of from this whole nightmare. See bug 725907.
(Assignee)

Updated

4 years ago
Assignee: general → nobody
You need to log in before you can comment on or make changes to this bug.