Use RuntimeInliner to provide the generator common code. (re-remove TODOs)

Use RuntimeInliner to provide the generator common code.

BUG=None
TEST=None

[arv, 2013-03-13 22:28:44]

LGTM

https://codereview.appspot.com/7487046/diff/2001/src/codegeneration/generator...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7487046/diff/2001/src/codegeneration/generator...
src/codegeneration/generator/GeneratorTransformer.js:88: * @param
{ErrorReporter} reporter
Add @param for runtimeInliner too

[usrbincc, 2013-03-14 21:22:14]

Updated JSDoc for GeneratorTransformer constructor.

I learned from this "patch series" experiment that if possible, you
should do your var renames and reindents early, preferably at the very
beginning. Anything requiring manual merge is an opportunity for error.
Other than that, I think it went relatively smoothly for a first try.

----

Patch order at a glance:

Make generator common code depend only upon constants.
https://codereview.appspot.com/7761043
Use generator 'return' to simplify the code for 'close()'
https://codereview.appspot.com/7519046
Checking undefined properties is slow, so cache in a boolean.
https://codereview.appspot.com/7663044
Use an inner object instead of captured vars.
https://codereview.appspot.com/7547045
Minimal diff for converting generator interface wrap to a function.
https://codereview.appspot.com/7547046
Use RuntimeInliner to provide the generator common code.
https://codereview.appspot.com/7487046

[usrbincc, 2013-03-15 16:21:58]

https://codereview.appspot.com/7487046/diff/6001/src/codegeneration/generator...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7487046/diff/6001/src/codegeneration/generator...
src/codegeneration/generator/GeneratorTransformer.js:239: // TODO: Look into if
this code can be shared between generator
While working on the next patch, I realized that this part snuck back in
during a merge snafu. Removing it again. Hopefully it doesn't reanimate.

[arv, 2013-03-17 17:03:06]

LGTM

https://codereview.appspot.com/7487046/diff/10002/src/codegeneration/generato...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7487046/diff/10002/src/codegeneration/generato...
src/codegeneration/generator/GeneratorTransformer.js:238: var $generatorWrap =
this.runtimeInliner_.get('generatorWrap',
no need to prefix this with $

[usrbincc, 2013-03-18 18:24:59]

https://codereview.appspot.com/7487046/diff/10002/src/codegeneration/generato...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7487046/diff/10002/src/codegeneration/generato...
src/codegeneration/generator/GeneratorTransformer.js:238: var $generatorWrap =
this.runtimeInliner_.get('generatorWrap',
On 2013/03/17 17:03:06, arv-chromium wrote:
> no need to prefix this with $

I went ahead and took out all $varName (that I happened to notice);
suddenly realized there were potential clashes with generated code.

https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generato...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generato...
src/codegeneration/generator/GeneratorTransformer.js:363: * @param
{RuntimeInliner} runtimeInliner
This is what happens when people are forced to manually type-check.

[arv, 2013-03-18 18:59:55]

LGTM

Are we ready to land these?

https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generato...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generato...
src/codegeneration/generator/GeneratorTransformer.js:363: * @param
{RuntimeInliner} runtimeInliner
On 2013/03/18 18:25:00, usrbincc wrote:
> This is what happens when people are forced to manually type-check.

Peter started this work... I'm looking forward to progress in that area.

[usrbincc, 2013-03-18 20:03:44]

> Are we ready to land these?

I think this patch series is ready to go as long as long as you don't
notice anything else. I'm pretty sure something will show up no matter
what, though. (Gotten somewhat resigned to that)

I'll go ahead and get the next issue (move main state machine code into
separate function from try-catch) merged with the latest from here.

https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generato...
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generato...
src/codegeneration/generator/GeneratorTransformer.js:363: * @param
{RuntimeInliner} runtimeInliner
> Peter started this work... I'm looking forward to progress in that area.

Compilers has always sort of been an area of interest, but it's never
been my strong point. Not 100% sure which area you're referring to, but
if you mean making the compiler use the JSDoc for type-checking, then
I wouldn't know where to start. It'd theoretically be trivial to turn
the JSDoc into runtime checks, though.

  /**
   * @param {number} a
   * @param {number} b
   * @return {string}
   */
  function addAndReturnString(a, b) {
    return String(a + b);
  }

  // generated code
  function addAndReturnString(a, b) {
    assert(typeof a === 'number');
    assert(typeof b === 'number');
    $ret = String(a + b);
    assert(typeof $ret === 'string');
    return $ret;
  }

[arv, 2013-03-18 20:19:25]

Submitted all the patches without any merge conflicts and all tests still
passes.

Thanks

Peter was adding TypeScript type annotation and he was going to add a
static type check pass. Adding runtime asserts is another option. I also
think those are valuable to be able to turn on for debug mode.


On Mon, Mar 18, 2013 at 4:03 PM, <usrbincc@yahoo.com> wrote:

>  Are we ready to land these?
>>
>
> I think this patch series is ready to go as long as long as you don't
> notice anything else. I'm pretty sure something will show up no matter
> what, though. (Gotten somewhat resigned to that)
>
> I'll go ahead and get the next issue (move main state machine code into
> separate function from try-catch) merged with the latest from here.
>
>
>
>
> https://codereview.appspot.**com/7487046/diff/20001/src/**
>
codegeneration/generator/**GeneratorTransformer.js<https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generator/GeneratorTransformer.js>
> File src/codegeneration/generator/**GeneratorTransformer.js (right):
>
> https://codereview.appspot.**com/7487046/diff/20001/src/**
>
codegeneration/generator/**GeneratorTransformer.js#**newcode363<https://codereview.appspot.com/7487046/diff/20001/src/codegeneration/generator/GeneratorTransformer.js#newcode363>
> src/codegeneration/generator/**GeneratorTransformer.js:363: * @param
> {RuntimeInliner} runtimeInliner
>
>> Peter started this work... I'm looking forward to progress in that
>>
> area.
>
> Compilers has always sort of been an area of interest, but it's never
> been my strong point. Not 100% sure which area you're referring to, but
> if you mean making the compiler use the JSDoc for type-checking, then
> I wouldn't know where to start. It'd theoretically be trivial to turn
> the JSDoc into runtime checks, though.
>
>   /**
>    * @param {number} a
>    * @param {number} b
>    * @return {string}
>    */
>   function addAndReturnString(a, b) {
>     return String(a + b);
>   }
>
>   // generated code
>   function addAndReturnString(a, b) {
>     assert(typeof a === 'number');
>     assert(typeof b === 'number');
>     $ret = String(a + b);
>     assert(typeof $ret === 'string');
>     return $ret;
>   }
>
>
https://codereview.appspot.**com/7487046/<https://codereview.appspot.com/7487...
>



-- 
erik