Committed as r1491

http://codereview.chromium.org/43075/diff/1/16
File src/jsregexp.cc (right):

http://codereview.chromium.org/43075/diff/1/16#newcode603
Line 603: matches = Factory::null_value();
On 2009/03/11 13:49:17, Mads Ager wrote:
> I know this is not your code, but why is there an assignment to a
local variable
> followed by a return?

Lasse has a patch waiting that also fixes this.

http://codereview.chromium.org/43075/diff/1/16#newcode740
Line 740: // Unimplemented on ARM, fall through to bytecode.
On 2009/03/11 13:54:03, Lasse Reichstein wrote:
> I'm all for factoring it differently, but I think we should wait until
after
> another pending patch that I have (or during), since it introduces
even more
> #ifdef ARM's.

I'll leave it alone for now.

http://codereview.chromium.org/43075/diff/1/10
File src/jsregexp.h (right):

http://codereview.chromium.org/43075/diff/1/10#newcode118
Line 118: static int GetCapture(FixedArray* array, int index) {
On 2009/03/11 13:49:17, Mads Ager wrote:
> This is a general comment on this code.  Don't let it stop you from
putting
> back, but this code does not match the style of the rest of the V8
code.  We
> usually put vertical space between functions to increase readability.
It seems
> that there is very little vertical spacing in this file.

Changed here, left alone other places in the file.

http://codereview.chromium.org/43075/diff/1/7
File src/regexp-delay.js (right):

http://codereview.chromium.org/43075/diff/1/7#newcode207
Line 207: throw MakeTypeError('method_called_on_incompatible',
['RegExp.prototype.test', this]);
On 2009/03/11 13:49:17, Mads Ager wrote:
> Long line, break it?

Yes.  It seems none of our linters catch >80 characters in .js files.

http://codereview.chromium.org/43075/diff/1/14
File src/string.js (right):

http://codereview.chromium.org/43075/diff/1/14#newcode302
Line 302:
On 2009/03/11 13:49:17, Mads Ager wrote:
> Any reason for the extra space here?

No.

http://codereview.chromium.org/43075

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to