Revision: 4455
Author: [email protected]
Date: Wed Apr 21 01:33:04 2010
Log: Fix incorrect handling of global RegExp properties for nested
replace-regexp-with-function.
Review URL: http://codereview.chromium.org/1695002
http://code.google.com/p/v8/source/detail?r=4455
Modified:
/branches/bleeding_edge/src/regexp.js
/branches/bleeding_edge/src/string.js
/branches/bleeding_edge/test/mjsunit/string-replace.js
=======================================
--- /branches/bleeding_edge/src/regexp.js Tue Apr 13 02:31:03 2010
+++ /branches/bleeding_edge/src/regexp.js Wed Apr 21 01:33:04 2010
@@ -115,7 +115,9 @@
function DoRegExpExec(regexp, string, index) {
- return %_RegExpExec(regexp, string, index, lastMatchInfo);
+ var result = %_RegExpExec(regexp, string, index, lastMatchInfo);
+ if (result !== null) lastMatchInfoOverride = null;
+ return result;
}
@@ -136,7 +138,7 @@
function CloneRegExpResult(array) {
- if (array == null) return null;
+ if (array == null) return null;
var length = array.length;
var answer = %_RegExpConstructResult(length, array.index, array.input);
for (var i = 0; i < length; i++) {
@@ -237,7 +239,7 @@
cache.type = 'exec';
return matchIndices; // No match.
}
-
+ lastMatchInfoOverride = null;
var result = BuildResultFromMatchInfo(matchIndices, s);
if (this.global) {
@@ -312,7 +314,7 @@
cache.answer = false;
return false;
}
-
+ lastMatchInfoOverride = null;
if (this.global) this.lastIndex = lastMatchInfo[CAPTURE1];
cache.answer = true;
return true;
@@ -340,7 +342,9 @@
// on the captures array of the last successful match and the subject
string
// of the last successful match.
function RegExpGetLastMatch() {
- if (lastMatchInfoOverride) { return lastMatchInfoOverride[0]; }
+ if (lastMatchInfoOverride !== null) {
+ return lastMatchInfoOverride[0];
+ }
var regExpSubject = LAST_SUBJECT(lastMatchInfo);
return SubString(regExpSubject,
lastMatchInfo[CAPTURE0],
=======================================
--- /branches/bleeding_edge/src/string.js Tue Apr 20 06:10:18 2010
+++ /branches/bleeding_edge/src/string.js Wed Apr 21 01:33:04 2010
@@ -175,9 +175,9 @@
// ECMA-262 section 15.5.4.10
function StringMatch(regexp) {
var subject = TO_STRING_INLINE(this);
- if (IS_REGEXP(regexp)) {
- if (!regexp.global) return regexp.exec(subject);
-
+ if (IS_REGEXP(regexp)) {
+ if (!regexp.global) return regexp.exec(subject);
+
var cache = regExpCache;
var saveAnswer = false;
@@ -435,63 +435,63 @@
// 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;
- 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;
- }
+
+ var res = %RegExpExecMultiple(regexp,
+ subject,
+ lastMatchInfo,
+ resultArray);
+ regexp.lastIndex = 0;
+ 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;
+ var override = [null, 0, subject];
+ while (i < len) {
+ var elem = res[i];
+ if (%_IsSmi(elem)) {
+ if (elem > 0) {
+ match_start = (elem >> 11) + (elem & 0x7ff);
} else {
- var func_result = replace.call(null, elem, match_start,
subject);
- if (!IS_STRING(func_result)) {
- func_result = NonStringToString(func_result);
- }
- res[i] = func_result;
- match_start += elem.length;
- }
- 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 = NonStringToString(func_result);
- }
- res[i] = func_result;
- }
- i++;
- }
- }
- var result = new ReplaceResultBuilder(subject, res);
- return result.generate();
- } finally {
- lastMatchInfoOverride = null;
- resultArray.length = 0;
- reusableReplaceArray = resultArray;
- }
+ match_start = res[++i] - elem;
+ }
+ } else {
+ override[0] = elem;
+ override[1] = match_start;
+ lastMatchInfoOverride = override;
+ var func_result = replace.call(null, elem, match_start, subject);
+ if (!IS_STRING(func_result)) {
+ func_result = NonStringToString(func_result);
+ }
+ res[i] = func_result;
+ match_start += elem.length;
+ }
+ 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 = NonStringToString(func_result);
+ }
+ res[i] = func_result;
+ }
+ i++;
+ }
+ }
+ var resultBuilder = new ReplaceResultBuilder(subject, res);
+ var result = resultBuilder.generate();
+ resultArray.length = 0;
+ reusableReplaceArray = resultArray;
+ return result;
} else { // Not a global regexp, no need to loop.
var matchInfo = DoRegExpExec(regexp, subject, 0);
if (IS_NULL(matchInfo)) return subject;
@@ -542,7 +542,6 @@
var s = TO_STRING_INLINE(this);
var match = DoRegExpExec(regexp, s, 0);
if (match) {
- lastMatchInfo = match;
return match[CAPTURE0];
}
return -1;
=======================================
--- /branches/bleeding_edge/test/mjsunit/string-replace.js Tue Mar 30
06:26:13 2010
+++ /branches/bleeding_edge/test/mjsunit/string-replace.js Wed Apr 21
01:33:04 2010
@@ -30,7 +30,7 @@
*/
function replaceTest(result, subject, pattern, replacement) {
- var name =
+ var name =
"\"" + subject + "\".replace(" + pattern + ", " + replacement + ")";
assertEquals(result, subject.replace(pattern, replacement), name);
}
@@ -114,8 +114,8 @@
replaceTest("xaxe$xcx", short, /b/g, "e$");
-replaceTest("[$$$1$$a1abb1bb0$002$3$03][$$$1$$b1bcc1cc0$002$3$03]c",
- "abc",
/(.)(?=(.))/g, "[$$$$$$1$$$$$11$01$2$21$02$020$002$3$03]");
+replaceTest("[$$$1$$a1abb1bb0$002$3$03][$$$1$$b1bcc1cc0$002$3$03]c",
+ "abc",
/(.)(?=(.))/g, "[$$$$$$1$$$$$11$01$2$21$02$020$002$3$03]");
// Replace with functions.
@@ -189,5 +189,21 @@
replaceTest("string null", "string x", /x/g, function() { return null; });
replaceTest("string undefined", "string x", /x/g, function() { return
undefined; });
-replaceTest("aundefinedbundefinedcundefined",
+replaceTest("aundefinedbundefinedcundefined",
"abc", /(.)|(.)/g, function(m, m1, m2, i, s) { return m1+m2;
});
+
+// Test nested calls to replace, including that it sets RegExp.$&
correctly.
+
+function replacer(m,i,s) {
+ assertEquals(m,RegExp['$&']);
+ return "[" + RegExp['$&'] + "-"
+ + m.replace(/./g,"$&$&") + "-"
+ + m.replace(/./g,function() { return RegExp['$&']; })
+ + "-" + RegExp['$&'] + "]";
+}
+
+replaceTest("[ab-aabb-ab-b][az-aazz-az-z]",
+ "abaz", /a./g, replacer);
+
+replaceTest("[ab-aabb-ab-b][az-aazz-az-z]",
+ "abaz", /a(.)/g, replacer);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
