From: yangguo@chromium.org Date: Fri, 25 Jan 2013 10:53:26 +0000 (+0000) Subject: Fix additional spec violations wrt RegExp.lastIndex. X-Git-Tag: upstream/4.7.83~15202 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=24ec13cbd2cf61c4d69852f5a51db8bc870d5d55;p=platform%2Fupstream%2Fv8.git 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 --- 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 regexp) { - // Reset lastIndex property to 0. - SetProperty(regexp, - isolate->factory()->last_index_symbol(), - Handle(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 --- 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 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 --- 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 --- 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 --- 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);