Closed Bug 958951 Opened 12 years ago Closed 12 years ago

Return IteratorResult object for completed generators instead of throwing

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: anba, Assigned: till)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [DocArea=JS])

Attachments

(1 file, 1 obsolete file)

Changes from November 8, 2013 Draft Rev 21: > Calling the next method of a completed generator returns a “done” result instead of throwing The reason for this change was to unify the behaviour of completed generator functions compared to array or collection objects iterators. Test case: --- js> function*g(){ } js> o = g(); o.next(); o.next() Expected: IteratorResult object `({value:(void 0), done:true})` Actual: throws TypeError "generator has already finished" ---
Right, this is 25.3.3.2, step 5 for .next() and 25.3.1.3, step 7 for .throw()
Attachment #8358948 - Flags: review?(jorendorff)
Assignee: nobody → till
Status: NEW → ASSIGNED
Comment on attachment 8358948 [details] [diff] [review] Return IteratorResult object for completed generators instead of throwing Review of attachment 8358948 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsiter.cpp @@ +1736,5 @@ > + RootedObject obj(cx, CreateItrResultObject(cx, args.get(0), true)); > + if (!obj) > + return false; > + args.rval().setObject(*obj); > + return true; 25.3.1.3, step 7 returns a completion record with [[type]]=throw, not [[type]]=normal. I guess that means using cx->setPendingException()?
> 25.3.1.3, step 7 returns a completion record with [[type]]=throw, not > [[type]]=normal. I guess that means using cx->setPendingException()? Yes it does, thanks! Also, of course the previous behavior was tested extensively, so this version contains various test changes. Try-servering here, in case any chrome code relies on the previous behavior, already: https://tbpl.mozilla.org/?tree=Try&rev=14913d4c2a18
Attachment #8358953 - Flags: review?(jorendorff)
Attachment #8358948 - Attachment is obsolete: true
Attachment #8358948 - Flags: review?(jorendorff)
Comment on attachment 8358953 [details] [diff] [review] Return IteratorResult object for completed generators instead of throwing. v2 Review of attachment 8358953 [details] [diff] [review]: ----------------------------------------------------------------- Great. ::: js/src/jsiter.cpp @@ +1707,5 @@ > JSGenerator *gen = thisObj->as<StarGeneratorObject>().getGenerator(); > > if (gen->state == JSGEN_CLOSED) { > + RootedValue val(cx, UndefinedValue()); > + RootedObject obj(cx, CreateItrResultObject(cx, val, true)); You can use UndefinedHandleValue here instead of an extra root. ::: js/src/tests/ecma_6/Generators/delegating-yield-6.js @@ +46,5 @@ > > assertEq(log, "indndndndndndv"); > > // Outer's dead, man. Outer's dead. > +assertDeepEq(outer.next.bind(outer)(), {value: undefined, done: true}); Please change `outer.next.bind(outer)()` to `outer.next()`.
Attachment #8358953 - Flags: review?(jorendorff) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: