Implementing __iterator__ as generator doesn't work

VERIFIED FIXED

Status

()

VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: nanto, Assigned: brendan)

Tracking

({verified1.8.1})

Trunk
x86
Windows XP
verified1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

12 years ago
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
(Reporter)

Comment 1

12 years ago
Created attachment 231267 [details]
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

12 years ago
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.

Updated

12 years ago
Blocks: 326466
(Assignee)

Comment 3

12 years ago
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
(Assignee)

Comment 4

12 years ago
Created attachment 231372 [details] [diff] [review]
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
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #231372 - Flags: review?(mrbkap)
(Assignee)

Comment 5

12 years ago
This is a JS1.7 blocker.

/be
Flags: blocking1.8.1?
(Assignee)

Updated

12 years ago
Blocks: 345558

Updated

12 years ago
Flags: blocking1.8.1? → blocking1.8.1+
Attachment #231372 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 6

12 years ago
Comment on attachment 231372 [details] [diff] [review]
proposed fix

Fixed on trunk.

/be
Attachment #231372 - Flags: approval1.8.1?
(Assignee)

Updated

12 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Attachment #231372 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 7

12 years ago
Fixed on the 1.8 branch too.

/be
Keywords: fixed1.8.1

Comment 8

12 years ago
Checking in regress-346021.js;
/cvsroot/mozilla/js/tests/js1_7/iterable/regress-346021.js,v  <--  regress-346021.js
initial revision: 1.1
Flags: in-testsuite+
The patch for this bug caused the regression in bug 346801.  Confirmed by local backout.

Comment 10

12 years ago
verified fixed 1.8, 1.9 windows/mac(ppc|tel)/linux 20060803
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1

Comment 11

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

Comment 12

12 years ago
(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

12 years ago
(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.
You need to log in before you can comment on or make changes to this bug.