346021 - Implementing __iterator__ as generator doesn't work

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[nanto_vi (TOYAMA Nao)]

For-loops and array comprehensions don't iterate yielded values if the __iterator__ property of the specified object is implemented as a generator function.  The generator is invoked but yielded values are not iterated without using Iterator().

js> var o = { __iterator__: function () { print(12); yield 42; } };
js> for (let i in Iterator(o)) print(i);
12
42
js> for (let i in o) print(i); // this doesn't iterate 42
12

Comment 1

[nanto_vi (TOYAMA Nao)]

Attachment: PEP 255 Example

Rewrite of PEP 255 Example in JavaScript.
http://www.python.org/dev/peps/pep-0255/

This, a pattern that __iterator__ returns a generator, also fails to work.
The result with strict option is as follows.

$ js -s generator-tree-20060713.js
mydocs/generator-tree-20060713.js:45: strict warning: reference to undefined property t.left
mydocs/generator-tree-20060713.js:47: strict warning: reference to undefined property t.label
mydocs/generator-tree-20060713.js:48: strict warning: reference to undefined property t.right

mydocs/generator-tree-20060713.js:70: strict warning: reference to undefined property node.label

Comment 2

[Nickolay_Ponomarev]

It works, just not how we expect it to work, see:
http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/d7571ff410e67528/24e39cbd75344230#24e39cbd75344230

Not sure if this is a bug or a weird design decision.

Comment 3

[Brendan Eich [:brendan]]

This is a bug, it should be fixed ASAP for 1.8.1, to have correct JS1.7 semantics that are future-proofed against JS2/ES4.  I wish I'd seen it sooner.  Have to find time to read m.d.t.js-engine.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: proposed fix

This patch merges JSITER_COMPAT and JSITER_HIDDEN into JSITER_ENUMERATE, which if set means the flagged iterator was implicitly created by a for-in loop whose right operand delegates to Object.prototype.__iterator__ to get or create its iterator.

This patch also tracks a late-breaking ES4 proposal change to avoid delegating to Error.prototype from StopIteration and GeneratorExit.

/be

Comment 5

[Brendan Eich [:brendan]]

This is a JS1.7 blocker.

/be

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 231372 [details] [diff] [review]
proposed fix

Fixed on trunk.

/be

Comment 7

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch too.

/be

Comment 8

[Bob Clary [:bc] (inactive)]

Checking in regress-346021.js;
/cvsroot/mozilla/js/tests/js1_7/iterable/regress-346021.js,v  <--  regress-346021.js
initial revision: 1.1

Comment 9

[Bill Gianopoulos [:WG9s]]

The patch for this bug caused the regression in bug 346801.  Confirmed by local backout.

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 1.8, 1.9 windows/mac(ppc|tel)/linux 20060803

Comment 11

[Bob Clary [:bc] (inactive)]

this is new since 20060818:
js1_7/iterable/regress-346021.js, result: TIMED OUT (900 seconds), windows/prune, firefox_1.9a1_2006082109_opt

Comment 12

[Brendan Eich [:brendan]]

(In reply to comment #11)
> this is new since 20060818:

Trunk?

> js1_7/iterable/regress-346021.js, result: TIMED OUT (900 seconds),
> windows/prune, firefox_1.9a1_2006082109_opt

What does this mean?  Is there a process you can backtrace?

/be

Comment 13

[Bob Clary [:bc] (inactive)]

(In reply to comment #12)
> (In reply to comment #11)
> > this is new since 20060818:
> 
> Trunk?

yes, trunk <-> 1.9a1

> 
> > js1_7/iterable/regress-346021.js, result: TIMED OUT (900 seconds),
> > windows/prune, firefox_1.9a1_2006082109_opt
> 
> What does this mean?  Is there a process you can backtrace?

essentially it is a temporary issue I can't (at the moment) reproduce but which had not occurred previously. The comment was mostly for me so I could keep track of it.