Return IteratorResult object for completed generators instead of throwing

RESOLVED FIXED in mozilla29

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: André Bargull, Assigned: till)

Tracking

({dev-doc-complete, site-compat})

Trunk
mozilla29
dev-doc-complete, site-compat
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [DocArea=JS])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

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

Comment 1

3 years ago
Created attachment 8358948 [details] [diff] [review]
Return IteratorResult object for completed generators instead of throwing

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)

Updated

3 years ago
Assignee: nobody → till
Status: NEW → ASSIGNED
(Reporter)

Comment 2

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

Comment 3

3 years ago
Created attachment 8358953 [details] [diff] [review]
Return IteratorResult object for completed generators instead of throwing. v2

> 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)
(Assignee)

Updated

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

Comment 5

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f177e032c15
https://hg.mozilla.org/mozilla-central/rev/7f177e032c15
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
https://developer.mozilla.org/en-US/Firefox/Releases/29/Site_Compatibility

This doc is outdated...
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Iterators_and_Generators
Keywords: site-compat
Depends on: 1053132
Also added to
https://developer.mozilla.org/en-US/Firefox/Releases/29#JavaScript
and
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/yield#IteratorResult_object_returned_instead_of_throwing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function*#IteratorResult_object_returned_instead_of_throwing
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.