arrange to create prototypes for generators

* src/generator.js: Add methods and intialization for generator meta-objects.

* src/contexts.h:
* src/bootstrapper.cc (InitializeExperimentalGlobal): Make generator
  meta-objects, and store maps for constructing generator functions
  and their prototypes.

* src/factory.h:
* src/factory.cc (MapForNewFunction): New helper.
  (NewFunctionFromSharedFunctionInfo): Use the new helper.

* src/heap.cc (AllocateFunctionPrototype, AllocateInitialMap): For
   generators, allocate appropriate prototypes and maps.

* src/code-stubs.h:
* src/arm/code-stubs-arm.h:
* src/arm/full-codegen-arm.h:
* src/ia32/code-stubs-ia32.h:
* src/ia32/full-codegen-ia32.h:
* src/x64/code-stubs-x64.h:
* src/x64/full-codegen-x64.h: Allow fast closure creation for generators,
using the appropriate map.

* test/mjsunit/harmony/builtins.js: Add a special case for
  GeneratorFunctionPrototype.prototype.__proto__.

BUG=
TEST=mjsunit/harmony/generators-runtime

Committed: http://code.google.com/p/v8/source/detail?r=14236

[wingo, 2013-03-28 17:41:31]

Based on https://codereview.chromium.org/13176002/ and
https://codereview.chromium.org/13179002/.

This changeset basically sets up the strange prototype structure described by
the standard.  It has a couple of failing tests however; it's strange, I set
some properties on snapshotted items in v8native.js like "prototype" but then
later I see completely different values.  Failures are marked with TODO in the
tests.

[wingo, 2013-04-02 08:34:46]

This issue needs some changes related to the new spec in
http://wiki.ecmascript.org/lib/exe/fetch.php?cache=cache&media=harmony:es6_ge....
 The new spec requires much less per-generator-function work so I will be doing
more of it in C++, and not calling out to a helper.  Hope to upload something
later today.

[wingo, 2013-04-02 12:30:54]

This updated patchset should conform to the newer TC39 drafts.  It also
consistently passes tests, unlike the previous patchset.

[rossberg, 2013-04-09 16:44:14]

First round of comments. I'm not sure I fully understand the way you initialize
these objects, see inline comments.

https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc#newco...
src/bootstrapper.cc:1478: if (FLAG_harmony_generators) {
Why do these have to be created in a different manner than the other Harmony
constructor functions (see InitializeExperimentalGlobals above, and respective
.js files)? In particular, it feels a bit dubious to initialize the maps in the
context _after_ generator.js has already executed.

https://codereview.chromium.org/13192004/diff/18001/src/factory.cc
File src/factory.cc (right):

https://codereview.chromium.org/13192004/diff/18001/src/factory.cc#newcode616
src/factory.cc:616: // they need their prototypes to be created eagerly.
I don't understand. Why can't you make the case distinction in
AllocateFunctionPrototype? Why does it have to be eager?

https://codereview.chromium.org/13192004/diff/18001/src/generators.js
File src/generators.js (right):

https://codereview.chromium.org/13192004/diff/18001/src/generators.js#newcode61
src/generators.js:61: %FunctionSetPrototype(GeneratorFunctionPrototype,
GeneratorIteratorPrototype);
Don't you also need to set GeneratorIteratorPrototype.constructor somewhere?

https://codereview.chromium.org/13192004/diff/18001/src/generators.js#newcode65
src/generators.js:65: $Array("next", GeneratorIteratorNext,
Nit: Use array literal here.

https://codereview.chromium.org/13192004/diff/18001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/13192004/diff/18001/src/heap.cc#newcode3972
src/heap.cc:3972: // Generator prototypes can share maps since they don't have
"constructor"
Hm, according to the class diagram they have.

https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/builtins.js
File test/mjsunit/builtins.js (right):

https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/builtins.js#...
test/mjsunit/builtins.js:58: if (name == "GeneratorFunctionPrototype")
Nit: use ?:

https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/harmony/gene...
File test/mjsunit/harmony/generators-runtime.js (right):

https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/harmony/gene...
test/mjsunit/harmony/generators-runtime.js:123: TestGeneratorFunctionInstance();
Nit: separating the calls from the definitions makes it easier to overlook a
missing call.

https://codereview.chromium.org/13192004/diff/18001/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):

https://codereview.chromium.org/13192004/diff/18001/tools/gyp/v8.gyp#newcode802
tools/gyp/v8.gyp:802: '../../src/generators.js'
Nit: I'd name this file generator.js (singular), in correspondence with the
others.

[wingo, 2013-04-10 08:13:44]

Thanks for the review.

>
https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc#newco...
> src/bootstrapper.cc:1478: if (FLAG_harmony_generators) {
> Why do these have to be created in a different manner than the other Harmony
> constructor functions (see InitializeExperimentalGlobals above, and respective
> .js files)? In particular, it feels a bit dubious to initialize the maps in
the
> context _after_ generator.js has already executed.

These initializations are not appropriate for InitializeExperimentalGlobal
because these generator meta-objects are not given public names, and so are on
the builtins object but not the global object.

I think given some of your other comments that my patch was not clear.  Let me
try to give an overview.

The current class diagram is here:
http://wiki.ecmascript.org/lib/exe/fetch.php?cache=cache&media=harmony:es6_ge...

Note that this is a different diagram than the one we were looking at in March.

The spec defines three "levels" of object, if we consider __proto__ to link
between levels of derived objects and their base objects.  Unfortunately the
spec does not provide clear vocabulary for how to talk about them, but I'll do
my best here:

 1. Base objects
     GeneratorFunction (called "(GeneratorFunction)" in the spec)
     GeneratorFunctionPrototype ("(Generator)")
     GeneratorIteratorPrototype ("(Generator).prototype")

These have .prototype and .constructor links between them.  These are the
objects defined by generators.js.

 2. Per-generator objects
     Derivations of GeneratorFunction -- these are the generator functions, e.g.
function*(){}.
     Derivations of GeneratorIteratorPrototype -- (function*(){}).prototype

These objects are made when generator function literals are instantiated.

 3. Generator iterators (or objects or instances)
     Derivations of per-generator prototypes, whose proto is
(function*(){}).prototype

These are made when a generator function is called.

To address your comment, as the diagram notes, the per-generator prototypes do
not have a constructor property, so all derivations of
GeneratorIteratorPrototype can share the same map, because they have the same
properties (no properties).  So the only thing we need to hold on to is the map
to use when creating those prototypes.

The other two maps are for generator functions, in strict and non-strict mode. 
They are not like the other maps because there are no other kinds of functions,
so there is nothing to compare to -- all other function literals have
Function.prototype as their prototype.  Hence the need to explicitly deal with
strict-mode and non-strict mode here.

> https://codereview.chromium.org/13192004/diff/18001/src/factory.cc
> File src/factory.cc (right):
> 
> https://codereview.chromium.org/13192004/diff/18001/src/factory.cc#newcode616
> src/factory.cc:616: // they need their prototypes to be created eagerly.
> I don't understand. Why can't you make the case distinction in
> AllocateFunctionPrototype? Why does it have to be eager?

I don't know.  Maybe lazy allocation can work.  I'll tell you why I did it this
way, though.

Generator functions are unlike other functions because they don't construct
their instances in the normal way; g() is the same as new g() and it will never
put any other set of properties on the returned generator.  The instance of a
generator has a special type (in a later patch), not JS_OBJECT_TYPE.  Even if
you explicitly set another prototype on the generator, the map should still
produce JS_GENERATOR_ITERATOR_TYPE objects.  There's no need for slack tracking
on generators.  I couldn't fit all of this in my head and still allow for lazy
initial_map() creation.  And indeed, the issue isn't so much the prototype as
the initial_map for generator instances; and can one lazily allocate the
prototype while eagerly allocating the initial_map?  That didn't appear possible
to me.

>
https://codereview.chromium.org/13192004/diff/18001/src/generators.js#newcode61
> src/generators.js:61: %FunctionSetPrototype(GeneratorFunctionPrototype,
> GeneratorIteratorPrototype);
> Don't you also need to set GeneratorIteratorPrototype.constructor somewhere?

Interesting question!  If I did so, it would provide a constructor property to
all generator instances, no?  That sounds like it could be a spec bug.

>
https://codereview.chromium.org/13192004/diff/18001/src/generators.js#newcode65
> src/generators.js:65: $Array("next", GeneratorIteratorNext,
> Nit: Use array literal here.

OK.

> https://codereview.chromium.org/13192004/diff/18001/src/heap.cc#newcode3972
> src/heap.cc:3972: // Generator prototypes can share maps since they don't have
> "constructor"
> Hm, according to the class diagram they have.

Could you perhaps be working from the 27 November diagram instead of the 31
March diagram>

> https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/builtins.js
> File test/mjsunit/builtins.js (right):
> 
>
https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/builtins.js#...
> test/mjsunit/builtins.js:58: if (name == "GeneratorFunctionPrototype")
> Nit: use ?:

OK

>
https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/harmony/gene...
> File test/mjsunit/harmony/generators-runtime.js (right):
> 
>
https://codereview.chromium.org/13192004/diff/18001/test/mjsunit/harmony/gene...
> test/mjsunit/harmony/generators-runtime.js:123:
TestGeneratorFunctionInstance();
> Nit: separating the calls from the definitions makes it easier to overlook a
> missing call.

OK

> https://codereview.chromium.org/13192004/diff/18001/tools/gyp/v8.gyp
> File tools/gyp/v8.gyp (right):
> 
>
https://codereview.chromium.org/13192004/diff/18001/tools/gyp/v8.gyp#newcode802
> tools/gyp/v8.gyp:802: '../../src/generators.js'
> Nit: I'd name this file generator.js (singular), in correspondence with the
> others.

OK

Thanks again for the review and for your future thoughts on lazy allocation and
on intializing the rat's nest of meta-objects.

[wingo, 2013-04-10 09:02:21]

Updated renaming "generator iterators" to "generator object", also addressing
nits.

Open questions about bootstrap.cc, lazy prototype allocation, and presence of
"constructor" on GeneratorObjectPrototype remain.

[rossberg, 2013-04-10 11:47:23]

>
https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc#newco...
> > src/bootstrapper.cc:1478: if (FLAG_harmony_generators) {
> > Why do these have to be created in a different manner than the other Harmony
> > constructor functions (see InitializeExperimentalGlobals above, and
respective
> > .js files)? In particular, it feels a bit dubious to initialize the maps in
> the
> > context _after_ generator.js has already executed.
> 
> These initializations are not appropriate for InitializeExperimentalGlobal
> because these generator meta-objects are not given public names, and so are on
> the builtins object but not the global object.

I see what you're saying, but nevertheless I think they belong there. They are
still "global", despite not being visible on the global object. Note that the
plain InitializeGlobal (for non-experimentals) also sets up a range of things
that do not correspond to properties of the global object.

(Maybe the function names Initialize[Experimental]Global aren't entirely
accurate. They should really be named Initialize[Experimental]Intrinsics, or
something. But who cares at this point... :) )

So, I think the creation/initialization of the generator intrinsics should
follow the pattern used for others, even if they are not installed on the global
object. That should also eliminate the need for your LOOKUP_NATIVE macro.


> I think given some of your other comments that my patch was not clear.  Let me
> try to give an overview.
> 
> The current class diagram is here:
>
http://wiki.ecmascript.org/lib/exe/fetch.php?cache=cache&media=harmony:es6_ge...
> 
> Note that this is a different diagram than the one we were looking at in
March.
> 
> The spec defines three "levels" of object, if we consider __proto__ to link
> between levels of derived objects and their base objects.  Unfortunately the
> spec does not provide clear vocabulary for how to talk about them, but I'll do
> my best here:
> 
>  1. Base objects
>      GeneratorFunction (called "(GeneratorFunction)" in the spec)
>      GeneratorFunctionPrototype ("(Generator)")
>      GeneratorIteratorPrototype ("(Generator).prototype")
> 
> These have .prototype and .constructor links between them.  These are the
> objects defined by generators.js.
> 
>  2. Per-generator objects
>      Derivations of GeneratorFunction -- these are the generator functions,
e.g.
> function*(){}.
>      Derivations of GeneratorIteratorPrototype -- (function*(){}).prototype
> 
> These objects are made when generator function literals are instantiated.
> 
>  3. Generator iterators (or objects or instances)
>      Derivations of per-generator prototypes, whose proto is
> (function*(){}).prototype
> 
> These are made when a generator function is called.
> 
> To address your comment, as the diagram notes, the per-generator prototypes do
> not have a constructor property, so all derivations of
> GeneratorIteratorPrototype can share the same map, because they have the same
> properties (no properties).  So the only thing we need to hold on to is the
map
> to use when creating those prototypes.
> 
> The other two maps are for generator functions, in strict and non-strict mode.

> They are not like the other maps because there are no other kinds of
functions,
> so there is nothing to compare to -- all other function literals have
> Function.prototype as their prototype.  Hence the need to explicitly deal with
> strict-mode and non-strict mode here.

Yes, all makes sense.


> > https://codereview.chromium.org/13192004/diff/18001/src/factory.cc
> > File src/factory.cc (right):
> > 
> >
https://codereview.chromium.org/13192004/diff/18001/src/factory.cc#newcode616
> > src/factory.cc:616: // they need their prototypes to be created eagerly.
> > I don't understand. Why can't you make the case distinction in
> > AllocateFunctionPrototype? Why does it have to be eager?
> 
> I don't know.  Maybe lazy allocation can work.  I'll tell you why I did it
this
> way, though.
> 
> Generator functions are unlike other functions because they don't construct
> their instances in the normal way; g() is the same as new g() and it will
never
> put any other set of properties on the returned generator.

FWIW, not sure if that matters, but the same is true for some built-ins, e.g.
Array.

> The instance of a
> generator has a special type (in a later patch), not JS_OBJECT_TYPE.  Even if
> you explicitly set another prototype on the generator, the map should still
> produce JS_GENERATOR_ITERATOR_TYPE objects.  There's no need for slack
tracking
> on generators.  I couldn't fit all of this in my head and still allow for lazy
> initial_map() creation.  And indeed, the issue isn't so much the prototype as
> the initial_map for generator instances; and can one lazily allocate the
> prototype while eagerly allocating the initial_map?  That didn't appear
possible
> to me.

Hm, I'm afraid I'm still confused how it follows from your observations that the
map has to be created eagerly.

Maybe you are right, but would it be reasonable to keep everything as symmetric
as possible for now (wrt ordinary functions), and defer any divergent
restructuring until the need for it becomes apparent?


>
https://codereview.chromium.org/13192004/diff/18001/src/generators.js#newcode61
> > src/generators.js:61: %FunctionSetPrototype(GeneratorFunctionPrototype,
> > GeneratorIteratorPrototype);
> > Don't you also need to set GeneratorIteratorPrototype.constructor somewhere?
> 
> Interesting question!  If I did so, it would provide a constructor property to
> all generator instances, no?  That sounds like it could be a spec bug.

Well, that seems to be what the diagram is saying. For the time being I'd assume
it's intentional. Admittedly, given the mess that prototype inheritance is I
never know for sure (and the diagram seems to have a bug at least in that
"(Generator).prototype" has a constructor link, but indicates no <<constructor>>
property -- but the same is true for f.prototype). Anyway, I'll ask on
es-discuss.


>
https://codereview.chromium.org/13192004/diff/18001/src/generators.js#newcode65
> > src/generators.js:65: $Array("next", GeneratorIteratorNext,
> > Nit: Use array literal here.
> 
> OK.
> 
> > https://codereview.chromium.org/13192004/diff/18001/src/heap.cc#newcode3972
> > src/heap.cc:3972: // Generator prototypes can share maps since they don't
have
> > "constructor"
> > Hm, according to the class diagram they have.
> 
> Could you perhaps be working from the 27 November diagram instead of the 31
> March diagram>

No, but I was confusing it with GeneratorIteratorPrototype. So ignore that
comment. :)

[rossberg, 2013-04-10 11:53:49]

On 2013/04/10 11:47:23, rossberg wrote:
>
https://codereview.chromium.org/13192004/diff/18001/src/generators.js#newcode61
> > > src/generators.js:61: %FunctionSetPrototype(GeneratorFunctionPrototype,
> > > GeneratorIteratorPrototype);
> > > Don't you also need to set GeneratorIteratorPrototype.constructor
somewhere?
> > 
> > Interesting question!  If I did so, it would provide a constructor property
to
> > all generator instances, no?  That sounds like it could be a spec bug.
> 
> Well, that seems to be what the diagram is saying. For the time being I'd
assume
> it's intentional. Admittedly, given the mess that prototype inheritance is I
> never know for sure (and the diagram seems to have a bug at least in that
> "(Generator).prototype" has a constructor link, but indicates no
<<constructor>>
> property -- but the same is true for f.prototype). Anyway, I'll ask on
> es-discuss.

I just saw that it was already discussed, and is indeed intentional. Also, that
trying to call it generates a TypeError because it's not callable.

[wingo, 2013-04-10 14:28:52]

Hi, a question about initialization of the rat's nest of meta-objects.  Thanks
:)

https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc#newco...
src/bootstrapper.cc:1478: if (FLAG_harmony_generators) {
On 2013/04/09 16:44:14, rossberg wrote:
> Why do these have to be created in a different manner than the other Harmony
> constructor functions (see InitializeExperimentalGlobals above, and respective
> .js files)? In particular, it feels a bit dubious to initialize the maps in
the
> context _after_ generator.js has already executed.

I have taken a look at this and I don't catch your meaning.  The code in my
patch makes maps not for the meta-objects themselves (the base level), but to
use when constructing per-generator objects (the middle level).  If I use
InstallFunction to make e.g. GeneratorFunctionObject in
InitializeExperimentalGlobals, that happens after generator.js runs, so I don't
have a chance to attach the next, send, etc functions on the
GeneratorFunctionObject.  I could of course do so in C++ but it seems better to
do that in JS, no?  And there's more to do there -- for example, I need to set
GeneratorFunctions's code to look like Function's, but to create a generator.

Have I misunderstood what you were getting at?

[wingo, 2013-04-10 14:44:28]

https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/13192004/diff/18001/src/bootstrapper.cc#newco...
src/bootstrapper.cc:1478: if (FLAG_harmony_generators) {
On 2013/04/10 14:28:52, wingo wrote:
> Have I misunderstood what you were getting at?

I think I misunderstood and found my error.  Please ignore the above comment.

[wingo, 2013-04-10 16:11:43]

Patch updated to create meta-objects in C++ using InstallFunction et al.

I'm not sure it's worth looking into lazy map creation -- that seems to be
something you can only do if the instance type is not JS_OBJECT_TYPE, and the
type will change in the next patch.  But perhaps it's worth it just to see that
the mechanism works correctly.  I'll look at it tomorrow.

[wingo, 2013-04-11 10:04:01]

Andreas, you were totally right: lazy map initialization appears to work fine. 
PTAL at the new patch.

Thanks!

[rossberg, 2013-04-11 13:05:19]

LGTM with nits

https://codereview.chromium.org/13192004/diff/54001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

https://codereview.chromium.org/13192004/diff/54001/src/arm/code-stubs-arm.cc...
src/arm/code-stubs-arm.cc:184: int map_index = is_generator_
Nit: it would be nice if this could be moved into some abstraction, since the
same logic is repeated at least 4 times (3 back-ends, and MapForNewFunction in
factory.cc).

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/builtins.js
File test/mjsunit/builtins.js (right):

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/builtins.js#...
test/mjsunit/builtins.js:57: var expected_prototype_of_prototype =
Nit: could inline this.

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/harmony/gene...
File test/mjsunit/harmony/generators-runtime.js (right):

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/harmony/gene...
test/mjsunit/harmony/generators-runtime.js:87: var expected_property_names =
["next", "send", "throw", "close", "constructor"];
Line too long.

[wingo, 2013-04-11 14:38:44]

New patches fixes nits, adds Context::FunctionMapIndex, also moves some more
initialization into generators.js.

https://codereview.chromium.org/13192004/diff/54001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

https://codereview.chromium.org/13192004/diff/54001/src/arm/code-stubs-arm.cc...
src/arm/code-stubs-arm.cc:184: int map_index = is_generator_
On 2013/04/11 13:05:20, rossberg wrote:
> Nit: it would be nice if this could be moved into some abstraction, since the
> same logic is repeated at least 4 times (3 back-ends, and MapForNewFunction in
> factory.cc).

Done.

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/builtins.js
File test/mjsunit/builtins.js (right):

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/builtins.js#...
test/mjsunit/builtins.js:57: var expected_prototype_of_prototype =
On 2013/04/11 13:05:20, rossberg wrote:
> Nit: could inline this.

Done.

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/harmony/gene...
File test/mjsunit/harmony/generators-runtime.js (right):

https://codereview.chromium.org/13192004/diff/54001/test/mjsunit/harmony/gene...
test/mjsunit/harmony/generators-runtime.js:87: var expected_property_names =
["next", "send", "throw", "close", "constructor"];
On 2013/04/11 13:05:20, rossberg wrote:
> Line too long.

Done.

[rossberg, 2013-04-11 14:47:17]

Looks good, I'll land it

[rossberg, 2013-04-11 14:55:50]

On 2013/04/11 14:47:17, rossberg wrote:
> Looks good, I'll land it

Hm, I'm getting several test failures from generators-runtime.js, could you look
into that?

[wingo, 2013-04-11 15:16:43]

On 2013/04/11 14:55:50, rossberg wrote:
> On 2013/04/11 14:47:17, rossberg wrote:
> > Looks good, I'll land it
> 
> Hm, I'm getting several test failures from generators-runtime.js, could you
look
> into that?

Strange: I didn't have them on my branch, but now that I've merged master I do
have them.  Will fix and sorry for the inconvenience.

[wingo, 2013-04-11 15:48:22]

On 2013/04/11 15:16:43, wingo wrote:
> On 2013/04/11 14:55:50, rossberg wrote:
> > On 2013/04/11 14:47:17, rossberg wrote:
> > > Looks good, I'll land it
> > 
> > Hm, I'm getting several test failures from generators-runtime.js, could you
> look
> > into that?
> 
> Strange: I didn't have them on my branch, but now that I've merged master I do
> have them.  Will fix and sorry for the inconvenience.

I bisected and https://v8.googlecode.com/svn/branches/bleeding_edge@14229
introduced the error it seems.  Working on it, but you might want to know that.

[wingo, 2013-04-11 16:02:22]

Updated patch explicitly adds constructor properties on meta-objects, adapting
to recent change in bleeding_edge's NewFunctionWithPrototype.

[rossberg, 2013-04-11 16:12:17]

On 2013/04/11 16:02:22, wingo wrote:
> Updated patch explicitly adds constructor properties on meta-objects, adapting
> to recent change in bleeding_edge's NewFunctionWithPrototype.

Ah, right. Sorry, I should have noticed that this might affect your CL.

Will land.

[rossberg, 2013-04-11 16:29:58]

Committed patchset #9 manually as r14236 (presubmit successful).

[Michael Starzinger, 2013-04-11 18:43:47]

Late drive-by.

https://codereview.chromium.org/13192004/diff/64005/src/generator.js
File src/generator.js (right):

https://codereview.chromium.org/13192004/diff/64005/src/generator.js#newcode28
src/generator.js:28: // This file relies on the fact that the following
declarations have been made
Late drive-by comment: Can we make this file "use strict". Even though it will
take longer to switch all old builtins strict I would advocate for writing new
ones in strict mode.