* Reapply revisions 1383, 1384, 1391, 1398, 1401, 1402,...

* Reapply revisions 1383, 1384, 1391, 1398, 1401, 1402,
  1418, and 1419 from bleeding_edge, reverted in 1429.
* Fix of $1 accessor on sliced strings.
* Fix of lastParen method when last parenthesis did not match.

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

[Mads Ager (chromium), 2009-03-11 13:49:17]

LGTM, thanks for finding and fixing the issues!

A couple of nits below.

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();
I know this is not your code, but why is there an assignment to a local variable
followed by a return?

http://codereview.chromium.org/43075/diff/1/16#newcode740
Line 740: // Unimplemented on ARM, fall through to bytecode.
Auch, this is hard to read.  Can we factor this differently.

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

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]);
Long line, break it?

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

http://codereview.chromium.org/43075/diff/1/14#newcode302
Line 302: 
Any reason for the extra space here?

[Lasse Reichstein, 2009-03-11 13:49:41]

LGTM

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

http://codereview.chromium.org/43075/diff/1/7#newcode138
Line 138: function DoRegExpExec(regexp, string, index) {
Is this used any more?

[Lasse Reichstein, 2009-03-11 13:51:53]

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

http://codereview.chromium.org/43075/diff/1/7#newcode138
Line 138: function DoRegExpExec(regexp, string, index) {
> Is this used any more?
Yes it is, just not in this file. It's called from string.js.

[Lasse Reichstein, 2009-03-11 13:54:03]

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

http://codereview.chromium.org/43075/diff/1/16#newcode740
Line 740: // Unimplemented on ARM, fall through to bytecode.
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.

[Erik Corry, 2009-03-11 14:01:06]

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.