Changeset 252683 in webkit


Ignore:
Timestamp:
Nov 19, 2019, 9:12:14 PM (5 years ago)
Author:
Ross Kirsling
Message:

Implement String.prototype.replaceAll
​https://bugs.webkit.org/show_bug.cgi?id=202471

Reviewed by Yusuke Suzuki.

JSTests:

  • stress/string-replaceall.js: Added.

Source/JavaScriptCore:

Implement the stage 3 proposal here:
​https://github.com/tc39/proposal-string-replaceall

String.prototype.replaceAll is the same as String.prototype.replace, except:

  1. When the first argument is a string, all instances of the search string are replaced.
  2. When the first argument is a non-global regular expression, a TypeError is thrown.
  • builtins/BuiltinNames.h:
  • builtins/StringPrototype.js:

(replaceAll): Added.

  • runtime/StringPrototype.cpp:

(JSC::StringPrototype::finishCreation):
(JSC::jsSpliceSubstringsWithSeparators): Add early out for single-replacement case.
(JSC::replaceUsingStringSearch): Add global replacement logic, following replaceUsingRegExpSearch.
(JSC::replace):
(JSC::stringProtoFuncReplaceUsingStringSearch):
(JSC::stringProtoFuncReplaceAllUsingStringSearch): Added.

Source/WTF:

  • wtf/text/StringCommon.h:

(WTF::findCommon):
Fix logic: "start > length" early out should come before "empty search string" early out.

LayoutTests:

  • js/Object-getOwnPropertyNames-expected.txt:
  • js/script-tests/Object-getOwnPropertyNames.js:

Grrr, why is this a layout test...

Location:
trunk
Files:
1 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r252680 r252683  
     12019-11-19  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Implement String.prototype.replaceAll
     4        https://bugs.webkit.org/show_bug.cgi?id=202471
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        * stress/string-replaceall.js: Added.
     9
    1102019-11-19  Robin Morisset  <rmorisset@apple.com>
    211

  • trunk/LayoutTests/ChangeLog

    r252681 r252683  
     12019-11-19  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Implement String.prototype.replaceAll
     4        https://bugs.webkit.org/show_bug.cgi?id=202471
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        * js/Object-getOwnPropertyNames-expected.txt:
     9        * js/script-tests/Object-getOwnPropertyNames.js:
     10        Grrr, why is this a layout test...
     11
    1122019-11-19  Youenn Fablet  <youenn@apple.com>
    213

  • trunk/LayoutTests/js/Object-getOwnPropertyNames-expected.txt

    r246567 r252683  
    5050PASS getSortedOwnPropertyNames(Array.prototype) is ['concat', 'constructor', 'copyWithin', 'entries', 'every', 'fill', 'filter', 'find', 'findIndex', 'flat', 'flatMap', 'forEach', 'includes', 'indexOf', 'join', 'keys', 'lastIndexOf', 'length', 'map', 'pop', 'push', 'reduce', 'reduceRight', 'reverse', 'shift', 'slice', 'some', 'sort', 'splice', 'toLocaleString', 'toString', 'unshift', 'values']
    5151PASS getSortedOwnPropertyNames(String) is ['fromCharCode', 'fromCodePoint', 'length', 'name', 'prototype', 'raw']
    52 PASS getSortedOwnPropertyNames(String.prototype) is ['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']
     52PASS getSortedOwnPropertyNames(String.prototype) is ['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'replaceAll', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']
    5353PASS getSortedOwnPropertyNames(Boolean) is ['length', 'name', 'prototype']
    5454PASS getSortedOwnPropertyNames(Boolean.prototype) is ['constructor', 'toString', 'valueOf']

  • trunk/LayoutTests/js/script-tests/Object-getOwnPropertyNames.js

    r246567 r252683  
    5959    "Array.prototype": "['concat', 'constructor', 'copyWithin', 'entries', 'every', 'fill', 'filter', 'find', 'findIndex', 'flat', 'flatMap', 'forEach', 'includes', 'indexOf', 'join', 'keys', 'lastIndexOf', 'length', 'map', 'pop', 'push', 'reduce', 'reduceRight', 'reverse', 'shift', 'slice', 'some', 'sort', 'splice', 'toLocaleString', 'toString', 'unshift', 'values']",
    6060    "String": "['fromCharCode', 'fromCodePoint', 'length', 'name', 'prototype', 'raw']",
    61     "String.prototype": "['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']",
     61    "String.prototype": "['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'replaceAll', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']",
    6262    "Boolean": "['length', 'name', 'prototype']",
    6363    "Boolean.prototype": "['constructor', 'toString', 'valueOf']",

  • trunk/Source/JavaScriptCore/ChangeLog

    r252680 r252683  
     12019-11-19  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Implement String.prototype.replaceAll
     4        https://bugs.webkit.org/show_bug.cgi?id=202471
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        Implement the stage 3 proposal here:
     9        https://github.com/tc39/proposal-string-replaceall
     10
     11        String.prototype.replaceAll is the same as String.prototype.replace, except:
     12        1. When the first argument is a string, all instances of the search string are replaced.
     13        2. When the first argument is a non-global regular expression, a TypeError is thrown.
     14
     15        * builtins/BuiltinNames.h:
     16        * builtins/StringPrototype.js:
     17        (replaceAll): Added.
     18        * runtime/StringPrototype.cpp:
     19        (JSC::StringPrototype::finishCreation):
     20        (JSC::jsSpliceSubstringsWithSeparators): Add early out for single-replacement case.
     21        (JSC::replaceUsingStringSearch): Add global replacement logic, following replaceUsingRegExpSearch.
     22        (JSC::replace):
     23        (JSC::stringProtoFuncReplaceUsingStringSearch):
     24        (JSC::stringProtoFuncReplaceAllUsingStringSearch): Added.
     25
    1262019-11-19  Robin Morisset  <rmorisset@apple.com>
    227

  • trunk/Source/JavaScriptCore/builtins/BuiltinNames.h

    r252032 r252683  
    133133    macro(replaceUsingRegExp) \
    134134    macro(replaceUsingStringSearch) \
     135    macro(replaceAllUsingStringSearch) \
    135136    macro(makeTypeError) \
    136137    macro(mapBucket) \

  • trunk/Source/JavaScriptCore/builtins/StringPrototype.js

    r251483 r252683  
    260260}
    261261
     262function replaceAll(search, replace)
     263{
     264    "use strict";
     265
     266    if (@isUndefinedOrNull(this))
     267        @throwTypeError("String.prototype.replaceAll requires |this| not to be null nor undefined");
     268
     269    if (search != null) {
     270        if (@isRegExp(search) && !@stringIncludesInternal.@call(@toString(search.flags), "g"))
     271            @throwTypeError("String.prototype.replaceAll argument must not be a non-global regular expression");
     272
     273        var replacer = search.@replaceSymbol;
     274        if (replacer !== @undefined) {
     275            if (!@hasObservableSideEffectsForStringReplace(search, replacer))
     276                return @toString(this).@replaceUsingRegExp(search, replace);
     277            return replacer.@call(search, this, replace);
     278        }
     279    }
     280
     281    var thisString = @toString(this);
     282    var searchString = @toString(search);
     283    return thisString.@replaceAllUsingStringSearch(searchString, replace);
     284}
     285
    262286function search(regexp)
    263287{

  • trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp

    r252374 r252683  
    7171EncodedJSValue JSC_HOST_CALL stringProtoFuncReplaceUsingRegExp(JSGlobalObject*, CallFrame*);
    7272EncodedJSValue JSC_HOST_CALL stringProtoFuncReplaceUsingStringSearch(JSGlobalObject*, CallFrame*);
     73EncodedJSValue JSC_HOST_CALL stringProtoFuncReplaceAllUsingStringSearch(JSGlobalObject*, CallFrame*);
    7374EncodedJSValue JSC_HOST_CALL stringProtoFuncSlice(JSGlobalObject*, CallFrame*);
    7475EncodedJSValue JSC_HOST_CALL stringProtoFuncSubstr(JSGlobalObject*, CallFrame*);
    … …  
    9899/* Source for StringConstructor.lut.h
    99100@begin stringPrototypeTable
    100     concat    JSBuiltin    DontEnum|Function 1
    101     match     JSBuiltin    DontEnum|Function 1
    102     matchAll  JSBuiltin    DontEnum|Function 1
    103     padStart  JSBuiltin    DontEnum|Function 1
    104     padEnd    JSBuiltin    DontEnum|Function 1
    105     repeat    JSBuiltin    DontEnum|Function 1
    106     replace   JSBuiltin    DontEnum|Function 2
    107     search    JSBuiltin    DontEnum|Function 1
    108     split     JSBuiltin    DontEnum|Function 1
    109     anchor    JSBuiltin    DontEnum|Function 1
    110     big       JSBuiltin    DontEnum|Function 0
    111     bold      JSBuiltin    DontEnum|Function 0
    112     blink     JSBuiltin    DontEnum|Function 0
    113     fixed     JSBuiltin    DontEnum|Function 0
    114     fontcolor JSBuiltin    DontEnum|Function 1
    115     fontsize  JSBuiltin    DontEnum|Function 1
    116     italics   JSBuiltin    DontEnum|Function 0
    117     link      JSBuiltin    DontEnum|Function 1
    118     small     JSBuiltin    DontEnum|Function 0
    119     strike    JSBuiltin    DontEnum|Function 0
    120     sub       JSBuiltin    DontEnum|Function 0
    121     sup       JSBuiltin    DontEnum|Function 0
     101    concat        JSBuiltin    DontEnum|Function 1
     102    match         JSBuiltin    DontEnum|Function 1
     103    matchAll      JSBuiltin    DontEnum|Function 1
     104    padStart      JSBuiltin    DontEnum|Function 1
     105    padEnd        JSBuiltin    DontEnum|Function 1
     106    repeat        JSBuiltin    DontEnum|Function 1
     107    replace       JSBuiltin    DontEnum|Function 2
     108    replaceAll    JSBuiltin    DontEnum|Function 2
     109    search        JSBuiltin    DontEnum|Function 1
     110    split         JSBuiltin    DontEnum|Function 1
     111    anchor        JSBuiltin    DontEnum|Function 1
     112    big           JSBuiltin    DontEnum|Function 0
     113    bold          JSBuiltin    DontEnum|Function 0
     114    blink         JSBuiltin    DontEnum|Function 0
     115    fixed         JSBuiltin    DontEnum|Function 0
     116    fontcolor     JSBuiltin    DontEnum|Function 1
     117    fontsize      JSBuiltin    DontEnum|Function 1
     118    italics       JSBuiltin    DontEnum|Function 0
     119    link          JSBuiltin    DontEnum|Function 1
     120    small         JSBuiltin    DontEnum|Function 0
     121    strike        JSBuiltin    DontEnum|Function 0
     122    sub           JSBuiltin    DontEnum|Function 0
     123    sup           JSBuiltin    DontEnum|Function 0
    122124@end
    123125*/
    … …  
    143145    JSC_NATIVE_INTRINSIC_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->builtinNames().replaceUsingRegExpPrivateName(), stringProtoFuncReplaceUsingRegExp, static_cast<unsigned>(PropertyAttribute::DontEnum), 2, StringPrototypeReplaceRegExpIntrinsic);
    144146    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->builtinNames().replaceUsingStringSearchPrivateName(), stringProtoFuncReplaceUsingStringSearch, static_cast<unsigned>(PropertyAttribute::DontEnum), 2);
     147    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->builtinNames().replaceAllUsingStringSearchPrivateName(), stringProtoFuncReplaceAllUsingStringSearch, static_cast<unsigned>(PropertyAttribute::DontEnum), 2);
    145148    JSC_NATIVE_INTRINSIC_FUNCTION_WITHOUT_TRANSITION("slice", stringProtoFuncSlice, static_cast<unsigned>(PropertyAttribute::DontEnum), 2, StringPrototypeSliceIntrinsic);
    146149    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("substr", stringProtoFuncSubstr, static_cast<unsigned>(PropertyAttribute::DontEnum), 2);
    … …  
    395398    }
    396399
     400    if (rangeCount == 2 && separatorCount == 1) {
     401        String leftPart(StringImpl::createSubstringSharingImpl(*source.impl(), substringRanges[0].position, substringRanges[0].length));
     402        String rightPart(StringImpl::createSubstringSharingImpl(*source.impl(), substringRanges[1].position, substringRanges[1].length));
     403        RELEASE_AND_RETURN(scope, jsString(globalObject, leftPart, separators[0], rightPart));
     404    }
     405
    397406    Checked<int, RecordOverflow> totalLength = 0;
    398407    bool allSeparators8Bit = true;
    … …  
    768777}
    769778
    770 static ALWAYS_INLINE JSString* replaceUsingStringSearch(VM& vm, JSGlobalObject* globalObject, JSString* jsString, JSValue searchValue, JSValue replaceValue)
     779static ALWAYS_INLINE JSString* replaceUsingStringSearch(VM& vm, JSGlobalObject* globalObject, CallFrame* callFrame, JSString* jsString, JSValue searchValue, JSValue replaceValue, bool isGlobal)
    771780{
    772781    auto scope = DECLARE_THROW_SCOPE(vm);
    … …  
    778787
    779788    size_t matchStart = string.find(searchString);
    780 
    781789    if (matchStart == notFound)
    782790        return jsString;
    … …  
    784792    CallData callData;
    785793    CallType callType = getCallData(vm, replaceValue, callData);
    786     if (callType != CallType::None) {
    787         MarkedArgumentBuffer args;
    788         auto* substring = jsSubstring(vm, string, matchStart, searchString.impl()->length());
     794    Optional<CachedCall> cachedCall;
     795    String replaceString;
     796    if (callType == CallType::None) {
     797        replaceString = replaceValue.toWTFString(globalObject);
    789798        RETURN_IF_EXCEPTION(scope, nullptr);
    790         args.append(substring);
    791         args.append(jsNumber(matchStart));
    792         args.append(jsString);
    793         ASSERT(!args.hasOverflowed());
    794         replaceValue = call(globalObject, replaceValue, callType, callData, jsUndefined(), args);
    795         RETURN_IF_EXCEPTION(scope, nullptr);
    796     }
    797 
    798     String replaceString = replaceValue.toWTFString(globalObject);
    799     RETURN_IF_EXCEPTION(scope, nullptr);
    800 
    801     StringImpl* stringImpl = string.impl();
    802     String leftPart(StringImpl::createSubstringSharingImpl(*stringImpl, 0, matchStart));
    803 
    804     size_t matchEnd = matchStart + searchString.impl()->length();
    805     int ovector[2] = { static_cast<int>(matchStart),  static_cast<int>(matchEnd)};
    806     String middlePart;
    807     if (callType != CallType::None)
    808         middlePart = replaceString;
    809     else {
    810         StringBuilder replacement(StringBuilder::OverflowHandler::RecordOverflow);
    811         substituteBackreferences(replacement, replaceString, string, ovector, 0);
    812         if (UNLIKELY(replacement.hasOverflowed()))
     799    } else {
     800        cachedCall.emplace(globalObject, callFrame, jsCast<JSFunction*>(replaceValue), 3);
     801        cachedCall->setThis(jsUndefined());
     802    }
     803
     804    size_t endOfLastMatch = 0;
     805    size_t searchStringLength = searchString.length();
     806    Vector<StringRange, 16> sourceRanges;
     807    Vector<String, 16> replacements;
     808    do {
     809        if (cachedCall) {
     810            auto* substring = jsSubstring(vm, string, matchStart, searchStringLength);
     811            RETURN_IF_EXCEPTION(scope, nullptr);
     812            cachedCall->clearArguments();
     813            cachedCall->appendArgument(substring);
     814            cachedCall->appendArgument(jsNumber(matchStart));
     815            cachedCall->appendArgument(jsString);
     816            if (UNLIKELY(cachedCall->hasOverflowedArguments()))
     817                OUT_OF_MEMORY(globalObject, scope);
     818            JSValue replacement = cachedCall->call();
     819            RETURN_IF_EXCEPTION(scope, nullptr);
     820            replaceString = replacement.toWTFString(globalObject);
     821            RETURN_IF_EXCEPTION(scope, nullptr);
     822        }
     823
     824        if (UNLIKELY(!sourceRanges.tryConstructAndAppend(endOfLastMatch, matchStart - endOfLastMatch)))
    813825            OUT_OF_MEMORY(globalObject, scope);
    814         middlePart = replacement.toString();
    815     }
    816 
    817     size_t leftLength = stringImpl->length() - matchEnd;
    818     String rightPart(StringImpl::createSubstringSharingImpl(*stringImpl, matchEnd, leftLength));
    819     RELEASE_AND_RETURN(scope, JSC::jsString(globalObject, leftPart, middlePart, rightPart));
     826
     827        size_t matchEnd = matchStart + searchStringLength;
     828        int ovector[2] = { static_cast<int>(matchStart),  static_cast<int>(matchEnd)};
     829        if (cachedCall) {
     830            replacements.append(replaceString);
     831            RETURN_IF_EXCEPTION(scope, nullptr);
     832        } else {
     833            StringBuilder replacement(StringBuilder::OverflowHandler::RecordOverflow);
     834            substituteBackreferences(replacement, replaceString, string, ovector, nullptr);
     835            if (UNLIKELY(replacement.hasOverflowed()))
     836                OUT_OF_MEMORY(globalObject, scope);
     837            replacements.append(replacement.toString());
     838        }
     839
     840        endOfLastMatch = matchEnd;
     841        if (!isGlobal)
     842            break;
     843        matchStart = string.find(searchString, UNLIKELY(!searchStringLength) ? endOfLastMatch + 1 : endOfLastMatch);
     844    } while (matchStart != notFound);
     845
     846    if (UNLIKELY(!sourceRanges.tryConstructAndAppend(endOfLastMatch, string.length() - endOfLastMatch)))
     847        OUT_OF_MEMORY(globalObject, scope);
     848    RELEASE_AND_RETURN(scope, jsSpliceSubstringsWithSeparators(globalObject, jsString, string, sourceRanges.data(), sourceRanges.size(), replacements.data(), replacements.size()));
    820849}
    821850
    … …  
    873902    if (searchValue.inherits<RegExpObject>(vm))
    874903        return replaceUsingRegExpSearch(vm, globalObject, callFrame, string, searchValue, replaceValue);
    875     return replaceUsingStringSearch(vm, globalObject, string, searchValue, replaceValue);
     904    constexpr bool isGlobal = false;
     905    return replaceUsingStringSearch(vm, globalObject, callFrame, string, searchValue, replaceValue, isGlobal);
    876906}
    877907
    … …  
    913943    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    914944
    915     RELEASE_AND_RETURN(scope, JSValue::encode(replaceUsingStringSearch(vm, globalObject, string, callFrame->argument(0), callFrame->argument(1))));
     945    constexpr bool isGlobal = false;
     946    RELEASE_AND_RETURN(scope, JSValue::encode(replaceUsingStringSearch(vm, globalObject, callFrame, string, callFrame->argument(0), callFrame->argument(1), isGlobal)));
     947}
     948
     949EncodedJSValue JSC_HOST_CALL stringProtoFuncReplaceAllUsingStringSearch(JSGlobalObject* globalObject, CallFrame* callFrame)
     950{
     951    VM& vm = globalObject->vm();
     952    auto scope = DECLARE_THROW_SCOPE(vm);
     953
     954    JSString* string = callFrame->thisValue().toString(globalObject);
     955    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     956
     957    constexpr bool isGlobal = true;
     958    RELEASE_AND_RETURN(scope, JSValue::encode(replaceUsingStringSearch(vm, globalObject, callFrame, string, callFrame->argument(0), callFrame->argument(1), isGlobal)));
    916959}
    917960

  • trunk/Source/WTF/ChangeLog

    r252642 r252683  
     12019-11-19  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Implement String.prototype.replaceAll
     4        https://bugs.webkit.org/show_bug.cgi?id=202471
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        * wtf/text/StringCommon.h:
     9        (WTF::findCommon):
     10        Fix logic: "start > length" early out should come before "empty search string" early out.
     11
    1122019-11-19  Yusuke Suzuki  <ysuzuki@apple.com>
    213

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

    r248831 r252683  
    571571    }
    572572
     573    if (start > haystack.length())
     574        return notFound;
     575
    573576    if (!needleLength)
    574577        return std::min(start, haystack.length());
    575578
    576     if (start > haystack.length())
    577         return notFound;
    578579    unsigned searchLength = haystack.length() - start;
    579580    if (needleLength > searchLength)
Note: See TracChangeset for help on using the changeset viewer.