Refactoring of RegExp interface to better support calling several times in a row.

Refactoring of RegExp interface to better support calling several times in a row.

[Lasse Reichstein, 2010-03-18 13:39:15]

Primarily a refactoring.
Also made native regexp code return correct indices, instead of relative to
start index. Since the introduction of the regexp exec stub, we had the
necessary values anyway.

[Søren Thygesen Gjesse, 2010-03-18 13:54:50]

http://codereview.chromium.org/1114001/diff/3001/4002
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1114001/diff/3001/4002#newcode10551
src/ia32/codegen-ia32.cc:10551: __ mov(edi, Operand(ecx, edx, times_int_size,
0));
Drive by - you should probably remove the comment and ASSERT_EQ.

[Erik Corry, 2010-03-19 11:04:11]

LGTM

Perhaps a RegistersProvided abstraction could reduce the ifdef ugliness?

http://codereview.chromium.org/1114001/diff/3001/4001
File src/arm/regexp-macro-assembler-arm.cc (right):

http://codereview.chromium.org/1114001/diff/3001/4001#newcode651
src/arm/regexp-macro-assembler-arm.cc:651: // Set r0 to address of char before
start of input
start of input -> match start position.

http://codereview.chromium.org/1114001/diff/3001/4005
File src/jsregexp.cc (right):

http://codereview.chromium.org/1114001/diff/3001/4005#newcode345
src/jsregexp.cc:345: Handle<String> pattern,
indent

http://codereview.chromium.org/1114001/diff/3001/4005#newcode373
src/jsregexp.cc:373: #endif  // V8_NATIVE_REGEXP
V8 ->  !V8

Actually I prefer the comment // ndef V8_NATIVE_REGEXP
which better reflects the contents of the #if in the #else.

http://codereview.chromium.org/1114001/diff/3001/4005#newcode384
src/jsregexp.cc:384: ASSERT(output.length() >=
please move this assert into the ifdef below

http://codereview.chromium.org/1114001/diff/3001/4005#newcode414
src/jsregexp.cc:414: // If result is RETRY, the string have changed
representation, and we
have -> has

http://codereview.chromium.org/1114001/diff/3001/4005#newcode417
src/jsregexp.cc:417: // the, potentially, differen subject (the string can
switch between
en -> ent

http://codereview.chromium.org/1114001/diff/3001/4005#newcode472
src/jsregexp.cc:472: 
2 blank lines

http://codereview.chromium.org/1114001/diff/3001/4006
File src/jsregexp.h (right):

http://codereview.chromium.org/1114001/diff/3001/4006#newcode81
src/jsregexp.h:81: Handle<String> pattern,
indent

http://codereview.chromium.org/1114001/diff/3001/4006#newcode104
src/jsregexp.h:104: // an exeception is thrown, and this function returns
negative.
Unclear comment.  You can't both throw an exception and return a value.

http://codereview.chromium.org/1114001/diff/3001/4006#newcode110
src/jsregexp.h:110: // If successful, returns RE_SUCCESS and have set the
capture positions
have -> set

http://codereview.chromium.org/1114001/diff/3001/4006#newcode113
src/jsregexp.h:113: // If execution fails, throws an exception and returns
RE_EXCEPTION.
Unclear again.

http://codereview.chromium.org/1114001/diff/3001/4007
File src/string.js (right):

http://codereview.chromium.org/1114001/diff/3001/4007#newcode419
src/string.js:419: 
Trailing spaces?

[Lasse Reichstein, 2010-03-19 11:25:42]

http://codereview.chromium.org/1114001/diff/3001/4001
File src/arm/regexp-macro-assembler-arm.cc (right):

http://codereview.chromium.org/1114001/diff/3001/4001#newcode651
src/arm/regexp-macro-assembler-arm.cc:651: // Set r0 to address of char before
start of input
On 2010/03/19 11:04:11, Erik Corry wrote:
> start of input -> match start position.

Actually "start of the input string". This is the thing I changed here, the "-1"
position is now relative to the entire string, not just the start of the match
test.

http://codereview.chromium.org/1114001/diff/3001/4002
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1114001/diff/3001/4002#newcode10551
src/ia32/codegen-ia32.cc:10551: __ mov(edi, Operand(ecx, edx, times_int_size,
0));
Done.

http://codereview.chromium.org/1114001/diff/3001/4005
File src/jsregexp.cc (right):

http://codereview.chromium.org/1114001/diff/3001/4005#newcode373
src/jsregexp.cc:373: #endif  // V8_NATIVE_REGEXP
Changed to ndef.

http://codereview.chromium.org/1114001/diff/3001/4005#newcode384
src/jsregexp.cc:384: ASSERT(output.length() >=
On 2010/03/19 11:04:11, Erik Corry wrote:
> please move this assert into the ifdef below

Done.

http://codereview.chromium.org/1114001/diff/3001/4005#newcode472
src/jsregexp.cc:472: 
Ignored, per offline discussion.

http://codereview.chromium.org/1114001/diff/3001/4006
File src/jsregexp.h (right):

http://codereview.chromium.org/1114001/diff/3001/4006#newcode81
src/jsregexp.h:81: Handle<String> pattern,
On 2010/03/19 11:04:11, Erik Corry wrote:
> indent

Done.

http://codereview.chromium.org/1114001/diff/3001/4006#newcode104
src/jsregexp.h:104: // an exeception is thrown, and this function returns
negative.
Reworded.

http://codereview.chromium.org/1114001/diff/3001/4006#newcode113
src/jsregexp.h:113: // If execution fails, throws an exception and returns
RE_EXCEPTION.
Reworded.

http://codereview.chromium.org/1114001/diff/3001/4007
File src/string.js (right):

http://codereview.chromium.org/1114001/diff/3001/4007#newcode419
src/string.js:419: 
On 2010/03/19 11:04:11, Erik Corry wrote:
> Trailing spaces?

Done.