Changeset 252758 in webkit
- Timestamp:
- Nov 21, 2019 6:16:57 PM (4 years ago)
- Location:
- trunk
- Files:
-
- 1 added
- 1 deleted
- 4 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JSTests/ChakraCore.yaml
r251819 r252758 1303 1303 cmd: runChakra :baseline, "NoException", "compare.baseline", [] 1304 1304 - path: ChakraCore/test/Strings/replace.js 1305 cmd: runChakra :baseline, "NoException", "replace.baseline -jsc", []1305 cmd: runChakra :baseline, "NoException", "replace.baseline", [] 1306 1306 - path: ChakraCore/test/Strings/trim.js 1307 1307 cmd: runChakra :baseline, "NoException", "trim.baseline", [] -
trunk/JSTests/ChangeLog
r252754 r252758 1 2019-11-21 Mark Lam <mark.lam@apple.com> 2 3 Fix broken String.prototype.replace() and replaceAll(). 4 https://bugs.webkit.org/show_bug.cgi?id=204479 5 <rdar://problem/57354854> 6 7 Reviewed by Ross Kirsling and Yusuke Suzuki. 8 9 * ChakraCore.yaml: 10 * ChakraCore/test/Strings/replace.baseline-jsc: Removed. 11 - We no longer need this because we've fixed the spec compliance issue in replace(). 12 * stress/string-replaceAll-2.js: Added. 13 1 14 2019-11-21 Yusuke Suzuki <ysuzuki@apple.com> 2 15 -
trunk/Source/JavaScriptCore/ChangeLog
r252756 r252758 1 2019-11-21 Mark Lam <mark.lam@apple.com> 2 3 Fix broken String.prototype.replace() and replaceAll(). 4 https://bugs.webkit.org/show_bug.cgi?id=204479 5 <rdar://problem/57354854> 6 7 Reviewed by Ross Kirsling and Yusuke Suzuki. 8 9 String.prototype.replace() regressed due to r252683: <https://trac.webkit.org/r252683> 10 for webkit.org/b/202471. The patch failed to handle InternalFunctions. 11 12 This patch also fixed a spec compliance bug for String.prototype.replace() i.e. 13 the replaceValue needs to be evaluated before we check if there's a match in the 14 source string. 15 Ref: 21.1.3.16-6 at https://www.ecma-international.org/ecma-262/10.0/#sec-string.prototype.replace 16 17 For String.prototype.replaceAll(), make sure it "behaves just like 18 String.prototype.replace if searchValue is a global regular expression". 19 Ref: https://github.com/tc39/proposal-string-replaceall 20 21 r252683 also made replaceUsingStringSearch() work the same way as 22 replaceUsingRegExpSearch(). I think this is the wrong trade off to make. 23 replaceUsingRegExpSearch() expects each search leg to do a RegExp search, which 24 is inherently expensive. We shouldn't make string searches slower just because 25 the RegExp search does it that way. 26 27 However, at https://bugs.webkit.org/show_bug.cgi?id=202471#c22, Ross pointed out 28 that JetStream 2 results appeared to be neutral. I think we should double check 29 with a micro-benchmark as well. I'll leave this for a later patch. For now, the 30 goal of this patch is simply to achieve correctness. 31 Ref: https://bugs.webkit.org/show_bug.cgi?id=204481 32 33 * runtime/StringPrototype.cpp: 34 (JSC::replaceUsingRegExpSearch): 35 (JSC::replaceUsingStringSearch): 36 1 37 2019-11-21 Per Arne Vollan <pvollan@apple.com> 2 38 -
trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp
r252754 r252758 665 665 groups->putDirect(vm, Identifier::fromString(vm, groupName), patternValue); 666 666 } 667 668 667 } 669 668 … … 788 787 RETURN_IF_EXCEPTION(scope, nullptr); 789 788 790 size_t matchStart = string.find(searchString);791 if (matchStart == notFound)792 return jsString;793 794 789 CallData callData; 795 790 CallType callType = getCallData(vm, replaceValue, callData); … … 800 795 RETURN_IF_EXCEPTION(scope, nullptr); 801 796 } else { 802 cachedCall.emplace(globalObject, callFrame, jsCast<JSFunction*>(replaceValue), 3); 803 cachedCall->setThis(jsUndefined()); 804 } 797 JSFunction* function = jsDynamicCast<JSFunction*>(vm, replaceValue); 798 if (function) { 799 cachedCall.emplace(globalObject, callFrame, function, 3); 800 cachedCall->setThis(jsUndefined()); 801 } 802 } 803 804 size_t matchStart = string.find(searchString); 805 if (matchStart == notFound) 806 return jsString; 805 807 806 808 size_t endOfLastMatch = 0; … … 809 811 Vector<String, 16> replacements; 810 812 do { 811 if (cachedCall) { 812 auto* substring = jsSubstring(vm, string, matchStart, searchStringLength); 813 RETURN_IF_EXCEPTION(scope, nullptr); 814 cachedCall->clearArguments(); 815 cachedCall->appendArgument(substring); 816 cachedCall->appendArgument(jsNumber(matchStart)); 817 cachedCall->appendArgument(jsString); 818 ASSERT(!cachedCall->hasOverflowedArguments()); 819 JSValue replacement = cachedCall->call(); 813 if (callType != CallType::None) { 814 JSValue replacement; 815 if (cachedCall) { 816 auto* substring = jsSubstring(vm, string, matchStart, searchStringLength); 817 RETURN_IF_EXCEPTION(scope, nullptr); 818 cachedCall->clearArguments(); 819 cachedCall->appendArgument(substring); 820 cachedCall->appendArgument(jsNumber(matchStart)); 821 cachedCall->appendArgument(jsString); 822 ASSERT(!cachedCall->hasOverflowedArguments()); 823 replacement = cachedCall->call(); 824 } else { 825 MarkedArgumentBuffer args; 826 auto* substring = jsSubstring(vm, string, matchStart, searchString.impl()->length()); 827 RETURN_IF_EXCEPTION(scope, nullptr); 828 args.append(substring); 829 args.append(jsNumber(matchStart)); 830 args.append(jsString); 831 ASSERT(!args.hasOverflowed()); 832 replacement = call(globalObject, replaceValue, callType, callData, jsUndefined(), args); 833 } 820 834 RETURN_IF_EXCEPTION(scope, nullptr); 821 835 replaceString = replacement.toWTFString(globalObject); … … 827 841 828 842 size_t matchEnd = matchStart + searchStringLength; 829 if (ca chedCall)843 if (callType != CallType::None) 830 844 replacements.append(replaceString); 831 845 else {
Note: See TracChangeset
for help on using the changeset viewer.