Comparison between original, modified and --disable-eval.

Companion CL to https://codereview.chromium.org/11188004/.

Patch Set 1 shows the version before the above CL.
Patch Set 2 shows the output after the CL.
Patch Set 3 shows the output with --disable-eval.

[kasperl, 2012-10-18 09:20:07]

Comments (indirectly) for https://codereview.chromium.org/11188004/.

https://codereview.chromium.org/11183040/diff/4001/comp.js
File comp.js (right):

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode14
comp.js:14: $$.StringBufferImpl = {'': function StringBufferImpl(_buffer,
_length) { this._buffer = _buffer; this._length = _length; },
Looks like we could use shorter argument names fairly easily. Maybe that's a job
for the minifier though. I think it would be nice (but larger) if we emitted
some newlines in the generated constructors too. Something ala:

   {'': function StringBufferImpl(...) {
     this._buffer = _buffer;
     this._length = _length;  
    },
    ...
   }

There's really no reason not to.

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode954
comp.js:954: var supportsProto = !!{}.__proto__;Isolate.$pendingClasses = {};
Would be nice with a newline after ;.

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode1006
comp.js:1006: function forceEfficientMap() {}
I think I'd prefer to call this ForceEfficientMap since it's (almost) used as a
constructor.

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode1008
comp.js:1008: new forceEfficientMap;
Not necessary.

[floitsch, 2012-10-18 10:33:08]

Compare Patch Set 3 and 5 to see how addressing the comments changed the output.

https://codereview.chromium.org/11183040/diff/4001/comp.js
File comp.js (right):

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode14
comp.js:14: $$.StringBufferImpl = {'': function StringBufferImpl(_buffer,
_length) { this._buffer = _buffer; this._length = _length; },
On 2012/10/18 09:20:07, kasperl wrote:
> Looks like we could use shorter argument names fairly easily. Maybe that's a
job
> for the minifier though. I think it would be nice (but larger) if we emitted
> some newlines in the generated constructors too. Something ala:
> 
>    {'': function StringBufferImpl(...) {
>      this._buffer = _buffer;
>      this._length = _length;  
>     },
>     ...
>    }
> 
> There's really no reason not to.

Done.

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode954
comp.js:954: var supportsProto = !!{}.__proto__;Isolate.$pendingClasses = {};
On 2012/10/18 09:20:07, kasperl wrote:
> Would be nice with a newline after ;.

Done.

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode1006
comp.js:1006: function forceEfficientMap() {}
On 2012/10/18 09:20:07, kasperl wrote:
> I think I'd prefer to call this ForceEfficientMap since it's (almost) used as
a
> constructor.

Done.

https://codereview.chromium.org/11183040/diff/4001/comp.js#newcode1008
comp.js:1008: new forceEfficientMap;
On 2012/10/18 09:20:07, kasperl wrote:
> Not necessary.

Benchmark says it is.