Issue 7447045: Implement 'return' in generators.

usrbincc

The pre-patches actually shouldn't affect this patch, so I've left them out here. Meant to ...

13 years, 3 months ago (2013-03-01 19:16:12 UTC) #1

The pre-patches actually shouldn't affect this patch, so
I've left them out here. Meant to get this patch up earlier
but got sidetracked.

Adding peterhal because this touches some state machine code
and adds some new code.

#--cut--
git checkout 710e75350af77936   # last merge with branch master

git checkout -b issue7456045-yield-expr-6-return-value

test -e issue7447045_1.diff || curl -sO \
  https://codereview.appspot.com/download/issue7447045_1.diff
openssl sha1 < issue7447045_1.diff \
  | { grep -q '7bff4f057755885fbbd00c9eb4d8de551029ca89$' &&
      git apply issue7447045_1.diff
    } || echo sha1 mismatch!

make test
#--cut--

----

Next up (rough order):
- Factoring out the common generator code.
- A mini library for async code based on traceur's
  'Deferred'. See below.
- Try micro-optimizing the generated generator (hereafter,
  tentatively referred to as gen-gen) code for V8; something
  about V8 not optimizing functions with try-catch, so try
  breaking out as much code as possible into functions that
  don't contain try-catch.
- Finally some demos. Sadly, this is all most people ever
  see.

----

#--cut--
# Rough cut at an async library.
cat > async.js <<"END"
function f(G) {
  var g = G();
  var w = new Deferred();
  function cbi(x) { return g.send(x).then(cb, eb); }
  function ebi(x) { return g.throw(x).then(cb, eb); }
  function rbi(e) {
    if (traceur.runtime.isStopIteration(e)) {
      w.callback(e.value);
    } else {
      w.errback(e);
    }
  }

  function cb(x) {
    try {
      return cbi(x);
    } catch(e) {
      rbi(e);
    }
  }
  function eb() {
    try {
      return ebi(x);
    } catch(e) {
      rbi(e);
    }
  }
  cb();
  return w;
}

var cc = 70;

function delay(t) {
  var d = new Deferred();
  setTimeout((function() {
    return d.callback(cc-=10);
  }), t);
  return d;
}

function* G() {
  var i, c;
  for (i=0;i<7;i++) {
    c = yield delay(500);
    console.log('%s, %s', i, c);
  }
  return 42;
}

f(G).then((x)=>console.log("ret:%s",x))
END

./traceur --out async.out.js async.js

{ printf "require('./src/node/traceur.js');\n"
  cat async.out.js
} > async.linked.js

node async.linked.js
#--cut--

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Ge...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Ge...
src/codegeneration/generator/GeneratorTransformer.js:148: // TODO: Is 'return'
allowed inside 'finally'?
I'm thinking it probably isn't, since it brings up the same
problems as 'yield', maybe even more. But just in case, I
had to ask.

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Yi...
File src/codegeneration/generator/YieldState.js (right):

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Yi...
src/codegeneration/generator/YieldState.js:49: return new this.constructor(
Necessary because currently ReturnState inherits from
YieldState, and so also uses this function.

https://codereview.appspot.com/7447045/diff/1/src/runtime/runtime.js
File src/runtime/runtime.js (right):

https://codereview.appspot.com/7447045/diff/1/src/runtime/runtime.js#newcode385
src/runtime/runtime.js:385: StopIterationLocal = new GeneratorReturnLocal();
In the absence of a real StopIteration, we want our object
to be an 'instanceof' GeneratorReturn, allowing Q and TaskJS
to properly recognize it as a type of StopIteration.

https://codereview.appspot.com/7447045/diff/1/src/runtime/runtime.js#newcode394
src/runtime/runtime.js:394: setStopIteration(global.StopIteration, global);
All these kinds of things should probably go into
setupGlobals() eventually.

Sign in to reply to this message.

arv

I'm not fully sure how this works. Why do we need a GeneratorReturnLocal? Your sample ...

13 years, 3 months ago (2013-03-05 17:27:04 UTC) #2

usrbincc

Read over this and honestly can't tell if this will come across as gibberish or ...

13 years, 3 months ago (2013-03-06 13:03:17 UTC) #3

Read over this and honestly can't tell if this will come
across as gibberish or not. It really is true about "not
having the time to make it shorter", so apologies.

----

> I'm not fully sure how this works. Why do we need a
> GeneratorReturnLocal?

Started writing some paragraphs and gave up on that.

One-sentence summary: if we don't have to work with Q.js and
Task.js, then we can do just about whatever we want.

But code is probably the easiest way.

Here is the extraction of a generator's return value. Q.js,
Task.js, and ES6 do the same overall thing.

  try {
    while (g.next()) {}
  } catch (e) {
    if (isStopIteration(e)) {
      if (e.value === undefined) {
        console.log('returned');
      } else {
        console.log('returned a value: %s', e.value);
      }
    } else {
      throw e;
    }
  }

Here is isStopIteration for Q.js:

  // straightforward implementation, not Q's, which uses the
  // fascinating 'uncurryThis'.
  function object_toString(obj) {
    Object.prototype.toString.call(obj);
  }
  ...
  object_toString(exception) === "[object StopIteration]" ||
    exception instanceof QReturnValue

and for Task.js:

  e instanceof TaskResult || e instanceof StopIteration

----

It's impossible to make an object obj such that

  obj !== StopIteration &&
    obj instanceof StopIteration &&
    obj.value === arbitraryReturnValue;
  obj !== StopIteration &&
    object_toString(obj) === '[object StopIteration]' &&
    obj.value === arbitraryReturnValue;

  Also note (ionmonkey):

    js> StopIteration.value = 42;
    42
    js> StopIteration.value === 42;
    false
    js> StopIteration.value === undefined;
    true

So we need to instead get traceur to pass:

  // Q.js
  obj instanceof QReturnValue &&
    obj.value === arbitraryReturnValue;
  // Task.js
  obj instanceof task.TaskResult &&
    obj.value === arbitraryReturnValue;

This is possible through library-specific settings:

  // Q.js
  <script>
    // Need to set this magic symbol before loading Q,
    // which will use it as its QReturnValue if it exists.
    // We cannot get Q's QReturnValue; we can only set it.
    var ReturnValue = traceur.runtime.GeneratorReturn;
  </script>
  <script src="lib/q.js"></script>

  // Task.js
  <script src="lib/task.js"></script>
  <script>
    // Only this simple in patch set 2. More fiddly in 1.
    // We cannot set TaskJS's TaskResult; we can only get
    // it.
    traceur.runtime.setGeneratorReturn(task.TaskResult);
  </script>

After that, you should be able to

  // generated code
  throw new traceur.runtime.GeneratorReturn(42);

and have it properly recognized as a generator return value
by Q or TaskJS.

  And this is a corner case, but I thought I'd mention it:
  throwing a real StopIteration is the only way to properly
  exit a for-in in Firefox. I believe this is not spec, so
  it's probably okay to ignore.

    function G1(n) {
      for (var i = 0; i < n; i++) {
        yield i;
      }
    }

    for (var i in G1(10)) {
      print(i);
    }

    function G2(n) {
      var i = 0;
      return {
        __iterator__: function() {
          return this;
        },
        next: function() {
          if (i < n)
            return i++;
          throw StopIteration;
        }
      };
    }

    for (var i in G2(10)) {
      print(i);
    }

  On the other hand, as browsers start half-implementing
  es6, things will get more interesting. I'm hoping tc39
  eventually decide to expose the 'StopIteration'
  constructor.

So that's why GeneratorReturn, though of course, anything
that satisfies the constraints given would work. Or if we
relax some of the constraints.

----

> Your sample code uses traceur.runtime, something we should
> not allow. In a perfect world we would not expose traceur
> and traceur.runtime to user code.

Searched around a little and found:

  http://wiki.ecmascript.org/doku.php?id=harmony:iterators

  There is a standard “@iter” module which provides the
  following functionality:

    isGenerator(x): returns true if x is a generator
    function, false otherwise.

    isStopIteration(x): returns true if x is an object whose
    [[Class]] internal property is “StopIteration”.

So it looks like isStopIteration has a home. I'll try doing
that in a follow-up once I figure out how standard modules
work in traceur.

  [time passes] Fixed a bug preventing import of standard
  modules in:

  #--cut--
  cat > iter.js <<"END"
  import * from '@iter';
  END
  ./traceur --out iter.out.js iter.js
  #--cut--

To be pedantic, GeneratorReturn (or equivalent) needs to be
exposed in a module somewhere also, but the spec is silent
on this.

----

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/GeneratorTra...
File src/codegeneration/GeneratorTransformPass.js (right):

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/GeneratorTra...
src/codegeneration/GeneratorTransformPass.js:359: if (!(finder.hasYield ||
isGenerator) && !finder.hasAsync) {
On 2013/03/05 17:27:04, arv-chromium wrote:
> Maybe we can skip the finder if isGenerator is true?

In the isGenerator case, you still need YieldFinder, because
of the finder.hasForIn later.

This function is definitely a little messy with all the
different paths for different options. And the new patch set
doesn't help, sad to say.

I did make this if-condition read more simply, though.

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Ge...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Ge...
src/codegeneration/generator/GeneratorTransformer.js:148: // TODO: Is 'return'
allowed inside 'finally'?
> function f() {
>   try {
>     return 4;
>   } finally {
>     return 3;
>   }
> }
> 
> f()  // 3
> 
> I'm fine ignoring this edge case for now

I'm really hoping that behavior is an oversight, and isn't
officially recognized as legal JS. That's just odd and
slightly disturbing. Ionmonkey and V8 both agree on the
result, oddly enough.

If that's official, though, then you'd expect 'yield' in
'finally' to also be well-defined; the 'finally' wins over
any 'yield' crossing it.

So yeah, hoping this thing eventually goes away. This just
adds another level of mental mapping you need to do. When is
'return x' not really a 'return x'?

https://codereview.appspot.com/7447045/diff/1/src/runtime/runtime.js
File src/runtime/runtime.js (right):

https://codereview.appspot.com/7447045/diff/1/src/runtime/runtime.js#newcode361
src/runtime/runtime.js:361: return '[object StopIteration]';
On 2013/03/05 17:27:04, arv-chromium wrote:
> I find this confusing. Why is this masquerading as a
> StopIteration.

Initially, because it was the simplest code that would pass
all the tests. Siren song, you know?

  Strictly speaking, it should be a constructor for
  StopIteration-class objects. But that's not possible.

Still not too happy with this "solution" (see the matching
update to isStopIteration in GeneratorReturn.js).

https://codereview.appspot.com/7447045/diff/9001/test/feature/Yield/Generator...
File test/feature/Yield/GeneratorReturn.js (right):

https://codereview.appspot.com/7447045/diff/9001/test/feature/Yield/Generator...
test/feature/Yield/GeneratorReturn.js:17: String(s).match(/^\[object
GeneratorReturn .*\]$/));
TODO: Implement '@iter' isStopIteration so that we don't
have to use this (now-extended) hack.

Sign in to reply to this message.

usrbincc

Split the patches for cleaner history. See Move util functions from DefaultParametersTransformer.js to src/semantics/util.js https://codereview.appspot.com/7509043/ ...

13 years, 3 months ago (2013-03-06 16:18:24 UTC) #4

arv

Inline On Wed, Mar 6, 2013 at 8:03 AM, <usrbincc@yahoo.com> wrote: > Read over this ...

13 years, 3 months ago (2013-03-06 16:22:06 UTC) #5

Inline

On Wed, Mar 6, 2013 at 8:03 AM,  <usrbincc@yahoo.com> wrote:
> Read over this and honestly can't tell if this will come
> across as gibberish or not. It really is true about "not
> having the time to make it shorter", so apologies.
>
> ----
>
>
>> I'm not fully sure how this works. Why do we need a
>> GeneratorReturnLocal?
>
>
> Started writing some paragraphs and gave up on that.
>
> One-sentence summary: if we don't have to work with Q.js and
> Task.js, then we can do just about whatever we want.
>
> But code is probably the easiest way.
>
> Here is the extraction of a generator's return value. Q.js,
> Task.js, and ES6 do the same overall thing.
>
>   try {
>     while (g.next()) {}
>   } catch (e) {
>     if (isStopIteration(e)) {
>       if (e.value === undefined) {
>         console.log('returned');
>       } else {
>         console.log('returned a value: %s', e.value);
>       }
>     } else {
>       throw e;
>     }
>   }
>
> Here is isStopIteration for Q.js:
>
>   // straightforward implementation, not Q's, which uses the
>   // fascinating 'uncurryThis'.
>   function object_toString(obj) {
>     Object.prototype.toString.call(obj);
>   }
>   ...
>   object_toString(exception) === "[object StopIteration]" ||
>     exception instanceof QReturnValue
>
> and for Task.js:
>
>   e instanceof TaskResult || e instanceof StopIteration
>
> ----
>
> It's impossible to make an object obj such that
>
>   obj !== StopIteration &&
>     obj instanceof StopIteration &&
>     obj.value === arbitraryReturnValue;

I didn't know how much I hated StopIteration before this.
StopIteration is an Object, not a Function. It does not have a
prototype so following the ES5 semanctics "o instanceof StopIteration"
should be false for all values of o.

I just had an aha moment. We can get the crazy instanceof behavior by
doing something like this:

function StopIteration() {
  throw new TypeError('StopIteration is not a constructor');
}
StopIteration.prototype.toString = function() {
  return '[object StopIteration]';
};
StopIteration.__proto__ = StopIteration.prototype;
Object.freeze(StopIteration.prototype);
Object.freeze(StopIteration);

now StopIteration === StopIteration && StopIteration instanceof StopIteration

>   obj !== StopIteration &&
>     object_toString(obj) === '[object StopIteration]' &&
>     obj.value === arbitraryReturnValue;

Any time there is a .value the object cannot be a StopIteration since
StopIteration is non extensible and has no value property

>
>   Also note (ionmonkey):
>
>     js> StopIteration.value = 42;
>     42
>     js> StopIteration.value === 42;
>     false
>     js> StopIteration.value === undefined;
>     true

This is easy. If StopIteration is non extensible or if value is non
writable you get this behvior

> obj = Object.preventExtensions({})
{}
> obj.value = 42
42
> obj.value === 42
false
> obj.value === undefined
true

> So we need to instead get traceur to pass:
>
>   // Q.js
>   obj instanceof QReturnValue &&
>     obj.value === arbitraryReturnValue;
>   // Task.js
>   obj instanceof task.TaskResult &&
>     obj.value === arbitraryReturnValue;
>
> This is possible through library-specific settings:
>
>   // Q.js
>   <script>
>     // Need to set this magic symbol before loading Q,
>     // which will use it as its QReturnValue if it exists.
>     // We cannot get Q's QReturnValue; we can only set it.
>     var ReturnValue = traceur.runtime.GeneratorReturn;

This is dubious. Where did the Q guys get this ReturnValue from?

>   </script>
>   <script src="lib/q.js"></script>
>
>   // Task.js
>   <script src="lib/task.js"></script>
>   <script>
>     // Only this simple in patch set 2. More fiddly in 1.
>     // We cannot set TaskJS's TaskResult; we can only get
>     // it.
>     traceur.runtime.setGeneratorReturn(task.TaskResult);
>   </script>
>
> After that, you should be able to
>
>   // generated code
>   throw new traceur.runtime.GeneratorReturn(42);
>
> and have it properly recognized as a generator return value
> by Q or TaskJS.
>
>   And this is a corner case, but I thought I'd mention it:
>   throwing a real StopIteration is the only way to properly
>   exit a for-in in Firefox. I believe this is not spec, so
>   it's probably okay to ignore.

It is ok to ignore, we do not want for-in to use iteration.

>     function G1(n) {
>       for (var i = 0; i < n; i++) {
>         yield i;
>       }
>     }
>
>     for (var i in G1(10)) {
>       print(i);
>     }
>
>     function G2(n) {
>       var i = 0;
>       return {
>         __iterator__: function() {
>           return this;
>         },
>         next: function() {
>           if (i < n)
>             return i++;
>           throw StopIteration;
>         }
>       };
>     }
>
>     for (var i in G2(10)) {
>       print(i);
>     }
>
>   On the other hand, as browsers start half-implementing
>   es6, things will get more interesting. I'm hoping tc39
>   eventually decide to expose the 'StopIteration'
>   constructor.

Why? StopIteration is a singleton. There should never be a way to have
multiple instances of it.

> So that's why GeneratorReturn, though of course, anything
> that satisfies the constraints given would work. Or if we
> relax some of the constraints.

I'm still confused. What is the ES6 behavior of return? How does
Task.js handle return since return is not supported in SpiderMonkey
generators?

> ----
>
>
>> Your sample code uses traceur.runtime, something we should
>> not allow. In a perfect world we would not expose traceur
>> and traceur.runtime to user code.
>
>
> Searched around a little and found:
>
>   http://wiki.ecmascript.org/doku.php?id=harmony:iterators
>
>   There is a standard “@iter” module which provides the
>   following functionality:
>
>     isGenerator(x): returns true if x is a generator
>     function, false otherwise.
>
>     isStopIteration(x): returns true if x is an object whose
>     [[Class]] internal property is “StopIteration”.
>
> So it looks like isStopIteration has a home. I'll try doing
> that in a follow-up once I figure out how standard modules
> work in traceur.
>
>   [time passes] Fixed a bug preventing import of standard
>   modules in:
>
>   #--cut--
>   cat > iter.js <<"END"
>   import * from '@iter';
>   END
>   ./traceur --out iter.out.js iter.js
>   #--cut--
>
> To be pedantic, GeneratorReturn (or equivalent) needs to be
> exposed in a module somewhere also, but the spec is silent
> on this.
>
> ----
>
>
>
>
>
https://codereview.appspot.com/7447045/diff/1/src/codegeneration/GeneratorTra...
> File src/codegeneration/GeneratorTransformPass.js (right):
>
>
https://codereview.appspot.com/7447045/diff/1/src/codegeneration/GeneratorTra...
> src/codegeneration/GeneratorTransformPass.js:359: if (!(finder.hasYield
> || isGenerator) && !finder.hasAsync) {
> On 2013/03/05 17:27:04, arv-chromium wrote:
>>
>> Maybe we can skip the finder if isGenerator is true?
>
>
> In the isGenerator case, you still need YieldFinder, because
> of the finder.hasForIn later.
>
> This function is definitely a little messy with all the
> different paths for different options. And the new patch set
> doesn't help, sad to say.
>
> I did make this if-condition read more simply, though.
>
>
>
https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Ge...
> File src/codegeneration/generator/GeneratorTransformer.js (right):
>
>
https://codereview.appspot.com/7447045/diff/1/src/codegeneration/generator/Ge...
> src/codegeneration/generator/GeneratorTransformer.js:148: // TODO: Is
> 'return' allowed inside 'finally'?
>>
>> function f() {
>>
>>    try {
>>      return 4;
>>    } finally {
>>      return 3;
>>    }
>> }
>
>
>> f()  // 3
>
>
>> I'm fine ignoring this edge case for now
>
>
> I'm really hoping that behavior is an oversight, and isn't
> officially recognized as legal JS. That's just odd and
> slightly disturbing. Ionmonkey and V8 both agree on the
> result, oddly enough.
>
> If that's official, though, then you'd expect 'yield' in
> 'finally' to also be well-defined; the 'finally' wins over
> any 'yield' crossing it.
>
> So yeah, hoping this thing eventually goes away. This just
> adds another level of mental mapping you need to do. When is
> 'return x' not really a 'return x'?
>
>
> https://codereview.appspot.com/7447045/diff/1/src/runtime/runtime.js
> File src/runtime/runtime.js (right):
>
>
https://codereview.appspot.com/7447045/diff/1/src/runtime/runtime.js#newcode361
> src/runtime/runtime.js:361: return '[object StopIteration]';
> On 2013/03/05 17:27:04, arv-chromium wrote:
>>
>> I find this confusing. Why is this masquerading as a
>> StopIteration.
>
>
> Initially, because it was the simplest code that would pass
> all the tests. Siren song, you know?
>
>   Strictly speaking, it should be a constructor for
>   StopIteration-class objects. But that's not possible.
>
> Still not too happy with this "solution" (see the matching
> update to isStopIteration in GeneratorReturn.js).
>
>
https://codereview.appspot.com/7447045/diff/9001/test/feature/Yield/Generator...
> File test/feature/Yield/GeneratorReturn.js (right):
>
>
https://codereview.appspot.com/7447045/diff/9001/test/feature/Yield/Generator...
> test/feature/Yield/GeneratorReturn.js:17: String(s).match(/^\[object
> GeneratorReturn .*\]$/));
> TODO: Implement '@iter' isStopIteration so that we don't
> have to use this (now-extended) hack.
>
> https://codereview.appspot.com/7447045/



--
erik

Sign in to reply to this message.

usrbincc

Here is the relevant part of harmony:generators. Note especially the object R, and how they ...

13 years, 3 months ago (2013-03-07 15:01:59 UTC) #6

Here is the relevant part of harmony:generators. Note
especially the object R, and how they don't specify a
mechanism to construct it.

http://wiki.ecmascript.org/doku.php?id=harmony:generators

  The semantics of return e inside a generator function is:

  Let V ?= Evaluate(e)
  Let K = the current execution context
  Let O = K.currentGenerator
  O.[[State]] := “closed”
  Let R = a new object with
    [[Class]] = “StopIteration”
  R.value := V
  Throw R

So GeneratorReturn is a mechanism to construct 'R'. But
[[Class]] seems to obscure the point; all I really want is
for my thrown exception to end the iteration, pass its
'.value' on to whoever asked for it, and not end up as a
surprise stack trace on someone's screen.

[[Class]] is just the official way to do it. As long as we
test everything through some agreed-upon "isStopIteration"
then we can make any (computable) set of objects work like
StopIteration. The problem is that we can't get everyone to
agree. Or can we?

----

Both the Q and TaskJS libraries allow two object types to
end an iteration, the native StopIteration, and their own
internal version of 'R'.

Their implementations of 'R' are compatible, as long as you
set it up correctly. So for all intents and purposes, we can
think of it as one 'R'.

Their 'isStopIteration' equivalents differ slightly, but
you never have to think about that as long as you only throw
'R'. Trying to pass the StopIteration arm of the test is
like trying to jump over a wall when there's a door there.
(I know it's "traceur" and all, but...)

So the only real issue is traceur generators co-existing
with native generators. Traceur for-of needs to handle
native generators throwing native StopIteration as well as
traceur generators throwing 'R'.

  Eventually I need to draw up a full compatibility matrix.
  Or decide on what parts to ignore.

Traceur generators never have to throw native StopIteration,
except as an aesthetic choice. 'R' in StopIteration clothing
works fine, and gives you more freedom in implementation.

Here is what each iteration structure accepts:

  traceur for-of: native StopIteration, 'R'
  traceur yield*: native StopIteration, 'R'
  Q.async       : native StopIteration, 'R'
  task.spawn    : native StopIteration, 'R'
  ion for-in    : native StopIteration
  native for-of : native StopIteration, native 'R' *
  native yield* : native StopIteration, native 'R' *

Here is what each 'return' or exit produces:

  traceur return: native StopIteration, 'R'
  Q.return      :                       'R'
  TaskResult    :                       'R'
  ion gen exit  : native StopIteration, 'R' **
  native return : native StopIteration, native 'R' *

* noting, of course, that this doesn't exist yet.
** using Q or TaskJS api.

The current code's strategy:

  if (we have native StopIteration) {
    if (we are returning 'undefined')
      throw native StopIteration
    else
      throw an 'R' with '.value' === retVal;
  } else {
    RStopIteration = an 'R' with '.value' === 'undefined'
    if we are returning 'undefined'
      throw RStopIteration;
    else
      throw an 'R' with '.value' === retVal;
  }

You could use RStopIteration in both cases, and it wouldn't
make a difference until such time as native for-of loops and
yield* become available.

  When those become available, traceur generators won't be
  able to return values unless there's a way to construct
  native 'R'.

  But by that time, I suppose you'd just run the entire code
  base native.

With a dual object-type system, it's much easier to tell
that it's going to work, because we don't modify any
existing StopIteration or 'R'; we simply use what's there or
make our own if it's not there.

With a single object-type system, we have to modify what's
there, and make sure that nothing broke in the process. It
may be possible, but it requires more thought and testing to
be sure.

----

All this is possibly circling the subject and not really
getting at it. The real issue is probably this:

Imagine the ideal harmony environment.
- How would we create 'R'?
- Would we expose the mechanism? Maybe a constructor?
- What would 'R' be like? toString, instanceof, typeof, etc.
  Immutable, etc.
- if R.value === undefined, should we be able to tell it
  apart from StopIteration aside from the fact that
  R !== StopIteration ?

From this set of ideals, we can try to figure out what's
practical to implement.

----

> I didn't know how much I hated StopIteration before this.

I'm starting to feel the same way, but for different
reasons. I'm ambivalent about the instanceof-object
weirdness, but the trouble current JS engines have
optimizing exception-based code makes an exception-based
iterator protocol troubling.

On another level, it's a little annoying you suddenly need
to explicitly test "real exception or just StopIteration?"
as soon as you start using send(), close(), throw(). There
is no complexity increase for explicit moveNext + current,
in comparison.

  Though I do like the immediacy of 'g.next()' similar to
  '*p++'.

  // one way to prevent the out-of-sync error /be was
  // concerned about.
  while (v = g.next()) {
    console.log(v.value);
  }

If they can make it fast, then I can live with those
interface niggles, though. Cue the hot/crazy analogy of
codinghorror. Besides, if it's fast, I have the freedom to
wrap it to look however I want.

  I'm sure JS engines can even special-case common
  StopIteration testing patterns if that helps.

Incidentally, it occurred to me that you never have to touch
StopIteration if you structure your generators just so:

  // traceur compat layer
  traceur.options.unstarredGenerators = true;
  var print = print || console.log.bind(console);

  // runs in traceur and ionmonkey
  function G(r, n) {
    for (var i = 0; i < n; i++) {
      r.current = i;
      yield true;
    }
    yield false;
    // We should never get here.
  }

  function DartIterator(G) {
    this.g_ = G(this, [].slice.call(arguments, 1));
  }

  DartIterator.prototype = {
    moveNext: function(x) {
      return this.g_.send(x);
    }
  };

  var dg = new DartIterator(G, 8);
  while (dg.moveNext()) {
    print(dg.current);
  }

So you can enjoy exception-free code with minimal overhead.

  Assuming nothing in the generator returns or throws an
  exception or otherwise exits. Hence, the "just so" caveat.

You may even be able to make "getIterator" (whatever it's
called) return something to make it work seamlessly with
native for-of.

----

> now StopIteration === StopIteration &&
>   StopIteration instanceof StopIteration

Just to make sure: I originally meant the unmodified
StopIteration when I was referring to the impossibilities.

  // ionmonkey
  var StopIterationNative, g;
  try {
    g = function() { yield g; }();
    g.next().next();
  } catch(e) { StopIterationNative = e; }

On the other hand, that is a pretty neat hack, and for
TaskJS, would result in a StopIteration that would be
indistinguishable in usage on the source code level.

The Q test is something you can only get with JS engine
support, because it refers to a magical, untouchable
property. (If you stop to think about it, there is a lot of
magical stuff in JS. You get used to it, but still...)

----

> This is dubious. Where did the Q guys get this ReturnValue
> from?

It's a hack, but it's still nice that they provide such a
mechanism at all. Otherwise I'd have to patch Q. Here's the
code in question:

  (Testing direct-linking to github)

  https://github.com/kriskowal/q/blob/master/q.js#L253

    var QReturnValue;
    if (typeof ReturnValue !== "undefined") {
      QReturnValue = ReturnValue;
    } else {
      QReturnValue = function (value) {
        this.value = value;
      };
    }

Sign in to reply to this message.

arv

On Thu, Mar 7, 2013 at 10:01 AM, <usrbincc@yahoo.com> wrote: > Here is the relevant ...

13 years, 3 months ago (2013-03-07 15:48:13 UTC) #7

On Thu, Mar 7, 2013 at 10:01 AM, <usrbincc@yahoo.com> wrote:

> Here is the relevant part of harmony:generators. Note
> especially the object R, and how they don't specify a
> mechanism to construct it.
>
>
http://wiki.ecmascript.org/**doku.php?id=harmony:generators<http://wiki.ecmas...
>
>   The semantics of return e inside a generator function is:
>
>   Let V ?= Evaluate(e)
>   Let K = the current execution context
>   Let O = K.currentGenerator
>   O.[[State]] := “closed”
>   Let R = a new object with
>     [[Class]] = “StopIteration”
>   R.value := V
>   Throw R
>
> So GeneratorReturn is a mechanism to construct 'R'. But
> [[Class]] seems to obscure the point; all I really want is
> for my thrown exception to end the iteration, pass its
> '.value' on to whoever asked for it, and not end up as a
> surprise stack trace on someone's screen.
>
> [[Class]] is just the official way to do it. As long as we
> test everything through some agreed-upon "isStopIteration"
> then we can make any (computable) set of objects work like
> StopIteration. The problem is that we can't get everyone to
> agree. Or can we?
>

Thanks, this makes sense.

Since the spec and implementations do not agree we will have to wait and
see.

Their 'isStopIteration' equivalents differ slightly, but
> you never have to think about that as long as you only throw
> 'R'. Trying to pass the StopIteration arm of the test is
> like trying to jump over a wall when there's a door there.
> (I know it's "traceur" and all, but...)
>

LOL

So the only real issue is traceur generators co-existing
> with native generators. Traceur for-of needs to handle
> native generators throwing native StopIteration as well as
> traceur generators throwing 'R'.
>
>   Eventually I need to draw up a full compatibility matrix.
>   Or decide on what parts to ignore.
>
> Traceur generators never have to throw native StopIteration,
> except as an aesthetic choice. 'R' in StopIteration clothing
> works fine, and gives you more freedom in implementation.
>
> Here is what each iteration structure accepts:
>
>   traceur for-of: native StopIteration, 'R'
>   traceur yield*: native StopIteration, 'R'
>   Q.async       : native StopIteration, 'R'
>   task.spawn    : native StopIteration, 'R'
>   ion for-in    : native StopIteration
>   native for-of : native StopIteration, native 'R' *
>   native yield* : native StopIteration, native 'R' *
>
> Here is what each 'return' or exit produces:
>
>   traceur return: native StopIteration, 'R'
>   Q.return      :                       'R'
>   TaskResult    :                       'R'
>   ion gen exit  : native StopIteration, 'R' **
>   native return : native StopIteration, native 'R' *
>
> * noting, of course, that this doesn't exist yet.
>

Mozilla is shipping for-of


> ** using Q or TaskJS api.
>
> The current code's strategy:
>
>   if (we have native StopIteration) {
>     if (we are returning 'undefined')
>       throw native StopIteration
>     else
>       throw an 'R' with '.value' === retVal;
>   } else {
>     RStopIteration = an 'R' with '.value' === 'undefined'
>     if we are returning 'undefined'
>       throw RStopIteration;
>     else
>       throw an 'R' with '.value' === retVal;
>   }
>
> You could use RStopIteration in both cases, and it wouldn't
> make a difference until such time as native for-of loops and
> yield* become available.
>
>   When those become available, traceur generators won't be
>   able to return values unless there's a way to construct
>   native 'R'.
>
>   But by that time, I suppose you'd just run the entire code
>   base native.
>
> With a dual object-type system, it's much easier to tell
> that it's going to work, because we don't modify any
> existing StopIteration or 'R'; we simply use what's there or
> make our own if it's not there.
>
> With a single object-type system, we have to modify what's
> there, and make sure that nothing broke in the process. It
> may be possible, but it requires more thought and testing to
> be sure.
>
> ----
>
> All this is possibly circling the subject and not really
> getting at it. The real issue is probably this:
>
> Imagine the ideal harmony environment.
> - How would we create 'R'?
> - Would we expose the mechanism? Maybe a constructor?
> - What would 'R' be like? toString, instanceof, typeof, etc.
>   Immutable, etc.
> - if R.value === undefined, should we be able to tell it
>   apart from StopIteration aside from the fact that
>   R !== StopIteration ?
>
> From this set of ideals, we can try to figure out what's
> practical to implement.
>
> ----
>
>
>  I didn't know how much I hated StopIteration before this.
>>
>
> I'm starting to feel the same way, but for different
> reasons. I'm ambivalent about the instanceof-object
> weirdness, but the trouble current JS engines have
> optimizing exception-based code makes an exception-based
> iterator protocol troubling.
>

Yesterday there was some epic twitter fights related to this (all between
Moz people).


>
> On another level, it's a little annoying you suddenly need
> to explicitly test "real exception or just StopIteration?"
> as soon as you start using send(), close(), throw(). There
> is no complexity increase for explicit moveNext + current,
> in comparison.
>
>   Though I do like the immediacy of 'g.next()' similar to
>   '*p++'.
>
>   // one way to prevent the out-of-sync error /be was
>   // concerned about.
>   while (v = g.next()) {
>     console.log(v.value);
>   }
>

This looks promising to me:

https://gist.github.com/jorendorff/5102818

Dave Herman pointed out that null cannot be used for the end case because
the end case needs to encode the return value.

https://gist.github.com/anonymous/5108939


> If they can make it fast, then I can live with those
> interface niggles, though. Cue the hot/crazy analogy of
> codinghorror. Besides, if it's fast, I have the freedom to
> wrap it to look however I want.
>
>   I'm sure JS engines can even special-case common
>   StopIteration testing patterns if that helps.
>
> Incidentally, it occurred to me that you never have to touch
> StopIteration if you structure your generators just so:
>
>   // traceur compat layer
>   traceur.options.**unstarredGenerators = true;
>   var print = print || console.log.bind(console);
>
>   // runs in traceur and ionmonkey
>   function G(r, n) {
>
>     for (var i = 0; i < n; i++) {
>       r.current = i;
>       yield true;
>     }
>     yield false;
>     // We should never get here.
>   }
>
>   function DartIterator(G) {
>     this.g_ = G(this, [].slice.call(arguments, 1));
>   }
>
>   DartIterator.prototype = {
>     moveNext: function(x) {
>       return this.g_.send(x);
>     }
>   };
>
>   var dg = new DartIterator(G, 8);
>   while (dg.moveNext()) {
>     print(dg.current);
>   }
>
> So you can enjoy exception-free code with minimal overhead.
>
>   Assuming nothing in the generator returns or throws an
>   exception or otherwise exits. Hence, the "just so" caveat.
>
> You may even be able to make "getIterator" (whatever it's
> called) return something to make it work seamlessly with
> native for-of.
>
> ----
>
>
>  now StopIteration === StopIteration &&
>>    StopIteration instanceof StopIteration
>>
>
> Just to make sure: I originally meant the unmodified
> StopIteration when I was referring to the impossibilities.
>
>   // ionmonkey
>   var StopIterationNative, g;
>   try {
>     g = function() { yield g; }();
>     g.next().next();
>   } catch(e) { StopIterationNative = e; }
>
> On the other hand, that is a pretty neat hack, and for
> TaskJS, would result in a StopIteration that would be
> indistinguishable in usage on the source code level.
>
> The Q test is something you can only get with JS engine
> support, because it refers to a magical, untouchable
> property. (If you stop to think about it, there is a lot of
> magical stuff in JS. You get used to it, but still...)
>
> ----
>
>
>  This is dubious. Where did the Q guys get this ReturnValue
>> from?
>>
>
> It's a hack, but it's still nice that they provide such a
> mechanism at all. Otherwise I'd have to patch Q. Here's the
> code in question:
>
>   (Testing direct-linking to github)
>
>  
https://github.com/kriskowal/**q/blob/master/q.js#L253<https://github.com/kri...
>
>     var QReturnValue;
>     if (typeof ReturnValue !== "undefined") {
>       QReturnValue = ReturnValue;
>     } else {
>       QReturnValue = function (value) {
>         this.value = value;
>       };
>     }
>
>
>
https://codereview.appspot.**com/7447045/<https://codereview.appspot.com/7447...
>

That was what I was referring to. Are we planning to set ReturnValue?


I'll take another look at the patch now. I think at this point whatever we
do to make this work is fine as a first step.

-- 
erik

Sign in to reply to this message.

arv

LGTM https://codereview.appspot.com/7447045/diff/21001/src/codegeneration/generator/GeneratorTransformer.js File src/codegeneration/generator/GeneratorTransformer.js (right): https://codereview.appspot.com/7447045/diff/21001/src/codegeneration/generator/GeneratorTransformer.js#newcode284 src/codegeneration/generator/GeneratorTransformer.js:284: throw new traceur.runtime.GeneratorReturn(${$YIELD_RETURN}); OK for now but I ...

13 years, 3 months ago (2013-03-07 16:21:59 UTC) #8

usrbincc

> Mozilla is shipping for-of That is a surprise. I haven't updated my local ionmonkey ...

13 years, 3 months ago (2013-03-07 16:41:06 UTC) #10

arv

> > Oops; forgot about return values. Ironic, considering this > patch. I kind of ...

13 years, 3 months ago (2013-03-07 19:37:11 UTC) #11

peterhal

C# doesn't support return in generators. On Thu, Mar 7, 2013 at 11:36 AM, Erik ...

13 years, 3 months ago (2013-03-07 20:03:58 UTC) #12

usrbincc

> Yesterday there was some epic twitter fights related to > this (all between Moz ...

13 years, 3 months ago (2013-03-08 17:01:51 UTC) #13

Message was sent while issue was closed.

> Yesterday there was some epic twitter fights related to
> this (all between Moz people).

About this. My understanding is that the current generator
protocol is pretty much set in stone, so is this dissent
probably just going to raise a hue and cry that amounts to
nothing?

> Task.js really needs return in generators. Python added
> them too.

Didn't realize Python had return in generators. But in what
I think is a stroke of wisdom, they don't allow return
values, just plain return.

  Checked python2 and python3, and they're both the same in
  this respect.

In that case, it becomes a control-flow statement only, and
so the "return in finally" thing doesn't seem nearly as odd.
There is no return value to overwrite.

----

Can't say that I understand Task.js very well, but since
there is no native generator 'return' yet, it's probably
only using it to return values (throwing 'R').

  True 'return' as control-flow can be useful when breaking
  out of nested try-catch blocks.

    // But this is a workaround.
    function* G() {
      returnLabel:
      do {
        try {
          try {
            break returnLabel; // get me outta here!
          } catch(e) { ... }
        } catch(e) { ... }
      } while(0);
    }

Taking a cue from Python, I think it's having a return value
that really complicates things.

If we only allowed plain 'return', we wouldn't have to deal
with 'R', and just plain StopIteration would be enough. No
need to worry about hidden constructors or any of that.

  This patch would have been much simpler without return
  values.

And if someone really needs to return something, they can
always set a value or do a callback somewhere. We just need
to have the producers and consumers agree to make it work.

So maybe the real issue is an attempt to standardize the
mechanism. But different mechanisms work better in different
situations. And even as a base-level option, I'm starting to
think return values over-complicate things in generators.

----

> C# doesn't support return in generators.

That made me curious, so I did a little searching.

Iterators (C# and Visual Basic)
http://msdn.microsoft.com/en-us/library/dscyy5s0.aspx

  You can use an Exit Function or Return statement (Visual
  Basic) or yield break statement (C#) to end the iteration.

I don't use .NET, so I don't know how accurate this info is
(I always take msdn with a grain of salt) but it seems that
they follow the Python route with no return values. I'm
starting to wonder if any other generator besides harmony's
has them.

Sign in to reply to this message.

peterhal

On 2013/03/08 17:01:51, usrbincc wrote: > > C# doesn't support return in generators. > > ...

13 years, 3 months ago (2013-03-08 18:58:20 UTC) #14

usrbincc

13 years, 3 months ago (2013-03-08 21:26:43 UTC) #15

Message was sent while issue was closed.

> Somehow I was under the impression that python allowed
> returning values from generators but I can't find any
> evidence of that. I agree that returning values from
> generators is overly complex.

While looking through

  comparing different iteration protocols
  https://gist.github.com/dherman/5101199

I stumbled upon

  PEP 380 -- Syntax for Delegating to a Subgenerator
  http://www.python.org/dev/peps/pep-0380/

and it looks like return values for generators have "Final"
status, and are slated for Python 3.3 -- the 'python3'
executable I checked with was Python 3.2.3 . Oh well.

  There was this other language that had lots of features,
  and you weren't supposed to pay for the features you
  didn't use, but someone always used one of them, so you
  did end up paying, at least in mental real estate.

  /rant

As with any language feature, there are probably good ways
to use generator return values. At this point, I just hope I
don't have to deal with too much code of the other variety.

Sign in to reply to this message.

Issue 7447045: Implement 'return' in generators. (Closed)

Description

Patch Set 1 #

Patch Set 2 : Fix GeneratorReturn.toString, excess YieldFinder, isUndefined(), indentation. #

Patch Set 3 : Identical to patch set 2 with semantics/util.js, etc split out. #

Patch Set 4 : Oops. Added missing brace. #

Messages