[webkit-changes] [220324] trunk

Sun, 06 Aug 2017 10:57:47 -0700

Title: [220324] trunk
Revision
220324
Author
utatane....@gmail.com
Date
2017-08-06 10:57:26 -0700 (Sun, 06 Aug 2017)

Log Message

Promise resolve and reject function should have length = 1
https://bugs.webkit.org/show_bug.cgi?id=175242

Reviewed by Saam Barati.

JSTests:

* stress/builtin-function-length.js: Added.
(shouldBe):
(shouldThrow):
(shouldBe.JSON.stringify.Object.getOwnPropertyDescriptor):
(shouldBe.JSON.stringify.Object.getOwnPropertyNames.Array.prototype.filter.sort):

Source/_javascript_Core:

Previously we have separate system for "length" and "name" for builtin functions.
The builtin functions do not use lazy reifying system. Instead, they have direct
properties when instantiating it. While the function created for properties (like
Array.prototype.filter) is created by JSFunction::createBuiltin(), function inside
these builtin functions are just created by JSFunction::create(). Since it does
not set any values for "length", these functions do not have "length" property.
So, the resolve and reject functions passed to Promise's executor do not have
"length" property.

This patch make builtin functions use standard lazy reifying system for "length".
So, "length" property of the builtin function just works as if the normal functions
do.

* runtime/JSFunction.cpp:
(JSC::JSFunction::createBuiltinFunction):
(JSC::JSFunction::getOwnPropertySlot):
(JSC::JSFunction::getOwnNonIndexPropertyNames):
(JSC::JSFunction::put):
(JSC::JSFunction::deleteProperty):
(JSC::JSFunction::defineOwnProperty):
(JSC::JSFunction::reifyLazyPropertyIfNeeded):
(JSC::JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded):
(JSC::JSFunction::reifyLazyLengthIfNeeded):
(JSC::JSFunction::reifyLazyBoundNameIfNeeded):
(JSC::JSFunction::reifyBoundNameIfNeeded): Deleted.
* runtime/JSFunction.h:

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (220323 => 220324)


--- trunk/JSTests/ChangeLog	2017-08-06 13:04:32 UTC (rev 220323)
+++ trunk/JSTests/ChangeLog	2017-08-06 17:57:26 UTC (rev 220324)
@@ -1,3 +1,16 @@
+2017-08-06  Yusuke Suzuki  <utatane....@gmail.com>
+
+        Promise resolve and reject function should have length = 1
+        https://bugs.webkit.org/show_bug.cgi?id=175242
+
+        Reviewed by Saam Barati.
+
+        * stress/builtin-function-length.js: Added.
+        (shouldBe):
+        (shouldThrow):
+        (shouldBe.JSON.stringify.Object.getOwnPropertyDescriptor):
+        (shouldBe.JSON.stringify.Object.getOwnPropertyNames.Array.prototype.filter.sort):
+
 2017-08-06  Oleksandr Skachkov  <gskach...@gmail.com>
 
         [ESNext] Async iteration - Implement Async Generator - parser

Added: trunk/JSTests/stress/builtin-function-length.js (0 => 220324)


--- trunk/JSTests/stress/builtin-function-length.js	                        (rev 0)
+++ trunk/JSTests/stress/builtin-function-length.js	2017-08-06 17:57:26 UTC (rev 220324)
@@ -0,0 +1,47 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+{
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(Array.prototype.filter).sort()), `["length","name"]`);
+    shouldBe(Array.prototype.filter.length, 1);
+    shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(Array.prototype.filter, 'length')), `{"value":1,"writable":false,"enumerable":false,"configurable":true}`);
+    shouldBe(delete Array.prototype.filter.length, true);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(Array.prototype.filter).sort()), `["name"]`);
+}
+
+{
+    shouldThrow(function () {
+        "use strict";
+        Array.prototype.forEach.length = 42;
+    }, `TypeError: Attempted to assign to readonly property.`);
+}
+
+{
+    var resolve = null;
+    var reject = null;
+    new Promise(function (arg0, arg1) {
+        resolve = arg0;
+        reject = arg1;
+    });
+    shouldBe(resolve.length, 1);
+    shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(resolve, 'length')), `{"value":1,"writable":false,"enumerable":false,"configurable":true}`);
+    shouldBe(reject.length, 1);
+    shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(reject, 'length')), `{"value":1,"writable":false,"enumerable":false,"configurable":true}`);
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (220323 => 220324)


--- trunk/Source/_javascript_Core/ChangeLog	2017-08-06 13:04:32 UTC (rev 220323)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-08-06 17:57:26 UTC (rev 220324)
@@ -1,3 +1,37 @@
+2017-08-06  Yusuke Suzuki  <utatane....@gmail.com>
+
+        Promise resolve and reject function should have length = 1
+        https://bugs.webkit.org/show_bug.cgi?id=175242
+
+        Reviewed by Saam Barati.
+
+        Previously we have separate system for "length" and "name" for builtin functions.
+        The builtin functions do not use lazy reifying system. Instead, they have direct
+        properties when instantiating it. While the function created for properties (like
+        Array.prototype.filter) is created by JSFunction::createBuiltin(), function inside
+        these builtin functions are just created by JSFunction::create(). Since it does
+        not set any values for "length", these functions do not have "length" property.
+        So, the resolve and reject functions passed to Promise's executor do not have
+        "length" property.
+
+        This patch make builtin functions use standard lazy reifying system for "length".
+        So, "length" property of the builtin function just works as if the normal functions
+        do.
+
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::createBuiltinFunction):
+        (JSC::JSFunction::getOwnPropertySlot):
+        (JSC::JSFunction::getOwnNonIndexPropertyNames):
+        (JSC::JSFunction::put):
+        (JSC::JSFunction::deleteProperty):
+        (JSC::JSFunction::defineOwnProperty):
+        (JSC::JSFunction::reifyLazyPropertyIfNeeded):
+        (JSC::JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded):
+        (JSC::JSFunction::reifyLazyLengthIfNeeded):
+        (JSC::JSFunction::reifyLazyBoundNameIfNeeded):
+        (JSC::JSFunction::reifyBoundNameIfNeeded): Deleted.
+        * runtime/JSFunction.h:
+
 2017-08-06  Oleksandr Skachkov  <gskach...@gmail.com>
 
         [ESNext] Async iteration - Implement Async Generator - parser

Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (220323 => 220324)


--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp	2017-08-06 13:04:32 UTC (rev 220323)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp	2017-08-06 17:57:26 UTC (rev 220324)
@@ -105,7 +105,6 @@
 {
     JSFunction* function = create(vm, executable, globalObject);
     function->putDirect(vm, vm.propertyNames->name, jsString(&vm, executable->name().string()), ReadOnly | DontEnum);
-    function->putDirect(vm, vm.propertyNames->length, jsNumber(executable->parameterCount()), ReadOnly | DontEnum);
     return function;
 }
 
@@ -113,7 +112,6 @@
 {
     JSFunction* function = create(vm, executable, globalObject);
     function->putDirect(vm, vm.propertyNames->name, jsString(&vm, name), ReadOnly | DontEnum);
-    function->putDirect(vm, vm.propertyNames->length, jsNumber(executable->parameterCount()), ReadOnly | DontEnum);
     return function;
 }
 
@@ -350,7 +348,7 @@
     VM& vm = exec->vm();
     JSFunction* thisObject = jsCast<JSFunction*>(object);
     if (thisObject->isHostOrBuiltinFunction()) {
-        thisObject->reifyBoundNameIfNeeded(vm, exec, propertyName);
+        thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
         return Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
     }
 
@@ -402,21 +400,27 @@
 {
     JSFunction* thisObject = jsCast<JSFunction*>(object);
     VM& vm = exec->vm();
-    if (!thisObject->isHostOrBuiltinFunction() && mode.includeDontEnumProperties()) {
-        // Make sure prototype has been reified.
-        PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
-        thisObject->methodTable(vm)->getOwnPropertySlot(thisObject, exec, vm.propertyNames->prototype, slot);
+    if (mode.includeDontEnumProperties()) {
+        if (!thisObject->isHostOrBuiltinFunction()) {
+            // Make sure prototype has been reified.
+            PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
+            thisObject->methodTable(vm)->getOwnPropertySlot(thisObject, exec, vm.propertyNames->prototype, slot);
 
-        if (thisObject->jsExecutable()->hasCallerAndArgumentsProperties()) {
-            propertyNames.add(vm.propertyNames->arguments);
-            propertyNames.add(vm.propertyNames->caller);
+            if (thisObject->jsExecutable()->hasCallerAndArgumentsProperties()) {
+                propertyNames.add(vm.propertyNames->arguments);
+                propertyNames.add(vm.propertyNames->caller);
+            }
+            if (!thisObject->hasReifiedLength())
+                propertyNames.add(vm.propertyNames->length);
+            if (!thisObject->hasReifiedName())
+                propertyNames.add(vm.propertyNames->name);
+        } else {
+            if (thisObject->isBuiltinFunction() && !thisObject->hasReifiedLength())
+                propertyNames.add(vm.propertyNames->length);
+            if (thisObject->inherits(vm, JSBoundFunction::info()) && !thisObject->hasReifiedName())
+                propertyNames.add(vm.propertyNames->name);
         }
-        if (!thisObject->hasReifiedLength())
-            propertyNames.add(vm.propertyNames->length);
-        if (!thisObject->hasReifiedName())
-            propertyNames.add(vm.propertyNames->name);
-    } else if (thisObject->isHostOrBuiltinFunction() && mode.includeDontEnumProperties() && thisObject->inherits(vm, JSBoundFunction::info()) && !thisObject->hasReifiedName())
-        propertyNames.add(exec->vm().propertyNames->name);
+    }
     Base::getOwnNonIndexPropertyNames(thisObject, exec, propertyNames, mode);
 }
 
@@ -433,7 +437,7 @@
     }
 
     if (thisObject->isHostOrBuiltinFunction()) {
-        LazyPropertyType propType = thisObject->reifyBoundNameIfNeeded(vm, exec, propertyName);
+        LazyPropertyType propType = thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
         if (propType == LazyPropertyType::IsLazyProperty)
             slot.disableCaching();
         scope.release();
@@ -475,12 +479,12 @@
 
 bool JSFunction::deleteProperty(JSCell* cell, ExecState* exec, PropertyName propertyName)
 {
+    VM& vm = exec->vm();
     JSFunction* thisObject = jsCast<JSFunction*>(cell);
     if (thisObject->isHostOrBuiltinFunction())
-        thisObject->reifyBoundNameIfNeeded(exec->vm(), exec, propertyName);
-    else if (exec->vm().deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) {
+        thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
+    else if (vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) {
         // For non-host functions, don't let these properties by deleted - except by DefineOwnProperty.
-        VM& vm = exec->vm();
         FunctionExecutable* executable = thisObject->jsExecutable();
         
         if (propertyName == exec->propertyNames().caller || propertyName == exec->propertyNames().arguments)
@@ -502,7 +506,7 @@
 
     JSFunction* thisObject = jsCast<JSFunction*>(object);
     if (thisObject->isHostOrBuiltinFunction()) {
-        thisObject->reifyBoundNameIfNeeded(vm, exec, propertyName);
+        thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
         scope.release();
         return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
     }
@@ -680,11 +684,9 @@
 
 JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
 {
-    if (propertyName == vm.propertyNames->length) {
-        if (!hasReifiedLength())
-            reifyLength(vm);
+    if (reifyLazyLengthIfNeeded(vm, exec, propertyName) == LazyPropertyType::IsLazyProperty)
         return LazyPropertyType::IsLazyProperty;
-    } else if (propertyName == vm.propertyNames->name) {
+    if (propertyName == vm.propertyNames->name) {
         if (!hasReifiedName())
             reifyName(vm, exec);
         return LazyPropertyType::IsLazyProperty;
@@ -692,8 +694,28 @@
     return LazyPropertyType::NotLazyProperty;
 }
 
-JSFunction::LazyPropertyType JSFunction::reifyBoundNameIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
 {
+    ASSERT(isHostOrBuiltinFunction());
+    if (isBuiltinFunction()) {
+        if (reifyLazyLengthIfNeeded(vm, exec, propertyName) == LazyPropertyType::IsLazyProperty)
+            return LazyPropertyType::IsLazyProperty;
+    }
+    return reifyLazyBoundNameIfNeeded(vm, exec, propertyName);
+}
+
+JSFunction::LazyPropertyType JSFunction::reifyLazyLengthIfNeeded(VM& vm, ExecState*, PropertyName propertyName)
+{
+    if (propertyName == vm.propertyNames->length) {
+        if (!hasReifiedLength())
+            reifyLength(vm);
+        return LazyPropertyType::IsLazyProperty;
+    }
+    return LazyPropertyType::NotLazyProperty;
+}
+
+JSFunction::LazyPropertyType JSFunction::reifyLazyBoundNameIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+{
     const Identifier& nameIdent = vm.propertyNames->name;
     if (propertyName != nameIdent)
         return LazyPropertyType::NotLazyProperty;

Modified: trunk/Source/_javascript_Core/runtime/JSFunction.h (220323 => 220324)


--- trunk/Source/_javascript_Core/runtime/JSFunction.h	2017-08-06 13:04:32 UTC (rev 220323)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.h	2017-08-06 17:57:26 UTC (rev 220324)
@@ -190,7 +190,9 @@
 
     enum class LazyPropertyType { NotLazyProperty, IsLazyProperty };
     LazyPropertyType reifyLazyPropertyIfNeeded(VM&, ExecState*, PropertyName);
-    LazyPropertyType reifyBoundNameIfNeeded(VM&, ExecState*, PropertyName);
+    LazyPropertyType reifyLazyPropertyForHostOrBuiltinIfNeeded(VM&, ExecState*, PropertyName);
+    LazyPropertyType reifyLazyLengthIfNeeded(VM&, ExecState*, PropertyName);
+    LazyPropertyType reifyLazyBoundNameIfNeeded(VM&, ExecState*, PropertyName);
 
     friend class LLIntOffsetsExtractor;
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to