Changeset 252721 in webkit

Timestamp:

Nov 20, 2019 5:16:22 PM (4 years ago)

Author:

Ross Kirsling

Message:

Unreviewed, address Darin's feedback on r252683.

JSTests:

• stress/string-replaceall.js:

Source/JavaScriptCore:

• runtime/StringPrototype.cpp:

(JSC::replaceUsingStringSearch):
(JSC::replace):
(JSC::stringProtoFuncReplaceUsingStringSearch):
(JSC::stringProtoFuncReplaceAllUsingStringSearch):

Source/WTF:

• wtf/text/StringCommon.h:

(WTF::findCommon):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/string-replaceall.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/StringPrototype.cpp (modified) (7 diffs)
• Source/WTF/ChangeLog (modified) (1 diff)
• Source/WTF/wtf/text/StringCommon.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-11-20  Ross Kirsling  <ross.kirsling@sony.com>
+
+        Unreviewed, address Darin's feedback on r252683.
+
+        * stress/string-replaceall.js:
+
 2019-11-20  Caio Lima  <ticaiolima@gmail.com>
 

trunk/JSTests/stress/string-replaceall.js

@@ -35 +35 @@
 search[Symbol.replace] = RegExp.prototype[Symbol.replace].bind(search);
 shouldBe('abcdefabcdefabc'.replaceAll(search, 'xyz'), 'abcxyzabcxyzabc');
+
+shouldBe('abc'.replaceAll('', 'z'), 'zazbzcz');
+shouldBe(''.replaceAll('', 'z'), 'z');
+shouldBe('abc'.replaceAll('', ''), 'abc');
+shouldBe(''.replaceAll('', ''), '');

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-11-20  Ross Kirsling  <ross.kirsling@sony.com>
+
+        Unreviewed, address Darin's feedback on r252683.
+
+        * runtime/StringPrototype.cpp:
+        (JSC::replaceUsingStringSearch):
+        (JSC::replace):
+        (JSC::stringProtoFuncReplaceUsingStringSearch):
+        (JSC::stringProtoFuncReplaceAllUsingStringSearch):
+
 2019-11-20  Caio Lima  <ticaiolima@gmail.com>
 

trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp

@@ -777 +777 @@
 }
 
-static ALWAYS_INLINE JSString* replaceUsingStringSearch(VM& vm, JSGlobalObject* globalObject, CallFrame* callFrame, JSString* jsString, JSValue searchValue, JSValue replaceValue, bool isGlobal)
+enum class ReplaceMode : bool { Single, Global };
+
+static ALWAYS_INLINE JSString* replaceUsingStringSearch(VM& vm, JSGlobalObject* globalObject, CallFrame* callFrame, JSString* jsString, JSValue searchValue, JSValue replaceValue, ReplaceMode mode)
 {
     auto scope = DECLARE_THROW_SCOPE(vm);
@@ -814 +816 @@
             cachedCall->appendArgument(jsNumber(matchStart));
             cachedCall->appendArgument(jsString);
-            if (UNLIKELY(cachedCall->hasOverflowedArguments()))
-                OUT_OF_MEMORY(globalObject, scope);
+            ASSERT(!cachedCall->hasOverflowedArguments());
             JSValue replacement = cachedCall->call();
             RETURN_IF_EXCEPTION(scope, nullptr);
@@ -826 +827 @@
 
         size_t matchEnd = matchStart + searchStringLength;
-        int ovector[2] = { static_cast<int>(matchStart),  static_cast<int>(matchEnd)};
-        if (cachedCall) {
+        if (cachedCall)
             replacements.append(replaceString);
-            RETURN_IF_EXCEPTION(scope, nullptr);
-        } else {
+        else {
             StringBuilder replacement(StringBuilder::OverflowHandler::RecordOverflow);
+            int ovector[2] = { static_cast<int>(matchStart),  static_cast<int>(matchEnd) };
             substituteBackreferences(replacement, replaceString, string, ovector, nullptr);
             if (UNLIKELY(replacement.hasOverflowed()))
@@ -839 +839 @@
 
         endOfLastMatch = matchEnd;
-        if (!isGlobal)
+        if (mode == ReplaceMode::Single)
             break;
-        matchStart = string.find(searchString, UNLIKELY(!searchStringLength) ? endOfLastMatch + 1 : endOfLastMatch);
+        matchStart = string.find(searchString, !searchStringLength ? endOfLastMatch + 1 : endOfLastMatch);
     } while (matchStart != notFound);
 
@@ -902 +902 @@
     if (searchValue.inherits<RegExpObject>(vm))
         return replaceUsingRegExpSearch(vm, globalObject, callFrame, string, searchValue, replaceValue);
-    constexpr bool isGlobal = false;
-    return replaceUsingStringSearch(vm, globalObject, callFrame, string, searchValue, replaceValue, isGlobal);
+    return replaceUsingStringSearch(vm, globalObject, callFrame, string, searchValue, replaceValue, ReplaceMode::Single);
 }
 
@@ -943 +942 @@
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
-    constexpr bool isGlobal = false;
-    RELEASE_AND_RETURN(scope, JSValue::encode(replaceUsingStringSearch(vm, globalObject, callFrame, string, callFrame->argument(0), callFrame->argument(1), isGlobal)));
+    RELEASE_AND_RETURN(scope, JSValue::encode(replaceUsingStringSearch(vm, globalObject, callFrame, string, callFrame->argument(0), callFrame->argument(1), ReplaceMode::Single)));
 }
 
@@ -955 +953 @@
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
-    constexpr bool isGlobal = true;
-    RELEASE_AND_RETURN(scope, JSValue::encode(replaceUsingStringSearch(vm, globalObject, callFrame, string, callFrame->argument(0), callFrame->argument(1), isGlobal)));
+    RELEASE_AND_RETURN(scope, JSValue::encode(replaceUsingStringSearch(vm, globalObject, callFrame, string, callFrame->argument(0), callFrame->argument(1), ReplaceMode::Global)));
 }
 

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2019-11-20  Ross Kirsling  <ross.kirsling@sony.com>
+
+        Unreviewed, address Darin's feedback on r252683.
+
+        * wtf/text/StringCommon.h:
+        (WTF::findCommon):
+
 2019-11-20  ChangSeok Oh  <changseok@webkit.org>
 

trunk/Source/WTF/wtf/text/StringCommon.h

@@ -575 +575 @@
 
     if (!needleLength)
-        return std::min(start, haystack.length());
+        return start;
 
     unsigned searchLength = haystack.length() - start;