Closed Bug 958951 Opened 10 years ago Closed 10 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+
https://hg.mozilla.org/mozilla-central/rev/7f177e032c15
Status: ASSIGNED → RESOLVED
Closed: 10 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: