Title: [208767] trunk/Source/_javascript_Core
Revision
208767
Author
[email protected]
Date
2016-11-15 16:19:54 -0800 (Tue, 15 Nov 2016)

Log 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):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (208766 => 208767)


--- trunk/Source/_javascript_Core/ChangeLog	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-11-16 00:19:54 UTC (rev 208767)
@@ -1,3 +1,48 @@
+2016-11-15  Mark Lam  <[email protected]>
+
+        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):
+
 2016-11-15  Filip Pizlo  <[email protected]>
 
         Unreviewed, remove bogus assertion.

Modified: trunk/Source/_javascript_Core/jsc.cpp (208766 => 208767)


--- trunk/Source/_javascript_Core/jsc.cpp	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/jsc.cpp	2016-11-16 00:19:54 UTC (rev 208767)
@@ -1531,6 +1531,9 @@
 
 static EncodedJSValue printInternal(ExecState* exec, FILE* out)
 {
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     if (test262AsyncTest) {
         JSValue value = exec->argument(0);
         if (value.isString() && WTF::equal(asString(value)->value(exec).impl(), "Test262:AsyncTestComplete"))
@@ -1543,7 +1546,9 @@
             if (EOF == fputc(' ', out))
                 goto fail;
 
-        if (fprintf(out, "%s", exec->uncheckedArgument(i).toString(exec)->view(exec).get().utf8().data()) < 0)
+        auto viewWithString = exec->uncheckedArgument(i).toString(exec)->viewWithUnderlyingString(*exec);
+        RETURN_IF_EXCEPTION(scope, encodedJSValue());
+        if (fprintf(out, "%s", viewWithString.view.utf8().data()) < 0)
             goto fail;
     }
 
@@ -1569,7 +1574,11 @@
 
 EncodedJSValue JSC_HOST_CALL functionDebug(ExecState* exec)
 {
-    fprintf(stderr, "--> %s\n", exec->argument(0).toString(exec)->view(exec).get().utf8().data());
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+    auto viewWithString = exec->argument(0).toString(exec)->viewWithUnderlyingString(*exec);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    fprintf(stderr, "--> %s\n", viewWithString.view.utf8().data());
     return JSValue::encode(jsUndefined());
 }
 

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2016-11-16 00:19:54 UTC (rev 208767)
@@ -667,7 +667,11 @@
         return JSValue::encode(slowJoin(*exec, thisObject, jsSeparator, length64));
     }
 
-    return JSValue::encode(fastJoin(*exec, thisObject, jsSeparator->view(exec).get(), length));
+    auto viewWithString = jsSeparator->viewWithUnderlyingString(*exec);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+
+    scope.release();
+    return JSValue::encode(fastJoin(*exec, thisObject, viewWithString.view, length));
 }
 
 EncodedJSValue JSC_HOST_CALL arrayProtoFuncPop(ExecState* exec)

Modified: trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp	2016-11-16 00:19:54 UTC (rev 208767)
@@ -123,13 +123,19 @@
         builder.append(prefix);
         builder.append(functionName.string());
         builder.append('(');
-        builder.append(args.at(0).toString(exec)->view(exec).get());
+        auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(*exec);
+        RETURN_IF_EXCEPTION(scope, nullptr);
+        builder.append(viewWithString.view);
         for (size_t i = 1; i < args.size() - 1; i++) {
             builder.appendLiteral(", ");
-            builder.append(args.at(i).toString(exec)->view(exec).get());
+            auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(*exec);
+            RETURN_IF_EXCEPTION(scope, nullptr);
+            builder.append(viewWithString.view);
         }
         builder.appendLiteral(") {\n");
-        builder.append(args.at(args.size() - 1).toString(exec)->view(exec).get());
+        viewWithString = args.at(args.size() - 1).toString(exec)->viewWithUnderlyingString(*exec);
+        RETURN_IF_EXCEPTION(scope, nullptr);
+        builder.append(viewWithString.view);
         builder.appendLiteral("\n}}");
         program = builder.toString();
     }

Modified: trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp	2016-11-16 00:19:54 UTC (rev 208767)
@@ -98,7 +98,12 @@
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
     // 9. Return CompareStrings(collator, X, Y).
-    return JSValue::encode(collator->compareStrings(*state, x->view(state).get(), y->view(state).get()));
+    auto xViewWithString = x->viewWithUnderlyingString(*state);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    auto yViewWithString = y->viewWithUnderlyingString(*state);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    scope.release();
+    return JSValue::encode(collator->compareStrings(*state, xViewWithString.view, yViewWithString.view));
 }
 
 EncodedJSValue JSC_HOST_CALL IntlCollatorPrototypeGetterCompare(ExecState* state)

Modified: trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h	2016-11-16 00:19:54 UTC (rev 208767)
@@ -287,7 +287,9 @@
 
     if (thisObject->isNeutered())
         return throwVMTypeError(exec, scope, typedArrayBufferHasBeenDetachedErrorMessage);
-    return joinWithSeparator(separatorString->view(exec).get());
+    auto viewWithString = separatorString->viewWithUnderlyingString(*exec);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    return joinWithSeparator(viewWithString.view);
 }
 
 template<typename ViewClass>

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp	2016-11-16 00:19:54 UTC (rev 208767)
@@ -65,10 +65,9 @@
     JSString* string = value.toStringOrNull(exec);
     if (UNLIKELY(!string))
         return { };
-    JSString::SafeView view = string->view(exec);
-    StringView stringView = view.get();
+    auto viewWithString = string->viewWithUnderlyingString(*exec);
     RETURN_IF_EXCEPTION(scope, { });
-    return callback(stringView);
+    return callback(viewWithString.view);
 }
 
 template<unsigned charactersCount>
@@ -718,7 +717,8 @@
 
 EncodedJSValue JSC_HOST_CALL globalFuncParseFloat(ExecState* exec)
 {
-    return JSValue::encode(jsNumber(parseFloat(exec->argument(0).toString(exec)->view(exec).get())));
+    auto viewWithString = exec->argument(0).toString(exec)->viewWithUnderlyingString(*exec);
+    return JSValue::encode(jsNumber(parseFloat(viewWithString.view)));
 }
 
 EncodedJSValue JSC_HOST_CALL globalFuncDecodeURI(ExecState* exec)

Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp	2016-11-16 00:19:54 UTC (rev 208767)
@@ -761,10 +761,9 @@
 
     if (!exec->argumentCount())
         return throwVMError(exec, scope, createError(exec, ASCIILiteral("JSON.parse requires at least one parameter")));
-    JSString::SafeView source = exec->uncheckedArgument(0).toString(exec)->view(exec);
+    auto viewWithString = exec->uncheckedArgument(0).toString(exec)->viewWithUnderlyingString(*exec);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    StringView view = source.get();
-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    StringView view = viewWithString.view;
 
     JSValue unfiltered;
     LocalScope localScope(vm);

Modified: trunk/Source/_javascript_Core/runtime/JSString.h (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/JSString.h	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSString.h	2016-11-16 00:19:54 UTC (rev 208767)
@@ -150,8 +150,6 @@
     AtomicString toAtomicString(ExecState*) const;
     RefPtr<AtomicStringImpl> toExistingAtomicString(ExecState*) const;
 
-    class SafeView;
-    SafeView view(ExecState*) const;
     StringViewWithUnderlyingString viewWithUnderlyingString(ExecState&) const;
 
     inline bool equal(ExecState*, JSString* other) const;
@@ -475,24 +473,6 @@
     friend JSString* jsString(ExecState*, const String&, const String&, const String&);
 };
 
-class JSString::SafeView {
-public:
-    explicit SafeView(ExecState&, const JSString&);
-    StringView get() const;
-
-    bool is8Bit() const { return m_string->is8Bit(); }
-    unsigned length() const { return m_string->length(); }
-
-private:
-    ExecState& m_state;
-
-    // The following pointer is marked "volatile" to make the compiler leave it on the stack
-    // or in a register as long as this object is alive, even after the last use of the pointer.
-    // That's needed to prevent garbage collecting the string and possibly deleting the block
-    // with the characters in it, and then using the StringView after that.
-    const JSString* volatile m_string;
-};
-
 JS_EXPORT_PRIVATE JSString* jsStringWithCacheSlowCase(VM&, StringImpl&);
 
 inline const StringImpl* JSString::tryGetValueImpl() const
@@ -759,22 +739,6 @@
     return isRope() && static_cast<const JSRopeString*>(this)->isSubstring();
 }
 
-inline JSString::SafeView::SafeView(ExecState& state, const JSString& string)
-    : m_state(state)
-    , m_string(&string)
-{
-}
-
-inline StringView JSString::SafeView::get() const
-{
-    return m_string->unsafeView(m_state);
-}
-
-ALWAYS_INLINE JSString::SafeView JSString::view(ExecState* exec) const
-{
-    return SafeView(*exec, *this);
-}
-
 // --- JSValue inlines ----------------------------
 
 inline bool JSValue::toBoolean(ExecState* exec) const

Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (208766 => 208767)


--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2016-11-16 00:19:54 UTC (rev 208767)
@@ -812,8 +812,8 @@
     ASSERT(repeatCount >= 0);
     ASSERT(!repeatCountValue.isDouble() || repeatCountValue.asDouble() == repeatCount);
 
-    JSString::SafeView safeView = string->view(exec);
-    StringView view = safeView.get();
+    auto viewWithString = string->viewWithUnderlyingString(*exec);
+    StringView view = viewWithString.view;
     ASSERT(view.length() == 1 && !scope.exception());
     UChar character = view[0];
     scope.release();
@@ -906,10 +906,9 @@
     JSValue thisValue = exec->thisValue();
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec, scope);
-    JSString::SafeView string = thisValue.toString(exec)->view(exec);
+    auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    StringView view = string.get();
-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    StringView view = viewWithString.view;
     JSValue a0 = exec->argument(0);
     if (a0.isUInt32()) {
         uint32_t i = a0.asUInt32();
@@ -931,11 +930,9 @@
     JSValue thisValue = exec->thisValue();
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec, scope);
-    JSString* jsString = thisValue.toString(exec);
+    auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    JSString::SafeView string = jsString->view(exec);
-    StringView view = string.get();
-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    StringView view = viewWithString.view;
     JSValue a0 = exec->argument(0);
     if (a0.isUInt32()) {
         uint32_t i = a0.asUInt32();
@@ -1035,7 +1032,11 @@
     if (thisJSString->length() < otherJSString->length() + pos)
         return JSValue::encode(jsNumber(-1));
 
-    size_t result = thisJSString->view(exec).get().find(otherJSString->view(exec).get(), pos);
+    auto thisViewWithString = thisJSString->viewWithUnderlyingString(*exec);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    auto otherViewWithString = otherJSString->viewWithUnderlyingString(*exec);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    size_t result = thisViewWithString.view.find(otherViewWithString.view, pos);
     if (result == notFound)
         return JSValue::encode(jsNumber(-1));
     return JSValue::encode(jsNumber(result));
@@ -2016,10 +2017,9 @@
     JSValue thisValue = exec->thisValue();
     if (!checkObjectCoercible(thisValue))
         return throwVMTypeError(exec, scope);
-    JSString::SafeView source = thisValue.toString(exec)->view(exec);
+    auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    StringView view = source.get();
-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    StringView view = viewWithString.view;
 
     UNormalizationMode form = UNORM_NFC;
     // Verify that the argument is provided and is not undefined.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to