Use simple concat of substrings instead of ReplaceStringBuilder for non-global replacements.

Use simple concat of substrings instead of ReplaceStringBuilder for non-global replacements.

BUG=
TEST=

Committed: https://code.google.com/p/v8/source/detail?r=11448

[Yang, 2012-04-25 14:29:37]

PTAL.

[Erik Corry, 2012-04-26 08:38:54]

LGTM with comments

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

http://codereview.chromium.org/10134057/diff/1/src/string.js#newcode400
src/string.js:400: // ReplaceResultBuilder
Comment out of date

http://codereview.chromium.org/10134057/diff/1/src/string.js#newcode406
src/string.js:406: if (start < 0) return;
This return seems wrong.  Should we not return a string?

The tests need beefing up to catch this.

[Yang, 2012-04-26 11:17:08]

Found dead code. Will upload a new revision without it.

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

http://codereview.chromium.org/10134057/diff/1/src/string.js#newcode400
src/string.js:400: // ReplaceResultBuilder
On 2012/04/26 08:38:54, Erik Corry wrote:
> Comment out of date

Done.

http://codereview.chromium.org/10134057/diff/1/src/string.js#newcode406
src/string.js:406: if (start < 0) return;
On 2012/04/26 08:38:54, Erik Corry wrote:
> This return seems wrong.  Should we not return a string?
> 
> The tests need beefing up to catch this.

Turns out this is dead code. addCaptureString is only called by
ExpandReplacement, which is only called for the String.replace(<string>, <string
including $-replacements>) case. In that case, numbered captures can not occur
since the search pattern is a string and not a regexp. The actual work for
replacing with numbered captures is implemented in CompiledReplacement

[Yang, 2012-04-26 11:17:57]

Please take another quick look.

[Erik Corry, 2012-04-26 11:25:01]

LGTM