Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Return IteratorResult object for completed generators instead of throwing

RESOLVED FIXED in mozilla29

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
3 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

4 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

4 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

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

Comment 2

4 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

4 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

4 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

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f177e032c15
https://hg.mozilla.org/mozilla-central/rev/7f177e032c15
Status: ASSIGNED → RESOLVED
Last Resolved: 4 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.