Changeset 208767 in webkit


Ignore:
Timestamp:
Nov 15, 2016 4:19:54 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

Remove JSString::SafeView and replace its uses with StringViewWithUnderlyingString.
​https://bugs.webkit.org/show_bug.cgi?id=164777

Reviewed by Geoffrey Garen.

JSString::SafeView no longer achieves its intended goal to make it easier to
handle strings safely. Its clients still need to do explicit exception checks in
order to be correct. We'll remove it and replace its uses with
StringViewWithUnderlyingString instead which serves to gets the a StringView
(which is what we really wanted from SafeView) and keeps the backing String alive
while the view is in use.

Also added some missing exception checks.

  • jsc.cpp:

(printInternal):
(functionDebug):

  • runtime/ArrayPrototype.cpp:

(JSC::arrayProtoFuncJoin):

  • runtime/FunctionConstructor.cpp:

(JSC::constructFunctionSkippingEvalEnabledCheck):

  • runtime/IntlCollatorPrototype.cpp:

(JSC::IntlCollatorFuncCompare):

  • runtime/JSGenericTypedArrayViewPrototypeFunctions.h:

(JSC::genericTypedArrayViewProtoFuncJoin):

  • runtime/JSGlobalObjectFunctions.cpp:

(JSC::toStringView):
(JSC::globalFuncParseFloat):

  • runtime/JSONObject.cpp:

(JSC::JSONProtoFuncParse):

  • runtime/JSString.h:

(JSC::JSString::SafeView::is8Bit): Deleted.
(JSC::JSString::SafeView::length): Deleted.
(JSC::JSString::SafeView::SafeView): Deleted.
(JSC::JSString::SafeView::get): Deleted.
(JSC::JSString::view): Deleted.

  • runtime/StringPrototype.cpp:

(JSC::stringProtoFuncRepeatCharacter):
(JSC::stringProtoFuncCharAt):
(JSC::stringProtoFuncCharCodeAt):
(JSC::stringProtoFuncIndexOf):
(JSC::stringProtoFuncNormalize):

Location:
trunk/Source/JavaScriptCore
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r208763 r208767  
     12016-11-15  Mark Lam  <mark.lam@apple.com>
     2
     3        Remove JSString::SafeView and replace its uses with StringViewWithUnderlyingString.
     4        https://bugs.webkit.org/show_bug.cgi?id=164777
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        JSString::SafeView no longer achieves its intended goal to make it easier to
     9        handle strings safely.  Its clients still need to do explicit exception checks in
     10        order to be correct.  We'll remove it and replace its uses with
     11        StringViewWithUnderlyingString instead which serves to gets the a StringView
     12        (which is what we really wanted from SafeView) and keeps the backing String alive
     13        while the view is in use.
     14
     15        Also added some missing exception checks.
     16
     17        * jsc.cpp:
     18        (printInternal):
     19        (functionDebug):
     20        * runtime/ArrayPrototype.cpp:
     21        (JSC::arrayProtoFuncJoin):
     22        * runtime/FunctionConstructor.cpp:
     23        (JSC::constructFunctionSkippingEvalEnabledCheck):
     24        * runtime/IntlCollatorPrototype.cpp:
     25        (JSC::IntlCollatorFuncCompare):
     26        * runtime/JSGenericTypedArrayViewPrototypeFunctions.h:
     27        (JSC::genericTypedArrayViewProtoFuncJoin):
     28        * runtime/JSGlobalObjectFunctions.cpp:
     29        (JSC::toStringView):
     30        (JSC::globalFuncParseFloat):
     31        * runtime/JSONObject.cpp:
     32        (JSC::JSONProtoFuncParse):
     33        * runtime/JSString.h:
     34        (JSC::JSString::SafeView::is8Bit): Deleted.
     35        (JSC::JSString::SafeView::length): Deleted.
     36        (JSC::JSString::SafeView::SafeView): Deleted.
     37        (JSC::JSString::SafeView::get): Deleted.
     38        (JSC::JSString::view): Deleted.
     39        * runtime/StringPrototype.cpp:
     40        (JSC::stringProtoFuncRepeatCharacter):
     41        (JSC::stringProtoFuncCharAt):
     42        (JSC::stringProtoFuncCharCodeAt):
     43        (JSC::stringProtoFuncIndexOf):
     44        (JSC::stringProtoFuncNormalize):
     45
    1462016-11-15  Filip Pizlo  <fpizlo@apple.com>
    247

  • trunk/Source/JavaScriptCore/jsc.cpp

    r208749 r208767  
    15321532static EncodedJSValue printInternal(ExecState* exec, FILE* out)
    15331533{
     1534    VM& vm = exec->vm();
     1535    auto scope = DECLARE_THROW_SCOPE(vm);
     1536
    15341537    if (test262AsyncTest) {
    15351538        JSValue value = exec->argument(0);
    … …  
    15441547                goto fail;
    15451548
    1546         if (fprintf(out, "%s", exec->uncheckedArgument(i).toString(exec)->view(exec).get().utf8().data()) < 0)
     1549        auto viewWithString = exec->uncheckedArgument(i).toString(exec)->viewWithUnderlyingString(*exec);
     1550        RETURN_IF_EXCEPTION(scope, encodedJSValue());
     1551        if (fprintf(out, "%s", viewWithString.view.utf8().data()) < 0)
    15471552            goto fail;
    15481553    }
    … …  
    15701575EncodedJSValue JSC_HOST_CALL functionDebug(ExecState* exec)
    15711576{
    1572     fprintf(stderr, "--> %s\n", exec->argument(0).toString(exec)->view(exec).get().utf8().data());
     1577    VM& vm = exec->vm();
     1578    auto scope = DECLARE_THROW_SCOPE(vm);
     1579    auto viewWithString = exec->argument(0).toString(exec)->viewWithUnderlyingString(*exec);
     1580    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     1581    fprintf(stderr, "--> %s\n", viewWithString.view.utf8().data());
    15731582    return JSValue::encode(jsUndefined());
    15741583}

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

    r207851 r208767  
    668668    }
    669669
    670     return JSValue::encode(fastJoin(*exec, thisObject, jsSeparator->view(exec).get(), length));
     670    auto viewWithString = jsSeparator->viewWithUnderlyingString(*exec);
     671    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     672
     673    scope.release();
     674    return JSValue::encode(fastJoin(*exec, thisObject, viewWithString.view, length));
    671675}
    672676

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

    r208052 r208767  
    124124        builder.append(functionName.string());
    125125        builder.append('(');
    126         builder.append(args.at(0).toString(exec)->view(exec).get());
     126        auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(*exec);
     127        RETURN_IF_EXCEPTION(scope, nullptr);
     128        builder.append(viewWithString.view);
    127129        for (size_t i = 1; i < args.size() - 1; i++) {
    128130            builder.appendLiteral(", ");
    129             builder.append(args.at(i).toString(exec)->view(exec).get());
     131            auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(*exec);
     132            RETURN_IF_EXCEPTION(scope, nullptr);
     133            builder.append(viewWithString.view);
    130134        }
    131135        builder.appendLiteral(") {\n");
    132         builder.append(args.at(args.size() - 1).toString(exec)->view(exec).get());
     136        viewWithString = args.at(args.size() - 1).toString(exec)->viewWithUnderlyingString(*exec);
     137        RETURN_IF_EXCEPTION(scope, nullptr);
     138        builder.append(viewWithString.view);
    133139        builder.appendLiteral("\n}}");
    134140        program = builder.toString();

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

    r206386 r208767  
    9999
    100100    // 9. Return CompareStrings(collator, X, Y).
    101     return JSValue::encode(collator->compareStrings(*state, x->view(state).get(), y->view(state).get()));
     101    auto xViewWithString = x->viewWithUnderlyingString(*state);
     102    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     103    auto yViewWithString = y->viewWithUnderlyingString(*state);
     104    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     105    scope.release();
     106    return JSValue::encode(collator->compareStrings(*state, xViewWithString.view, yViewWithString.view));
    102107}
    103108

  • trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h

    r208209 r208767  
    288288    if (thisObject->isNeutered())
    289289        return throwVMTypeError(exec, scope, typedArrayBufferHasBeenDetachedErrorMessage);
    290     return joinWithSeparator(separatorString->view(exec).get());
     290    auto viewWithString = separatorString->viewWithUnderlyingString(*exec);
     291    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     292    return joinWithSeparator(viewWithString.view);
    291293}
    292294

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

    r208712 r208767  
    6666    if (UNLIKELY(!string))
    6767        return { };
    68     JSString::SafeView view = string->view(exec);
    69     StringView stringView = view.get();
     68    auto viewWithString = string->viewWithUnderlyingString(*exec);
    7069    RETURN_IF_EXCEPTION(scope, { });
    71     return callback(stringView);
     70    return callback(viewWithString.view);
    7271}
    7372
    … …  
    719718EncodedJSValue JSC_HOST_CALL globalFuncParseFloat(ExecState* exec)
    720719{
    721     return JSValue::encode(jsNumber(parseFloat(exec->argument(0).toString(exec)->view(exec).get())));
     720    auto viewWithString = exec->argument(0).toString(exec)->viewWithUnderlyingString(*exec);
     721    return JSValue::encode(jsNumber(parseFloat(viewWithString.view)));
    722722}
    723723

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

    r208699 r208767  
    762762    if (!exec->argumentCount())
    763763        return throwVMError(exec, scope, createError(exec, ASCIILiteral("JSON.parse requires at least one parameter")));
    764     JSString::SafeView source = exec->uncheckedArgument(0).toString(exec)->view(exec);
     764    auto viewWithString = exec->uncheckedArgument(0).toString(exec)->viewWithUnderlyingString(*exec);
    765765    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    766     StringView view = source.get();
    767     RETURN_IF_EXCEPTION(scope, encodedJSValue());
     766    StringView view = viewWithString.view;
    768767
    769768    JSValue unfiltered;

  • trunk/Source/JavaScriptCore/runtime/JSString.h

    r208699 r208767  
    151151    RefPtr<AtomicStringImpl> toExistingAtomicString(ExecState*) const;
    152152
    153     class SafeView;
    154     SafeView view(ExecState*) const;
    155153    StringViewWithUnderlyingString viewWithUnderlyingString(ExecState&) const;
    156154
    … …  
    476474};
    477475
    478 class JSString::SafeView {
    479 public:
    480     explicit SafeView(ExecState&, const JSString&);
    481     StringView get() const;
    482 
    483     bool is8Bit() const { return m_string->is8Bit(); }
    484     unsigned length() const { return m_string->length(); }
    485 
    486 private:
    487     ExecState& m_state;
    488 
    489     // The following pointer is marked "volatile" to make the compiler leave it on the stack
    490     // or in a register as long as this object is alive, even after the last use of the pointer.
    491     // That's needed to prevent garbage collecting the string and possibly deleting the block
    492     // with the characters in it, and then using the StringView after that.
    493     const JSString* volatile m_string;
    494 };
    495 
    496476JS_EXPORT_PRIVATE JSString* jsStringWithCacheSlowCase(VM&, StringImpl&);
    497477
    … …  
    760740}
    761741
    762 inline JSString::SafeView::SafeView(ExecState& state, const JSString& string)
    763     : m_state(state)
    764     , m_string(&string)
    765 {
    766 }
    767 
    768 inline StringView JSString::SafeView::get() const
    769 {
    770     return m_string->unsafeView(m_state);
    771 }
    772 
    773 ALWAYS_INLINE JSString::SafeView JSString::view(ExecState* exec) const
    774 {
    775     return SafeView(*exec, *this);
    776 }
    777 
    778742// --- JSValue inlines ----------------------------
    779743

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

    r208699 r208767  
    813813    ASSERT(!repeatCountValue.isDouble() || repeatCountValue.asDouble() == repeatCount);
    814814
    815     JSString::SafeView safeView = string->view(exec);
    816     StringView view = safeView.get();
     815    auto viewWithString = string->viewWithUnderlyingString(*exec);
     816    StringView view = viewWithString.view;
    817817    ASSERT(view.length() == 1 && !scope.exception());
    818818    UChar character = view[0];
    … …  
    907907    if (!checkObjectCoercible(thisValue))
    908908        return throwVMTypeError(exec, scope);
    909     JSString::SafeView string = thisValue.toString(exec)->view(exec);
    910     RETURN_IF_EXCEPTION(scope, encodedJSValue());
    911     StringView view = string.get();
    912     RETURN_IF_EXCEPTION(scope, encodedJSValue());
     909    auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
     910    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     911    StringView view = viewWithString.view;
    913912    JSValue a0 = exec->argument(0);
    914913    if (a0.isUInt32()) {
    … …  
    932931    if (!checkObjectCoercible(thisValue))
    933932        return throwVMTypeError(exec, scope);
    934     JSString* jsString = thisValue.toString(exec);
    935     RETURN_IF_EXCEPTION(scope, encodedJSValue());
    936     JSString::SafeView string = jsString->view(exec);
    937     StringView view = string.get();
    938     RETURN_IF_EXCEPTION(scope, encodedJSValue());
     933    auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
     934    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     935    StringView view = viewWithString.view;
    939936    JSValue a0 = exec->argument(0);
    940937    if (a0.isUInt32()) {
    … …  
    10361033        return JSValue::encode(jsNumber(-1));
    10371034
    1038     size_t result = thisJSString->view(exec).get().find(otherJSString->view(exec).get(), pos);
     1035    auto thisViewWithString = thisJSString->viewWithUnderlyingString(*exec);
     1036    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     1037    auto otherViewWithString = otherJSString->viewWithUnderlyingString(*exec);
     1038    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     1039    size_t result = thisViewWithString.view.find(otherViewWithString.view, pos);
    10391040    if (result == notFound)
    10401041        return JSValue::encode(jsNumber(-1));
    … …  
    20172018    if (!checkObjectCoercible(thisValue))
    20182019        return throwVMTypeError(exec, scope);
    2019     JSString::SafeView source = thisValue.toString(exec)->view(exec);
    2020     RETURN_IF_EXCEPTION(scope, encodedJSValue());
    2021     StringView view = source.get();
    2022     RETURN_IF_EXCEPTION(scope, encodedJSValue());
     2020    auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
     2021    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     2022    StringView view = viewWithString.view;
    20232023
    20242024    UNormalizationMode form = UNORM_NFC;
Note: See TracChangeset for help on using the changeset viewer.