Fix incorrect handling of global RegExp properties for nested replace-regexp-with...

Review URL: http://codereview.chromium.org/1695002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4455 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/regexp.js b/src/regexp.js
index 9929b11..24e3309 100644 (file)
--- a/src/regexp.js
+++ b/src/regexp.js
@@ -115,7 +115,9 @@ function CompileRegExp(pattern, flags) {
 
 
 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 @@ var regExpCache = new RegExpCache();
 
 
 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 @@ function RegExpExec(string) {
     cache.type = 'exec';
     return matchIndices;        // No match.
   }
-
+  lastMatchInfoOverride = null;
   var result = BuildResultFromMatchInfo(matchIndices, s);
 
   if (this.global) {
@@ -312,7 +314,7 @@ function RegExpTest(string) {
     cache.answer = false;
     return false;
   }
-
+  lastMatchInfoOverride = null;
   if (this.global) this.lastIndex = lastMatchInfo[CAPTURE1];
   cache.answer = true;
   return true;
@@ -340,7 +342,9 @@ function RegExpToString() {
 // 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],

diff --git a/src/string.js b/src/string.js
index daa179b..9433249 100644 (file)
--- a/src/string.js
+++ b/src/string.js
@@ -175,9 +175,9 @@ function StringLocaleCompare(other) {
 // 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 @@ function StringReplaceRegExpWithFunction(subject, regexp, replace) {
       // 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;
+            match_start = res[++i] - elem;
           }
-          i++;
+        } 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;
         }
-      } 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++;
+      }
+    } 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);
           }
-          i++;
+          res[i] = func_result;
         }
+        i++;
       }
-      var result = new ReplaceResultBuilder(subject, res);
-      return result.generate();
-    } finally {
-      lastMatchInfoOverride = null;
-      resultArray.length = 0;
-      reusableReplaceArray = resultArray;
     }
+    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 @@ function StringSearch(re) {
   var s = TO_STRING_INLINE(this);
   var match = DoRegExpExec(regexp, s, 0);
   if (match) {
-    lastMatchInfo = match;
     return match[CAPTURE0];
   }
   return -1;

diff --git a/test/mjsunit/string-replace.js b/test/mjsunit/string-replace.js
index a555c20..9e4f559 100644 (file)
--- a/test/mjsunit/string-replace.js
+++ b/test/mjsunit/string-replace.js
@@ -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/, "e$");
 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 true", "string x", /x/g, function() { return true; });
 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);