Re: Use RuntimeInliner to provide the generator common code. (re-remove TODOs) (issue 7487046)

[usrb...@yahoo.com]

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

https://codereview.appspot.com/7487046/diff/10002/src/codegeneration/generator/GeneratorTransformer.js#newcode238
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/generator/GeneratorTransformer.js
File src/codegeneration/generator/GeneratorTransformer.js (right):

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

https://codereview.appspot.com/7487046/

[a...@chromium.org]

LGTM

Are we ready to land these?

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
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.

https://codereview.appspot.com/7487046/

[usrb...@yahoo.com]

> 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
File src/codegeneration/generator/GeneratorTransformer.js (right):

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/

[Erik Arvidsson]

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.

--
erik