Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(270)

Unified Diff: src/string.js

Issue 1109010: Run string replace regexp with function in C++ code loop. (Closed)
Patch Set: Fix to also work in debug mode. Created 10 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« src/runtime.cc ('K') | « src/runtime.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/string.js
diff --git a/src/string.js b/src/string.js
index 6bb19e9548c00e76c56abd4d2353126dad60bbe1..9272d44475c0beeebe99a4d09c86f13fb149ef0a 100644
--- a/src/string.js
+++ b/src/string.js
@@ -405,97 +405,91 @@ function addCaptureString(builder, matchInfo, index) {
builder.addSpecialSlice(start, end);
};
+// TODO(lrn): This array will survive indefinitely if replace is never
+// called again. However, it will be empty, since the contents are cleared
+// in the finally block.
+var reusableReplaceArray = $Array(16);
// Helper function for replacing regular expressions with the result of a
-// function application in String.prototype.replace. The function application
-// must be interleaved with the regexp matching (contrary to ECMA-262
-// 15.5.4.11) to mimic SpiderMonkey and KJS behavior when the function uses
-// the static properties of the RegExp constructor. Example:
-// 'abcd'.replace(/(.)/g, function() { return RegExp.$1; }
-// should be 'abcd' and not 'dddd' (or anything else).
+// function application in String.prototype.replace.
function StringReplaceRegExpWithFunction(subject, regexp, replace) {
- var matchInfo = DoRegExpExec(regexp, subject, 0);
- if (IS_NULL(matchInfo)) return subject;
-
- var result = new ReplaceResultBuilder(subject);
- // There's at least one match. If the regexp is global, we have to loop
- // over all matches. The loop is not in C++ code here like the one in
- // RegExp.prototype.exec, because of the interleaved function application.
- // Unfortunately, that means this code is nearly duplicated, here and in
- // jsregexp.cc.
if (regexp.global) {
- var previous = 0;
- var startOfMatch;
- if (NUMBER_OF_CAPTURES(matchInfo) == 2) {
- // Both branches contain essentially the same loop except for the call
- // to the replace function. The branch is put outside of the loop for
- // speed
- do {
- startOfMatch = matchInfo[CAPTURE0];
- result.addSpecialSlice(previous, startOfMatch);
- previous = matchInfo[CAPTURE1];
- var match = SubString(subject, startOfMatch, previous);
- // Don't call directly to avoid exposing the built-in global object.
- result.add(replace.call(null, match, startOfMatch, subject));
- // Can't use matchInfo any more from here, since the function could
- // overwrite it.
- // Continue with the next match.
- // Increment previous if we matched an empty string, as per ECMA-262
- // 15.5.4.10.
- if (previous == startOfMatch) {
- // Add the skipped character to the output, if any.
- if (previous < subject.length) {
- result.addSpecialSlice(previous, previous + 1);
- }
- previous++;
- // Per ECMA-262 15.10.6.2, if the previous index is greater than the
- // string length, there is no match
- if (previous > subject.length) {
- return result.generate();
- }
- }
- matchInfo = DoRegExpExec(regexp, subject, previous);
- } while (!IS_NULL(matchInfo));
+ var resultArray = reusableReplaceArray;
+ if (resultArray) {
+ reusableReplaceArray = null;
} else {
- do {
- startOfMatch = matchInfo[CAPTURE0];
- result.addSpecialSlice(previous, startOfMatch);
- previous = matchInfo[CAPTURE1];
- result.add(ApplyReplacementFunction(replace, matchInfo, subject));
- // Can't use matchInfo any more from here, since the function could
- // overwrite it.
- // Continue with the next match.
- // Increment previous if we matched an empty string, as per ECMA-262
- // 15.5.4.10.
- if (previous == startOfMatch) {
- // Add the skipped character to the output, if any.
- if (previous < subject.length) {
- result.addSpecialSlice(previous, previous + 1);
+ // Inside a nested replace (replace called from the replacement function
+ // of another replace) or we have failed to set the reusable array
+ // back due to an exception in a replacement function. Create a new
+ // array to use in the future, or until the original is written back.
+ resultArray = $Array(16);
+ }
+ try {
+ // Must handle exceptions thrown by the replace functions correctly,
+ // including unregistering global regexps.
+ var res = %RegExpExecMultiple(regexp,
+ subject,
+ lastMatchInfo,
+ resultArray);
+ regexp.lastIndex = 0; // why this?
fschneider 2010/03/25 10:26:06 Clarify this comment.
Lasse Reichstein 2010/03/26 11:18:26 Removed. There is no doubt why if one reads the sp
+ if (IS_NULL(res)) {
+ // No matches at all.
+ return subject;
+ }
+ var len = res.length;
+ var i = 0;
+ if (NUMBER_OF_CAPTURES(lastMatchInfo) == 2) {
+ var match_start = 0;
+ while (i < len) {
+ var elem = res[i];
+ if (%_IsSmi(elem)) {
+ if (elem > 0) {
+ match_start = (elem >> 11) + (elem & 0x7ff);
+ } else {
+ match_start = res[++i] - elem;
+ }
+ } else {
+ var func_result = replace.call(null, elem, match_start, subject);
+ if (!IS_STRING(func_result)) func_result = TO_STRING(func_result);
+ res[i] = func_result;
+ match_start += elem.length;
}
- previous++;
- // Per ECMA-262 15.10.6.2, if the previous index is greater than the
- // string length, there is no match
- if (previous > subject.length) {
- return result.generate();
+ i++;
+ }
+ } else {
+ while (i < len) {
+ var elem = res[i];
+ if (!%_IsSmi(elem)) {
+ // elem must be an Array.
+ // Use the apply argument as backing for global RegExp properties.
+ lastMatchInfoOverride = elem;
+ var func_result = replace.apply(null, elem);
+ if (!IS_STRING(func_result)) func_result = TO_STRING(func_result);
+ res[i] = func_result;
}
+ i++;
}
- matchInfo = DoRegExpExec(regexp, subject, previous);
- } while (!IS_NULL(matchInfo));
+ }
+ var result = new ReplaceResultBuilder(subject, res);
+ return result.generate();
+ } finally {
+ lastMatchInfoOverride = null;
+ resultArray.length = 0;
+ reusableReplaceArray = resultArray;
}
-
- // Tack on the final right substring after the last match.
- result.addSpecialSlice(previous, subject.length);
-
} else { // Not a global regexp, no need to loop.
+ var matchInfo = DoRegExpExec(regexp, subject, 0);
+ if (IS_NULL(matchInfo)) return subject;
+
+ var result = new ReplaceResultBuilder(subject);
result.addSpecialSlice(0, matchInfo[CAPTURE0]);
var endOfMatch = matchInfo[CAPTURE1];
result.add(ApplyReplacementFunction(replace, matchInfo, subject));
// Can't use matchInfo any more from here, since the function could
// overwrite it.
result.addSpecialSlice(endOfMatch, subject.length);
+ return result.generate();
}
-
- return result.generate();
}
@@ -896,7 +890,11 @@ function StringSup() {
// ReplaceResultBuilder support.
function ReplaceResultBuilder(str) {
- this.elements = new $Array();
+ if (%_ArgumentsLength() > 1) {
+ this.elements = %_Arguments(1);
+ } else {
+ this.elements = new $Array();
+ }
this.special_string = str;
}
« src/runtime.cc ('K') | « src/runtime.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698