New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

semantics of yield* in throw case #293

Closed
GeorgNeis opened this Issue Jan 18, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@GeorgNeis
Contributor

GeorgNeis commented Jan 18, 2016

In the semantics of yield* (14.4.14) in step 5.b.ii.5.b (the throw case), it returns a return completion record.

If I understand correctly, this means that if I define

function* inner() { try {yield 1} catch(e) {return}; yield 2 }
function* outer() { yield* inner(); yield 3 }
x = outer()

then I should get the following interaction:

> x.next()
{value: 1, done: false}
> x.throw("bla")
{value: undefined, done: true}

Is this intended?

V8 and Firefox behave as if 5.b.ii.5.b would just return a normal completion (like in 5.a.iii):

> x.next()
{value: 1, done: false}
> x.throw("bla")
{value: 3, done: false}

Edge does something else entirely:

> x.next()
{value: 1, done: false}
> x.throw("bla")
Exception: bla

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing Jan 18, 2016

Contributor

It looks to me like this is a spec bug, and 5.b.ii.5.a should just say

  • Return ? IteratorValue(innerResult).
Contributor

zenparsing commented Jan 18, 2016

It looks to me like this is a spec bug, and 5.b.ii.5.a should just say

  • Return ? IteratorValue(innerResult).
@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Jan 18, 2016

Member

see see https://bugs.ecmascript.org/show_bug.cgi?id=3526

The spec. originally (or at least at one time, read the whole bug thread) had the equivalent to @zenparsing’s proposed change. That was replaced with the current behavior as a fix for Comment #8. This change added both steps 5.b.ii.5.a&b (using the current draft steps) and 5.c.vii.1&2.

But it appears that the 5.b.ii.5.a&b change may have been unnecessary as Comment #8 is about invoking return rather than throw on the outer iterator. Basically that says that the current V8&FF semantics make the most sense and @zenparsing's fix is appropriate. The Edge semantics seem wrong as the exception has already been handle by the inner catch.

But this is all pretty subtle stuff, particularly when you get finally blocks involved. I recall a discussion on either es-discuss or in TC39 meeting notes where the conclusion was that totally transparent delegation via yield* (as originally envisioned) was impossible because of issues related to things like this. I haven't found those notes yet but it would be nice to review them to see if they contain anything that relates to this.

Member

allenwb commented Jan 18, 2016

see see https://bugs.ecmascript.org/show_bug.cgi?id=3526

The spec. originally (or at least at one time, read the whole bug thread) had the equivalent to @zenparsing’s proposed change. That was replaced with the current behavior as a fix for Comment #8. This change added both steps 5.b.ii.5.a&b (using the current draft steps) and 5.c.vii.1&2.

But it appears that the 5.b.ii.5.a&b change may have been unnecessary as Comment #8 is about invoking return rather than throw on the outer iterator. Basically that says that the current V8&FF semantics make the most sense and @zenparsing's fix is appropriate. The Edge semantics seem wrong as the exception has already been handle by the inner catch.

But this is all pretty subtle stuff, particularly when you get finally blocks involved. I recall a discussion on either es-discuss or in TC39 meeting notes where the conclusion was that totally transparent delegation via yield* (as originally envisioned) was impossible because of issues related to things like this. I haven't found those notes yet but it would be nice to review them to see if they contain anything that relates to this.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Jan 19, 2016

Member

@zenparsing's fix seems good. The old fix to the return path (5.c.vii. 1&2) seems correct. With this fix in place, IIUC, yield* always propagates errors and only propagates returns when .return is called on the outer generator, and otherwise the value is a normal completion with the inner generator's return value.

Member

bterlson commented Jan 19, 2016

@zenparsing's fix seems good. The old fix to the return path (5.c.vii. 1&2) seems correct. With this fix in place, IIUC, yield* always propagates errors and only propagates returns when .return is called on the outer generator, and otherwise the value is a normal completion with the inner generator's return value.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Feb 18, 2016

Member

Unless anyone objects (even weakly) I'll go ahead and apply @zenparsing's fix here (which is what implementations do anyway, other than Chakra which is bugged in a different way, see Microsoft/ChakraCore#137).

Member

bterlson commented Feb 18, 2016

Unless anyone objects (even weakly) I'll go ahead and apply @zenparsing's fix here (which is what implementations do anyway, other than Chakra which is bugged in a different way, see Microsoft/ChakraCore#137).

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Feb 19, 2016

Member

Sounds good to me

Member

littledan commented Feb 19, 2016

Sounds good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment