Changeset 206082 in webkit

Timestamp:

Sep 18, 2016 10:40:11 AM (8 years ago)

Author:

Yusuke Suzuki

Message:

[JSC] Do not need to use defineProperty to define methods for object literals
​https://bugs.webkit.org/show_bug.cgi?id=162111

Reviewed by Saam Barati.

JSTests:

• stress/object-literal-methods.js: Added.

(shouldBe):
(throw.new.Error.let.object.get name):
(throw.new.Error):
(shouldBe.let.object.get name):
(shouldBe.let.object.get prototype):
(shouldBe.let.object.get 42):

Source/JavaScriptCore:

When we receive the following code,

var object = { method() { } };

currently, we use defineProperty to define "method" function for "object".
This patch replaces it with the ordinary put_by_id_direct / put_by_val_direct
because the following 2 conditions are met.

1. While methods in classes have special attributes ({configurable: true, writable: true, enumerable: false}), the attributes of methods in object literals is just the same to the other normal properties ({configurable: true, writable: true, enumerable: true}). This means that we can use the usual put_by_id_direct / put_by_val_direct to define method properties for object literals.

2. Furthermore, all the own properties that can reside in objects created by object literals have {configurable: true}. So there is no need to check conflict by defineProperty. Always overwriting is OK.

let name = 'method';
var object = { get [name]() { }, method() { } };
Latter method wins.

On the other hand, in class syntax, conflict check is necessary since "prototype" own property is defined as {configurable: false}.

class Hello { static prototype() { } } Should throw error by defineProperty's check.

This means that conflict check done in defneProperty is not necessary for object literals' properties.

• bytecompiler/NodesCodegen.cpp:

(JSC::PropertyListNode::emitPutConstantProperty):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/object-literal-methods.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2016-09-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Do not need to use defineProperty to define methods for object literals
+        https://bugs.webkit.org/show_bug.cgi?id=162111
+
+        Reviewed by Saam Barati.
+
+        * stress/object-literal-methods.js: Added.
+        (shouldBe):
+        (throw.new.Error.let.object.get name):
+        (throw.new.Error):
+        (shouldBe.let.object.get name):
+        (shouldBe.let.object.get prototype):
+        (shouldBe.let.object.get 42):
+
 2016-09-16  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-09-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Do not need to use defineProperty to define methods for object literals
+        https://bugs.webkit.org/show_bug.cgi?id=162111
+
+        Reviewed by Saam Barati.
+
+        When we receive the following code,
+
+            var object = { method() { } };
+
+        currently, we use defineProperty to define "method" function for "object".
+        This patch replaces it with the ordinary put_by_id_direct / put_by_val_direct
+        because the following 2 conditions are met.
+
+        1. While methods in classes have special attributes ({configurable: true, writable: true, enumerable: false}),
+           the attributes of methods in object literals is just the same to the other normal properties ({configurable: true, writable: true, enumerable: true}).
+           This means that we can use the usual put_by_id_direct / put_by_val_direct to define method properties for object literals.
+
+        2. Furthermore, all the own properties that can reside in objects created by object literals have {configurable: true}.
+           So there is no need to check conflict by defineProperty. Always overwriting is OK.
+
+                let name = 'method';
+                var object = { get [name]() { }, method() { } };
+                // Latter method wins.
+
+            On the other hand, in class syntax, conflict check is necessary since "prototype" own property is defined as {configurable: false}.
+
+                class Hello { static prototype() { } }  // Should throw error by defineProperty's check.
+
+            This means that conflict check done in defneProperty is not necessary for object literals' properties.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::PropertyListNode::emitPutConstantProperty):
+
 2016-09-16  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -588 +588 @@
 {
     RefPtr<RegisterID> value = generator.emitNode(node.m_assign);
-    if (node.needsSuperBinding()) {
+    if (node.needsSuperBinding())
         emitPutHomeObject(generator, value.get(), newObj);
 
+    if (node.isClassProperty()) {
+        ASSERT(node.needsSuperBinding());
         RefPtr<RegisterID> propertyNameRegister;
         if (node.name())
@@ -597 +599 @@
             propertyNameRegister = generator.emitNode(node.m_expression);
 
-        unsigned attributes = BytecodeGenerator::PropertyConfigurable | BytecodeGenerator::PropertyWritable;
-        if (!node.isClassProperty())
-            attributes |= BytecodeGenerator::PropertyEnumerable;
         generator.emitSetFunctionNameIfNeeded(node.m_assign, value.get(), propertyNameRegister.get());
-        generator.emitCallDefineProperty(newObj, propertyNameRegister.get(),
-            value.get(), nullptr, nullptr, attributes, m_position);
+        generator.emitCallDefineProperty(newObj, propertyNameRegister.get(), value.get(), nullptr, nullptr, BytecodeGenerator::PropertyConfigurable | BytecodeGenerator::PropertyWritable, m_position);
         return;
     }