Fix additional spec violations wrt RegExp.lastIndex.

R=svenpanne@chromium.org
BUG=v8:2437

Review URL: https://chromiumcodereview.appspot.com/12033099

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

diff --git a/src/objects-inl.h b/src/objects-inl.h
index ef506fa..16627e2 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -5075,17 +5075,6 @@ void JSRegExp::SetDataAtUnchecked(int index, Object* value, Heap* heap) {
 }
 
 
-void JSRegExp::ResetLastIndex(Isolate* isolate,
-                              Handle<JSRegExp> regexp) {
-  // Reset lastIndex property to 0.
-  SetProperty(regexp,
-              isolate->factory()->last_index_symbol(),
-              Handle<Smi>(Smi::FromInt(0)),
-              ::NONE,
-              kNonStrictMode);
-}
-
-
 ElementsKind JSObject::GetElementsKind() {
   ElementsKind kind = map()->elements_kind();
 #if DEBUG

diff --git a/src/objects.h b/src/objects.h
index 958e1ef..975fdf8 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -6622,8 +6622,6 @@ class JSRegExp: public JSObject {
   inline void SetDataAtUnchecked(int index, Object* value, Heap* heap);
   inline Type TypeTagUnchecked();
 
-  static inline void ResetLastIndex(Isolate* isolate, Handle<JSRegExp> regexp);
-
   static int code_index(bool is_ascii) {
     if (is_ascii) {
       return kIrregexpASCIICodeIndex;

diff --git a/src/runtime.cc b/src/runtime.cc
index 5047dbd..e8003b6 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -2912,8 +2912,7 @@ MUST_USE_RESULT static MaybeObject* StringReplaceAtomRegExpWithString(
 
   int matches = indices.length();
   if (matches == 0) {
-    JSRegExp::ResetLastIndex(isolate, pattern_regexp);
-    return *subject;
+    return isolate->heap()->undefined_value();
   }
 
   // Detect integer overflow.
@@ -3015,8 +3014,7 @@ MUST_USE_RESULT static MaybeObject* StringReplaceRegExpWithString(
   int32_t* current_match = global_cache.FetchNext();
   if (current_match == NULL) {
     if (global_cache.HasException()) return Failure::Exception();
-    JSRegExp::ResetLastIndex(isolate, regexp);
-    return *subject;
+    return isolate->heap()->undefined_value();
   }
 
   // Guessing the number of parts that the final result string is built
@@ -3114,8 +3112,7 @@ MUST_USE_RESULT static MaybeObject* StringReplaceRegExpWithEmptyString(
   int32_t* current_match = global_cache.FetchNext();
   if (current_match == NULL) {
     if (global_cache.HasException()) return Failure::Exception();
-    JSRegExp::ResetLastIndex(isolate, regexp);
-    return *subject;
+    return isolate->heap()->undefined_value();
   }
 
   int start = current_match[0];

diff --git a/src/string.js b/src/string.js
index 60a5abe..b39976c 100644 (file)
--- a/src/string.js
+++ b/src/string.js
@@ -194,6 +194,7 @@ function StringMatch(regexp) {
     // lastMatchInfo is defined in regexp.js.
     var result = %StringMatch(subject, regexp, lastMatchInfo);
     if (result !== null) lastMatchInfoOverride = null;
+    regexp.lastIndex = 0;
     return result;
   }
   // Non-regexp argument.
@@ -244,13 +245,19 @@ function StringReplace(search, replace) {
       }
     } else {
       if (lastMatchInfoOverride == null) {
-        return %StringReplaceRegExpWithString(subject,
-                                              search,
-                                              TO_STRING_INLINE(replace),
-                                              lastMatchInfo);
+        var answer = %StringReplaceRegExpWithString(subject,
+                                                    search,
+                                                    TO_STRING_INLINE(replace),
+                                                    lastMatchInfo);
+        if (IS_UNDEFINED(answer)) {  // No match.  Return subject string.
+          search.lastIndex = 0;
+          return subject;
+        }
+        if (search.global) search.lastIndex = 0;
+        return answer;
       } else {
         // We use this hack to detect whether StringReplaceRegExpWithString
-        // found at least one hit.  In that case we need to remove any
+        // found at least one hit. In that case we need to remove any
         // override.
         var saved_subject = lastMatchInfo[LAST_SUBJECT_INDEX];
         lastMatchInfo[LAST_SUBJECT_INDEX] = 0;
@@ -258,11 +265,17 @@ function StringReplace(search, replace) {
                                                     search,
                                                     TO_STRING_INLINE(replace),
                                                     lastMatchInfo);
+        if (IS_UNDEFINED(answer)) {  // No match.  Return subject string.
+          search.lastIndex = 0;
+          lastMatchInfo[LAST_SUBJECT_INDEX] = saved_subject;
+          return subject;
+        }
         if (%_IsSmi(lastMatchInfo[LAST_SUBJECT_INDEX])) {
           lastMatchInfo[LAST_SUBJECT_INDEX] = saved_subject;
         } else {
           lastMatchInfoOverride = null;
         }
+        if (search.global) search.lastIndex = 0;
         return answer;
       }
     }

diff --git a/test/mjsunit/regress/regress-2437.js b/test/mjsunit/regress/regress-2437.js
index 06b69b2..c82293a 100644 (file)
--- a/test/mjsunit/regress/regress-2437.js
+++ b/test/mjsunit/regress/regress-2437.js
@@ -25,6 +25,14 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+// Summary of the spec: lastIndex is reset to 0 if
+// - a regexp fails to match, regardless of global or non-global.
+// - a global regexp is used in a function that returns multiple results,
+//   such as String.prototype.replace or String.prototype.match, since it
+//   repeats the regexp until it fails to match.
+// Otherwise lastIndex is only set when a global regexp matches, to the index
+// after the match.
+
 // Test Regexp.prototype.exec
 r = /a/;
 r.lastIndex = 1;
@@ -73,3 +81,76 @@ r.lastIndex = 1;
 "zzzz".replace(r, function() { return ""; });
 assertEquals(0, r.lastIndex);
 
+// Regexp functions that returns multiple results:
+// A global regexp always resets lastIndex regardless of whether it matches.
+r = /a/g;
+r.lastIndex = -1;
+"0123abcd".replace(r, "x");
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = -1;
+"01234567".replace(r, "x");
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = -1;
+"0123abcd".match(r);
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = -1;
+"01234567".match(r);
+assertEquals(0, r.lastIndex);
+
+// A non-global regexp resets lastIndex iff it does not match.
+r = /a/;
+r.lastIndex = -1;
+"0123abcd".replace(r, "x");
+assertEquals(-1, r.lastIndex);
+
+r.lastIndex = -1;
+"01234567".replace(r, "x");
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = -1;
+"0123abcd".match(r);
+assertEquals(-1, r.lastIndex);
+
+r.lastIndex = -1;
+"01234567".match(r);
+assertEquals(0, r.lastIndex);
+
+// Also test RegExp.prototype.exec and RegExp.prototype.test
+r = /a/g;
+r.lastIndex = 1;
+r.exec("01234567");
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = 1;
+r.exec("0123abcd");
+assertEquals(5, r.lastIndex);
+
+r = /a/;
+r.lastIndex = 1;
+r.exec("01234567");
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = 1;
+r.exec("0123abcd");
+assertEquals(1, r.lastIndex);
+
+r = /a/g;
+r.lastIndex = 1;
+r.test("01234567");
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = 1;
+r.test("0123abcd");
+assertEquals(5, r.lastIndex);
+
+r = /a/;
+r.lastIndex = 1;
+r.test("01234567");
+assertEquals(0, r.lastIndex);
+
+r.lastIndex = 1;
+r.test("0123abcd");
+assertEquals(1, r.lastIndex);