[regexp] Avoid unneeded accesses to lastIndex

This implements https://github.com/tc39/ecma262/pull/627/.

BUG=v8:5360

Review-Url: https://codereview.chromium.org/2339443002
Cr-Commit-Position: refs/heads/master@{#39402}

src/js/regexp.js

@@ -172,29 +172,26 @@ function RegExpSubclassExecJS(string) {
   }
 
   string = TO_STRING(string);
-  var lastIndex = this.lastIndex;
-
-  // Conversion is required by the ES2015 specification (RegExpBuiltinExec
-  // algorithm, step 4) even if the value is discarded for non-global RegExps.
-  var i = TO_LENGTH(lastIndex);
 
+  var lastIndex;
   var global = TO_BOOLEAN(REGEXP_GLOBAL(this));
   var sticky = TO_BOOLEAN(REGEXP_STICKY(this));
   var updateLastIndex = global || sticky;
   if (updateLastIndex) {
-    if (i > string.length) {
+    lastIndex = TO_LENGTH(this.lastIndex);
+    if (lastIndex > string.length) {
       this.lastIndex = 0;
       return null;
     }
   } else {
-    i = 0;
+    lastIndex = 0;
   }
 
   // matchIndices is either null or the RegExpLastMatchInfo array.
-  var matchIndices = %_RegExpExec(this, string, i, RegExpLastMatchInfo);
+  var matchIndices = %_RegExpExec(this, string, lastIndex, RegExpLastMatchInfo);
 
   if (IS_NULL(matchIndices)) {
-    this.lastIndex = 0;
+    if (updateLastIndex) this.lastIndex = 0;
     return null;
   }
 
@@ -851,9 +848,10 @@ function RegExpSubclassSearch(string) {
   }
   string = TO_STRING(string);
   var previousLastIndex = this.lastIndex;
-  this.lastIndex = 0;
+  if (previousLastIndex != 0) this.lastIndex = 0;
   var result = RegExpSubclassExec(this, string);
-  this.lastIndex = previousLastIndex;
+  var currentLastIndex = this.lastIndex;
+  if (currentLastIndex != previousLastIndex) this.lastIndex = previousLastIndex;
   if (IS_NULL(result)) return -1;
   return result.index;
 }

test/mjsunit/regexp.js

@@ -564,21 +564,21 @@ log = [];
 re.lastIndex = fakeLastIndex;
 var result = re.exec(fakeString);
 assertEquals(["str"], result);
-assertEquals(["ts", "li"], log);
+assertEquals(["ts"], log);
 
 // Again, to check if caching interferes.
 log = [];
 re.lastIndex = fakeLastIndex;
 result = re.exec(fakeString);
 assertEquals(["str"], result);
-assertEquals(["ts", "li"], log);
+assertEquals(["ts"], log);
 
 // And one more time, just to be certain.
 log = [];
 re.lastIndex = fakeLastIndex;
 result = re.exec(fakeString);
 assertEquals(["str"], result);
-assertEquals(["ts", "li"], log);
+assertEquals(["ts"], log);
 
 // Now with a global regexp, where lastIndex is actually used.
 re = /str/g;

test/mjsunit/regress/regress-2437.js

@@ -26,7 +26,7 @@
 // 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 or sticky regexp fails to match.
 // - 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.
@@ -37,19 +37,19 @@
 r = /a/;
 r.lastIndex = 1;
 r.exec("zzzz");
-assertEquals(0, r.lastIndex);
+assertEquals(1, r.lastIndex);
 
 // Test Regexp.prototype.test
 r = /a/;
 r.lastIndex = 1;
 r.test("zzzz");
-assertEquals(0, r.lastIndex);
+assertEquals(1, r.lastIndex);
 
 // Test String.prototype.match
 r = /a/;
 r.lastIndex = 1;
 "zzzz".match(r);
-assertEquals(0, r.lastIndex);
+assertEquals(1, r.lastIndex);
 
 // Test String.prototype.replace with atomic regexp and empty string.
 r = /a/;
@@ -116,7 +116,7 @@ assertEquals(-1, r.lastIndex);
 
 r.lastIndex = -1;
 "01234567".match(r);
-assertEquals(0, r.lastIndex);
+assertEquals(-1, r.lastIndex);
 
 // Also test RegExp.prototype.exec and RegExp.prototype.test
 r = /a/g;
@@ -131,7 +131,7 @@ assertEquals(5, r.lastIndex);
 r = /a/;
 r.lastIndex = 1;
 r.exec("01234567");
-assertEquals(0, r.lastIndex);
+assertEquals(1, r.lastIndex);
 
 r.lastIndex = 1;
 r.exec("0123abcd");
@@ -149,7 +149,7 @@ assertEquals(5, r.lastIndex);
 r = /a/;
 r.lastIndex = 1;
 r.test("01234567");
-assertEquals(0, r.lastIndex);
+assertEquals(1, r.lastIndex);
 
 r.lastIndex = 1;
 r.test("0123abcd");

test/mjsunit/regress/regress-2438.js

@@ -27,14 +27,20 @@
 
 function testSideEffects(subject, re) {
   var counter = 0;
+  var expected_counter = 0;
+  const accesses_lastindex = (re.global || re.sticky);
   var side_effect_object = { valueOf: function() { return counter++; } };
   re.lastIndex = side_effect_object;
   re.exec(subject);
-  assertEquals(1, counter);
+
+  if (accesses_lastindex) expected_counter++;
+  assertEquals(expected_counter, counter);
 
   re.lastIndex = side_effect_object;
   re.test(subject);
-  assertEquals(2, counter);
+
+  if (accesses_lastindex) expected_counter++;
+  assertEquals(expected_counter, counter);
 }
 
 testSideEffects("zzzz", /a/);

test/test262/test262.status

@@ -126,6 +126,11 @@
 
   ###### END REGEXP SUBCLASSING SECTION ######
 
+  # https://bugs.chromium.org/p/v8/issues/detail?id=5360
+  'built-ins/RegExp/prototype/Symbol.match/builtin-coerce-lastindex-err': [FAIL],
+  'built-ins/RegExp/prototype/Symbol.match/builtin-failure-set-lastindex': [FAIL],
+  'built-ins/RegExp/prototype/Symbol.search/set-lastindex-restore': [FAIL],
+
   # https://code.google.com/p/v8/issues/detail?id=4360
   'intl402/Collator/10.1.1_1': [FAIL],
   'intl402/DateTimeFormat/12.1.1_1': [FAIL],