Simplified replace JS a little.

Simplified replace JS.
Refactored code so global/non-global regexps are handled in separate functions.
Inlined ApplyReplaceFunction at its only call point.

[Lasse Reichstein, 2010-05-12 14:26:14]

Small JS-change.

[Lasse Reichstein, 2010-05-12 14:29:48]

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

http://codereview.chromium.org/1994019/diff/1/2#newcode399
src/string.js:399: // If start isn't valid, return undefined.
lastCaptureInfo holds alternating start/end points of captures, with -1 for
nonparticipating captures (the part of the regexp containing the capture isn't
part of the actual match).
We never return a result with a non-negative start point and negative end (and
if anyone deliberately munges lastCaptureInfo, SubString checks its arguments
too)

[Lasse Reichstein, 2010-05-12 14:31:40]

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

http://codereview.chromium.org/1994019/diff/1/2#newcode522
src/string.js:522: return %_CallFunction(global, s, index, subject, replace);
Is "global" the correct receiver here? (Is it the global object or the global
receiver?)

[antonm, 2010-05-12 14:35:05]

drive-by

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

http://codereview.chromium.org/1994019/diff/1/2#newcode522
src/string.js:522: return %_CallFunction(global, s, index, subject, replace);
On 2010/05/12 14:31:40, Lasse Reichstein wrote:
> Is "global" the correct receiver here? (Is it the global object or the global
> receiver?)

I'd used %GlobalReceiver().

Funny, right now I am running performance tests on a patch which adds those
%_CallFunction to all the places I spotted with .call :)

[antonm, 2010-05-13 11:43:17]

LGTM

http://codereview.chromium.org/1994019/diff/7001/8001
File src/string.js (right):

http://codereview.chromium.org/1994019/diff/7001/8001#newcode406
src/string.js:406: if (start < 0) return;
you omit end check, is it fine?  is lastCaptureInfo sorted?

http://codereview.chromium.org/1994019/diff/7001/8001#newcode449
src/string.js:449: return subject;
could reusableResultArray be restored at that point?

http://codereview.chromium.org/1994019/diff/7001/8001#newcode470
src/string.js:470: %_CallFunction(receiver, elem, match_start, subject,
replace);
do we have a cheap way to ensure replace is function here?  something like debug
mode assert?  This separate function is far enough for me to make tracing the
check somewhat difficult.

http://codereview.chromium.org/1994019/diff/7001/8001#newcode517
src/string.js:517: var s = SubString(subject, index, matchInfo[CAPTURE1]);
maybe user endOfMatch here instead of matchInfo[CAPTURE1]?

http://codereview.chromium.org/1994019/diff/7001/8001#newcode522
src/string.js:522: var parameters = $Array(m + 2);
not for this CL, but I'd be curious to see if push is faster than direct
indexing.

[Lasse Reichstein, 2010-05-13 12:13:11]

http://codereview.chromium.org/1994019/diff/7001/8001
File src/string.js (right):

http://codereview.chromium.org/1994019/diff/7001/8001#newcode406
src/string.js:406: if (start < 0) return;
It should be fine. If lastCaptureInfo was created by us (in the RegExp engine),
then (start < 0) iff (end < 0).
If someone malicious has changed lastCaptureInfo, then we will just get invalid
values for SubString, which tests its arguments, and, at most, returns a
different substring.

http://codereview.chromium.org/1994019/diff/7001/8001#newcode449
src/string.js:449: return subject;
It could and probably should. 
(There used to be a try-finally around it, and I can see I was a little careless
when I removed it).

http://codereview.chromium.org/1994019/diff/7001/8001#newcode470
src/string.js:470: %_CallFunction(receiver, elem, match_start, subject,
replace);
I don't think we have a debug assert (Something like %_Assert(expr,str)) would
be a logical idea). In this case, we only know it's a function because this
function is only called if replace is a function (hence its name
"...WithFunction").

http://codereview.chromium.org/1994019/diff/7001/8001#newcode517
src/string.js:517: var s = SubString(subject, index, matchInfo[CAPTURE1]);
Good catch.